Fix PUT merge handling for polymorphic properties.

We now replace the original value of polymorphic properties with the newly deserialized one if the type of the new one is different from the old one. We still copy all JSON ignored properties to the new instance to make sure that non-exposed, server-side state is retained. We apply the same handling for explicitly immutable source and/or target types.

Fixes #2130.

Author: odrotbohm

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

@@ -42,6 +42,7 @@
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 
@@ -124,6 +125,7 @@ public <T> T readPut(final ObjectNode source, T target, final ObjectMapper mappe
 	 * @param mapper must not be {@literal null}.
 	 * @return
 	 */
+	@Nullable
 	<T> T mergeForPut(T source, T target, final ObjectMapper mapper) {
 
 		Assert.notNull(mapper, "ObjectMapper must not be null!");
@@ -132,41 +134,53 @@ <T> T mergeForPut(T source, T target, final ObjectMapper mapper) {
 			return source;
 		}
 
-		Class<? extends Object> type = target.getClass();
+		boolean isTypeChange = !source.getClass().isInstance(target);
 
-		return entities.getPersistentEntity(type) //
-				.filter(it -> !it.isImmutable()) //
+		boolean immutableTarget = entities.getPersistentEntity(target.getClass())
+				.map(PersistentEntity::isImmutable)
+				.orElse(true); // Not a Spring Data managed type -> no detailed merging
+
+		return entities.getPersistentEntity(isTypeChange ? source.getClass() : target.getClass()) //
 				.map(it -> {
 
+					MappedProperties properties = MappedProperties.forDeserialization(it, mapper);
+
+					if (isTypeChange || immutableTarget || it.isImmutable()) {
+
+						copyRemainingProperties(properties.getIgnoredProperties(), target, source);
+
+						return source;
+					}
+
 					MergingPropertyHandler propertyHandler = new MergingPropertyHandler(source, target, it, mapper);
 
 					it.doWithProperties(propertyHandler);
 					it.doWithAssociations(new LinkedAssociationSkippingAssociationHandler(associationLinks, propertyHandler));
 
 					// Need to copy unmapped properties as the PersistentProperty model currently does not contain any transient
 					// properties
-					copyRemainingProperties(propertyHandler.getProperties(), source, target);
+					copyRemainingProperties(properties.getSpringDataUnmappedProperties(), source, target);
 
 					return target;
 
 				}).orElse(source);
 	}
 
 	/**
-	 * Copies the unmapped properties of the given {@link MappedProperties} from the source object to the target instance.
+	 * Copies the given properties from the source object to the target instance.
 	 *
-	 * @param properties must not be {@literal null}.
+	 * @param propertyNames must not be {@literal null}.
 	 * @param source must not be {@literal null}.
 	 * @param target must not be {@literal null}.
 	 */
-	private static void copyRemainingProperties(MappedProperties properties, Object source, Object target) {
+	private static void copyRemainingProperties(Iterable<String> propertyNames, Object source, Object target) {
 
 		PropertyAccessor sourceFieldAccessor = PropertyAccessorFactory.forDirectFieldAccess(source);
 		PropertyAccessor sourcePropertyAccessor = PropertyAccessorFactory.forBeanPropertyAccess(source);
 		PropertyAccessor targetFieldAccessor = PropertyAccessorFactory.forDirectFieldAccess(target);
 		PropertyAccessor targetPropertyAccessor = PropertyAccessorFactory.forBeanPropertyAccess(target);
 
-		for (String property : properties.getSpringDataUnmappedProperties()) {
+		for (String property : propertyNames) {
 
 			// If there's a field we can just copy it.
 			if (targetFieldAccessor.isWritableProperty(property)) {

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

@@ -199,6 +199,16 @@ public Iterable<String> getSpringDataUnmappedProperties() {
 		return result;
 	}
 
+	/**
+	 * Returns all property names of ignored properties.
+	 *
+	 * @return will never be {@literal null}.
+	 * @since 3.5.11, 3.6.4
+	 */
+	public Iterable<String> getIgnoredProperties() {
+		return ignoredPropertyNames;
+	}
+
 	/**
 	 * Returns whether the given {@link PersistentProperty} is mapped, i.e. known to both Jackson and Spring Data.
 	 *

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/package-info.java

@@ -13,8 +13,5 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-/**
- *
- * @author Oliver Gierke
- */
+@org.springframework.lang.NonNullApi
 package org.springframework.data.rest.webmvc.json;

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

@@ -55,6 +55,9 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.fasterxml.jackson.annotation.JsonTypeInfo.As;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
@@ -104,6 +107,8 @@ void setUp() {
 		mappingContext.getPersistentEntity(Note.class);
 		mappingContext.getPersistentEntity(WithNullCollection.class);
 		mappingContext.getPersistentEntity(ArrayHolder.class);
+		mappingContext.getPersistentEntity(Apple.class);
+		mappingContext.getPersistentEntity(Pear.class);
 		mappingContext.afterPropertiesSet();
 
 		this.entities = new PersistentEntities(Collections.singleton(mappingContext));
@@ -587,6 +592,32 @@ void arraysCanBeResizedDuringMerge() throws Exception {
 		assertThat(updated.array).containsExactly("new");
 	}
 
+	@Test // #2130
+	void writesPolymorphicArrayWithSwitchedItemForPut() throws Exception {
+
+		Apple apple = new Apple();
+		apple.apple = "apple";
+		apple.color = "red";
+		apple.ignored = "ignored";
+
+		Pear pear = new Pear();
+		pear.pear = "pear";
+
+		Fruit result = reader.mergeForPut(pear, apple, new ObjectMapper());
+
+		assertThat(result).isInstanceOfSatisfying(Pear.class, it -> {
+
+			// Exposed property is wiped as expected for PUT
+			assertThat(it.color).isNull();
+
+			// Non-exposed state is transferred
+			assertThat(it.ignored).isEqualTo("ignored");
+
+			// Type specific state applied, too
+			assertThat(it.pear).isEqualTo("pear");
+		});
+	}
+
 	@SuppressWarnings("unchecked")
 	private static <T> T as(Object source, Class<T> type) {
 
@@ -816,4 +847,32 @@ static class WithNullCollection {
 	static class ArrayHolder {
 		String[] array;
 	}
+
+	// DATAREST-1026
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Basket {
+
+		@Id Long id;
+		List<Fruit> fruits;
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = As.PROPERTY, property = "type")
+	@JsonSubTypes({ @JsonSubTypes.Type(name = "Apple", value = Apple.class),
+			@JsonSubTypes.Type(name = "Pear", value = Pear.class) })
+	static class Fruit {
+		String color;
+		@JsonIgnore String ignored;
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Apple extends Fruit {
+		String apple;
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Pear extends Fruit {
+		String pear;
+	}
 }

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

@@ -115,6 +115,11 @@ void exposesExistanceOfCatchAllMethod() {
 		assertThat(properties.isWritableProperty("someRandomProperty")).isTrue();
 	}
 
+	@Test // #2130
+	void exposesIgnoredProperties() {
+		assertThat(properties.getIgnoredProperties()).contains("notExposedByJackson");
+	}
+
 	static class Sample {
 
 		public @Transient String notExposedBySpringData;