Add namespace aware parsing for <hello> #50
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # See https://editorconfig.org/ | ||
| [*] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| indent_size = 4 | ||
| indent_style = space | ||
| insert_final_newline = false | ||
| max_line_length = 120 | ||
| tab_width = 4 | ||
|
|
||
| [{*.bat,*.cmd}] | ||
| end_of_line = crlf | ||
|
|
||
| [*.java] | ||
| ij_java_blank_lines_after_imports = 1 | ||
| ij_java_blank_lines_before_imports = 1 | ||
| ij_java_class_count_to_use_import_on_demand = 999 | ||
| ij_java_imports_layout = *,|,javax.**,java.**,|,$* | ||
| ij_java_layout_static_imports_separately = true | ||
| ij_java_names_count_to_use_import_on_demand = 999 | ||
| ij_java_use_single_class_imports = true | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| package net.juniper.netconf.element; | ||
|
|
||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.Singular; | ||
| import lombok.ToString; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import net.juniper.netconf.NetconfConstants; | ||
| import org.w3c.dom.Document; | ||
| import org.w3c.dom.Element; | ||
| import org.w3c.dom.Node; | ||
| import org.w3c.dom.NodeList; | ||
| import org.xml.sax.InputSource; | ||
| import org.xml.sax.SAXException; | ||
|
|
||
| import javax.xml.parsers.DocumentBuilderFactory; | ||
| import javax.xml.parsers.ParserConfigurationException; | ||
| import javax.xml.transform.OutputKeys; | ||
| import javax.xml.transform.Transformer; | ||
| import javax.xml.transform.TransformerException; | ||
| import javax.xml.transform.TransformerFactory; | ||
| import javax.xml.transform.dom.DOMSource; | ||
| import javax.xml.transform.stream.StreamResult; | ||
| import javax.xml.xpath.XPath; | ||
| import javax.xml.xpath.XPathConstants; | ||
| import javax.xml.xpath.XPathExpressionException; | ||
| import javax.xml.xpath.XPathFactory; | ||
| import java.io.IOException; | ||
| import java.io.StringReader; | ||
| import java.io.StringWriter; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
| * Class to represent a NETCONF hello element - https://datatracker.ietf.org/doc/html/rfc6241#section-8.1 | ||
| */ | ||
| @Data | ||
| @Slf4j | ||
| public class Hello { | ||
|
|
||
| @ToString.Exclude | ||
| @EqualsAndHashCode.Exclude | ||
| private final Document document; | ||
|
|
||
| @ToString.Exclude | ||
| private final String xml; | ||
|
|
||
| private final String sessionId; | ||
|
|
||
| @Singular("capability") | ||
| private final List<String> capabilities; | ||
|
|
||
| public boolean hasCapability(final String capability) { | ||
| return capabilities.contains(capability); | ||
| } | ||
|
|
||
| public String toXML() { | ||
| return xml; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a Hello object based on the supplied XML. | ||
| * | ||
| * @param xml The XML of the NETCONF <hello> | ||
| * @return an new, immutable, Hello object. | ||
| * @throws ParserConfigurationException If the XML parser cannot be created | ||
| * @throws IOException If the XML cannot be read | ||
| * @throws SAXException If the XML cannot be parsed | ||
| * @throws XPathExpressionException If there is a problem in the parsing expressions | ||
| */ | ||
| public static Hello from(final String xml) | ||
| throws ParserConfigurationException, IOException, SAXException, XPathExpressionException { | ||
|
|
||
| final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); | ||
| documentBuilderFactory.setNamespaceAware(true); | ||
| final Document document = documentBuilderFactory.newDocumentBuilder() | ||
| .parse(new InputSource(new StringReader(xml))); | ||
| final XPath xPath = XPathFactory.newInstance().newXPath(); | ||
| final String sessionId = xPath.evaluate("/*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='hello']/*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='session-id']", document); | ||
|
Contributor
There was a problem hiding this comment. that is a super long string. Would it make sense to add this to the NetconfConstants.java?
Contributor
Author
There was a problem hiding this comment. There’s certainly some refactoring that could be done here. However, I plan to do it with the implementation. My current thinking is that both Hello and RpcReply will inherit from AbstractNetconfElement which will contain various utility methods that can be shared between them. So I’d like to put it off for now, if that’s OK. |
||
| final HelloBuilder builder = Hello.builder() | ||
| .originalDocument(document) | ||
| .sessionId(sessionId); | ||
| final NodeList capabilities = (NodeList) xPath.evaluate("/*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='hello']/*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='capabilities']/*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='capability']", document, XPathConstants.NODESET); | ||
|
Contributor
There was a problem hiding this comment. that is a super long string. Would it make sense to add this to the NetconfConstants.java? |
||
| for (int i = 0; i < capabilities.getLength(); i++) { | ||
|
Contributor
There was a problem hiding this comment. nit, this is fine.. I would have been tempted to do something like capabilites.foreach(capability -> {
Contributor
Author
There was a problem hiding this comment. Unfortunately the NodeList doesn’t implement iterable so it has to be an old-style ‘for’ statement. Though it does highlight that there is a lot of reliance on old-style, pretty much deprecated javax packages. But I think changing the public API of the library is beyond the scope of this PR! |
||
| final Node node = capabilities.item(i); | ||
| builder.capability(node.getTextContent()); | ||
| } | ||
| final Hello hello = builder.build(); | ||
| if (log.isInfoEnabled()) { | ||
| log.info("hello is: {}", hello.toXML()); | ||
| } | ||
| return hello; | ||
| } | ||
|
|
||
| @Builder | ||
| private Hello( | ||
| final Document originalDocument, | ||
| final String namespacePrefix, | ||
| final String sessionId, | ||
| @Singular("capability") final List<String> capabilities) { | ||
| this.sessionId = sessionId; | ||
| this.capabilities = capabilities; | ||
| if (originalDocument != null) { | ||
| this.document = originalDocument; | ||
| } else { | ||
| this.document = createDocument(namespacePrefix, sessionId, capabilities); | ||
| } | ||
| this.xml = createXml(document); | ||
| } | ||
|
|
||
| private static Document createDocument( | ||
| final String namespacePrefix, | ||
| final String sessionId, | ||
| final List<String> capabilities) { | ||
|
|
||
| final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); | ||
| documentBuilderFactory.setNamespaceAware(true); | ||
| final Document createdDocument; | ||
| try { | ||
| createdDocument = documentBuilderFactory.newDocumentBuilder().newDocument(); | ||
| } catch (final ParserConfigurationException e) { | ||
| throw new IllegalStateException("Unable to create document builder", e); | ||
| } | ||
|
|
||
| final Element helloElement | ||
| = createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "hello"); | ||
|
Contributor
There was a problem hiding this comment. When I see four lines that are very similar, I wonder if it could be turned into a private function. it looks like the difference is the name space name? So if you could pass in a Document and name space name and it would return an Element? I'd also be tempted to make the name space name an enum and use that instead of the string. |
||
| helloElement.setPrefix(namespacePrefix); | ||
| createdDocument.appendChild(helloElement); | ||
|
|
||
| final Element capabilitiesElement | ||
| = createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "capabilities"); | ||
| capabilitiesElement.setPrefix(namespacePrefix); | ||
| capabilities.forEach(capability -> { | ||
| final Element capabilityElement = | ||
| createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "capability"); | ||
| capabilityElement.setTextContent(capability); | ||
| capabilityElement.setPrefix(namespacePrefix); | ||
| capabilitiesElement.appendChild(capabilityElement); | ||
| }); | ||
| helloElement.appendChild(capabilitiesElement); | ||
|
|
||
| if (sessionId != null) { | ||
| final Element sessionIdElement | ||
| = createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "session-id"); | ||
| sessionIdElement.setPrefix(namespacePrefix); | ||
| sessionIdElement.setTextContent(sessionId); | ||
| helloElement.appendChild(sessionIdElement); | ||
| } | ||
| return createdDocument; | ||
| } | ||
|
|
||
| private static String createXml(final Document document) { | ||
| try { | ||
| final TransformerFactory transformerFactory = TransformerFactory.newInstance(); | ||
| final Transformer transformer = transformerFactory.newTransformer(); | ||
| transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); | ||
| final StringWriter stringWriter = new StringWriter(); | ||
| transformer.transform(new DOMSource(document), new StreamResult(stringWriter)); | ||
| return stringWriter.toString(); | ||
| } catch (final TransformerException e) { | ||
| throw new IllegalStateException("Unable to transform document to XML", e); | ||
| } | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure that this file should be in the remote repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a deliberate choice by me; an earlier comment from @peterjhill highlighted that I had inadvertently shuffled around the order of import statements. This file should stop that happening for in the future for users of IntelliJ at least by configuring it to use the defaults used in the project.
The file also defines standard editorconfig settings for things like line-endings, tab width etc., also in line with the project. These should work with most, if not all IDEs, and should make it easier for contributors to maintain the project style.
I'm happy to remove it if the consensus is it is unnecessary.