fixed #1598

FasterXML · Apr 18, 2017 · 3c8efbe · 3c8efbe
1 parent 201398c
commit 3c8efbe
Show file tree

Hide file tree

Showing 14 changed files with 112 additions and 48 deletions.
diff --git a/src/main/java/com/fasterxml/jackson/databind/PropertyMetadata.java b/src/main/java/com/fasterxml/jackson/databind/PropertyMetadata.java
@@ -1,6 +1,6 @@
 package com.fasterxml.jackson.databind;
 
-import com.fasterxml.jackson.annotation.JsonSetter;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
 
 /**
@@ -32,7 +32,7 @@ public class PropertyMetadata
      * @since 2.9
      */
     public final static class MergeInfo
-    // NOTE: need not be Serializable, not peristed
+    // NOTE: need not be Serializable, not persisted
     {
         public final AnnotatedMember getter;
 
@@ -106,7 +106,7 @@ public static MergeInfo createForPropertyOverride(AnnotatedMember getter) {
      * 
      * @since 2.9
      */
-    protected JsonSetter.Nulls _valueNulls, _contentNulls;
+    protected Nulls _valueNulls, _contentNulls;
 
     /*
     /**********************************************************
@@ -118,7 +118,7 @@ public static MergeInfo createForPropertyOverride(AnnotatedMember getter) {
      * @since 2.9
      */
     protected PropertyMetadata(Boolean req, String desc, Integer index, String def,
-            MergeInfo mergeInfo, JsonSetter.Nulls valueNulls, JsonSetter.Nulls contentNulls)
+            MergeInfo mergeInfo, Nulls valueNulls, Nulls contentNulls)
     {
         _required = req;
         _description = desc;
@@ -187,8 +187,8 @@ public PropertyMetadata withMergeInfo(MergeInfo mergeInfo) {
     /**
      * @since 2.9
      */
-    public PropertyMetadata withNulls(JsonSetter.Nulls valueNulls,
-            JsonSetter.Nulls contentNulls) {
+    public PropertyMetadata withNulls(Nulls valueNulls,
+            Nulls contentNulls) {
         return new PropertyMetadata(_required, _description, _index, _defaultValue,
                 _mergeInfo, valueNulls, contentNulls);
     }
@@ -266,10 +266,10 @@ public PropertyMetadata withRequired(Boolean b) {
     /**
      * @since 2.9
      */
-    public JsonSetter.Nulls getValueNulls() { return _valueNulls; }
+    public Nulls getValueNulls() { return _valueNulls; }
 
     /**
      * @since 2.9
      */
-    public JsonSetter.Nulls getContentNulls() { return _contentNulls; }
+    public Nulls getContentNulls() { return _contentNulls; }
 }
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/SettableBeanProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/SettableBeanProperty.java
@@ -525,11 +525,9 @@ public final Object deserialize(JsonParser p, DeserializationContext ctxt) throw
     public final Object deserializeWith(JsonParser p, DeserializationContext ctxt,
             Object toUpdate) throws IOException
     {
-        JsonToken t = p.getCurrentToken();
-
         // 20-Oct-2016, tatu: Not 100% sure what to do; probably best to simply return
         //   null value and let caller decide what to do.
-        if (t == JsonToken.VALUE_NULL) {
+        if (p.hasToken(JsonToken.VALUE_NULL)) {
             // ... except for "skip nulls" case which should just do that:
             if (NullsConstantProvider.isSkipper(_nullProvider)) {
                 return toUpdate;

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/MergingSettableBeanProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/MergingSettableBeanProperty.java
@@ -80,11 +80,9 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt,
             newValue = delegate.deserializeWith(p, ctxt, oldValue);
         }
         if (newValue != oldValue) {
-            // 31-Oct-2016, tatu: Basically should just ignore as null can't really
-            //    contribute to merging.
-            if (newValue != null) {
-                delegate.set(instance, newValue);
-            }
+            // 18-Apr-2017, tatu: Null handling should occur within delegate, which may
+            //     set/skip/transform it, or throw an exception.
+            delegate.set(instance, newValue);
         }
     }
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java
@@ -5,7 +5,7 @@
 import java.util.Arrays;
 
 import com.fasterxml.jackson.annotation.JsonFormat;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.core.*;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.annotation.JacksonStdImpl;

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java
@@ -4,7 +4,7 @@
 import java.util.*;
 
 import com.fasterxml.jackson.annotation.JsonFormat;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.core.*;
 import com.fasterxml.jackson.core.io.NumberInput;
 import com.fasterxml.jackson.databind.*;

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java
@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonSetter;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.cfg.ConfigOverride;
 import com.fasterxml.jackson.databind.cfg.MapperConfig;
@@ -241,8 +242,8 @@ public PropertyMetadata getMetadata() {
     protected PropertyMetadata _getSetterInfo(PropertyMetadata metadata)
     {
         boolean needMerge = true;
-        JsonSetter.Nulls valueNulls = null;
-        JsonSetter.Nulls contentNulls = null;
+        Nulls valueNulls = null;
+        Nulls contentNulls = null;
 
         // Slightly confusing: first, annotations should be accessed via primary member
         // (mutator); but accessor is needed for actual merge operation. So:

diff --git a/src/test/java/com/fasterxml/jackson/databind/ObjectMapperTest.java b/src/test/java/com/fasterxml/jackson/databind/ObjectMapperTest.java
@@ -6,8 +6,7 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonSetter;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
-
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.core.*;
 import com.fasterxml.jackson.core.util.MinimalPrettyPrinter;
 

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsForContentTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsForContentTest.java
@@ -3,7 +3,7 @@
 import java.util.*;
 
 import com.fasterxml.jackson.annotation.JsonSetter;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.exc.InvalidNullException;
@@ -15,17 +15,17 @@ public class NullConversionsForContentTest extends BaseMapTest
     static class NullContentFail<T> {
         public T nullsOk;
 
-        @JsonSetter(contentNulls=JsonSetter.Nulls.FAIL)
+        @JsonSetter(contentNulls=Nulls.FAIL)
         public T noNulls;
     }
 
     static class NullContentAsEmpty<T> {
-        @JsonSetter(contentNulls=JsonSetter.Nulls.AS_EMPTY)
+        @JsonSetter(contentNulls=Nulls.AS_EMPTY)
         public T values;
     }
 
     static class NullContentSkip<T> {
-        @JsonSetter(contentNulls=JsonSetter.Nulls.SKIP)
+        @JsonSetter(contentNulls=Nulls.SKIP)
         public T values;
     }
 

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsGenericTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsGenericTest.java
@@ -4,6 +4,7 @@
 import java.util.Map;
 
 import com.fasterxml.jackson.annotation.JsonSetter;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.core.type.TypeReference;
 
 import com.fasterxml.jackson.databind.*;
@@ -17,14 +18,14 @@ static class GeneralEmpty<T> {
 //        @JsonSetter(nulls=JsonSetter.Nulls.AS_EMPTY)
         T value;
 
-        @JsonSetter(nulls=JsonSetter.Nulls.AS_EMPTY)
+        @JsonSetter(nulls=Nulls.AS_EMPTY)
         public void setValue(T v) {
             value = v;
         }
     }
 
     static class NoCtorWrapper {
-        @JsonSetter(nulls=JsonSetter.Nulls.AS_EMPTY)
+        @JsonSetter(nulls=Nulls.AS_EMPTY)
         public NoCtorPOJO value;
     }
 

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsPojoTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsPojoTest.java
@@ -1,7 +1,7 @@
 package com.fasterxml.jackson.databind.deser.filter;
 
 import com.fasterxml.jackson.annotation.JsonSetter;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.exc.InvalidNullException;
 
@@ -11,14 +11,14 @@ public class NullConversionsPojoTest extends BaseMapTest
     static class NullFail {
         public String nullsOk = "a";
 
-        @JsonSetter(nulls=JsonSetter.Nulls.FAIL)
+        @JsonSetter(nulls=Nulls.FAIL)
         public String noNulls = "b";
     }
 
     static class NullAsEmpty {
         public String nullsOk = "a";
 
-        @JsonSetter(nulls=JsonSetter.Nulls.AS_EMPTY)
+        @JsonSetter(nulls=Nulls.AS_EMPTY)
         public String nullAsEmpty = "b";
     }
 

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsSkipTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsSkipTest.java
@@ -1,7 +1,7 @@
 package com.fasterxml.jackson.databind.deser.filter;
 
 import com.fasterxml.jackson.annotation.JsonSetter;
-import com.fasterxml.jackson.annotation.JsonSetter.Nulls;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.*;
 
 // for [databind#1402]; configurable null handling, specifically with SKIP
@@ -10,7 +10,7 @@ public class NullConversionsSkipTest extends BaseMapTest
     static class NullSkipField {
         public String nullsOk = "a";
 
-        @JsonSetter(nulls=JsonSetter.Nulls.SKIP)
+        @JsonSetter(nulls=Nulls.SKIP)
         public String noNulls = "b";
     }
 
@@ -22,7 +22,7 @@ public void setNullsOk(String v) {
             _nullsOk = v;
         }
 
-        @JsonSetter(nulls=JsonSetter.Nulls.SKIP)
+        @JsonSetter(nulls=Nulls.SKIP)
         public void setNoNulls(String v) {
             _noNulls = v;
         }

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/merge/MergeWithNullTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/merge/MergeWithNullTest.java
@@ -1,23 +1,46 @@
 package com.fasterxml.jackson.databind.deser.merge;
 
 import com.fasterxml.jackson.annotation.JsonMerge;
-
+import com.fasterxml.jackson.annotation.JsonSetter;
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.BaseMapTest;
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 public class MergeWithNullTest extends BaseMapTest
 {
-    static class Config {
+    static class ConfigDefault {
+        @JsonMerge
+        public AB loc = new AB(1, 2);
+
+        protected ConfigDefault() { }
+        public ConfigDefault(int a, int b) {
+            loc = new AB(a, b);
+        }
+    }
+
+    static class ConfigSkipNull {
         @JsonMerge
+        @JsonSetter(nulls=Nulls.SKIP)
         public AB loc = new AB(1, 2);
 
-        protected Config() { }
-        public Config(int a, int b) {
+        protected ConfigSkipNull() { }
+        public ConfigSkipNull(int a, int b) {
             loc = new AB(a, b);
         }
     }
 
+    static class ConfigAllowNullOverwrite {
+        @JsonMerge
+        @JsonSetter(nulls=Nulls.SET)
+        public AB loc = new AB(1, 2);
+
+        protected ConfigAllowNullOverwrite() { }
+        public ConfigAllowNullOverwrite(int a, int b) {
+            loc = new AB(a, b);
+        }
+    }
+
     // another variant where all we got is a getter
     static class NoSetterConfig {
         AB _value = new AB(2, 3);
@@ -37,21 +60,66 @@ public AB(int a0, int b0) {
         }
     }
 
+    /*
+    /**********************************************************
+    /* Test methods
+    /**********************************************************
+     */
+
     private final ObjectMapper MAPPER = new ObjectMapper()
             // 26-Oct-2016, tatu: Make sure we'll report merge problems by default
             .disable(MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE)
     ;
 
-    // Also: nulls better not override
-    public void testBeanMergingWithNull() throws Exception
+    public void testBeanMergingWithNullDefault() throws Exception
     {
-        Config config = MAPPER.readerForUpdating(new Config(5, 7))
+        // By default `null` should simply overwrite value
+        ConfigDefault config = MAPPER.readerForUpdating(new ConfigDefault(5, 7))
                 .readValue(aposToQuotes("{'loc':null}"));
+        assertNotNull(config);
+        assertNull(config.loc);
+
+        // but it should be possible to override setting to, say, skip
+
+        // First: via specific type override
+        // important! We'll specify for value type to be merged
+        ObjectMapper mapper = new ObjectMapper();
+        mapper.configOverride(AB.class)
+            .setSetterInfo(JsonSetter.Value.forValueNulls(Nulls.SKIP));
+        config = mapper.readerForUpdating(new ConfigDefault(137, -3))
+                .readValue(aposToQuotes("{'loc':null}"));
+        assertNotNull(config.loc);
+        assertEquals(137, config.loc.a);
+        assertEquals(-3, config.loc.b);
+
+        // Second: by global defaults
+        mapper = new ObjectMapper();
+        mapper.setDefaultSetterInfo(JsonSetter.Value.forValueNulls(Nulls.SKIP));
+        config = mapper.readerForUpdating(new ConfigDefault(12, 34))
+                .readValue(aposToQuotes("{'loc':null}"));
+        assertNotNull(config.loc);
+        assertEquals(12, config.loc.a);
+        assertEquals(34, config.loc.b);
+    }
+
+    public void testBeanMergingWithNullSkip() throws Exception
+    {
+        ConfigSkipNull config = MAPPER.readerForUpdating(new ConfigSkipNull(5, 7))
+                .readValue(aposToQuotes("{'loc':null}"));
+        assertNotNull(config);
         assertNotNull(config.loc);
         assertEquals(5, config.loc.a);
         assertEquals(7, config.loc.b);
     }
 
+    public void testBeanMergingWithNullSet() throws Exception
+    {
+        ConfigAllowNullOverwrite config = MAPPER.readerForUpdating(new ConfigAllowNullOverwrite(5, 7))
+                .readValue(aposToQuotes("{'loc':null}"));
+        assertNotNull(config);
+        assertNull(config.loc);
+    }
+
     public void testSetterlessMergingWithNull() throws Exception
     {
         NoSetterConfig input = new NoSetterConfig();

diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/PropertyMetadataTest.java b/src/test/java/com/fasterxml/jackson/databind/introspect/PropertyMetadataTest.java
@@ -1,7 +1,6 @@
 package com.fasterxml.jackson.databind.introspect;
 
-import com.fasterxml.jackson.annotation.JsonSetter;
-
+import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.databind.*;
 
 public class PropertyMetadataTest extends BaseMapTest
@@ -49,10 +48,10 @@ public void testPropertyMetadata()
         assertNull(md.getDefaultValue());
         assertEquals(Boolean.FALSE, md.getRequired());
 
-        md = md.withNulls(JsonSetter.Nulls.AS_EMPTY,
-                JsonSetter.Nulls.FAIL);
-        assertEquals(JsonSetter.Nulls.AS_EMPTY, md.getValueNulls());
-        assertEquals(JsonSetter.Nulls.FAIL, md.getContentNulls());
+        md = md.withNulls(Nulls.AS_EMPTY,
+                Nulls.FAIL);
+        assertEquals(Nulls.AS_EMPTY, md.getValueNulls());
+        assertEquals(Nulls.FAIL, md.getContentNulls());
 
         assertFalse(md.hasDefaultValue());
         assertSame(md, md.withDefaultValue(null));

diff --git a/src/test/java/com/fasterxml/jackson/failing/TestObjectIdWithUnwrapping1298.java b/src/test/java/com/fasterxml/jackson/failing/TestObjectIdWithUnwrapping1298.java
@@ -76,7 +76,7 @@ public void testObjectIdWithRepeatedChild() throws Exception
 //                .writerWithDefaultPrettyPrinter()
                 .writeValue(sw, parents);
         } catch (Exception e) {
-            fail("Failed output so far: " + sw);
+            fail("Failed with "+e.getClass().getName()+", output so far: " + sw);
         }
     }
 }