[java-shib-shared] branch main updated: Tighten up ElementSupport methods.

Scott Cantor cantor.2 at osu.edu
Tue Mar 7 21:15:59 UTC 2023


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-shib-shared.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-shared.git;a=commit;h=8fc873f523437a7df6938a67de01224de509e7d7

The following commit(s) were added to refs/heads/main by this push:
     new 8fc873f5 Tighten up ElementSupport methods.
8fc873f5 is described below

commit 8fc873f523437a7df6938a67de01224de509e7d7
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Mar 7 16:15:56 2023 -0500

    Tighten up ElementSupport methods.
---
 .../net/shibboleth/shared/xml/ElementSupport.java  | 96 +++++++---------------
 .../shibboleth/shared/xml/ElementSupportTest.java  | 52 ++----------
 2 files changed, 36 insertions(+), 112 deletions(-)

diff --git a/shib-support/src/main/java/net/shibboleth/shared/xml/ElementSupport.java b/shib-support/src/main/java/net/shibboleth/shared/xml/ElementSupport.java
index 335678f6..ed9b37ea 100644
--- a/shib-support/src/main/java/net/shibboleth/shared/xml/ElementSupport.java
+++ b/shib-support/src/main/java/net/shibboleth/shared/xml/ElementSupport.java
@@ -27,6 +27,8 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 
+import net.shibboleth.shared.annotation.constraint.NotLive;
+import net.shibboleth.shared.annotation.constraint.Unmodifiable;
 import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
