Fix #2430

FasterXML · Aug 23, 2019 · 6c3144d · 6c3144d
1 parent 2c64915
commit 6c3144d
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 31 deletions.
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -24,6 +24,7 @@ Project: jackson-databind
  (reported by dejanlokar1@github)
 #2425: Add global config override setting for `@JsonFormat.lenient()`
 #2428: Use "activateDefaultTyping" over "enableDefaultTyping" in 2.10 with new methods
+#2430: Change `ObjectMapper.valueToTree()` to convert `null` to `NullNode`
 
 2.10.0.pr1 (19-Jul-2019)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java b/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
@@ -2856,17 +2856,24 @@ public JsonParser treeAsTokens(TreeNode n) {
     public <T> T treeToValue(TreeNode n, Class<T> valueType)
         throws JsonProcessingException
     {
-        _assertNotNull("n", n);
-       try {
+        if (n == null) {
+            return null;
+        }
+        try {
             // 25-Jan-2019, tatu: [databind#2220] won't prevent existing coercions here
             // Simple cast when we just want to cast to, say, ObjectNode
             if (TreeNode.class.isAssignableFrom(valueType)
                     && valueType.isAssignableFrom(n.getClass())) {
                 return (T) n;
             }
+            final JsonToken tt = n.asToken();
+            // 22-Aug-2019, tatu: [databind#2430] Consider "null node" (minor optimization)
+            if (tt == JsonToken.VALUE_NULL) {
+                return null;
+            }
             // 20-Apr-2016, tatu: Another thing: for VALUE_EMBEDDED_OBJECT, assume similar
             //    short-cut coercion
-            if (n.asToken() == JsonToken.VALUE_EMBEDDED_OBJECT) {
+            if (tt == JsonToken.VALUE_EMBEDDED_OBJECT) {
                 if (n instanceof POJONode) {
                     Object ob = ((POJONode) n).getPojo();
                     if ((ob == null) || valueType.isInstance(ob)) {
@@ -2900,13 +2907,18 @@ public <T> T treeToValue(TreeNode n, Class<T> valueType)
      * @param <T> Actual node type; usually either basic {@link JsonNode} or
      *  {@link com.fasterxml.jackson.databind.node.ObjectNode}
      * @param fromValue Bean value to convert
-     * @return Root node of the resulting JSON tree
+     *
+     * @return (non-null) Root node of the resulting JSON tree: in case of {@code null} value,
+     *    node for which {@link JsonNode#isNull()} returns {@code true}.
      */
     @SuppressWarnings({ "unchecked", "resource" })
     public <T extends JsonNode> T valueToTree(Object fromValue)
         throws IllegalArgumentException
     {
-        if (fromValue == null) return null;
+        // [databind#2430]: `null` should become "null node":
+        if (fromValue == null) {
+            return (T) getNodeFactory().nullNode();
+        }
         TokenBuffer buf = new TokenBuffer(this, false);
         if (isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)) {
             buf = buf.forceUseOfBigDecimal(true);
@@ -2921,7 +2933,7 @@ public <T extends JsonNode> T valueToTree(Object fromValue)
             throw new IllegalArgumentException(e.getMessage(), e);
         }
         return (T) result;
-    } 
+    }
 
     /*
     /**********************************************************

diff --git a/src/test/java/com/fasterxml/jackson/databind/node/TestConversions.java b/src/test/java/com/fasterxml/jackson/databind/node/TestConversions.java
@@ -81,6 +81,32 @@ static class LongContainer1940 {
         public Long longObj;
     }
 
+    // [databind#433]
+    static class CustomSerializedPojo implements JsonSerializable
+    {
+        private final ObjectNode node = JsonNodeFactory.instance.objectNode();
+
+        public void setFoo(final String foo) {
+            node.put("foo", foo);
+        }
+
+        @Override
+        public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException
+        {
+            jgen.writeTree(node);
+        }
+
+        @Override
+        public void serializeWithType(JsonGenerator g,
+                SerializerProvider provider, TypeSerializer typeSer) throws IOException
+        {
+            WritableTypeId typeIdDef = new WritableTypeId(this, JsonToken.START_OBJECT);
+            typeSer.writeTypePrefix(g, typeIdDef);
+            serialize(g, provider);
+            typeSer.writeTypePrefix(g, typeIdDef);
+        }    
+    }
+
     /*
     /**********************************************************
     /* Unit tests
@@ -241,31 +267,6 @@ public void testBigDecimalAsPlainStringTreeConversion() throws Exception
         assertTrue(tree.has("pi"));
     }
 
-    static class CustomSerializedPojo implements JsonSerializable
-    {
-        private final ObjectNode node = JsonNodeFactory.instance.objectNode();
-
-        public void setFoo(final String foo) {
-            node.put("foo", foo);
-        }
-
-        @Override
-        public void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException
-        {
-            jgen.writeTree(node);
-        }
-
-        @Override
-        public void serializeWithType(JsonGenerator g,
-                SerializerProvider provider, TypeSerializer typeSer) throws IOException
-        {
-            WritableTypeId typeIdDef = new WritableTypeId(this, JsonToken.START_OBJECT);
-            typeSer.writeTypePrefix(g, typeIdDef);
-            serialize(g, provider);
-            typeSer.writeTypePrefix(g, typeIdDef);
-        }    
-    }
-
     // [databind#433]
     public void testBeanToTree() throws Exception
     {
@@ -314,4 +315,16 @@ public void testBufferedLongViaCoercion() throws Exception {
         LongContainer1940 obj = MAPPER.treeToValue(tree, LongContainer1940.class);
         assertEquals(Long.valueOf(EXP), obj.longObj);
     }
+
+    public void testConversionsOfNull() throws Exception
+    {
+        // First: `null` value should become `NullNode`
+        JsonNode n = MAPPER.valueToTree(null);
+        assertNotNull(n);
+        assertTrue(n.isNull());
+
+        // and vice versa
+        Object pojo = MAPPER.treeToValue(n, Root.class);
+        assertNull(pojo);
+    }
 }