From 3c8efbe5f572cd77d008c413b27a91947f0c6a59 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 18 Apr 2017 16:17:26 -0700 Subject: [PATCH] fixed #1598 --- .../jackson/databind/PropertyMetadata.java | 16 ++-- .../databind/deser/SettableBeanProperty.java | 4 +- .../impl/MergingSettableBeanProperty.java | 8 +- .../std/PrimitiveArrayDeserializers.java | 2 +- .../databind/deser/std/StdDeserializer.java | 2 +- .../introspect/POJOPropertyBuilder.java | 5 +- .../jackson/databind/ObjectMapperTest.java | 3 +- .../filter/NullConversionsForContentTest.java | 8 +- .../filter/NullConversionsGenericTest.java | 5 +- .../deser/filter/NullConversionsPojoTest.java | 6 +- .../deser/filter/NullConversionsSkipTest.java | 6 +- .../deser/merge/MergeWithNullTest.java | 82 +++++++++++++++++-- .../introspect/PropertyMetadataTest.java | 11 ++- .../TestObjectIdWithUnwrapping1298.java | 2 +- 14 files changed, 112 insertions(+), 48 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/PropertyMetadata.java b/src/main/java/com/fasterxml/jackson/databind/PropertyMetadata.java index 547e4c2ce2..9b57186c2f 100644 --- 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 index 750763860a..ff77c1326f 100644 --- 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 index 22c4c4e9e1..db9fda775b 100644 --- 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 index da19a5b946..099e8491e2 100644 --- 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 index 4b1c564fb4..fa1deae001 100644 --- 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 index 7051862fdc..602e88d925 100644 --- 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 index d016068d34..f59eabce6c 100644 --- 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 index c40b71e3ba..f6f6106888 100644 --- 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 { public T nullsOk; - @JsonSetter(contentNulls=JsonSetter.Nulls.FAIL) + @JsonSetter(contentNulls=Nulls.FAIL) public T noNulls; } static class NullContentAsEmpty { - @JsonSetter(contentNulls=JsonSetter.Nulls.AS_EMPTY) + @JsonSetter(contentNulls=Nulls.AS_EMPTY) public T values; } static class NullContentSkip { - @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 index 2b0babc2b7..0d9bbcb20c 100644 --- 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 { // @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 index 255a8272f5..b04389aa30 100644 --- 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 index da30951175..218b9f5bf6 100644 --- 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 index 7f24f5805d..4f1db0ef58 100644 --- 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 index 31cc7c0b7f..84dcc54cbf 100644 --- 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 index 666dfe772a..d039140222 100644 --- 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); } } }