Fix #2237

FasterXML · Aug 29, 2019 · 8f0b80d · 8f0b80d
1 parent ad74329
commit 8f0b80d
Show file tree

Hide file tree

Showing 10 changed files with 247 additions and 5 deletions.
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -6,6 +6,8 @@ Project: jackson-databind
 
 2.10.0.pr2 (not yet released)
 
+#2237: Add "required" methods in `JsonNode`: `required(String | int)`,
+  `requiredAt(JsonPointer)`
 #2331: `JsonMappingException` through nested getter with generic wildcard return type
  (reported by sunchezz89@github)
 #2336: `MapDeserializer` can not merge `Map`s with polymorphic values

diff --git a/src/main/java/com/fasterxml/jackson/databind/JsonNode.java b/src/main/java/com/fasterxml/jackson/databind/JsonNode.java
@@ -404,7 +404,7 @@ public final boolean isBinary() {
      * @since 2.0
      */
     public boolean canConvertToLong() { return false; }
-    
+
     /*
     /**********************************************************
     /* Public API, straight value access
@@ -492,7 +492,7 @@ public byte[] binaryValue() throws IOException {
      *   nodes.
      */
     public long longValue() { return 0L; }
-    
+
     /**
      * Returns 32-bit floating value for this node, <b>if and only if</b>
      * this node is numeric ({@link #isNumber} returns true). For other
@@ -676,6 +676,68 @@ public boolean asBoolean(boolean defaultValue) {
         return defaultValue;
     }
 
+    /*
+    /**********************************************************************
+    /* Public API, extended traversal (2.10) with "required()"
+    /**********************************************************************
+     */
+
+    /**
+     * @since 2.10
+     */
+    public <T extends JsonNode> T require() {
+        return _this();
+    }
+
+    /**
+     * @since 2.10
+     */
+    public <T extends JsonNode> T requireNonNull() {
+        return _this();
+    }
+
+    /**
+     * @since 2.10
+     */
+    public JsonNode required(String fieldName) {
+        return _reportRequiredViolation("Node of type `%s` has no fields", getClass().getName());
+    }
+
+    /**
+     * @since 2.10
+     */
+    public JsonNode required(int index) {
+        return _reportRequiredViolation("Node of type `%s` has no indexed values", getClass().getName());
+    }
+
+    /**
+     * @since 2.10
+     */
+    public JsonNode requiredAt(String pathExpr) {
+        return requiredAt(JsonPointer.compile(pathExpr));
+    }
+
+    /**
+     * @since 2.10
+     */
+    public final JsonNode requiredAt(final JsonPointer pathExpr) {
+        JsonPointer currentExpr = pathExpr;
+        JsonNode curr = this;
+
+        // Note: copied from `at()`
+        while (true) {
+            if (currentExpr.matches()) {
+                return curr;
+            }
+            curr = curr._at(currentExpr);
+            if (curr == null) {
+                _reportRequiredViolation("No node at '%s' (unmatched part: '%s')",
+                        pathExpr, currentExpr);
+            }
+            currentExpr = currentExpr.tail();
+        }
+    }
+
     /*
     /**********************************************************
     /* Public API, value find / existence check methods
@@ -1000,4 +1062,25 @@ public String toPrettyString() {
      */
     @Override
     public abstract boolean equals(Object o);
+
+    /*
+    /**********************************************************************
+    /* Helper methods,  for sub-classes
+    /**********************************************************************
+     */
+
+    // @since 2.10
+    @SuppressWarnings("unchecked")
+    protected <T extends JsonNode> T _this() {
+        return (T) this;
+    }
+
+    /**
+     * Helper method that throws {@link IllegalArgumentException} as a result of
+     * violating "required-constraint" for this node (for {@link #require() or related
+     * methods).
+     */
+    protected <T> T _reportRequiredViolation(String msgTemplate, Object...args) {
+        throw new IllegalArgumentException(String.format(msgTemplate, args));
+    }
 }
diff --git a/src/main/java/com/fasterxml/jackson/databind/node/ArrayNode.java b/src/main/java/com/fasterxml/jackson/databind/node/ArrayNode.java
@@ -132,6 +132,15 @@ public JsonNode path(int index) {
         return MissingNode.getInstance();
     }
 
