From 28f8e0d397c887bf7d02f33681829b8be420d082 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 4 Nov 2019 09:09:59 +0100 Subject: [PATCH] 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. --- .../rest/webmvc/json/DomainObjectReader.java | 3 +- .../rest/webmvc/json/MappedProperties.java | 62 ++++++++++++++----- .../json/MappedPropertiesUnitTests.java | 28 ++++++++- 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java index 6ebea3a7c..489b15d44 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java @@ -227,7 +227,8 @@ public class DomainObjectReader { JsonNode child = entry.getValue(); String fieldName = entry.getKey(); - if (!mappedProperties.hasPersistentPropertyForField(fieldName)) { + if (!mappedProperties.isWritableProperty(fieldName)) { + i.remove(); continue; } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java index 563f7bffd..35953b6f3 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java @@ -26,17 +26,16 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.util.Assert; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.databind.BeanDescription; import com.fasterxml.jackson.databind.DeserializationConfig; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationConfig; -import com.fasterxml.jackson.databind.introspect.BasicClassIntrospector; import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; import com.fasterxml.jackson.databind.introspect.ClassIntrospector; @@ -50,11 +49,11 @@ import com.fasterxml.jackson.databind.introspect.ClassIntrospector; @RequiredArgsConstructor(access = AccessLevel.PRIVATE) class MappedProperties { - private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector(); - private final Map, BeanPropertyDefinition> propertyToFieldName; private final Map> fieldNameToProperty; private final Set unmappedProperties; + private final Set ignoredPropertyNames; + private final boolean anySetterFound; /** * Creates a new {@link MappedProperties} instance for the given {@link PersistentEntity} and {@link BeanDescription}. @@ -62,27 +61,33 @@ class MappedProperties { * @param entity must not be {@literal null}. * @param description must not be {@literal null}. */ - private MappedProperties(PersistentEntity> entity, BeanDescription description, - Predicate> filter) { + private MappedProperties(PersistentEntity> entity, BeanDescription description) { Assert.notNull(entity, "Entity must not be null!"); Assert.notNull(description, "BeanDescription must not be null!"); - this.propertyToFieldName = new HashMap, BeanPropertyDefinition>(); - this.fieldNameToProperty = new HashMap>(); - this.unmappedProperties = new HashSet(); + this.propertyToFieldName = new HashMap<>(); + this.fieldNameToProperty = new HashMap<>(); + this.unmappedProperties = new HashSet<>(); + + this.anySetterFound = description.findAnySetterAccessor() != null; + + // We need to call this method after findAnySetterAccessor above as that triggers the + // collection of ignored properties in the first place. See + // https://github.com/FasterXML/jackson-databind/issues/2531 + + this.ignoredPropertyNames = description.getIgnoredPropertyNames(); for (BeanPropertyDefinition property : description.findProperties()) { - if (description.getIgnoredPropertyNames().contains(property.getName())) { + if (ignoredPropertyNames.contains(property.getName())) { continue; } Optional> persistentProperty = // Optional.ofNullable(entity.getPersistentProperty(property.getInternalName())); - persistentProperty// - .filter(filter) // + persistentProperty // .ifPresent(it -> { propertyToFieldName.put(it, property); fieldNameToProperty.put(property.getName(), it); @@ -105,10 +110,11 @@ class MappedProperties { public static MappedProperties forDeserialization(PersistentEntity entity, ObjectMapper mapper) { DeserializationConfig config = mapper.getDeserializationConfig(); - BeanDescription description = INTROSPECTOR.forDeserialization(config, mapper.constructType(entity.getType()), + ClassIntrospector introspector = config.getClassIntrospector(); + BeanDescription description = introspector.forDeserialization(config, mapper.constructType(entity.getType()), config); - return new MappedProperties(entity, description, it -> it.isWritable()); + return new MappedProperties(entity, description); } /** @@ -122,13 +128,15 @@ class MappedProperties { public static MappedProperties forSerialization(PersistentEntity entity, ObjectMapper mapper) { SerializationConfig config = mapper.getSerializationConfig(); - BeanDescription description = INTROSPECTOR.forSerialization(config, mapper.constructType(entity.getType()), config); + ClassIntrospector introspector = config.getClassIntrospector(); + BeanDescription description = introspector.forSerialization(config, mapper.constructType(entity.getType()), config); - return new MappedProperties(entity, description, it -> true); + return new MappedProperties(entity, description); } public static MappedProperties none() { - return new MappedProperties(Collections.emptyMap(), Collections.emptyMap(), Collections.emptySet()); + return new MappedProperties(Collections.emptyMap(), Collections.emptyMap(), Collections.emptySet(), + Collections.emptySet(), false); } /** @@ -196,4 +204,24 @@ class MappedProperties { return propertyToFieldName.containsKey(property); } + + /** + * Returns whether the property is actually writable. I.e. whether there's a non-read-only property on the target type + * or there's a catch all method annotated with {@link JsonAnySetter}. + * + * @param name must not be {@literal null} or empty. + * @return + */ + public boolean isWritableProperty(String name) { + + Assert.hasText(name, "Property name must not be null or empty!"); + + if (ignoredPropertyNames.contains(name)) { + return false; + } + + PersistentProperty property = fieldNameToProperty.get(name); + + return property != null ? property.isWritable() : anySetterFound; + } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java index bc0917fc6..ed24ff4ab 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java @@ -23,6 +23,7 @@ import org.springframework.data.annotation.Transient; import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext; import org.springframework.data.mapping.PersistentEntity; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty.Access; @@ -90,7 +91,7 @@ public class MappedPropertiesUnitTests { MappedProperties properties = MappedProperties.forDeserialization(entity, mapper); - assertThat(properties.hasPersistentPropertyForField("anotherReadOnlyProperty")).isFalse(); + assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse(); assertThat(properties.getPersistentProperty("readOnlyProperty")).isNull(); properties = MappedProperties.forSerialization(entity, mapper); @@ -99,6 +100,21 @@ public class MappedPropertiesUnitTests { assertThat(properties.getPersistentProperty("readOnlyProperty")).isNotNull(); } + @Test // DATAREST-1440 + public void exposesExistanceOfCatchAllMethod() { + + PersistentEntity entity = context.getRequiredPersistentEntity(SampleWithJsonAnySetter.class); + + MappedProperties properties = MappedProperties.forDeserialization(entity, mapper); + + assertThat(properties.isWritableProperty("someProperty")).isTrue(); + assertThat(properties.isWritableProperty("readOnlyProperty")).isFalse(); + assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse(); + + // Due to @JsonAnySetter + assertThat(properties.isWritableProperty("someRandomProperty")).isTrue(); + } + static class Sample { public @Transient String notExposedBySpringData; @@ -108,4 +124,14 @@ public class MappedPropertiesUnitTests { public @JsonProperty(access = Access.READ_ONLY) String readOnlyProperty; public @ReadOnlyProperty String anotherReadOnlyProperty; } + + static class SampleWithJsonAnySetter { + + public String someProperty; + public @JsonProperty(access = Access.READ_ONLY) String readOnlyProperty; + public @ReadOnlyProperty String anotherReadOnlyProperty; + + @JsonAnySetter + public void set(String key, String value) {} + } }