From 8875a9cb5fc680245eb92de4755de0c47c612672 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 22 Jul 2024 20:25:35 -0700 Subject: [PATCH 1/6] Try to fix #4626, #4630 (start with failing tests) --- .../RecordWithIgnoreOverride3992Test.java | 10 +++++ .../RecordWithOverriddenInclude4630Test.java | 45 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenInclude4630Test.java diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java index faf005c4f2..a1e6478241 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java @@ -50,4 +50,14 @@ public void testHelloRecord() throws Exception { HelloRecord result = MAPPER.readValue(json, HelloRecord.class); assertNotNull(result); } + + // [databind#4626] + @Test + public void testDeserialize() throws Exception { + HelloRecord expected = new HelloRecord("hello", null); + + assertEquals(expected, MAPPER.readValue(a2q("{'text':'hello'}"), HelloRecord.class)); + assertEquals(expected, MAPPER.readValue(a2q("{'text':'hello','hidden':null}"), HelloRecord.class)); + assertEquals(expected, MAPPER.readValue(a2q("{'text':'hello','hidden':{'all': []}}"), HelloRecord.class)); + } } diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenInclude4630Test.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenInclude4630Test.java new file mode 100644 index 0000000000..a1b677f2ab --- /dev/null +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenInclude4630Test.java @@ -0,0 +1,45 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonIncludeProperties; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class RecordWithOverriddenInclude4630Test extends DatabindTestUtil +{ + record Id2Name(int id, String name) { + } + + record RecordWithJsonIncludeProperties(@JsonIncludeProperties("id") Id2Name child) { + @Override + public Id2Name child() { + return child; + } + } + + record RecordWithJsonIgnoreProperties(@JsonIgnoreProperties("name") Id2Name child) { + @Override + public Id2Name child() { + return child; + } + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + // [databind#4630] + @Test + public void testSerializeJsonIncludeProperties() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithJsonIncludeProperties(new Id2Name(123, "Bob"))); + assertEquals(a2q("{'child':{'id':123}}"), json); + } + + // [databind#4630] + @Test + public void testSerializeJsonIgnoreProperties() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithJsonIgnoreProperties(new Id2Name(123, "Bob"))); + assertEquals(a2q("{'child':{'id':123}}"), json); + } +} From 509f41bdf5d94eb8f0f194ec6fef3321352d8dd4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 22 Jul 2024 20:31:28 -0700 Subject: [PATCH 2/6] First part of fix, for #4630 --- .../jackson/databind/introspect/POJOPropertiesCollector.java | 4 ++-- .../databind/records/RecordWithIgnoreOverride3992Test.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 924492686a..6bd9dd8c78 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -1303,8 +1303,8 @@ protected void _removeUnwantedAccessor(Map props) { // 15-Jan-2023, tatu: Avoid pulling in mutators for Records; Fields mostly // since there should not be setters. - final boolean inferMutators = !isRecordType() - && _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); + // 22-Jul-2024, tatu: Actually do pull them to fix [databind#4630] + final boolean inferMutators = _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); Iterator it = props.values().iterator(); while (it.hasNext()) { diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java index a1e6478241..0bd4a91921 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithIgnoreOverride3992Test.java @@ -39,7 +39,7 @@ void add(Recursion recursion) { // [databind#3992] @Test - public void testHelloRecord() throws Exception { + public void testHelloRecord3992() throws Exception { Recursion beanWithRecursion = new Recursion(); beanWithRecursion.add(beanWithRecursion); String json = MAPPER.writer() @@ -53,7 +53,7 @@ public void testHelloRecord() throws Exception { // [databind#4626] @Test - public void testDeserialize() throws Exception { + public void testDeserializeWithOverride4626() throws Exception { HelloRecord expected = new HelloRecord("hello", null); assertEquals(expected, MAPPER.readValue(a2q("{'text':'hello'}"), HelloRecord.class)); From 52c6102dbb7683e6b6d51c43722ef814966ff716 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 22 Jul 2024 21:07:46 -0700 Subject: [PATCH 3/6] ... --- .../introspect/POJOPropertiesCollector.java | 18 +++++++++++++----- .../introspect/POJOPropertyBuilder.java | 9 +++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 6bd9dd8c78..2e4ec6d677 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -438,9 +438,9 @@ protected void collectAll() // altogether (unless we find a good reason to detect them) // 17-Apr-2023: Need Records' fields for serialization for cases // like [databind#3628], [databind#3895] and [databind#3992] - if (!isRecordType() || _forSerialization) { - _addFields(props); // note: populates _fieldRenameMappings - } + // 22-Jul-2024, tatu: ... and for deserialization sometimes too [databind#4626] + _addFields(props); // note: populates _fieldRenameMappings + _addMethods(props); // 25-Jan-2016, tatu: Avoid introspecting (constructor-)creators for non-static // inner classes, see [databind#1502] @@ -455,7 +455,7 @@ protected void collectAll() // since logic relies on knowing exactly which accessor has which annotation _removeUnwantedProperties(props); // and then remove unneeded accessors (wrt read-only, read-write) - _removeUnwantedAccessor(props); + _removeUnwantedAccessors(props); // Rename remaining properties _renameProperties(props); @@ -485,6 +485,14 @@ protected void collectAll() property.trimByVisibility(); } + // 22-Jul-2024, tatu: And now drop Record Fields once their effect (annotations) + // have worn off + if (_isRecordType) { + for (POJOPropertyBuilder property : props.values()) { + property.removeFields(); + } + } + // and, if required, apply wrapper name: note, MUST be done after // annotations are merged. if (_config.isEnabled(MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME)) { @@ -1299,7 +1307,7 @@ protected void _removeUnwantedProperties(Map props) * based on read/write settings and rules for "pulling in" accessors * (or not). */ - protected void _removeUnwantedAccessor(Map props) + protected void _removeUnwantedAccessors(Map props) { // 15-Jan-2023, tatu: Avoid pulling in mutators for Records; Fields mostly // since there should not be setters. 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 dc0c21fdba..5cf537169d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java @@ -996,6 +996,15 @@ public void removeConstructors() { _ctorParameters = null; } + /** + * Mutator that will simply drop any fields property may have. + * + * @since 2.18 + */ + public void removeFields() { + _fields = null; + } + /** * Method called to trim unnecessary entries, such as implicit * getter if there is an explict one available. This is important From 2164436a39e9fe759c8f2c4eceeab5b17605b4fa Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 24 Jul 2024 19:05:29 -0700 Subject: [PATCH 4/6] Fix the fix, now all passing --- release-notes/VERSION-2.x | 6 ++++++ .../databind/introspect/POJOPropertiesCollector.java | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index f248d4a06a..e203be20b2 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -59,6 +59,12 @@ Project: jackson-databind (reported by Eduard G) #4617: Record property serialization order not preserved (reported by @GeorgiPetkov) +#4626: `@JsonIgnore` on Record property ignored for deserialization, if + there is getter override + (reported by Sim Y-T) +#4630: `@JsonIncludeProperties`, `@JsonIgnoreProperties` ignored when serializing + Records, if there is getter override + (reported by Sim Y-T) #4634: `@JsonAnySetter` not working when annotated on both constructor parameter & field (contributed by Sim Y-T) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 2e4ec6d677..e547995e54 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -485,9 +485,9 @@ protected void collectAll() property.trimByVisibility(); } - // 22-Jul-2024, tatu: And now drop Record Fields once their effect (annotations) - // have worn off - if (_isRecordType) { + // 22-Jul-2024, tatu: And now drop Record Fields once their effect + // (annotations) has been applied. But just for deserialization + if (_isRecordType && !_forSerialization) { for (POJOPropertyBuilder property : props.values()) { property.removeFields(); } From 80777bc5b2de43467518f5f9057fd7e755c10873 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 24 Jul 2024 19:07:43 -0700 Subject: [PATCH 5/6] Remove an unnecessary change --- .../jackson/databind/introspect/POJOPropertiesCollector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index e547995e54..bc5267f3cf 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -1311,8 +1311,8 @@ protected void _removeUnwantedAccessors(Map props) { // 15-Jan-2023, tatu: Avoid pulling in mutators for Records; Fields mostly // since there should not be setters. - // 22-Jul-2024, tatu: Actually do pull them to fix [databind#4630] - final boolean inferMutators = _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); + final boolean inferMutators = _isRecordType + && _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); Iterator it = props.values().iterator(); while (it.hasNext()) { From 1907cfb93375b0287fb51192e44d4d095b08abe4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 24 Jul 2024 19:09:07 -0700 Subject: [PATCH 6/6] ... uhh. Ok, was not unnecessary after all --- .../jackson/databind/introspect/POJOPropertiesCollector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index bc5267f3cf..e547995e54 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -1311,8 +1311,8 @@ protected void _removeUnwantedAccessors(Map props) { // 15-Jan-2023, tatu: Avoid pulling in mutators for Records; Fields mostly // since there should not be setters. - final boolean inferMutators = _isRecordType - && _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); + // 22-Jul-2024, tatu: Actually do pull them to fix [databind#4630] + final boolean inferMutators = _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); Iterator it = props.values().iterator(); while (it.hasNext()) {