+    @Override
+    public JsonNode required(int index) {
+        if ((index >= 0) && (index < _children.size())) {
+            return _children.get(index);
+        }
+        return _reportRequiredViolation("No value at index #%d [0, %d) of `ArrayNode`",
+                index, _children.size());
+    }
+
     @Override
     public boolean equals(Comparator<JsonNode> comparator, JsonNode o)
     {

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/BaseJsonNode.java b/src/main/java/com/fasterxml/jackson/databind/node/BaseJsonNode.java
@@ -49,6 +49,24 @@ public final JsonNode findPath(String fieldName)
     // Also, force (re)definition (2.7)
     @Override public abstract int hashCode();
 
+    /*
+    /**********************************************************************
+    /* Improved required-ness checks for standard JsonNode implementations
+    /**********************************************************************
+     */
+
+    @Override
+    public JsonNode required(String fieldName) {
+        return _reportRequiredViolation("Node of type `%s` has no fields",
+                getClass().getSimpleName());
+    }
+
+    @Override
+    public JsonNode required(int index) {
+        return _reportRequiredViolation("Node of type `%s` has no indexed values",
+                getClass().getSimpleName());
+    }
+
     /*
     /**********************************************************
     /* Support for traversal-as-stream

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/BigIntegerNode.java b/src/main/java/com/fasterxml/jackson/databind/node/BigIntegerNode.java
@@ -17,9 +17,9 @@ public class BigIntegerNode
     private final static BigInteger MAX_INTEGER = BigInteger.valueOf(Integer.MAX_VALUE);
     private final static BigInteger MIN_LONG = BigInteger.valueOf(Long.MIN_VALUE);
     private final static BigInteger MAX_LONG = BigInteger.valueOf(Long.MAX_VALUE);
-    
+
     final protected BigInteger _value;
-    
+
     /*
     /**********************************************************
     /* Construction

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/MissingNode.java b/src/main/java/com/fasterxml/jackson/databind/node/MissingNode.java
@@ -106,6 +106,18 @@ public boolean equals(Object o)
         return (o == this);
     }
 
+    @SuppressWarnings("unchecked")
+    @Override
+    public JsonNode require() {
+        return _reportRequiredViolation("require() called on `MissingNode`");
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public JsonNode requireNonNull() {
+        return _reportRequiredViolation("requireNonNull() called on `MissingNode`");
+    }
+
     @Override
     public int hashCode() {
         return JsonNodeType.MISSING.ordinal();

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/NullNode.java b/src/main/java/com/fasterxml/jackson/databind/node/NullNode.java
@@ -3,6 +3,7 @@
 import java.io.IOException;
 
 import com.fasterxml.jackson.core.*;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.SerializerProvider;
 
 
@@ -42,6 +43,12 @@ public JsonNodeType getNodeType() {
     @Override public String asText(String defaultValue) { return defaultValue; }
     @Override public String asText() { return "null"; }
 
+    @SuppressWarnings("unchecked")
+    @Override
+    public JsonNode requireNonNull() {
+        return _reportRequiredViolation("requireNonNull() called on `NullNode`");
+    }
+
     // as with MissingNode, not considered number node; hence defaults are returned if provided
 
     /*

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/ObjectNode.java b/src/main/java/com/fasterxml/jackson/databind/node/ObjectNode.java
@@ -129,6 +129,15 @@ public JsonNode path(String fieldName)
         return MissingNode.getInstance();
     }
 
+    @Override
+    public JsonNode required(String fieldName) {
+        JsonNode n = _children.get(fieldName);
+        if (n != null) {
+            return n;
+        }
+        return _reportRequiredViolation("No value for property '%s' of `ObjectNode`", fieldName);
+    }
+
     /**
      * Method to use for accessing all fields (with both names
      * and values) of this JSON Object.

diff --git a/src/test/java/com/fasterxml/jackson/databind/node/ObjectNodeTest.java b/src/test/java/com/fasterxml/jackson/databind/node/ObjectNodeTest.java
@@ -68,7 +68,7 @@ static class MyValue
     /**********************************************************
      */
 
-    private final ObjectMapper MAPPER = objectMapper();
+    private final ObjectMapper MAPPER = sharedMapper();
 
     public void testSimpleObject() throws Exception
     {

diff --git a/src/test/java/com/fasterxml/jackson/databind/node/RequiredAccessorTest.java b/src/test/java/com/fasterxml/jackson/databind/node/RequiredAccessorTest.java
@@ -0,0 +1,102 @@
+package com.fasterxml.jackson.databind.node;
+
+import com.fasterxml.jackson.core.JsonPointer;
+import com.fasterxml.jackson.databind.BaseMapTest;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class RequiredAccessorTest
+    extends BaseMapTest
+{
+    private final ObjectMapper MAPPER = sharedMapper();
+
+    private final JsonNode TEST_OBJECT, TEST_ARRAY;
+
+    public RequiredAccessorTest() throws Exception {
+        TEST_OBJECT = MAPPER.readTree(aposToQuotes(
+ "{ 'data' : { 'primary' : 15, 'vector' : [ 'yes', false ], 'nullable' : null  },\n"
++"  'array' : [ true,   {\"messsage\":'hello', 'value' : 42, 'misc' : [1, 2] }, null, 0.25 ]\n"
++"}"
+        ));
+        TEST_ARRAY = MAPPER.readTree(aposToQuotes(
+ "[ true, { 'data' : { 'primary' : 15, 'vector' : [ 'yes', false ]  } }, 0.25, 'last' ]"
+        ));
+    }
+
+    public void testIMPORTANT() {
+        _checkRequiredAt(TEST_OBJECT, "/data/weird/and/more", "/weird/and/more");
+    }
+
+    public void testRequiredAtObjectOk() throws Exception {
+        assertNotNull(TEST_OBJECT.requiredAt("/array"));
+        assertNotNull(TEST_OBJECT.requiredAt("/array/0"));
+        assertTrue(TEST_OBJECT.requiredAt("/array/0").isBoolean());
+        assertNotNull(TEST_OBJECT.requiredAt("/array/1/misc/1"));
+        assertEquals(2, TEST_OBJECT.requiredAt("/array/1/misc/1").intValue());
+    }
+
+    public void testRequiredAtArrayOk() throws Exception {
+        assertTrue(TEST_ARRAY.requiredAt("/0").isBoolean());
+        assertTrue(TEST_ARRAY.requiredAt("/1").isObject());
+        assertNotNull(TEST_ARRAY.requiredAt("/1/data/primary"));
+        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");
+
+        _checkRequiredAt(TEST_OBJECT, "/data/vector/other/3", "/other/3");
+    }
+
+    public void testRequiredAtFailOnArray() throws Exception {
+        _checkRequiredAt(TEST_ARRAY, "/1/data/vector/25", "/25");
+    }
+
+    private void _checkRequiredAt(JsonNode doc, String fullPath, String mismatchPart) {
+        try {
+            doc.requiredAt(fullPath);
+        } 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());
+        assertSame(TEST_OBJECT, TEST_OBJECT.requireNonNull());
+        assertSame(TEST_OBJECT, TEST_OBJECT.requiredAt(""));
+        assertSame(TEST_OBJECT, TEST_OBJECT.requiredAt(JsonPointer.compile("")));
+
+        assertSame(TEST_OBJECT.get("data"), TEST_OBJECT.required("data"));
+        assertSame(TEST_ARRAY.get(0), TEST_ARRAY.required(0));
+        assertSame(TEST_ARRAY.get(3), TEST_ARRAY.required(3));
+
+        // check diff between missing, null nodes
+        TEST_OBJECT.path("data").path("nullable").require();
+
+        try {
+            JsonNode n = TEST_OBJECT.path("data").path("nullable").requireNonNull();
+            fail("Should not pass; got: "+n);
+        } catch (IllegalArgumentException e) {
+            verifyException(e, "requireNonNull() called on `NullNode`");
+        }
+    }
+
+    public void testSimpleRequireFail() throws Exception {
+        try {
+            TEST_OBJECT.required("bogus");
+            fail("Should not pass");
+        } catch (IllegalArgumentException e) {
+            verifyException(e, "No value for property 'bogus'");
+        }
+
+        try {
+            TEST_ARRAY.required("bogus");
+            fail("Should not pass");
+        } catch (IllegalArgumentException e) {
+            verifyException(e, "Node of type `ArrayNode` has no fields");
+        }
+    }
+}