From b082811e75cbf8063f8df2162a84b1c31e0b9602 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 16 Jan 2023 20:31:11 -0800 Subject: [PATCH 1/4] Fix #190: update Afterburner to use same logic as jackson-databind default for custom filters --- .../ser/ObjectFieldPropertyWriter.java | 8 +- .../ser/ObjectMethodPropertyWriter.java | 8 +- .../ser/StringFieldPropertyWriter.java | 8 +- .../ser/StringMethodPropertyWriter.java | 8 +- .../ser/filter/JsonIncludeCustomTest.java | 160 ++++++++++++++++++ .../ser/filter/JsonIncludeCustomTest.java | 140 +++++++++++++++ 6 files changed, 320 insertions(+), 12 deletions(-) create mode 100644 afterburner/src/test/java/com/fasterxml/jackson/module/afterburner/ser/filter/JsonIncludeCustomTest.java create mode 100644 blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java diff --git a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectFieldPropertyWriter.java b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectFieldPropertyWriter.java index ec22b91d0..aba2e68f3 100644 --- a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectFieldPropertyWriter.java +++ b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectFieldPropertyWriter.java @@ -49,12 +49,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } diff --git a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectMethodPropertyWriter.java b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectMethodPropertyWriter.java index fe3373300..ea6da95e6 100644 --- a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectMethodPropertyWriter.java +++ b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/ObjectMethodPropertyWriter.java @@ -49,12 +49,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } diff --git a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringFieldPropertyWriter.java b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringFieldPropertyWriter.java index fcb6a12b5..fa5b03ad5 100644 --- a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringFieldPropertyWriter.java +++ b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringFieldPropertyWriter.java @@ -48,12 +48,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } diff --git a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringMethodPropertyWriter.java b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringMethodPropertyWriter.java index 7e6bdf954..763713890 100644 --- a/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringMethodPropertyWriter.java +++ b/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/ser/StringMethodPropertyWriter.java @@ -48,12 +48,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } diff --git a/afterburner/src/test/java/com/fasterxml/jackson/module/afterburner/ser/filter/JsonIncludeCustomTest.java b/afterburner/src/test/java/com/fasterxml/jackson/module/afterburner/ser/filter/JsonIncludeCustomTest.java new file mode 100644 index 000000000..20b8c5a62 --- /dev/null +++ b/afterburner/src/test/java/com/fasterxml/jackson/module/afterburner/ser/filter/JsonIncludeCustomTest.java @@ -0,0 +1,160 @@ +package com.fasterxml.jackson.module.afterburner.ser.filter; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import com.fasterxml.jackson.annotation.*; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.module.afterburner.AfterburnerTestBase; + +// Tests for [databind#888] +public class JsonIncludeCustomTest extends AfterburnerTestBase +{ + static class FooFilter { + @Override + public boolean equals(Object other) { + if (other == null) { // do NOT filter out nulls + return false; + } + // in fact, only filter out exact String "foo" + return "foo".equals(other); + } + } + + // for testing prob with `equals(null)` which SHOULD be allowed + static class BrokenFilter { + @Override + public boolean equals(Object other) { + /*String str = */ other.toString(); + return false; + } + } + + static class FooBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=FooFilter.class) + public String value; + + public FooBean(String v) { value = v; } + } + + static class FooMapBean { + @JsonInclude(content=JsonInclude.Include.CUSTOM, + contentFilter=FooFilter.class) + public Map stuff = new LinkedHashMap(); + + public FooMapBean add(String key, String value) { + stuff.put(key, value); + return this; + } + } + + static class BrokenBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=BrokenFilter.class) + public String value; + + public BrokenBean(String v) { value = v; } + } + + static class BrokenBean2 { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=BrokenFilter.class) + public Map value; + + public BrokenBean2(Map v) { value = v; } + } + + // [databind#3481] + static class CountingFooFilter { + public final static AtomicInteger counter = new AtomicInteger(0); + + @Override + public boolean equals(Object other) { + counter.incrementAndGet(); + return "foo".equals(other); + } + } + + static class CountingFooBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=CountingFooFilter.class) + public String value; + + public CountingFooBean(String v) { value = v; } + } + + /* + /********************************************************** + /* Test methods, success + /********************************************************** + */ + + private final ObjectMapper MAPPER = newObjectMapper(); + + public void testSimpleCustomFilter() throws Exception + { + assertEquals(a2q("{'value':'x'}"), MAPPER.writeValueAsString(new FooBean("x"))); + assertEquals("{}", MAPPER.writeValueAsString(new FooBean("foo"))); + } + + public void testCustomFilterWithMap() throws Exception + { + FooMapBean input = new FooMapBean() + .add("a", "1") + .add("b", "foo") + .add("c", "2"); + + assertEquals(a2q("{'stuff':{'a':'1','c':'2'}}"), MAPPER.writeValueAsString(input)); + } + + // [databind#3481] + public void testRepeatedCalls() throws Exception + { + CountingFooFilter.counter.set(0); + + assertEquals(a2q("{'value':'x'}"), + MAPPER.writeValueAsString(new CountingFooBean("x"))); + assertEquals(1, CountingFooFilter.counter.get()); + + assertEquals("{}", MAPPER.writeValueAsString(new CountingFooBean("foo"))); + assertEquals(2, CountingFooFilter.counter.get()); + + // except filter will be called again for `null`s, as per [databind#3481] + assertEquals(a2q("{'value':null}"), MAPPER.writeValueAsString(new CountingFooBean(null))); + assertEquals(3, CountingFooFilter.counter.get()); + } + + /* + /********************************************************** + /* Test methods, fail handling + /********************************************************** + */ + + public void testBrokenFilterString() throws Exception + { + try { + String json = MAPPER.writeValueAsString(new BrokenBean(null)); + fail("Should not pass, produced: "+json); + } catch (JsonMappingException e) { + // 20-Jun-2022, tatu: Actual message seems to vary across JDKs... + verifyException(e, "Problem determining whether filter"); + verifyException(e, "should filter out `null` values"); + } + } + + public void testBrokenFilterMap() throws Exception + { + try { + String json = MAPPER.writeValueAsString(new BrokenBean2(null)); + fail("Should not pass, produced: "+json); + } catch (JsonMappingException e) { + // 20-Jun-2022, tatu: Actual message seems to vary across JDKs... + verifyException(e, "Problem determining whether filter"); + verifyException(e, "should filter out `null` values"); + } + } +} diff --git a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java new file mode 100644 index 000000000..9882776b7 --- /dev/null +++ b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java @@ -0,0 +1,140 @@ +package com.fasterxml.jackson.module.blackbird.ser.filter; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import com.fasterxml.jackson.annotation.*; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.module.blackbird.BlackbirdTestBase; + +// Tests for [databind#888] +public class JsonIncludeCustomTest extends BlackbirdTestBase +{ + static class FooFilter { + @Override + public boolean equals(Object other) { + if (other == null) { // do NOT filter out nulls + return false; + } + // in fact, only filter out exact String "foo" + return "foo".equals(other); + } + } + + // for testing prob with `equals(null)` which SHOULD be allowed + static class BrokenFilter { + @Override + public boolean equals(Object other) { + /*String str = */ other.toString(); + return false; + } + } + + static class FooBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=FooFilter.class) + public String value; + + public FooBean(String v) { value = v; } + } + + static class FooMapBean { + @JsonInclude(content=JsonInclude.Include.CUSTOM, + contentFilter=FooFilter.class) + public Map stuff = new LinkedHashMap(); + + public FooMapBean add(String key, String value) { + stuff.put(key, value); + return this; + } + } + + static class BrokenBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=BrokenFilter.class) + public String value; + + public BrokenBean(String v) { value = v; } + } + + // [databind#3481] + static class CountingFooFilter { + public final static AtomicInteger counter = new AtomicInteger(0); + + @Override + public boolean equals(Object other) { + counter.incrementAndGet(); + return "foo".equals(other); + } + } + + static class CountingFooBean { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=CountingFooFilter.class) + public String value; + + public CountingFooBean(String v) { value = v; } + } + + /* + /********************************************************** + /* Test methods, success + /********************************************************** + */ + + private final ObjectMapper MAPPER = newObjectMapper(); + + public void testSimpleCustomFilter() throws Exception + { + assertEquals(a2q("{'value':'x'}"), MAPPER.writeValueAsString(new FooBean("x"))); + assertEquals("{}", MAPPER.writeValueAsString(new FooBean("foo"))); + } + + public void testCustomFilterWithMap() throws Exception + { + FooMapBean input = new FooMapBean() + .add("a", "1") + .add("b", "foo") + .add("c", "2"); + + assertEquals(a2q("{'stuff':{'a':'1','c':'2'}}"), MAPPER.writeValueAsString(input)); + } + + // [databind#3481] + public void testRepeatedCalls() throws Exception + { + CountingFooFilter.counter.set(0); + + assertEquals(a2q("{'value':'x'}"), + MAPPER.writeValueAsString(new CountingFooBean("x"))); + assertEquals(1, CountingFooFilter.counter.get()); + + assertEquals("{}", MAPPER.writeValueAsString(new CountingFooBean("foo"))); + assertEquals(2, CountingFooFilter.counter.get()); + + // except filter will be called again for `null`s, as per [databind#3481] + assertEquals(a2q("{'value':null}"), MAPPER.writeValueAsString(new CountingFooBean(null))); + assertEquals(3, CountingFooFilter.counter.get()); + } + + /* + /********************************************************** + /* Test methods, fail handling + /********************************************************** + */ + + public void testBrokenFilter() throws Exception + { + try { + String json = MAPPER.writeValueAsString(new BrokenBean(null)); + fail("Should not pass, produced: "+json); + } catch (JsonMappingException e) { + // 20-Jun-2022, tatu: Actual message seems to vary across JDKs... + verifyException(e, "Problem determining whether filter"); + verifyException(e, "should filter out `null` values"); + } + } +} From b219dfafdd73f6c4db2e4cdcaf366d159bc1cf89 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 16 Jan 2023 20:36:31 -0800 Subject: [PATCH 2/4] minor test update --- .../ser/filter/JsonIncludeCustomTest.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java index 9882776b7..c6f58a447 100644 --- a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java +++ b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/ser/filter/JsonIncludeCustomTest.java @@ -60,6 +60,14 @@ static class BrokenBean { public BrokenBean(String v) { value = v; } } + static class BrokenBean2 { + @JsonInclude(value=JsonInclude.Include.CUSTOM, + valueFilter=BrokenFilter.class) + public Map value; + + public BrokenBean2(Map v) { value = v; } + } + // [databind#3481] static class CountingFooFilter { public final static AtomicInteger counter = new AtomicInteger(0); @@ -125,8 +133,8 @@ public void testRepeatedCalls() throws Exception /* Test methods, fail handling /********************************************************** */ - - public void testBrokenFilter() throws Exception + + public void testBrokenFilterString() throws Exception { try { String json = MAPPER.writeValueAsString(new BrokenBean(null)); @@ -137,4 +145,16 @@ public void testBrokenFilter() throws Exception verifyException(e, "should filter out `null` values"); } } + + public void testBrokenFilterMap() throws Exception + { + try { + String json = MAPPER.writeValueAsString(new BrokenBean2(null)); + fail("Should not pass, produced: "+json); + } catch (JsonMappingException e) { + // 20-Jun-2022, tatu: Actual message seems to vary across JDKs... + verifyException(e, "Problem determining whether filter"); + verifyException(e, "should filter out `null` values"); + } + } } From 385ce16f315bfaa8d00f6fa0192632b9eed317ce Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 16 Jan 2023 20:40:33 -0800 Subject: [PATCH 3/4] Add fixes to Blackbird, even if we cannot quite reproduce the failure. --- .../module/blackbird/ser/ObjectPropertyWriter.java | 8 +++++--- .../module/blackbird/ser/StringPropertyWriter.java | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/ObjectPropertyWriter.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/ObjectPropertyWriter.java index ea5666a3e..f2ee09c10 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/ObjectPropertyWriter.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/ObjectPropertyWriter.java @@ -57,12 +57,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/StringPropertyWriter.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/StringPropertyWriter.java index 68b24f06e..145dd07d3 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/StringPropertyWriter.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/StringPropertyWriter.java @@ -57,12 +57,14 @@ public final void serializeAsField(Object bean, JsonGenerator gen, SerializerPro } // Null (etc) handling; copied from super-class impl if (value == null) { + // 20-Jun-2022, tatu: Defer checking of null, see [databind#3481] + if ((_suppressableValue != null) + && prov.includeFilterSuppressNulls(_suppressableValue)) { + return; + } if (_nullSerializer != null) { gen.writeFieldName(_fastName); _nullSerializer.serialize(null, gen, prov); - } else if (!_suppressNulls) { - gen.writeFieldName(_fastName); - prov.defaultSerializeNull(gen); } return; } From cd7450a98e9e63aada8d776e83dddcd4afe56e7e Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 16 Jan 2023 20:42:33 -0800 Subject: [PATCH 4/4] Update release notes --- release-notes/VERSION-2.x | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index d7f4177ce..2301779df 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -21,7 +21,8 @@ Active maintainers: 2.15.0 (not yet released) -- +#190: Filter annotated by JsonInclude.Include.CUSTOM does not get called if + property is null with Afterburner/Blackbird module registered 2.14.1 (21-Nov-2022)