Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore Records' immutable fields in BeanDeserializerFactory instead of ignoring it in POJOPropertiesCollector, to preserve any annotation info the fields may be carrying. #4627

Merged
merged 7 commits into from
Jul 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
// [databind#3736] Pointless to create a SettableBeanProperty for an immutable field
return null;
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
}
prop = new FieldProperty(propDef, type, typeDeser, beanDesc.getClassAnnotations(), field);
}
JsonDeserializer<?> deser = findDeserializerFromAnnotation(ctxt, mutator);
if (deser == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public int hashCode() {
return Objects.hashCode(_field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

@yihtserns yihtserns Jul 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really understand this change, but I reverted it simply because it was part of #3737 even though it has no effect on #4626.

But I just found out that reverting this fixed #4630.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

Iterator<POJOPropertyBuilder> it = props.values().iterator();

while (it.hasNext()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/*
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}