Skip to content

Commit 28f8e0d

Browse files
odrotbohmodrotbohm
committed
DATAREST-1440 - Revisited removal of fields in incoming payloads.
We now only remove fields from the payload in case there's no @JsonAnySetter on the target entity.
1 parent 8689c1b commit 28f8e0d

File tree

3 files changed

+74
-19
lines changed

3 files changed

+74
-19
lines changed

‎spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,8 @@ <T> T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exception {
227227
JsonNode child = entry.getValue();
228228
String fieldName = entry.getKey();
229229

230-
if (!mappedProperties.hasPersistentPropertyForField(fieldName)) {
230+
if (!mappedProperties.isWritableProperty(fieldName)) {
231+
231232
i.remove();
232233
continue;
233234
}

‎spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,16 @@
2626
import java.util.Map;
2727
import java.util.Optional;
2828
import java.util.Set;
29-
import java.util.function.Predicate;
3029

3130
import org.springframework.data.mapping.PersistentEntity;
3231
import org.springframework.data.mapping.PersistentProperty;
3332
import org.springframework.util.Assert;
3433

34+
import com.fasterxml.jackson.annotation.JsonAnySetter;
3535
import com.fasterxml.jackson.databind.BeanDescription;
3636
import com.fasterxml.jackson.databind.DeserializationConfig;
3737
import com.fasterxml.jackson.databind.ObjectMapper;
3838
import com.fasterxml.jackson.databind.SerializationConfig;
39-
import com.fasterxml.jackson.databind.introspect.BasicClassIntrospector;
4039
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
4140
import com.fasterxml.jackson.databind.introspect.ClassIntrospector;
4241

@@ -50,39 +49,45 @@
5049
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
5150
class MappedProperties {
5251

53-
private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector();
54-
5552
private final Map<PersistentProperty<?>, BeanPropertyDefinition> propertyToFieldName;
5653
private final Map<String, PersistentProperty<?>> fieldNameToProperty;
5754
private final Set<BeanPropertyDefinition> unmappedProperties;
55+
private final Set<String> ignoredPropertyNames;
56+
private final boolean anySetterFound;
5857

5958
/**
6059
* Creates a new {@link MappedProperties} instance for the given {@link PersistentEntity} and {@link BeanDescription}.
6160
*
6261
* @param entity must not be {@literal null}.
6362
* @param description must not be {@literal null}.
6463
*/
65-
private MappedProperties(PersistentEntity<?, ? extends PersistentProperty<?>> entity, BeanDescription description,
66-
Predicate<PersistentProperty<?>> filter) {
64+
private MappedProperties(PersistentEntity<?, ? extends PersistentProperty<?>> entity, BeanDescription description) {
6765

6866
Assert.notNull(entity, "Entity must not be null!");
6967
Assert.notNull(description, "BeanDescription must not be null!");
7068

71-
this.propertyToFieldName = new HashMap<PersistentProperty<?>, BeanPropertyDefinition>();
72-
this.fieldNameToProperty = new HashMap<String, PersistentProperty<?>>();
73-
this.unmappedProperties = new HashSet<BeanPropertyDefinition>();
69+
this.propertyToFieldName = new HashMap<>();
70+
this.fieldNameToProperty = new HashMap<>();
71+
this.unmappedProperties = new HashSet<>();
72+
73+
this.anySetterFound = description.findAnySetterAccessor() != null;
74+
75+
// We need to call this method after findAnySetterAccessor above as that triggers the
76+
// collection of ignored properties in the first place. See
77+
// https://github.com/FasterXML/jackson-databind/issues/2531
78+
79+
this.ignoredPropertyNames = description.getIgnoredPropertyNames();
7480

7581
for (BeanPropertyDefinition property : description.findProperties()) {
7682

77-
if (description.getIgnoredPropertyNames().contains(property.getName())) {
83+
if (ignoredPropertyNames.contains(property.getName())) {
7884
continue;
7985
}
8086

8187
Optional<? extends PersistentProperty<?>> persistentProperty = //
8288
Optional.ofNullable(entity.getPersistentProperty(property.getInternalName()));
8389

84-
persistentProperty//
85-
.filter(filter) //
90+
persistentProperty //
8691
.ifPresent(it -> {
8792
propertyToFieldName.put(it, property);
8893
fieldNameToProperty.put(property.getName(), it);
@@ -105,10 +110,11 @@ private MappedProperties(PersistentEntity<?, ? extends PersistentProperty<?>> en
105110
public static MappedProperties forDeserialization(PersistentEntity<?, ?> entity, ObjectMapper mapper) {
106111

107112
DeserializationConfig config = mapper.getDeserializationConfig();
108-
BeanDescription description = INTROSPECTOR.forDeserialization(config, mapper.constructType(entity.getType()),
113+
ClassIntrospector introspector = config.getClassIntrospector();
114+
BeanDescription description = introspector.forDeserialization(config, mapper.constructType(entity.getType()),
109115
config);
110116

111-
return new MappedProperties(entity, description, it -> it.isWritable());
117+
return new MappedProperties(entity, description);
112118
}
113119

114120
/**
@@ -122,13 +128,15 @@ public static MappedProperties forDeserialization(PersistentEntity<?, ?> entity,
122128
public static MappedProperties forSerialization(PersistentEntity<?, ?> entity, ObjectMapper mapper) {
123129

124130
SerializationConfig config = mapper.getSerializationConfig();
125-
BeanDescription description = INTROSPECTOR.forSerialization(config, mapper.constructType(entity.getType()), config);
131+
ClassIntrospector introspector = config.getClassIntrospector();
132+
BeanDescription description = introspector.forSerialization(config, mapper.constructType(entity.getType()), config);
126133

127-
return new MappedProperties(entity, description, it -> true);
134+
return new MappedProperties(entity, description);
128135
}
129136

130137
public static MappedProperties none() {
131-
return new MappedProperties(Collections.emptyMap(), Collections.emptyMap(), Collections.emptySet());
138+
return new MappedProperties(Collections.emptyMap(), Collections.emptyMap(), Collections.emptySet(),
139+
Collections.emptySet(), false);
132140
}
133141

134142
/**
@@ -196,4 +204,24 @@ public boolean isMappedProperty(PersistentProperty<?> property) {
196204

197205
return propertyToFieldName.containsKey(property);
198206
}
207+
208+
/**
209+
* Returns whether the property is actually writable. I.e. whether there's a non-read-only property on the target type
210+
* or there's a catch all method annotated with {@link JsonAnySetter}.
211+
*
212+
* @param name must not be {@literal null} or empty.
213+
* @return
214+
*/
215+
public boolean isWritableProperty(String name) {
216+
217+
Assert.hasText(name, "Property name must not be null or empty!");
218+
219+
if (ignoredPropertyNames.contains(name)) {
220+
return false;
221+
}
222+
223+
PersistentProperty<?> property = fieldNameToProperty.get(name);
224+
225+
return property != null ? property.isWritable() : anySetterFound;
226+
}
199227
}

‎spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
2424
import org.springframework.data.mapping.PersistentEntity;
2525

26+
import com.fasterxml.jackson.annotation.JsonAnySetter;
2627
import com.fasterxml.jackson.annotation.JsonIgnore;
2728
import com.fasterxml.jackson.annotation.JsonProperty;
2829
import com.fasterxml.jackson.annotation.JsonProperty.Access;
@@ -90,7 +91,7 @@ public void doesNotRegardReadOnlyPropertyForDeserialization() {
9091

9192
MappedProperties properties = MappedProperties.forDeserialization(entity, mapper);
9293

93-
assertThat(properties.hasPersistentPropertyForField("anotherReadOnlyProperty")).isFalse();
94+
assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse();
9495
assertThat(properties.getPersistentProperty("readOnlyProperty")).isNull();
9596

9697
properties = MappedProperties.forSerialization(entity, mapper);
@@ -99,6 +100,21 @@ public void doesNotRegardReadOnlyPropertyForDeserialization() {
99100
assertThat(properties.getPersistentProperty("readOnlyProperty")).isNotNull();
100101
}
101102

103+
@Test // DATAREST-1440
104+
public void exposesExistanceOfCatchAllMethod() {
105+
106+
PersistentEntity<?, ?> entity = context.getRequiredPersistentEntity(SampleWithJsonAnySetter.class);
107+
108+
MappedProperties properties = MappedProperties.forDeserialization(entity, mapper);
109+
110+
assertThat(properties.isWritableProperty("someProperty")).isTrue();
111+
assertThat(properties.isWritableProperty("readOnlyProperty")).isFalse();
112+
assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse();
113+
114+
// Due to @JsonAnySetter
115+
assertThat(properties.isWritableProperty("someRandomProperty")).isTrue();
116+
}
117+
102118
static class Sample {
103119

104120
public @Transient String notExposedBySpringData;
@@ -108,4 +124,14 @@ static class Sample {
108124
public @JsonProperty(access = Access.READ_ONLY) String readOnlyProperty;
109125
public @ReadOnlyProperty String anotherReadOnlyProperty;
110126
}
127+
128+
static class SampleWithJsonAnySetter {
129+
130+
public String someProperty;
131+
public @JsonProperty(access = Access.READ_ONLY) String readOnlyProperty;
132+
public @ReadOnlyProperty String anotherReadOnlyProperty;
133+
134+
@JsonAnySetter
135+
public void set(String key, String value) {}
136+
}
111137
}

0 commit comments

Comments
 (0)