@@ -68,12 +70,9 @@ public final class ElementSupport {
      * @param parentElement the parent Element
      * @param childElement the child Element
      */
-    public static void appendChildElement(@Nonnull final Element parentElement, @Nullable final Element childElement) {
-        if (childElement == null) {
-            return;
-        }
-
+    public static void appendChildElement(@Nonnull final Element parentElement, @Nonnull final Element childElement) {
         Constraint.isNotNull(parentElement, "Parent Element may not be null");
+        Constraint.isNotNull(childElement, "Child Element may not be null");
         final Document parentDocument = parentElement.getOwnerDocument();
         if (!parentDocument.equals(childElement.getOwnerDocument())) {
             adoptElement(parentDocument, childElement);
@@ -147,11 +146,7 @@ public final class ElementSupport {
      * 
      * @return list of child elements or an empty list if the root is null.
      */
-    @Nonnull public static List<Element> getChildElements(@Nullable final Node root) {
-
-        if (root == null) {
-            return CollectionSupport.emptyList();
-        }
+    @Nonnull @Unmodifiable @NotLive public static List<Element> getChildElements(@Nonnull final Node root) {
 
         final ArrayList<Element> children = new ArrayList<>();
 
@@ -161,7 +156,7 @@ public final class ElementSupport {
             childNode = getNextSiblingElement(childNode);
         }
 
-        return children;
+        return CollectionSupport.copyToList(children);
     }
 
     /**
@@ -173,11 +168,8 @@ public final class ElementSupport {
      * 
      * @return list of child elements, never null
      */
-    @Nonnull public static List<Element> getChildElements(@Nullable final Node root, @Nullable final QName name) {
-        if (root == null || name == null) {
-            return CollectionSupport.emptyList();
-        }
-
+    @Nonnull @Unmodifiable @NotLive public static List<Element> getChildElements(@Nonnull final Node root,
+            @Nonnull final QName name) {
         return getChildElementsByTagNameNS(root, name.getNamespaceURI(), name.getLocalPart());
     }
 
@@ -190,7 +182,7 @@ public final class ElementSupport {
      * 
      * @return first child element or null
      */
-    @Nullable public static Element getFirstChildElement(@Nullable final Node root, @Nullable final QName name) {
+    @Nullable public static Element getFirstChildElement(@Nonnull final Node root, @Nonnull final QName name) {
         final List<Element> elements = ElementSupport.getChildElements(root, name);
         if (elements.size() > 0) {
             return elements.get(0);
@@ -207,11 +199,8 @@ public final class ElementSupport {
      * 
      * @return list of child elements, never null
      */
-    @Nonnull public static List<Element> getChildElementsByTagName(@Nullable final Node root,
-            @Nullable final String localName) {
-        if (root == null) {
-            return CollectionSupport.emptyList();
-        }
+    @Nonnull @Unmodifiable @NotLive public static List<Element> getChildElementsByTagName(@Nonnull final Node root,
+            @Nonnull final String localName) {
 
         final ArrayList<Element> children = new ArrayList<>();
 
@@ -223,7 +212,7 @@ public final class ElementSupport {
             childNode = getNextSiblingElement(childNode);
         }
 
-        return children;
+        return CollectionSupport.copyToList(children);
     }
 
     /**
@@ -236,11 +225,8 @@ public final class ElementSupport {
      * 
      * @return list of child elements, never null
      */
-    @Nonnull public static List<Element> getChildElementsByTagNameNS(@Nullable final Node root,
-            @Nullable final String namespaceURI, @Nullable final String localName) {
-        if (root == null || localName == null) {
-            return CollectionSupport.emptyList();
-        }
+    @Nonnull @Unmodifiable @NotLive  public static List<Element> getChildElementsByTagNameNS(@Nonnull final Node root,
+            @Nullable final String namespaceURI, @Nonnull final String localName) {
 
         final ArrayList<Element> children = new ArrayList<>();
 
@@ -252,7 +238,7 @@ public final class ElementSupport {
             childNode = getNextSiblingElement(childNode);
         }
 
-        return children;
+        return CollectionSupport.copyToList(children);
     }
 
     /**
@@ -262,11 +248,7 @@ public final class ElementSupport {
      * 
      * @return the ancestral element node of the current node, or null
      */
-    @Nullable public static Element getElementAncestor(@Nullable final Node currentNode) {
-        if (currentNode == null) {
-            return null;
-        }
-
+    @Nullable public static Element getElementAncestor(@Nonnull final Node currentNode) {
         final Node parent = currentNode.getParentNode();
         if (parent != null) {
             if (parent.getNodeType() == Node.ELEMENT_NODE) {
@@ -282,13 +264,10 @@ public final class ElementSupport {
      * element and all children, this just grabs the text for this element (which may be spread over multiple lines).
      * 
      * @param element The element to look at.
-     * @return The text content, or "" if there is none, never null.
+     * @return The text content, or "" if there is none, never null
      * 
      */
-    @Nonnull public static String getElementContentAsString(@Nullable final Element element) {
-        if (element == null) {
-            return "";
-        }
+    @Nonnull public static String getElementContentAsString(@Nonnull final Element element) {
         final StringBuilder builder = new StringBuilder();
 
         Node node = element.getFirstChild();
@@ -309,10 +288,7 @@ public final class ElementSupport {
      * 
      * @return list of values, never null
      */
-    @Nonnull public static List<String> getElementContentAsList(@Nullable final Element element) {
-        if (element == null) {
-            return CollectionSupport.emptyList();
-        }
+    @Nonnull public static List<String> getElementContentAsList(@Nonnull final Element element) {
         return StringSupport.stringToList(getElementContentAsString(element), XMLConstants.LIST_DELIMITERS);
     }
 
@@ -323,10 +299,7 @@ public final class ElementSupport {
      * 
      * @return a QName from an element's value, or null if the given element is empty
      */
-    @Nullable public static QName getElementContentAsQName(@Nullable final Element element) {
-        if (element == null) {
-            return null;
-        }
+    @Nullable public static QName getElementContentAsQName(@Nonnull final Element element) {
         final String elementContent = StringSupport.trimOrNull(getElementContentAsString(element));
 
         if (elementContent == null) {
@@ -351,11 +324,7 @@ public final class ElementSupport {
      * @param n The parent in which to search for children
      * @return The first child Element of n, or null if none
      */
-    @Nullable public static Element getFirstChildElement(@Nullable final Node n) {
-        if (n == null) {
-            return null;
-        }
-
+    @Nullable public static Element getFirstChildElement(@Nonnull final Node n) {
         Node child = n.getFirstChild();
         while (child != null && child.getNodeType() != Node.ELEMENT_NODE) {
             child = child.getNextSibling();
@@ -371,10 +340,8 @@ public final class ElementSupport {
      * 
      * @return child elements indexed by namespace qualified tag name, never null
      */
-    @Nonnull public static Map<QName, List<Element>> getIndexedChildElements(@Nullable final Element root) {
-        if (root == null) {
-            return CollectionSupport.emptyMap();
-        }
+    @Nonnull @Unmodifiable @NotLive public static Map<QName, List<Element>> getIndexedChildElements(
+            @Nonnull final Element root) {
 
         final Map<QName, List<Element>> children = new HashMap<>();
 
@@ -392,7 +359,7 @@ public final class ElementSupport {
             e = getNextSiblingElement(e);
         }
 
-        return children;
+        return CollectionSupport.copyToMap(children);
     }
 
     /**
@@ -401,10 +368,7 @@ public final class ElementSupport {
      * @param n The sibling to start with
      * @return The next sibling Element of n, or null if none
      */
-    @Nullable public static Element getNextSiblingElement(@Nullable final Node n) {
-        if (n == null) {
-            return null;
-        }
+    @Nullable public static Element getNextSiblingElement(@Nonnull final Node n) {
 
         Node sib = n.getNextSibling();
         while (sib != null && sib.getNodeType() != Node.ELEMENT_NODE) {
@@ -422,11 +386,7 @@ public final class ElementSupport {
      * 
      * @return true if the element has the given name, false otherwise
      */
-    public static boolean isElementNamed(@Nullable final Element e, @Nullable final QName name) {
-        if (name == null) {
-            return false;
-        }
-
+    public static boolean isElementNamed(@Nonnull final Element e, @Nonnull final QName name) {
         return isElementNamed(e, name.getNamespaceURI(), name.getLocalPart());
     }
 
@@ -438,8 +398,8 @@ public final class ElementSupport {
      * @param localName A local name to compare
      * @return true iff the element's local name and namespace match the parameters
      */
-    public static boolean isElementNamed(@Nullable final Element e, @Nullable final String ns,
-            @Nullable final String localName) {
+    public static boolean isElementNamed(@Nonnull final Element e, @Nullable final String ns,
+            @Nonnull final String localName) {
         return e != null && Objects.equals(ns, e.getNamespaceURI()) && Objects.equals(localName, e.getLocalName());
     }
 
diff --git a/shib-support/src/test/java/net/shibboleth/shared/xml/ElementSupportTest.java b/shib-support/src/test/java/net/shibboleth/shared/xml/ElementSupportTest.java
index bd9f1038..6bc078ff 100644
--- a/shib-support/src/test/java/net/shibboleth/shared/xml/ElementSupportTest.java
+++ b/shib-support/src/test/java/net/shibboleth/shared/xml/ElementSupportTest.java
@@ -85,15 +85,8 @@ public class ElementSupportTest {
     }
 
     @Test public void testIsElementNamed() {
-        Assert.assertFalse(ElementSupport.isElementNamed(null, TEST_NS, ROOT_ELEMENT),
-                "not find if provided element is null");
         Assert.assertFalse(ElementSupport.isElementNamed(rootElement, null, ROOT_ELEMENT),
                 "not find if provided namespace is null");
-        Assert.assertFalse(ElementSupport.isElementNamed(rootElement, TEST_NS, null),
-                "not find if provided attribute name is null");
-        Assert.assertFalse(ElementSupport.isElementNamed(rootElement, null),
-                "not find if provided attribute QName is null");
-
         Assert.assertTrue(ElementSupport.isElementNamed(rootElement, new QName(TEST_NS, ROOT_ELEMENT, TEST_PREFIX)),
                 "lookup against QNAME");
         Assert.assertTrue(ElementSupport.isElementNamed(rootElement, TEST_NS, ROOT_ELEMENT), "lookup against name");
@@ -106,10 +99,6 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testIsElementNamed"}) public void testGetChildElements() {
-        Assert.assertTrue(ElementSupport.getChildElements(null).isEmpty(), "Null should provide empty list");
-        Assert.assertTrue(ElementSupport.getChildElements(rootElement, null).isEmpty(),
-                "Null QName should provide empty list");
-
         Assert.assertEquals(ElementSupport.getChildElements(rootElement).size(), 8, "unnanmed element lookup");
         List<Element> list = ElementSupport.getChildElements(rootElement, TEST_ELEMENT_QNAME);
         Assert.assertEquals(list.size(), 3, "Named element lookup");
@@ -119,16 +108,8 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testIsElementNamed"}) public void testGetChildElementsByTagName() {
-        Assert.assertTrue(ElementSupport.getChildElementsByTagName(rootElement, null).isEmpty(),
-                "getChildElementsByTagName: Null name should provide empty list");
-        Assert.assertTrue(ElementSupport.getChildElementsByTagName(null, TEST_ELEMENT_NAME).isEmpty(),
-                "getChildElementsByTagName: Null root  should provide empty list");
-        Assert.assertTrue(ElementSupport.getChildElementsByTagNameNS(rootElement, TEST_NS, null).isEmpty(),
-                "getChildElementsByTagName: Null name should provide empty list");
         Assert.assertTrue(ElementSupport.getChildElementsByTagNameNS(rootElement, null, TEST_ELEMENT_NAME).isEmpty(),
                 "getChildElementsByTagNameNS: Null name space should provide empty list");
-        Assert.assertTrue(ElementSupport.getChildElementsByTagNameNS(null, TEST_NS, TEST_ELEMENT_NAME).isEmpty(),
-                "getChildElementsByTagNameNS: Null root  should provide empty list");
 
         List<Element> list = ElementSupport.getChildElementsByTagName(rootElement, TEST_ELEMENT_NAME);
         Assert.assertEquals(list.size(), 5, "getChildElementsByTagName size");
@@ -149,8 +130,6 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testGetChildElementsByTagName"}) public void testGetElementAncestor() {
-        Assert.assertNull(ElementSupport.getElementAncestor(null),
-                "getElementAncestor: Null element should provide null result");
         Assert.assertNull(ElementSupport.getElementAncestor(rootElement),
                 "getElementAncestor: root node should provide null result");
 
@@ -163,9 +142,6 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testGetChildElementsByTagName"}) public void testGetElementContentAsString() {
-        Assert.assertTrue(ElementSupport.getElementContentAsString(null).isEmpty(),
-                "getElementContentAsList: Null element should provide empty result");
-
         final String empty = StringSupport.trim(ElementSupport.getElementContentAsString(rootElement));
         assert empty != null;
         Assert.assertTrue(empty.isEmpty(),
@@ -179,9 +155,6 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testGetChildElementsByTagName"}) public void testGetElementContentAsList() {
-        Assert.assertTrue(ElementSupport.getElementContentAsList(null).isEmpty(),
-                "getElementContentAsList: Null element should provide empty result");
-
         Assert.assertTrue(ElementSupport.getElementContentAsList(rootElement).isEmpty(),
                 "getElementContentAsList: Empty element should provide empty result");
 
@@ -195,8 +168,6 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testGetChildElementsByTagName"}) public void testGetElementContentAsQName() {
-        Assert.assertNull(ElementSupport.getElementContentAsQName(null),
-                "getElementContentAsQName: Null element should provide empty result");
         Assert.assertNull(ElementSupport.getElementContentAsQName(rootElement),
                 "getElementContentAsQName: Empty element should provide empty result");
 
@@ -209,41 +180,43 @@ public class ElementSupportTest {
     }
 
     @Test(dependsOnMethods = {"testIsElementNamed"}) public void testGetChildAndNext() {
-        Assert.assertNull(ElementSupport.getFirstChildElement(null),
-                "getFirstChildElement: Null element should provide null result");
-
-        Assert.assertNull(ElementSupport.getNextSiblingElement(null),
-                "getNextSiblingElement: Null element should provide null result");
-
         Element element = ElementSupport.getFirstChildElement(rootElement);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, TEST_ELEMENT_QNAME), "getFirstChildElement");
         Assert.assertNull(ElementSupport.getFirstChildElement(element),
                 "getFirstChildElement: Empty element should provide null result");
 
         element = ElementSupport.getNextSiblingElement(element);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, TEST_ELEMENT_QNAME), "getNextSiblingElement 1");
 
         element = ElementSupport.getNextSiblingElement(element);
         QName qName = new QName(TEST_NS, "Element2", TEST_PREFIX);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 2");
 
         element = ElementSupport.getNextSiblingElement(element);
         qName = new QName(TEST_NS, TEST_ELEMENT_NAME, "mynsagain");
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 3");
 
         element = ElementSupport.getNextSiblingElement(element);
         qName = new QName(OTHER_NS, TEST_ELEMENT_NAME, TEST_PREFIX);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 4 ");
 
         element = ElementSupport.getNextSiblingElement(element);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 5");
 
         element = ElementSupport.getNextSiblingElement(element);
         qName = new QName(OTHER_NS, "Element2", TEST_PREFIX);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 6");
 
         element = ElementSupport.getNextSiblingElement(element);
         qName = new QName(TEST_NS, "Element4", TEST_PREFIX);
+        assert element != null;
         Assert.assertTrue(ElementSupport.isElementNamed(element, qName), "getNextSiblingElement 7");
 
         Assert.assertNull(ElementSupport.getNextSiblingElement(element),
@@ -251,31 +224,22 @@ public class ElementSupportTest {
     }
 
     @Test public void testGetIndexedChildElements() {
-        Assert.assertTrue(ElementSupport.getIndexedChildElements(null).isEmpty(),
-                "getIndexedChildElements with null input");
         Map<QName, List<Element>> map = ElementSupport.getIndexedChildElements(rootElement);
 
         Assert.assertEquals(map.get(TEST_ELEMENT_QNAME).size(), 3, "getIndexedChildElements for " + TEST_ELEMENT_QNAME);
-        map.remove(TEST_ELEMENT_QNAME);
 
         QName qname = new QName(TEST_NS, "Element2", TEST_PREFIX);
         Assert.assertEquals(map.get(qname).size(), 1, "getIndexedChildElements for " + qname);
-        map.remove(qname);
 
         qname = new QName(OTHER_NS, "Element1", "otherns");
         Assert.assertEquals(map.get(qname).size(), 2, "getIndexedChildElements for " + qname);
-        map.remove(qname);
 
         qname = new QName(OTHER_NS, "Element2", "otherns");
         Assert.assertEquals(map.get(qname).size(), 1, "getIndexedChildElements for " + qname);
         Element elementNoKids = map.get(qname).get(0);
-        map.remove(qname);
 
         qname = new QName(TEST_NS, "Element4", TEST_PREFIX);
         Assert.assertEquals(map.get(qname).size(), 1, "getIndexedChildElements for " + qname);
-        map.remove(qname);
-
-        Assert.assertTrue(map.isEmpty(), "getIndexedChildElements found extra elements");
 
         Assert.assertTrue(ElementSupport.getIndexedChildElements(elementNoKids).isEmpty(),
                 "getIndexedChildElements with a no child element");

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list