Fixed #3003

release-notes/VERSION-2.x

@@ -19,6 +19,7 @@ Project: jackson-databind
  (reported, fix suggested by SunYiJun)
 #2990: Breaking API change in `BasicClassIntrospector` (2.12.0)
  (reported, fix contributed by Faron D)
+#3003: `JsonNode.requiredAt()` does NOT fail on some path expressions
 
 2.12.0 (29-Nov-2020)
 

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

@@ -241,6 +241,16 @@ public final JsonNode at(String jsonPtrExpr) {
         return at(JsonPointer.compile(jsonPtrExpr));
     }
 
+    /**
+     * Helper method used by other methods for traversing the next step
+     * of given path expression, and returning matching value node if any:
+     * if no match, {@code null} is returned.
+     *
+     * @param ptr Path expression to use
+     *
+     * @return Either matching {@link JsonNode} for the first step of path or
+     *    {@code null} if no match (including case that this node is not a container)
+     */
     protected abstract JsonNode _at(JsonPointer ptr);
 
     /*
@@ -746,17 +756,19 @@ public <T extends JsonNode> T requireNonNull() throws IllegalArgumentException {
      * and can be used to check that this node is an {@code ObjectNode} (that is, represents
      * JSON Object value) and has value for specified property with key {@code fieldName}
      * (but note that value may be explicit JSON null value).
-     * If this node is Object Node and has value for specified property, {@code this} is returned
-     * to allow chaining; otherwise {@link IllegalArgumentException} is thrown.
+     * If this node is Object Node and has value for specified property, matching value
+     * is returned; otherwise {@link IllegalArgumentException} is thrown.
      *
-     * @return {@code this} node to allow chaining
+     * @param propertyName Name of property to access
+     *
+     * @return Value of the specified property of this Object node
      *
      * @throws IllegalArgumentException if this node is not an Object node or if it does not
      *   have value for specified property
      *
      * @since 2.10
      */
