Added more tests

Author: Peter Hill

src/main/java/net/juniper/netconf/Device.java

@@ -612,9 +612,9 @@ public BufferedReader runShellCommandRunning(String command)
     }
 
     /**
-     * Send an RPC(as String object) over the default Netconf session and get
-     * the response as an XML object.
-     * <p>
+     * Send an RPC(as String object) over the default Netconf session and get the
+     * response as an XML object.
+     * <p>Convenience overload for raw‑string payloads.</p>
      *
      * @param rpcContent RPC content to be sent. For example, to send an rpc
      *                   &lt;rpc&gt;&lt;get-chassis-inventory/&gt;&lt;/rpc&gt;, the
@@ -637,7 +637,7 @@ public XML executeRPC(String rpcContent) throws SAXException, IOException {
     /**
      * Send an RPC(as XML object) over the Netconf session and get the response
      * as an XML object.
-     * <p>
+     * <p>Use when the payload is already assembled as an {@link XML} helper object.</p>
      *
      * @param rpc RPC to be sent. Use the XMLBuilder to create RPC as an
      *            XML object.
@@ -656,7 +656,8 @@ public XML executeRPC(XML rpc) throws SAXException, IOException {
     /**
      * Send an RPC(as Document object) over the Netconf session and get the
      * response as an XML object.
-     * <p>
+     * <p>Accepts a DOM {@link org.w3c.dom.Document} that represents the full
+     * &lt;rpc&gt; element.</p>
      *
      * @param rpcDoc RPC content to be sent, as a org.w3c.dom.Document object.
      * @return RPC reply sent by Netconf server
@@ -691,7 +692,7 @@ public BufferedReader executeRPCRunning(String rpcContent) throws IOException {
     /**
      * Send an RPC(as XML object) over the Netconf session and get the response
      * as a BufferedReader.
-     * <p>
+     * <p>Streams the reply incrementally, suitable for large responses.</p>
      *
      * @param rpc RPC to be sent. Use the XMLBuilder to create RPC as an
      *            XML object.
@@ -709,15 +710,23 @@ public BufferedReader executeRPCRunning(XML rpc) throws IOException {
     }
 
     /**
-     * Send an RPC(as Document object) over the Netconf session and get the
-     * response as a BufferedReader.
+     * Sends an RPC (as a DOM {@link Document}) over the active NETCONF session
+     * and returns a {@link BufferedReader} for streaming the reply.
      * <p>
+     * Use this variant when you need to consume the server response
+     * <em>incrementally</em>&nbsp;&mdash; for example, when the RPC produces a
+     * large dataset or when you want to start processing output before the
+     * device finishes sending the final <code>]]&gt;]]&gt;</code> prompt.
+     * </p>
      *
-     * @param rpcDoc RPC content to be sent, as a org.w3c.dom.Document object.
-     * @return RPC reply sent by Netconf server as a BufferedReader. This is
-     * useful if we want continuous stream of output, rather than wait
-     * for whole output till command execution completes.
-     * @throws java.io.IOException If there are errors communicating with the Netconf server.
+     * @param rpcDoc the complete &lt;rpc&gt; element encoded as a DOM
+     *               {@link Document}; must not be {@code null}
+     *
+     * @return a {@link BufferedReader} connected to the server’s reply stream
+     *
+     * @throws IOException           if an I/O error occurs while sending the
+     *                               request or reading the reply
+     * @throws IllegalStateException if no NETCONF connection is established
      */
     public BufferedReader executeRPCRunning(Document rpcDoc) throws IOException {
         if (netconfSession == null) {
@@ -1257,9 +1266,15 @@ public boolean isStrictHostKeyChecking() {
     // Getters for fields
 
     /**
-     * Returns the JSch SSH client used by this device.
+     * Returns the {@link JSch} instance that backs this {@code Device}.
      * <p>
-     * Defensive copy not possible; caller must not modify the returned instance.
+     * <strong>Note&nbsp;–</strong> the returned object is the live instance
+     * used for all SSH operations; creating a defensive copy is not feasible,
+     * so callers <em>must not</em> modify its global state (e.g.&nbsp;changing
+     * the identity repository or host‑key repository) once the {@code Device}
+     * has been built.
+     *
+     * @return the underlying {@link JSch} SSH client
      */
     public JSch getSshClient() {
         // Defensive copy not possible; document that caller must not modify

src/main/java/net/juniper/netconf/NetconfConstants.java

@@ -1,6 +1,14 @@
 package net.juniper.netconf;
 
 /**
+ * Centralised collection of string literals and protocol constants used
+ * throughout the NETCONF client library.
+ * <p>
+ * The class is {@code final} and has a private constructor – it cannot be
+ * instantiated or extended.  All members are {@code public static final}
+ * to encourage direct use without additional indirection.
+ * </p>
+ *
  * @author Jonas Glass
  */
 public class NetconfConstants {
@@ -55,6 +63,8 @@ public class NetconfConstants {
     /** UTF‑8 charset literal used throughout the library. */
     public static final String CHARSET_UTF8 = "utf-8";
 
-    private NetconfConstants() {
-    }
+    /**
+     * Not instantiable – utility holder only.
+     */
+    private NetconfConstants() { /* no‑op */ }
 }

src/main/java/net/juniper/netconf/NetconfSession.java

@@ -803,6 +803,7 @@ public void commit() throws IOException, SAXException {
      *
      * @param seconds the <confirm-timeout> in seconds; if {@code seconds
      *                <= 0} the device’s default (600 s) is used
+     * @throws IOException if communication with the device fails
      *
      * @deprecated Prefer {@link #commitConfirm(long, String)} which adds
      * the <code>&lt;persist&gt;</code> / <code>&lt;persist-id&gt;</code>

src/main/java/net/juniper/netconf/XML.java

@@ -380,47 +380,49 @@ public String findValue(List<String> list) {
         for (int k=0; k<list.size(); k++) {
             nextElementFound = false;
             String nextElementName = list.get(k);
-            if (!nextElementName.contains("~")){
-                try {
-                    NodeList nextElementList = nextElement.
-                            getElementsByTagName(nextElementName);
-                    /* If the next to next(n2n) element is a filter based on
-                     * text value, then do the required filtering.
+            if (!nextElementName.contains("~")) {
+                NodeList nextElementList =
+                        nextElement != null
+                                ? nextElement.getElementsByTagName(nextElementName)
+                                : null;
+                if (nextElementList == null || nextElementList.getLength() == 0) {
+                    logger.fine("Element '" + nextElementName + "' not found in findValue()");
+                    return null;
+                }
+
+                /* If the next‑to‑next (n2n) element is a filter based on
+                 * text value, then do the required filtering.
+                 */
+                String n2nString = null;
+                if (k<list.size()-1)
+                    n2nString = list.get(k+1);
+                if (n2nString != null && n2nString.contains("~")) {
+                    /* Since the n2n element is a filter based on text
+                     * value( decided by '~')
+                     * we now traverse the entire NodeList to find the
+                     * correct nextElement
+                     * based on the text value of the filter.
                      */
-                    String n2nString = null;
-                    if (k<list.size()-1)
-                        n2nString = list.get(k+1);
-                    if (n2nString != null && n2nString.contains("~")) {
-                        /* Since the n2n element is a filter based on text
-                         * value( decided by '~')
-                         * we now traverse the entire NodeList to find the
-                         * correct nextElement
-                         * based on the text value of the filter.
-                         */
-                        String n2nText = n2nString.substring(n2nString.
-                                indexOf("~") + 1);
-                        String n2nElementName = n2nString.substring(0,
-                                n2nString.indexOf("~"));
-                        for (int i=0; i<nextElementList.getLength(); i++) {
-                            nextElement = (Element)nextElementList.item(i);
-                            NodeList nodes = nextElement.getElementsByTagName(n2nElementName);
-                            if (nodes.getLength() == 0) continue;
-                            Element n2nElement = (Element) nodes.item(0);
-                            if (n2nElement == null || n2nElement.getFirstChild() == null) continue;
-                            String text = trim(n2nElement.getFirstChild().getNodeValue());
-                            if (text.equals(n2nText)) {
-                                nextElementFound = true;
-                                break;
-                            }
+                    String n2nText = n2nString.substring(n2nString.
+                            indexOf("~") + 1);
+                    String n2nElementName = n2nString.substring(0,
+                            n2nString.indexOf("~"));
+                    for (int i=0; i<nextElementList.getLength(); i++) {
+                        nextElement = (Element)nextElementList.item(i);
+                        NodeList nodes = nextElement.getElementsByTagName(n2nElementName);
+                        if (nodes.getLength() == 0) continue;
+                        Element n2nElement = (Element) nodes.item(0);
+                        if (n2nElement == null || n2nElement.getFirstChild() == null) continue;
+                        String text = trim(n2nElement.getFirstChild().getNodeValue());
+                        if (text.equals(n2nText)) {
+                            nextElementFound = true;
+                            break;
                         }
-                        if (!nextElementFound)
-                            return null;
-                    } else {
-                        nextElement = (Element)nextElementList.item(0);
                     }
-                } catch (NullPointerException e) {
-                    logger.warning("NullPointerException in findValue at element: " + nextElementName);
-                    return null;
+                    if (!nextElementFound)
+                        return null;
+                } else {
+                    nextElement = (Element)nextElementList.item(0);
                 }
             }
         }
@@ -458,57 +460,58 @@ public List<Node> findNodes(List<String> list) {
             nextElementFound = false;
             String nextElementName = list.get(k);
             if (!nextElementName.contains("~")) {
-                try {
-                    NodeList nextElementList = nextElement.
-                            getElementsByTagName(nextElementName);
-                    /* If the next to next(n2n) element is a filter based on
-                     * text value,
-                     * then do the required filtering.
-                     * For example,
-                     * ....
-                     *     <physical-interface>
-                     *         <name>ge-1/0/0</name>
-                     *         <logical-interface>
-                     *             ....
-                     * In this case, the list passed to findValue function
-                     * should contain (..,"physical-interface","name~ge-1/0/0",
-                     * "logical-interface",..)
-                     * This will fetch me the required element.
+                NodeList nextElementList =
+                        nextElement != null
+                                ? nextElement.getElementsByTagName(nextElementName)
+                                : null;
+                if (nextElementList == null || nextElementList.getLength() == 0) {
+                    logger.fine("Element '" + nextElementName + "' not found in findNodes()");
+                    return null;
+                }
+                /* If the next to next(n2n) element is a filter based on
+                 * text value,
+                 * then do the required filtering.
+                 * For example,
+                 * ....
+                 *     <physical-interface>
+                 *         <name>ge-1/0/0</name>
+                 *         <logical-interface>
+                 *             ....
+                 * In this case, the list passed to findValue function
+                 * should contain (..,"physical-interface","name~ge-1/0/0",
+                 * "logical-interface",..)
+                 * This will fetch me the required element.
+                 */
+                String n2nString = null;
+                if (k<list.size()-1)
+                    n2nString = list.get(k+1);
+                if (n2nString != null && n2nString.contains("~")) {
+                    /* Since the n2n element is a filter based on text value
+                     * ( decided by '~')
+                     * we now traverse the entire NodeList to find the
+                     * correct nextElement
+                     * based on the text value of the filter.
                      */
-                    String n2nString = null;
-                    if (k<list.size()-1)
-                        n2nString = list.get(k+1);
-                    if (n2nString != null && n2nString.contains("~")) {
-                        /* Since the n2n element is a filter based on text value
-                         * ( decided by '~')
-                         * we now traverse the entire NodeList to find the
-                         * correct nextElement
-                         * based on the text value of the filter.
-                         */
-                        String n2nText = n2nString.substring(n2nString.
-                                indexOf("~") + 1);
-                        String n2nElementName = n2nString.substring(0,
-                                n2nString.indexOf("~"));
-                        for (int i=0; i<nextElementList.getLength(); i++) {
-                            nextElement = (Element)nextElementList.item(i);
-                            Element n2nElement = (Element)nextElement.
-                                    getElementsByTagName(n2nElementName).item(0);
-                            String text = n2nElement.getFirstChild().
-                                    getNodeValue();
-                            text = trim(text);
-                            if (text.equals(n2nText)) {
-                                nextElementFound = true;
-                                break;
-                            }
+                    String n2nText = n2nString.substring(n2nString.
+                            indexOf("~") + 1);
+                    String n2nElementName = n2nString.substring(0,
+                            n2nString.indexOf("~"));
+                    for (int i=0; i<nextElementList.getLength(); i++) {
+                        nextElement = (Element)nextElementList.item(i);
+                        Element n2nElement = (Element)nextElement.
+                                getElementsByTagName(n2nElementName).item(0);
+                        String text = n2nElement.getFirstChild().
+                                getNodeValue();
+                        text = trim(text);
+                        if (text.equals(n2nText)) {
+                            nextElementFound = true;
+                            break;
                         }
-                        if (!nextElementFound)
-                            return null;
-                    } else {
-                        nextElement = (Element)nextElementList.item(0);
                     }
-                } catch (NullPointerException e) {
-                    logger.warning("NullPointerException in findNodes at element: " + nextElementName);
-                    return null;
+                    if (!nextElementFound)
+                        return null;
+                } else {
+                    nextElement = (Element)nextElementList.item(0);
                 }
             }
         }