From 8f46d9f413ba1efb3e84c28d9998f8341661cfef 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 | 51 ++++++++++++++++--- .../json/DomainObjectReaderUnitTests.java | 31 +++++++++++ 2 files changed, 76 insertions(+), 6 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 593e1a50c..0c98c7d2e 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,11 @@ 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 +311,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 +333,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 +342,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 +356,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 +449,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 { 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 92ecc2bba..e4c27a76d 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 @@ -113,6 +113,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)); @@ -684,6 +685,26 @@ class DomainObjectReaderUnitTests { assertThat(result.inner.hidden).isNull(); } + @Test + void deserializesNewNestedEntitiesCorrectly() throws Exception { + + var mapper = new ObjectMapper(); + var node = mapper.readTree("{ \"list\" : [ { \"value\" : \"Foo\" }, { \"value\" : \"Bar\" }] }"); + + var nested = new BugModel.NestedModel(); + nested.value = "FooBar"; + + var model = new BugModel(); + model.list = new ArrayList<>(); + model.list.add(nested); + + var 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) { @@ -966,4 +987,14 @@ class DomainObjectReaderUnitTests { } } } + + // GH-2287 + static class BugModel { + + public List list; + + static class NestedModel { + public String value; + } + } }