From 571dd40b82d58445c4da10ff4021af636b0ed49f Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 30 Aug 2023 21:26:40 +0200 Subject: [PATCH] Reinstate new object addition for collections in patch operations. The removal of manual collection value appendance for primitives in the context of GH-2261 lead to complex object to be appended onto a collection not being deserialized at all. This deserialization is now reinstantiated with the value to be added looked up by reading the parent node into the collections element type using a JSON Pointer expression of /$propertyName/$index. This is done to make sure that @JsonDeserialize annotations on the property kick in for the deserialization of the individual elements. We now also shortcut the entire merge algorithm for collections that are empty, contain primitives or enums. Fixes GH-2287. --- .../rest/webmvc/json/DomainObjectReader.java | 54 ++++++++++++++++--- .../json/DomainObjectReaderUnitTests.java | 32 +++++++++++ 2 files changed, 78 insertions(+), 8 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 94b75e14a..f85b96b4f 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.rest.webmvc.json; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; @@ -24,6 +25,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.function.IntFunction; import org.springframework.beans.PropertyAccessor; import org.springframework.beans.PropertyAccessorFactory; @@ -44,6 +46,7 @@ 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.ClassUtils; import org.springframework.util.ObjectUtils; import com.fasterxml.jackson.databind.JsonNode; @@ -264,7 +267,10 @@ public class DomainObjectReader { if (child.isArray()) { - if (handleArray(child, it, mapper, property.getTypeInformation())) { + IntFunction rawValues = index -> readRawCollectionElement(property.getComponentType(), fieldName, + index, root, mapper); + + if (handleArray(child, it, mapper, property.getTypeInformation(), rawValues)) { i.remove(); } @@ -304,6 +310,16 @@ public class DomainObjectReader { return mapper.readerForUpdating(target).readValue(root); } + private static Object readRawCollectionElement(Class elementType, String fieldName, int index, JsonNode root, + ObjectMapper mapper) { + + try { + return mapper.readerFor(elementType).at("/" + fieldName + "/" + index).readValue(root); + } catch (IOException o_O) { + throw new RuntimeException(o_O); + } + } + /** * Handles the given {@link JsonNode} by treating it as {@link ArrayNode} and the given source value as * {@link Collection}-like value. Looks up the actual type to handle from the potentially available first element, @@ -316,7 +332,8 @@ public class DomainObjectReader { * @return * @throws Exception */ - private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation collectionType) { + private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation collectionType, + IntFunction rawValues) { Collection collection = ifCollection(source); @@ -324,7 +341,8 @@ public class DomainObjectReader { return false; } - return execute(() -> handleArrayNode((ArrayNode) node, collection, mapper, collectionType.getComponentType())); + return execute( + () -> handleArrayNode((ArrayNode) node, collection, mapper, collectionType.getComponentType(), rawValues)); } /** @@ -337,27 +355,47 @@ public class DomainObjectReader { * @return whether an object merge has been applied to the {@link ArrayNode}. */ private boolean handleArrayNode(ArrayNode array, Collection collection, ObjectMapper mapper, - TypeInformation componentType) throws Exception { + TypeInformation componentType, IntFunction rawValues) throws Exception { Assert.notNull(array, "ArrayNode must not be null!"); Assert.notNull(collection, "Source collection must not be null!"); Assert.notNull(mapper, "ObjectMapper must not be null!"); + // Empty collection? Primitive? Enum? No need to merge. + if (array.isEmpty() + || collection.isEmpty() + || ClassUtils.isPrimitiveOrWrapper(componentType.getType()) + || componentType.getType().isEnum()) { + return false; + } + // We need an iterator for the original collection. // We might modify it but we want to keep iterating over the original collection. Iterator value = new ArrayList(collection).iterator(); boolean nestedObjectFound = false; + int i = 0; for (JsonNode jsonNode : array) { + int current = i++; + + // We need to append new elements if (!value.hasNext()) { + + nestedObjectFound = true; + + // Use pre-read values if available. Deserialize node otherwise. + collection.add(rawValues != null + ? rawValues.apply(current) + : mapper.treeToValue(jsonNode, componentType.getType())); + break; } Object next = value.next(); if (ArrayNode.class.isInstance(jsonNode)) { - return handleArray(jsonNode, next, mapper, getTypeToMap(value, componentType)); + return handleArray(jsonNode, next, mapper, getTypeToMap(value, componentType), null); } if (ObjectNode.class.isInstance(jsonNode)) { @@ -410,7 +448,7 @@ public class DomainObjectReader { } else if (value instanceof ArrayNode && sourceValue != null) { - handleArray(value, sourceValue, mapper, getTypeToMap(sourceValue, typeToMap)); + handleArray(value, sourceValue, mapper, getTypeToMap(sourceValue, typeToMap), null); } else { @@ -685,8 +723,8 @@ public class DomainObjectReader { result = mergeCollections(property, sourceValue, targetValue, mapper); } else if (property.isEntity()) { - result = targetValue.isPresent() ? targetValue.flatMap(t -> sourceValue.map(s -> mergeForPut(s, t, mapper))) - : sourceValue; + result = targetValue.isPresent() ? targetValue.flatMap(t -> sourceValue.map(s -> mergeForPut(s, t, mapper))) + : sourceValue; } else { result = sourceValue; } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java index 6ec07d9d6..520b20f9b 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java @@ -49,6 +49,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.rest.core.mapping.ResourceMappings; +import org.springframework.data.rest.webmvc.json.DomainObjectReaderUnitTests.BugModel.NestedModel; import org.springframework.data.rest.webmvc.mapping.Associations; import com.fasterxml.jackson.annotation.JsonAutoDetect; @@ -113,6 +114,7 @@ class DomainObjectReaderUnitTests { mappingContext.getPersistentEntity(Apple.class); mappingContext.getPersistentEntity(Pear.class); mappingContext.getPersistentEntity(WithCustomMappedPrimitiveCollection.class); + mappingContext.getPersistentEntity(BugModel.class); mappingContext.afterPropertiesSet(); this.entities = new PersistentEntities(Collections.singleton(mappingContext)); @@ -682,6 +684,26 @@ class DomainObjectReaderUnitTests { assertThat(result.inner.hidden).isNull(); } + @Test + void deserializesNewNestedEntitiesCorrectly() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + JsonNode node = mapper.readTree("{ \"list\" : [ { \"value\" : \"Foo\" }, { \"value\" : \"Bar\" }] }"); + + NestedModel nested = new BugModel.NestedModel(); + nested.value = "FooBar"; + + BugModel model = new BugModel(); + model.list = new ArrayList<>(); + model.list.add(nested); + + BugModel result = reader.doMerge((ObjectNode) node, model, mapper); + + assertThat(result.list) + .extracting(it -> it.value) + .containsExactly("Foo", "Bar"); + } + @SuppressWarnings("unchecked") private static T as(Object source, Class type) { @@ -968,4 +990,14 @@ class DomainObjectReaderUnitTests { } } } + + // GH-2287 + static class BugModel { + + public List list; + + static class NestedModel { + public String value; + } + } }