Fix regression from #4008, optimize ObjectNode.findValue(s) and findParent(s) (#4230)

Author: JooHyukKim

release-notes/VERSION-2.x

@@ -13,6 +13,9 @@ Project: jackson-databind
 #4216: Primitive array deserializer cannot being captured by `DeserializerModifier`
  (reported by @SakuraKoi)
  (fix contributed by Joo-Hyuk K)
+#4229 JsonNode findValues and findParents missing expected values in 2.16.0
+ (reported by @gcookemoto)
+ (fix contributed by Joo-Hyuk K)
 
 2.16.0 (15-Nov-2023)
 

src/main/java/com/fasterxml/jackson/databind/node/ObjectNode.java

@@ -382,37 +382,32 @@ public JsonNode findValue(String propertyName)
     @Override
     public List<JsonNode> findValues(String propertyName, List<JsonNode> foundSoFar)
     {
-        JsonNode jsonNode = _children.get(propertyName);
-        if (jsonNode != null) {
-            if (foundSoFar == null) {
-                foundSoFar = new ArrayList<>();
+        for (Map.Entry<String, JsonNode> entry : _children.entrySet()) {
+            if (propertyName.equals(entry.getKey())) {
+                if (foundSoFar == null) {
+                    foundSoFar = new ArrayList<JsonNode>();
+                }
+                foundSoFar.add(entry.getValue());
+            } else { // only add children if parent not added
+                foundSoFar = entry.getValue().findValues(propertyName, foundSoFar);
             }
-            foundSoFar.add(jsonNode);
-            return foundSoFar;
-        }
-
-        // only add children if parent not added
-        for (JsonNode child : _children.values()) {
-            foundSoFar = child.findValues(propertyName, foundSoFar);
         }
         return foundSoFar;
     }
 
     @Override
     public List<String> findValuesAsText(String propertyName, List<String> foundSoFar)
     {
-        JsonNode jsonNode = _children.get(propertyName);
-        if (jsonNode != null) {
-            if (foundSoFar == null) {
-                foundSoFar = new ArrayList<>();
+        for (Map.Entry<String, JsonNode> entry : _children.entrySet()) {
+            if (propertyName.equals(entry.getKey())) {
+                if (foundSoFar == null) {
+                    foundSoFar = new ArrayList<String>();
+                }
+                foundSoFar.add(entry.getValue().asText());
+            } else { // only add children if parent not added
+                foundSoFar = entry.getValue().findValuesAsText(propertyName,
+                    foundSoFar);
             }
-            foundSoFar.add(jsonNode.asText());
-            return foundSoFar;
-        }
-
-        // only add children if parent not added
-        for (JsonNode child : _children.values()) {
-            foundSoFar = child.findValuesAsText(propertyName, foundSoFar);
         }
         return foundSoFar;
     }
@@ -436,18 +431,16 @@ public ObjectNode findParent(String propertyName)
     @Override
     public List<JsonNode> findParents(String propertyName, List<JsonNode> foundSoFar)
     {
-        JsonNode jsonNode = _children.get(propertyName);
-        if (jsonNode != null) {
-            if (foundSoFar == null) {
-                foundSoFar = new ArrayList<>();
+        for (Map.Entry<String, JsonNode> entry : _children.entrySet()) {
+            if (propertyName.equals(entry.getKey())) {
+                if (foundSoFar == null) {
+                    foundSoFar = new ArrayList<JsonNode>();
+                }
+                foundSoFar.add(this);
+            } else { // only add children if parent not added
+                foundSoFar = entry.getValue()
+                    .findParents(propertyName, foundSoFar);
             }
-            foundSoFar.add(this);
-            return foundSoFar;
-        }
-
-        // only add children if parent not added
-        for (JsonNode child : _children.values()) {
-            foundSoFar = child.findParents(propertyName, foundSoFar);
         }
         return foundSoFar;
     }

src/test/java/com/fasterxml/jackson/databind/node/MissingValues4229Test.java

@@ -0,0 +1,64 @@
+package com.fasterxml.jackson.databind.node;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import static com.fasterxml.jackson.databind.BaseMapTest.jsonMapperBuilder;
+import static com.fasterxml.jackson.databind.BaseTest.a2q;
+
+// [databind#4229] JsonNode findValues and findParents missing expected values
+public class MissingValues4229Test
+{
+
+    private final String JSON = a2q("{"
+            + "    'target': 'target1'," // Found in <= 2.15.3 and 2.16.0
+            + "    'object1': {"
+            + "        'target': 'target2' " // Found in <= 2.15.3, but not in 2.16.0
+            + "    },"
+            + "    'object2': {"
+            + "        'target': { " // Found in <= 2.15.3, but not in 2.16.0
+            + "            'target': 'ignoredAsParentIsTarget'" // Expect not to be found (as sub-tree search ends when parent is found)
+            + "        }"
+            + "    }"
+            + "}");
+
+    private final ObjectMapper objectMapper = jsonMapperBuilder()
+            .configure(JsonParser.Feature.ALLOW_COMMENTS, true)
+            .build();
+
+    @Test
+    public void testFindValues() throws Exception
+    {
+        JsonNode rootNode = objectMapper.readTree(JSON);
+
+        List<JsonNode> expectedNodes = new ArrayList<>();
+        expectedNodes.add(rootNode.at("/target"));
+        expectedNodes.add(rootNode.at("/object1/target"));
+        expectedNodes.add(rootNode.at("/object2/target"));
+
+        List<JsonNode> actualNodes = rootNode.findValues("target");
+
+        Assertions.assertEquals(expectedNodes, actualNodes);
+    }
+
+    @Test
+    public void testFindParents() throws Exception
+    {
+        JsonNode rootNode = objectMapper.readTree(JSON);
+
+        List<JsonNode> expectedNodes = new ArrayList<>();
+        expectedNodes.add(rootNode.at(""));
+        expectedNodes.add(rootNode.at("/object1"));
+        expectedNodes.add(rootNode.at("/object2"));
+
+        List<JsonNode> foundNodes = rootNode.findParents("target");
+
+        Assertions.assertEquals(expectedNodes, foundNodes);
+    }
+}