-    public JsonNode required(String fieldName) throws IllegalArgumentException {
+    public JsonNode required(String propertyName) throws IllegalArgumentException {
         return _reportRequiredViolation("Node of type `%s` has no fields", getClass().getName());
     }
 
@@ -768,10 +780,12 @@ public JsonNode required(String fieldName) throws IllegalArgumentException {
      * and can be used to check that this node is an {@code ArrayNode} (that is, represents
      * JSON Array value) and has value for specified {@code index}
      * (but note that value may be explicit JSON null value).
-     * If this node is Array Node and has value for specified index, {@code this} is returned
-     * to allow chaining; otherwise {@link IllegalArgumentException} is thrown.
+     * If this node is Array Node and has value for specified index, value at index
+     * is returned; otherwise {@link IllegalArgumentException} is thrown.
      *
-     * @return {@code this} node to allow chaining
+     * @param index Index of the value of this Array node to access
+     *
+     * @return Value at specified index of this Array node
      *
      * @throws IllegalArgumentException if this node is not an Array node or if it does not
      *   have value for specified index
@@ -790,10 +804,12 @@ public JsonNode required(int index) throws IllegalArgumentException {
      * and can be used to check that there is an actual value node at specified {@link JsonPointer}
      * starting from {@code this} node
      * (but note that value may be explicit JSON null value).
-     * If such value node exists {@code this} is returned
-     * to allow chaining; otherwise {@link IllegalArgumentException} is thrown.
+     * If such value node exists it is returned;
+     * otherwise {@link IllegalArgumentException} is thrown.
      *
-     * @return {@code this} node to allow chaining
+     * @param pathExpr {@link JsonPointer} expression (as String) to use for finding value node
+     *
+     * @return Matching value node for given expression
      *
      * @throws IllegalArgumentException if no value node exists at given {@code JSON Pointer} path
      *
@@ -811,10 +827,12 @@ public JsonNode requiredAt(String pathExpr) throws IllegalArgumentException {
      * and can be used to check that there is an actual value node at specified {@link JsonPointer}
      * starting from {@code this} node
      * (but note that value may be explicit JSON null value).
-     * If such value node exists {@code this} is returned
-     * to allow chaining; otherwise {@link IllegalArgumentException} is thrown.
+     * If such value node exists it is returned;
+     * otherwise {@link IllegalArgumentException} is thrown.
      *
-     * @return {@code this} node to allow chaining
+     * @param path {@link JsonPointer} expression to use for finding value node
+     *
+     * @return Matching value node for given expression
      *
      * @throws IllegalArgumentException if no value node exists at given {@code JSON Pointer} path
      *

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

@@ -23,9 +23,9 @@ protected ValueNode() { }
 
     @Override
     protected JsonNode _at(JsonPointer ptr) {
-        // will only allow direct matches, but no traversal through
-        // (base class checks for direct match)
-        return MissingNode.getInstance();
+        // 02-Jan-2020, tatu: As per [databind#3003] must return `null` and NOT
+        //    "missing node"
+        return null;
     }
 
     /**

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

@@ -8,7 +8,7 @@
 public class RequiredAccessorTest
     extends BaseMapTest
 {
-    private final ObjectMapper MAPPER = sharedMapper();
+    private final ObjectMapper MAPPER = newJsonMapper();
 
     private final JsonNode TEST_OBJECT, TEST_ARRAY;
 
@@ -24,7 +24,7 @@ public RequiredAccessorTest() throws Exception {
     }
 
     public void testIMPORTANT() {
-        _checkRequiredAt(TEST_OBJECT, "/data/weird/and/more", "/weird/and/more");
+        _checkRequiredAtFail(TEST_OBJECT, "/data/weird/and/more", "/weird/and/more");
     }
 
     public void testRequiredAtObjectOk() throws Exception {
@@ -42,26 +42,30 @@ public void testRequiredAtArrayOk() throws Exception {
         assertNotNull(TEST_ARRAY.requiredAt("/1/data/vector/1"));
     }
 
-    public void testRequiredAtFailOnObject() throws Exception {
-        _checkRequiredAt(TEST_OBJECT, "/0", "/0");
-        _checkRequiredAt(TEST_OBJECT, "/bogus", "/bogus");
-        _checkRequiredAt(TEST_OBJECT, "/data/weird/and/more", "/weird/and/more");
+    public void testRequiredAtFailOnObjectBasic() throws Exception {
+        _checkRequiredAtFail(TEST_OBJECT, "/0", "/0");
+        _checkRequiredAtFail(TEST_OBJECT, "/bogus", "/bogus");
+        _checkRequiredAtFail(TEST_OBJECT, "/data/weird/and/more", "/weird/and/more");
 
-        _checkRequiredAt(TEST_OBJECT, "/data/vector/other/3", "/other/3");
+        _checkRequiredAtFail(TEST_OBJECT, "/data/vector/other/3", "/other/3");
+
+        _checkRequiredAtFail(TEST_OBJECT, "/data/primary/more", "/more");
     }
 
     public void testRequiredAtFailOnArray() throws Exception {
-        _checkRequiredAt(TEST_ARRAY, "/1/data/vector/25", "/25");
+        _checkRequiredAtFail(TEST_ARRAY, "/1/data/vector/25", "/25");
+        _checkRequiredAtFail(TEST_ARRAY, "/0/data/x", "/data/x");
     }
 
-    private void _checkRequiredAt(JsonNode doc, String fullPath, String mismatchPart) {
+    private void _checkRequiredAtFail(JsonNode doc, String fullPath, String mismatchPart) {
         try {
-            doc.requiredAt(fullPath);
+            JsonNode n = doc.requiredAt(fullPath);
+            fail("Should NOT pass: got node ("+n.getClass().getSimpleName()+") -> {"+n+"}");
         } catch (IllegalArgumentException e) {
             verifyException(e, "No node at '"+fullPath+"' (unmatched part: '"+mismatchPart+"')");
         }
     }
-    
+
     public void testSimpleRequireOk() throws Exception {
         // first basic working accessors on node itself
         assertSame(TEST_OBJECT, TEST_OBJECT.require());
@@ -99,4 +103,16 @@ public void testSimpleRequireFail() throws Exception {
             verifyException(e, "Node of type `ArrayNode` has no fields");
         }
     }
+
+    // [databind#3005]
+    public void testRequiredAtFailOnObjectScalar3005() throws Exception
+    {
+        JsonNode n = MAPPER.readTree("{\"simple\":5}");
+        try {
+            JsonNode match = n.requiredAt("/simple/property");
+            fail("Should NOT pass: got node ("+match.getClass().getSimpleName()+") -> {"+match+"}");
+        } catch (IllegalArgumentException e) {
+            verifyException(e, "No node at '/simple/property' (unmatched part: '/property')");
+        }
+    }
 }