From f1b1078c28f35922812507a1ad74db0ac9243ced Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Fri, 19 Jul 2024 23:32:27 +0800 Subject: [PATCH 1/6] Add test cases to verify that Records' deserialization will ignore a property when its accessor method is annotated with @JsonIgnore. --- .../databind/records/RecordWithJsonIgnoreTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java index 9c372884e9..4fb02ee485 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java @@ -81,9 +81,11 @@ public void testSerializeJsonIgnoreAccessorRecord() throws Exception { @Test public void testDeserializeJsonIgnoreAccessorRecord() throws Exception { - RecordWithIgnoreAccessor value = MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", - RecordWithIgnoreAccessor.class); - assertEquals(new RecordWithIgnoreAccessor(123, null), value); + RecordWithIgnoreAccessor expected = new RecordWithIgnoreAccessor(123, null); + + assertEquals(expected, MAPPER.readValue("{\"id\":123}", RecordWithIgnoreAccessor.class)); + assertEquals(expected, MAPPER.readValue("{\"id\":123,\"name\":null}", RecordWithIgnoreAccessor.class)); + assertEquals(expected, MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnoreAccessor.class)); } /* From 2096c424f983a7bbd80a0571a66a9848602a2d5e Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Sat, 20 Jul 2024 02:43:46 +0800 Subject: [PATCH 2/6] Add test cases to demonstrate that deserialization is not ignoring a property when a Record's component has @JsonIgnore + the corresponding accessor method is overridden without @JsonIgnore. --- .../records/RecordWithIgnoreOverride3992Test.java | 10 ++++++++++ 1 file changed, 10 insertions(+) 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)); + } } From 2b4cf9f5b9187a60ae83fd35636cb6a47a535092 Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Sat, 20 Jul 2024 02:50:09 +0800 Subject: [PATCH 3/6] Ignore Records' immutable fields in BeanDeserializerFactory instead of ignoring it in POJOPropertiesCollector, to preserve any annotation info the fields may be carrying. --- .../databind/deser/BeanDeserializerFactory.java | 8 ++++++-- .../jackson/databind/introspect/AnnotatedField.java | 8 ++++++++ .../introspect/POJOPropertiesCollector.java | 13 ++----------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index 80d9d492c2..cb9cb872a5 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -977,8 +977,12 @@ protected SettableBeanProperty constructSettableProperty(DeserializationContext beanDesc.getClassAnnotations(), (AnnotatedMethod) mutator); } else { // 08-Sep-2016, tatu: wonder if we should verify it is `AnnotatedField` to be safe? - prop = new FieldProperty(propDef, type, typeDeser, - beanDesc.getClassAnnotations(), (AnnotatedField) mutator); + AnnotatedField field = (AnnotatedField) mutator; + if (field.isImmutable()) { + // [databind#3736] Pointless to create a SettableBeanProperty for an immutable field + return null; + } + prop = new FieldProperty(propDef, type, typeDeser, beanDesc.getClassAnnotations(), field); } JsonDeserializer deser = findDeserializerFromAnnotation(ctxt, mutator); if (deser == null) { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java index be182b72bf..371c212f0a 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java @@ -128,6 +128,14 @@ public Object getValue(Object pojo) throws IllegalArgumentException */ public boolean isTransient() { return Modifier.isTransient(getModifiers()); } + /** + * @since 2.18 + */ + public boolean isImmutable() { + // Records' fields can't mutated via reflection (JDK-8247517) + return _typeContext.resolveType(getDeclaringClass()).isRecordType(); + } + @Override public int hashCode() { return Objects.hashCode(_field); 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..14481405a7 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -434,13 +434,7 @@ protected void collectAll() // First: gather basic accessors LinkedHashMap props = new LinkedHashMap(); - // 15-Jan-2023, tatu: [databind#3736] Let's avoid detecting fields of Records - // 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 - } + _addFields(props); // note: populates _fieldRenameMappings _addMethods(props); // 25-Jan-2016, tatu: Avoid introspecting (constructor-)creators for non-static // inner classes, see [databind#1502] @@ -1301,10 +1295,7 @@ protected void _removeUnwantedProperties(Map props) */ 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); + final boolean inferMutators = _config.isEnabled(MapperFeature.INFER_PROPERTY_MUTATORS); Iterator it = props.values().iterator(); while (it.hasNext()) { From 2ce22e31e2bee6eb9c6ee06daeccfdec3c4de11c Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Mon, 22 Jul 2024 01:36:04 +0800 Subject: [PATCH 4/6] Add test cases to demonstrate that serialization is not ignoring a property when a Record's component has @JsonIncludeProperties/@JsonIgnoreProperties + the corresponding accessor method is overridden. --- .../RecordWithOverriddenAccessorTest.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenAccessorTest.java diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenAccessorTest.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenAccessorTest.java new file mode 100644 index 0000000000..f0626445c1 --- /dev/null +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordWithOverriddenAccessorTest.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 RecordWithOverriddenAccessorTest 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 8eb2f55d13c183e2f54e9e24d9201c77d7c82676 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 21 Jul 2024 15:21:12 -0700 Subject: [PATCH 5/6] Change the way (immutable) Record field is dropped --- .../jackson/databind/deser/BeanDeserializerFactory.java | 6 ++++-- .../jackson/databind/introspect/AnnotatedField.java | 8 -------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index cb9cb872a5..53bdb1fa7d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -978,8 +978,10 @@ protected SettableBeanProperty constructSettableProperty(DeserializationContext } else { // 08-Sep-2016, tatu: wonder if we should verify it is `AnnotatedField` to be safe? AnnotatedField field = (AnnotatedField) mutator; - if (field.isImmutable()) { - // [databind#3736] Pointless to create a SettableBeanProperty for an immutable field + // [databind#3736] Pointless to create a SettableBeanProperty for an immutable field + // Records' fields can't mutated via reflection (JDK-8247517) + // (also see [databind#4626] + if (beanDesc.isRecordType()) { return null; } prop = new FieldProperty(propDef, type, typeDeser, beanDesc.getClassAnnotations(), field); diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java index 371c212f0a..be182b72bf 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java @@ -128,14 +128,6 @@ public Object getValue(Object pojo) throws IllegalArgumentException */ public boolean isTransient() { return Modifier.isTransient(getModifiers()); } - /** - * @since 2.18 - */ - public boolean isImmutable() { - // Records' fields can't mutated via reflection (JDK-8247517) - return _typeContext.resolveType(getDeclaringClass()).isRecordType(); - } - @Override public int hashCode() { return Objects.hashCode(_field); From ae1083d33d5ca2c89af2192bfa2cdd3da4f4268b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 21 Jul 2024 15:28:08 -0700 Subject: [PATCH 6/6] Update release notes --- release-notes/CREDITS-2.x | 5 +++-- release-notes/VERSION-2.x | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 8961e82598..c612a32684 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -10,6 +10,7 @@ Co-Authors (with only partial listings below): * Joo Hyuk Kim (JooHyukKim@github) * PJ Fanning (pjfanning@github) +* Sim Yih Tsern (yihtsern@github) ---------------------------------------------------------------------------- @@ -1624,9 +1625,9 @@ Sim Yih Tsern (yihtsern@github) * Contributed fix for #3897: 2.15.0 breaks deserialization when POJO/Record only has a single field and is marked `Access.WRITE_ONLY` (2.15.1) - * Contributed fux fix #3968: Records with additional constructors failed to deserialize + * Contributed fix fix #3968: Records with additional constructors failed to deserialize (2.15.3) - + ... and many more Ajay Siwach (Siwach16@github) * Contributed #3637: Add enum features into `@JsonFormat.Feature` diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 251ffdb7ac..23e1b40e3d 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -57,6 +57,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 + (contributed by @yihtserns) +#4630: `@JsonIncludeProperties`, `@JsonIgnoreProperties` ignored when serializing Records, + if there is getter override + (contributed by @yihtserns) 2.17.2 (05-Jul-2024)