From b1ada0a4cab9bd6d332336f505be27d295965c92 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Fri, 29 Mar 2024 00:11:45 +0900 Subject: [PATCH 1/5] Implement --- .../databind/deser/impl/FieldProperty.java | 5 + .../databind/deser/SkipNulls4441Test.java | 108 ++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java index b3eb596583..25768fd785 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java @@ -186,6 +186,11 @@ public Object deserializeSetAndReturn(JsonParser p, @Override public void set(Object instance, Object value) throws IOException { + if (value == null) { + if (_skipNulls) { + return; + } + } try { _field.set(instance, value); } catch (Exception e) { diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java new file mode 100644 index 0000000000..7b2f33caf9 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java @@ -0,0 +1,108 @@ +package com.fasterxml.jackson.databind.deser; + +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; +import org.junit.jupiter.api.Test; + +import java.beans.ConstructorProperties; +import java.util.ArrayList; +import java.util.List; + +import static com.fasterxml.jackson.databind.BaseTest.a2q; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +// [databind#4441] @JsonSetter(nulls = Nulls.SKIP) doesn't work in some situations +public class SkipNulls4441Test { + + static class Outer { + @JsonSetter(nulls = Nulls.SKIP) + private final List listMiddle = new ArrayList<>(); + + public Outer() { } + + public List getListMiddle() { + return listMiddle; + } + } + + static class Middle { + @JsonSetter(nulls = Nulls.SKIP) + private final List listInner = new ArrayList<>(); + private final String field1; + + @ConstructorProperties({"field1"}) + public Middle(String field1) { + this.field1 = field1; + } + + public List getListInner() { + return listInner; + } + + public String getField1() { + return field1; + } + } + + static class Inner { + private final String field1; + + @ConstructorProperties({"field1"}) + public Inner(String field1) { + this.field1 = field1; + } + + public String getField1() { + return field1; + } + } + + private final ObjectMapper objectMapper = JsonMapper.builder().build(); + + @Test + public void testJSON() throws Exception { + // Passes + // For some reason, if most-inner "list1" field is null in the end, it works + testSkipNullsWith( + a2q("{" + + " 'listMiddle': [" + + " {" + + " 'field1': 'data', " + + " 'listInner': null " + + " }" + + " ]" + + "}") + ); + + // Fails + // But if it's null in the beginning, it doesn't work + testSkipNullsWith( + a2q("{" + + " 'listMiddle': [" + + " {" + + " 'listInner': null, " + + " 'field1': 'data' " + + " }" + + " ]" + + "}") + ); + } + + private void testSkipNullsWith(String JSON) throws Exception { + Outer outer = objectMapper.readValue(JSON, Outer.class); + + validateNotNull(outer); + validateNotNull(outer.getListMiddle()); + for (Middle middle : outer.getListMiddle()) { + validateNotNull(middle); + validateNotNull(middle.getField1()); + validateNotNull(middle.getListInner()); + } + } + + private static void validateNotNull(Object o) { + assertNotNull(o); + } +} From b0ae6a3db1c8bfc9cbe534c416db989135a4406c Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Fri, 29 Mar 2024 00:25:32 +0900 Subject: [PATCH 2/5] Add case for class also --- .../databind/deser/SkipNulls4441Test.java | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java index 7b2f33caf9..2eb5dd160e 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java @@ -90,18 +90,49 @@ public void testJSON() throws Exception { ); } + @Test + public void testDeserializeMiddleOnly() throws Exception { + // Passes + // For some reason, if most-inner "list1" field is null in the end, it works + testSkipNullsWithMiddleOnly( + a2q("{" + + " 'field1': 'data', " + + " 'listInner': null " + + "}") + ); + + // Fails + // But if it's null in the beginning, it doesn't work + testSkipNullsWithMiddleOnly( + a2q("{" + + " 'listInner': null, " + + " 'field1': 'data' " + + "}") + ); + } + private void testSkipNullsWith(String JSON) throws Exception { Outer outer = objectMapper.readValue(JSON, Outer.class); validateNotNull(outer); validateNotNull(outer.getListMiddle()); for (Middle middle : outer.getListMiddle()) { - validateNotNull(middle); - validateNotNull(middle.getField1()); - validateNotNull(middle.getListInner()); + testMiddle(middle); } } + private void testSkipNullsWithMiddleOnly(String s) throws Exception { + Middle middle = objectMapper.readValue(s, Middle.class); + + testMiddle(middle); + } + + private void testMiddle(Middle middle) { + validateNotNull(middle); + validateNotNull(middle.getField1()); + validateNotNull(middle.getListInner()); + } + private static void validateNotNull(Object o) { assertNotNull(o); } From bff3865f8e56cc373641684057404d309c68f097 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Fri, 29 Mar 2024 09:46:36 +0900 Subject: [PATCH 3/5] Add test for method also --- .../databind/deser/SkipNulls4441Test.java | 126 +++++++++++------- 1 file changed, 81 insertions(+), 45 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java index 2eb5dd160e..d772bafabe 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java @@ -20,7 +20,8 @@ static class Outer { @JsonSetter(nulls = Nulls.SKIP) private final List listMiddle = new ArrayList<>(); - public Outer() { } + public Outer() { + } public List getListMiddle() { return listMiddle; @@ -59,69 +60,98 @@ public String getField1() { } } + // for method setter level Nulls.SKIP + static class OuterSetter { + private final List listMiddle = new ArrayList<>(); + + public OuterSetter() { + } + + @JsonSetter(nulls = Nulls.SKIP) + public void setListMiddle(List listMiddle) { + this.listMiddle.addAll(listMiddle); + } + + public List getListMiddle() { + return listMiddle; + } + } + + static class MiddleSetter { + private final List listInner = new ArrayList<>(); + private final String field1; + + @ConstructorProperties({"field1"}) + public MiddleSetter(String field1) { + this.field1 = field1; + } + + @JsonSetter(nulls = Nulls.SKIP) + public void setListInner(List listInner) { + this.listInner.addAll(listInner); + } + + public List getListInner() { + return listInner; + } + + public String getField1() { + return field1; + } + } + + static class InnerSetter { + private final String field1; + + @ConstructorProperties({"field1"}) + public InnerSetter(String field1) { + this.field1 = field1; + } + + public String getField1() { + return field1; + } + } + private final ObjectMapper objectMapper = JsonMapper.builder().build(); + private final String NULL_ENDING_JSON = a2q("{" + + " 'field1': 'data', " + + " 'listInner': null " + + "}"); + + private final String NULL_BEGINNING_JSON = a2q("{" + + " 'listInner': null, " + + " 'field1': 'data' " + + "}"); + @Test - public void testJSON() throws Exception { + public void testFields() throws Exception { // Passes // For some reason, if most-inner "list1" field is null in the end, it works - testSkipNullsWith( - a2q("{" + - " 'listMiddle': [" + - " {" + - " 'field1': 'data', " + - " 'listInner': null " + - " }" + - " ]" + - "}") - ); - + _testFieldNullSkip(NULL_ENDING_JSON); // Fails // But if it's null in the beginning, it doesn't work - testSkipNullsWith( - a2q("{" + - " 'listMiddle': [" + - " {" + - " 'listInner': null, " + - " 'field1': 'data' " + - " }" + - " ]" + - "}") - ); + _testFieldNullSkip(NULL_BEGINNING_JSON); } @Test - public void testDeserializeMiddleOnly() throws Exception { + public void testMethods() throws Exception { // Passes // For some reason, if most-inner "list1" field is null in the end, it works - testSkipNullsWithMiddleOnly( - a2q("{" + - " 'field1': 'data', " + - " 'listInner': null " + - "}") - ); - + _testMethodNullSkip(NULL_ENDING_JSON); // Fails // But if it's null in the beginning, it doesn't work - testSkipNullsWithMiddleOnly( - a2q("{" + - " 'listInner': null, " + - " 'field1': 'data' " + - "}") - ); + _testMethodNullSkip(NULL_BEGINNING_JSON); } - private void testSkipNullsWith(String JSON) throws Exception { - Outer outer = objectMapper.readValue(JSON, Outer.class); + private void _testMethodNullSkip(String s) throws Exception { + MiddleSetter middle = objectMapper.readValue(s, MiddleSetter.class); - validateNotNull(outer); - validateNotNull(outer.getListMiddle()); - for (Middle middle : outer.getListMiddle()) { - testMiddle(middle); - } + testMiddleSetter(middle); } - private void testSkipNullsWithMiddleOnly(String s) throws Exception { + private void _testFieldNullSkip(String s) throws Exception { Middle middle = objectMapper.readValue(s, Middle.class); testMiddle(middle); @@ -133,6 +163,12 @@ private void testMiddle(Middle middle) { validateNotNull(middle.getListInner()); } + private void testMiddleSetter(MiddleSetter middle) { + validateNotNull(middle); + validateNotNull(middle.getField1()); + validateNotNull(middle.getListInner()); + } + private static void validateNotNull(Object o) { assertNotNull(o); } From fd36b9fb681d632cb56bc67ae481d25ee9cfef46 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Fri, 29 Mar 2024 09:56:35 +0900 Subject: [PATCH 4/5] Clean up test --- .../databind/deser/SkipNulls4441Test.java | 36 +++---------------- 1 file changed, 5 insertions(+), 31 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java index d772bafabe..5c22908b55 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/SkipNulls4441Test.java @@ -9,6 +9,7 @@ import java.beans.ConstructorProperties; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import static com.fasterxml.jackson.databind.BaseTest.a2q; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -16,18 +17,6 @@ // [databind#4441] @JsonSetter(nulls = Nulls.SKIP) doesn't work in some situations public class SkipNulls4441Test { - static class Outer { - @JsonSetter(nulls = Nulls.SKIP) - private final List listMiddle = new ArrayList<>(); - - public Outer() { - } - - public List getListMiddle() { - return listMiddle; - } - } - static class Middle { @JsonSetter(nulls = Nulls.SKIP) private final List listInner = new ArrayList<>(); @@ -60,25 +49,8 @@ public String getField1() { } } - // for method setter level Nulls.SKIP - static class OuterSetter { - private final List listMiddle = new ArrayList<>(); - - public OuterSetter() { - } - - @JsonSetter(nulls = Nulls.SKIP) - public void setListMiddle(List listMiddle) { - this.listMiddle.addAll(listMiddle); - } - - public List getListMiddle() { - return listMiddle; - } - } - static class MiddleSetter { - private final List listInner = new ArrayList<>(); + private List listInner = new ArrayList<>(); private final String field1; @ConstructorProperties({"field1"}) @@ -88,7 +60,9 @@ public MiddleSetter(String field1) { @JsonSetter(nulls = Nulls.SKIP) public void setListInner(List listInner) { - this.listInner.addAll(listInner); + // null passed here + Objects.requireNonNull(listInner); + this.listInner = listInner; } public List getListInner() { From 5359291bd92119111fe070dd6a667d9d5ea71d4a Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Fri, 29 Mar 2024 10:04:15 +0900 Subject: [PATCH 5/5] Apply to MethodProperty also --- .../jackson/databind/deser/impl/FieldProperty.java | 1 + .../jackson/databind/deser/impl/MethodProperty.java | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java index 25768fd785..2968d63be1 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java @@ -186,6 +186,7 @@ public Object deserializeSetAndReturn(JsonParser p, @Override public void set(Object instance, Object value) throws IOException { + // [databind#4441] 2.17 Fix `@JsonSetter(Nulls.SKIP)` field is not skipped up to this point if (value == null) { if (_skipNulls) { return; diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/MethodProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/MethodProperty.java index 69af26514f..33fdf20866 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/MethodProperty.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/MethodProperty.java @@ -178,6 +178,12 @@ public Object deserializeSetAndReturn(JsonParser p, @Override public final void set(Object instance, Object value) throws IOException { + // [databind#4441] 2.17 Fix `@JsonSetter(Nulls.SKIP)` Method is not skipped up to this point + if (value == null) { + if (_skipNulls) { + return; + } + } try { _setter.invoke(instance, value); } catch (Exception e) {