From aafba8790fad349095b9e59ee5082386f34d601e Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 25 Jul 2024 08:36:59 -0700 Subject: [PATCH] Try to fix #4626, #4630 (start with failing tests) (#4635) --- release-notes/VERSION-2.x | 6 +++ .../introspect/POJOPropertiesCollector.java | 22 ++++++--- .../introspect/POJOPropertyBuilder.java | 9 ++++ .../RecordWithIgnoreOverride3992Test.java | 12 ++++- .../RecordWithOverriddenInclude4630Test.java | 45 +++++++++++++++++++ 5 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenInclude4630Test.java 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 924492686a..e547995e54 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) has been applied. But just for deserialization + if (_isRecordType && !_forSerialization) { + 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,12 +1307,12 @@ 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. - 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/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 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..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() @@ -50,4 +50,14 @@ public void testHelloRecord() throws Exception { HelloRecord result = MAPPER.readValue(json, HelloRecord.class); assertNotNull(result); } + + // [databind#4626] + @Test + public void testDeserializeWithOverride4626() 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); + } +}