Fix #4096: change JsonNode.withObject(String) to accept non-expression as property name (#4132)

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -73,6 +73,8 @@ Project: jackson-databind
 #4090: Support sequenced collections (JDK 21)S
  (contributed by @pjfanning)
 #4095: Add `withObjectProperty(String)`, `withArrayProperty(String)` in `JsonNode`
+#4096: Change `JsonNode.withObject(String)` to work similar to `withArray()`
+  wrt argument
 #4122: Do not resolve wildcards if upper bound is too non-specific
  (contributed by @yawkat)
 

src/main/java/com/fasterxml/jackson/databind/JsonNode.java

@@ -1139,20 +1139,31 @@ public final List<JsonNode> findParents(String fieldName)
      */
 
     /**
-     * Short-cut equivalent to:
+     * Method that works in one of possible ways, depending on whether
+     * {@code exprOrProperty} is a valid {@link JsonPointer} expression or
+     * not (valid expression is either empty String {@code ""} or starts
+     * with leading slash {@code /} character).
+     * If it is, works as a short-cut to:
      *<pre>
-     *   withObject(JsonPointer.compile(expr);
+     *  withObject(JsonPointer.compile(exprOrProperty));
+     *</pre>
+     * If it is NOT a valid {@link JsonPointer} expression, value is taken
+     * as a literal Object property name and calls is alias for
+     *<pre>
+     *  withObjectProperty(exprOrProperty);
      *</pre>
-     * see {@link #withObject(JsonPointer)} for full explanation.
      *
-     * @param expr {@link JsonPointer} expression to use
+     * @param exprOrProperty {@link JsonPointer} expression to use (if valid as one),
+     *    or, if not (no leading "/"), property name to match.
      *
      * @return {@link ObjectNode} found or created
      *
-     * @since 2.14
+     * @since 2.14 (but semantics before 2.16 did NOT allow for non-JsonPointer expressions)
      */
-    public final ObjectNode withObject(String expr) {
-        return withObject(JsonPointer.compile(expr));
+    public ObjectNode withObject(String exprOrProperty) {
+        // To avoid abstract method, base implementation just fails
+        throw new UnsupportedOperationException("`withObject(String)` not implemented by `"
+                +getClass().getName()+"`");
     }
 
     /**

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

@@ -89,6 +89,15 @@ public ObjectNode with(String exprOrProperty) {
         return result;
     }
 
+    @Override
+    public ObjectNode withObject(String exprOrProperty) {
+        JsonPointer ptr = _jsonPointerIfValid(exprOrProperty);
+        if (ptr != null) {
+            return withObject(ptr);
+        }
+        return withObjectProperty(exprOrProperty);
+    }
+
     @Override
     public ObjectNode withObjectProperty(String propName) {
         JsonNode child = _children.get(propName);

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

@@ -318,7 +318,7 @@ public void testInvalidWithObject() throws Exception
             root.withObject("/prop");
             fail("Expected exception");
         } catch (UnsupportedOperationException e) {
-            verifyException(e, "Cannot replace context node (of type");
+            verifyException(e, "`withObject(String)` not implemented");
             verifyException(e, "ArrayNode");
         }
         // also: should fail of we already have non-object property

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

@@ -198,10 +198,11 @@ private void _verifyObjectReplaceFail(JsonNode doc, JsonPointer ptr, OverwriteMo
 
     /*
     /**********************************************************************
-    /* Test methods, withObjectProperty()
+    /* Test methods, withObjectProperty()/withObject(exprOrProperty)
     /**********************************************************************
      */
 
+    // [databind#4095]
     public void testWithObjectProperty() throws Exception
     {
         ObjectNode root = MAPPER.createObjectNode();
@@ -226,7 +227,7 @@ public void testWithObjectProperty() throws Exception
         ObjectNode match3 = root2.withObjectProperty("b");
         assertNotSame(match, match3);
         assertEquals("{\"b\":{}}", root2.toString());
-        
+
         // and then failing case
         JsonNode root3 = MAPPER.readTree("{\"c\": 123}");
         try {
@@ -237,6 +238,46 @@ public void testWithObjectProperty() throws Exception
         }
     }
 
+    // [databind#4096]
+    public void testWithObjectAdnExprOrProp() throws Exception
+    {
+        ObjectNode root = MAPPER.createObjectNode();
+
+        // First: create new property value
+        ObjectNode match = root.withObject("a");
+        assertTrue(match.isObject());
+        assertEquals(a2q("{}"), match.toString());
+        match.put("value", 42);
+        assertEquals(a2q("{'a':{'value':42}}"), root.toString());
+
+        // and then with JsonPointer expr
+        match = root.withObject("/a/b");
+        assertTrue(match.isObject());
+        assertEquals(a2q("{}"), match.toString());
+        assertEquals(a2q("{'a':{'value':42,'b':{}}}"), root.toString());
+
+        // Then existing prop:
+        assertEquals(a2q("{'value':42,'b':{}}"),
+                root.withObject("a").toString());
+        assertEquals(a2q("{}"),
+                root.withObject("/a/b").toString());
+
+        // and then failing case
+        JsonNode root3 = MAPPER.readTree("{\"c\": 123}");
+        try {
+            root3.withObject("c");
+            fail("Should not pass");
+        } catch (UnsupportedOperationException e) {
+            verifyException(e, "Cannot replace `JsonNode` of type ");
+        }
+        try {
+            root3.withObject("/c");
+            fail("Should not pass");
+        } catch (UnsupportedOperationException e) {
+            verifyException(e, "Cannot replace `JsonNode` of type ");
+        }
+    }
+
     /*
     /**********************************************************************
     /* Test methods, withArray()
@@ -359,10 +400,11 @@ public void testWithArray3882() throws Exception
 
     /*
     /**********************************************************************
-    /* Test methods, withArrayProperty()
+    /* Test methods, withArrayProperty()/withArray(exprOrProperty)
     /**********************************************************************
      */
 
+    // [databind#4095]
     public void testWithArrayProperty() throws Exception
     {
         ObjectNode root = MAPPER.createObjectNode();
@@ -396,4 +438,33 @@ public void testWithArrayProperty() throws Exception
             verifyException(e, "Cannot replace `JsonNode` of type ");
         }
     }
+
+    // [databind#4096]
+    public void testWithArrayAndExprOrProp() throws Exception
+    {
+        ObjectNode root = MAPPER.createObjectNode();
+
+        // First: create new property value
+        ArrayNode match = root.withArray("a");
+        assertTrue(match.isArray());
+        assertEquals(a2q("[]"), match.toString());
+        match.add(42);
+        assertEquals(a2q("{'a':[42]}"), root.toString());
+
+        match = root.withArray("/b");
+        assertEquals(a2q("{'a':[42],'b':[]}"), root.toString());
+
+        // Second: match existing Object property
+        assertEquals(a2q("[42]"), root.withArray("a").toString());
+        assertEquals(a2q("[42]"), root.withArray("/a").toString());
+
+        // and then failing case
+        JsonNode root3 = MAPPER.readTree("{\"c\": 123}");
+        try {
+            root3.withArrayProperty("c");
+            fail("Should not pass");
+        } catch (UnsupportedOperationException e) {
+            verifyException(e, "Cannot replace `JsonNode` of type ");
+        }
+    }
 }