Ignore Records' immutable fields in BeanDeserializerFactory instead o…

…f ignoring it in POJOPropertiesCollector, to preserve any annotation info the fields may be carrying. (#4627)

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`

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)
 

src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java

@@ -977,8 +977,14 @@ 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;
+            // [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);
         }
         JsonDeserializer<?> deser = findDeserializerFromAnnotation(ctxt, mutator);
         if (deser == null) {

src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -434,13 +434,7 @@ protected void collectAll()
         // First: gather basic accessors
         LinkedHashMap<String, POJOPropertyBuilder> props = new LinkedHashMap<String, POJOPropertyBuilder>();
 
-        // 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<String, POJOPropertyBuilder> props)
      */
     protected void _removeUnwantedAccessor(Map<String, POJOPropertyBuilder> 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<POJOPropertyBuilder> it = props.values().iterator();
 
         while (it.hasNext()) {

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));
+    }
 }

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));
     }
 
     /*

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);
+    }
+}