From 8f0b80d2408396e1d97daad96730ab91f8f0b420 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 29 Aug 2019 15:24:29 -0700 Subject: [PATCH] Fix #2237 --- release-notes/VERSION-2.x | 2 + .../fasterxml/jackson/databind/JsonNode.java | 87 ++++++++++++++- .../jackson/databind/node/ArrayNode.java | 9 ++ .../jackson/databind/node/BaseJsonNode.java | 18 ++++ .../jackson/databind/node/BigIntegerNode.java | 4 +- .../jackson/databind/node/MissingNode.java | 12 +++ .../jackson/databind/node/NullNode.java | 7 ++ .../jackson/databind/node/ObjectNode.java | 9 ++ .../jackson/databind/node/ObjectNodeTest.java | 2 +- .../databind/node/RequiredAccessorTest.java | 102 ++++++++++++++++++ 10 files changed, 247 insertions(+), 5 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/node/RequiredAccessorTest.java diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index a39175279f..430bfbc542 100644 --- 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 index ef59906701..5cbb9be39c 100644 --- 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, if and only if * 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 require() { + return _this(); + } + + /** + * @since 2.10 + */ + public 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 _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 _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 index 54bdedcf8f..8587436f75 100644 --- 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 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 index a96e57cfee..82da773e7e 100644 --- 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 index 39083c5792..a8a6ebb657 100644 --- 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 index 69499c5e60..7827ee151f 100644 --- 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 index cb8f288659..0621dc47ce 100644 --- 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 index c241423150..85634c37bd 100644 --- 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 index d819d2514a..18c913fa8c 100644 --- 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 new file mode 100644 index 0000000000..22f239ad40 --- /dev/null +++ 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"); + } + } +}