Clean up XmlElement namespace handing

Use foo()/findFoo()/getFoo() to simplify naming of methods, cutting down
on ceremony around namespace/namespaceAttribute accesses. This allows us
to forgo Optional wrapping and reduce code duplication.

Change-Id: Ib3a9393b78c406dcbee85d3d91f338295fac0d11
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/customrpc/RpcMapping.java b/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/customrpc/RpcMapping.java
index c62107f4925f56bd36ba31e171bcbc8f427b860c..c4161b7a0f9756e7f05213a216bd014d9012403d 100644 (file)
--- a/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/customrpc/RpcMapping.java
+++ b/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/customrpc/RpcMapping.java
@@ -15,6 +15,7 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Stream;
 import javax.xml.bind.JAXB;
@@ -77,7 +78,7 @@ class RpcMapping {
         private final int hashCode;
 
         Request(final XmlElement element) {
-            this.xmlElement = element;
+            xmlElement = element;
             hashCode = XmlUtil.toString(element)
                     .replaceAll("message-id=.*(>| )", "") //message id is variable, remove it
                     .replaceAll("\\s+", "") //remove whitespaces
@@ -93,7 +94,7 @@ class RpcMapping {
                 return false;
             }
             final Request request = (Request) obj;
-            return documentEquals(this.xmlElement, request.xmlElement);
+            return documentEquals(xmlElement, request.xmlElement);
         }
 
         @Override
@@ -102,7 +103,7 @@ class RpcMapping {
         }
 
         private static boolean documentEquals(final XmlElement e1, final XmlElement e2) {
-            if (!e1.getNamespaceOptionally().equals(e2.getNamespaceOptionally())) {
+            if (!Objects.equals(e1.namespace(), e2.namespace())) {
                 return false;
             }
             if (!e1.getName().equals(e2.getName())) {

diff --git a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NetconfHelloMessage.java b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NetconfHelloMessage.java
index ddacf11f5e0435ed732063ed9856813469f88a90..3fa7b69c0f26a92a618ea23a5e8ca3c6c9476fd2 100644 (file)
--- a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NetconfHelloMessage.java
+++ b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NetconfHelloMessage.java
@@ -95,9 +95,8 @@ public final class NetconfHelloMessage extends NetconfMessage {
             return false;
         }
 
-        final Optional<String> optNamespace = element.getNamespaceOptionally();
+        final var namespace = element.namespace();
         // accept even if hello has no namespace
-        return optNamespace.isEmpty()
-                || XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0.equals(optNamespace.orElseThrow());
+        return namespace == null || XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0.equals(namespace);
     }
 }

diff --git a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/xml/XmlElement.java b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/xml/XmlElement.java
index d94f92295fc37c0662a3f7f50c572f7ac82e149a..7151e2ec73a561daa189a2862d28e54b31c7e46b 100644 (file)
--- a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/xml/XmlElement.java
+++ b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/xml/XmlElement.java
@@ -20,12 +20,11 @@ import java.util.Optional;
 import java.util.function.Predicate;
 import javax.xml.XMLConstants;
 import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.netconf.api.DocumentedException;
 import org.opendaylight.yangtools.yang.common.ErrorSeverity;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.w3c.dom.Attr;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -36,9 +35,7 @@ import org.w3c.dom.Text;
 import org.xml.sax.SAXException;
 
 public final class XmlElement {
-    public static final String DEFAULT_NAMESPACE_PREFIX = "";
-
-    private static final Logger LOG = LoggerFactory.getLogger(XmlElement.class);
+    public static final @NonNull String DEFAULT_NAMESPACE_PREFIX = "";
 
     private final Element element;
 
@@ -99,9 +96,9 @@ public final class XmlElement {
 
         // namespace does not have to be defined on this element but inherited
         if (!namespaces.containsKey(DEFAULT_NAMESPACE_PREFIX)) {
-            Optional<String> namespaceOptionally = getNamespaceOptionally();
-            if (namespaceOptionally.isPresent()) {
-                namespaces.put(DEFAULT_NAMESPACE_PREFIX, namespaceOptionally.orElseThrow());
+            final var namespace = namespace();
+            if (namespace != null) {
+                namespaces.put(DEFAULT_NAMESPACE_PREFIX, namespace);
             }
         }
 
@@ -135,11 +132,8 @@ public final class XmlElement {
     }
 
     public String getName() {
-        final String localName = element.getLocalName();
-        if (!Strings.isNullOrEmpty(localName)) {
-            return localName;
-        }
-        return element.getTagName();
+        final var localName = element.getLocalName();
+        return Strings.isNullOrEmpty(localName) ? element.getTagName() : localName;
     }
 
     public String getAttribute(final String attributeName) {
@@ -208,11 +202,8 @@ public final class XmlElement {
 
     public List<XmlElement> getChildElementsWithinNamespace(final String namespace) {
         return getChildElementsInternal(e -> {
-            try {
-                return XmlElement.fromDomElement(e).getNamespace().equals(namespace);
-            } catch (final MissingNameSpaceException e1) {
-                return false;
-            }
+            final var elementNamespace = namespace(e);
+            return elementNamespace != null && elementNamespace.equals(namespace);
         });
     }
 
@@ -253,28 +244,28 @@ public final class XmlElement {
     }
 
     public Optional<XmlElement> getOnlyChildElementWithSameNamespaceOptionally(final String childName) {
-        Optional<String> namespace = getNamespaceOptionally();
-        if (namespace.isPresent()) {
-            List<XmlElement> children = getChildElementsWithinNamespace(namespace.orElseThrow());
-            children = Lists.newArrayList(Collections2.filter(children,
-                xmlElement -> xmlElement.getName().equals(childName)));
-            if (children.size() != 1) {
-                return Optional.empty();
-            }
-            return Optional.of(children.get(0));
+        final var namespace = namespace();
+        if (namespace == null) {
+            return Optional.empty();
         }
-        return Optional.empty();
+
+        List<XmlElement> children = getChildElementsWithinNamespace(namespace);
+        children = Lists.newArrayList(Collections2.filter(children,
+            xmlElement -> xmlElement.getName().equals(childName)));
+        if (children.size() != 1) {
+            return Optional.empty();
+        }
+        return Optional.of(children.get(0));
     }
 
+    // FIXME: if we do not have a namespace this method always returns Optional.empty(). Why?!
     public Optional<XmlElement> getOnlyChildElementWithSameNamespaceOptionally() {
-        Optional<XmlElement> child = getOnlyChildElementOptionally();
-        // FIXME: a beautiful example of how NOT to use Optional
-        if (child.isPresent()
-            && child.orElseThrow().getNamespaceOptionally().isPresent()
-            && getNamespaceOptionally().isPresent()
-            && getNamespaceOptionally().orElseThrow().equals(
-                child.orElseThrow().getNamespaceOptionally().orElseThrow())) {
-            return child;
+        final var optChild = getOnlyChildElementOptionally();
+        if (optChild.isPresent()) {
+            final var namespace = namespace();
+            if (namespace != null && namespace.equals(optChild.orElseThrow().namespace())) {
+                return optChild;
+            }
         }
         return Optional.empty();
     }
@@ -335,51 +326,54 @@ public final class XmlElement {
         return Optional.empty();
     }
 
-    public String getNamespaceAttribute() throws MissingNameSpaceException {
-        String attribute = element.getAttribute(XMLConstants.XMLNS_ATTRIBUTE);
-        if (attribute.isEmpty() || attribute.equals(DEFAULT_NAMESPACE_PREFIX)) {
-            throw new MissingNameSpaceException(String.format("Element %s must specify namespace", toString()),
-                    ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, ErrorSeverity.ERROR);
+    public @Nullable String namespaceAttribute() {
+        final var attribute = element.getAttribute(XMLConstants.XMLNS_ATTRIBUTE);
+        return attribute.isEmpty() ? null : attribute;
+    }
+
+    public Optional<String> findNamespaceAttribute() {
+        return Optional.ofNullable(namespaceAttribute());
+    }
+
+    public @NonNull String getNamespaceAttribute() throws MissingNameSpaceException {
+        final var attribute = namespaceAttribute();
+        if (attribute == null) {
+            throw new MissingNameSpaceException("Element " + this + " must specify namespace",
+                ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, ErrorSeverity.ERROR);
         }
         return attribute;
     }
 
+    @Deprecated(since = "5.0.6", forRemoval = true)
     public Optional<String> getNamespaceAttributeOptionally() {
-        String attribute = element.getAttribute(XMLConstants.XMLNS_ATTRIBUTE);
-        if (attribute.isEmpty() || attribute.equals(DEFAULT_NAMESPACE_PREFIX)) {
-            return Optional.empty();
-        }
-        return Optional.of(attribute);
+        return findNamespaceAttribute();
     }
 
-    public Optional<String> getNamespaceOptionally() {
-        String namespaceURI = element.getNamespaceURI();
-        if (Strings.isNullOrEmpty(namespaceURI)) {
-            return Optional.empty();
-        } else {
-            return Optional.of(namespaceURI);
-        }
+    public @Nullable String namespace() {
+        return namespace(element);
     }
 
-    public String getNamespace() throws MissingNameSpaceException {
-        return getNamespaceOptionally()
-            .orElseThrow(() -> new MissingNameSpaceException("No namespace defined for " + this,
-                ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, ErrorSeverity.ERROR));
+    private static @Nullable String namespace(final Element element) {
+        final var namespaceURI = element.getNamespaceURI();
+        return namespaceURI == null || namespaceURI.isEmpty() ? null : namespaceURI;
     }
 
-    @Override
-    public String toString() {
-        final StringBuilder sb = new StringBuilder("XmlElement{");
-        sb.append("name='").append(getName()).append('\'');
-        if (element.getNamespaceURI() != null) {
-            try {
-                sb.append(", namespace='").append(getNamespace()).append('\'');
-            } catch (final MissingNameSpaceException e) {
-                LOG.trace("Missing namespace for element.");
-            }
+    public Optional<String> findNamespace() {
+        return Optional.ofNullable(namespace());
+    }
+
+    @Deprecated(since = "5.0.6", forRemoval = true)
+    public Optional<String> getNamespaceOptionally() {
+        return findNamespace();
+    }
+
+    public @NonNull String getNamespace() throws MissingNameSpaceException {
+        final var namespace = namespace();
+        if (namespace == null) {
+            throw new MissingNameSpaceException("No namespace defined for " + this,
+                ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, ErrorSeverity.ERROR);
         }
-        sb.append('}');
-        return sb.toString();
+        return namespace;
     }
 
     /**
@@ -437,6 +431,10 @@ public final class XmlElement {
         checkUnrecognisedElements(List.of(), additionalRecognisedElements);
     }
 
+    public boolean hasNamespace() {
+        return namespaceAttribute() != null || namespace() != null;
+    }
+
     @Override
     public boolean equals(final Object obj) {
         return this == obj || obj instanceof XmlElement other && element.isEqualNode(other.element);
@@ -447,7 +445,13 @@ public final class XmlElement {
         return element.hashCode();
     }
 
-    public boolean hasNamespace() {
-        return getNamespaceAttributeOptionally().isPresent() || getNamespaceOptionally().isPresent();
+    @Override
+    public String toString() {
+        final var sb = new StringBuilder("XmlElement{").append("name='").append(getName()).append('\'');
+        final var namespace = namespace();
+        if (namespace != null) {
+            sb.append(", namespace='").append(namespace).append('\'');
+        }
+        return sb.append('}').toString();
     }
 }

diff --git a/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/xml/XmlElementTest.java b/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/xml/XmlElementTest.java
index 787b9d1cf2e35e5ed4b3fea4812573702e5df213..69d15824cdebfb59bfb2f56f366d006e2436ca07 100644 (file)
--- a/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/xml/XmlElementTest.java
+++ b/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/xml/XmlElementTest.java
@@ -67,7 +67,7 @@ public class XmlElementTest {
         assertTrue(xmlElement.hasNamespace());
         assertEquals("namespace", xmlElement.getNamespace());
         assertEquals("namespace", xmlElement.getNamespaceAttribute());
-        assertEquals(Optional.of("namespace"), xmlElement.getNamespaceOptionally());
+        assertEquals(Optional.of("namespace"), xmlElement.findNamespace());
 
         assertEquals("value1", xmlElement.getAttribute("attr1", "attrNamespace"));
         assertEquals("value2", xmlElement.getAttribute("attr2"));

diff --git a/protocol/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/SubtreeFilter.java b/protocol/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/SubtreeFilter.java
index 4f6ac5839483b2e702e0c69e49e6accbe2ddeaf4..f1a6ec5bae067dd268b958915571b5eabe67f43c 100644 (file)
--- a/protocol/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/SubtreeFilter.java
+++ b/protocol/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/SubtreeFilter.java
@@ -8,6 +8,7 @@
 package org.opendaylight.netconf.util.messages;
 
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import javax.xml.XMLConstants;
 import org.opendaylight.netconf.api.DocumentedException;
@@ -179,10 +180,8 @@ public final class SubtreeFilter {
      * If filter node has no children and has text content, it also must match.
      */
     private static MatchingResult matches(final XmlElement src, final XmlElement filter) throws DocumentedException {
-        boolean tagMatch = src.getName().equals(filter.getName())
-                && src.getNamespaceOptionally().equals(filter.getNamespaceOptionally());
         MatchingResult result = null;
-        if (tagMatch) {
+        if (src.getName().equals(filter.getName()) && Objects.equals(src.namespace(), filter.namespace())) {
             // match text content
             Optional<String> maybeText = filter.getOnlyTextContentOptionally();
             if (maybeText.isPresent()) {