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 ef9a75f23..36b4ae7f8 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 @@ -19,6 +19,7 @@ import lombok.NonNull; import lombok.RequiredArgsConstructor; import java.io.InputStream; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -48,6 +49,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; * @author Oliver Gierke * @author Mark Paluch * @author Craig Andrews + * @author Mathias Düsterhöft * @since 2.2 */ @RequiredArgsConstructor @@ -175,7 +177,7 @@ public class DomainObjectReader { if (child.isArray()) { - boolean nestedObjectFound = handleArrayNode((ArrayNode) child, asCollection(rawValue), mapper); + boolean nestedObjectFound = handleArrayNode((ArrayNode) child, asCollection(rawValue), property.getComponentType(), mapper); if (nestedObjectFound) { i.remove(); @@ -224,29 +226,35 @@ public class DomainObjectReader { * * @param array the source {@link ArrayNode}m, must not be {@literal null}. * @param collection the actual collection values, must not be {@literal null}. - * @param mapper the {@link ObjectMapper} to use, must not be {@literal null}. - * @return whether an object merge has been applied to the {@link ArrayNode}. + * @param componentType the item type of the collection + * @param mapper the {@link ObjectMapper} to use, must not be {@literal null}. @return whether an object merge has been applied to the {@link ArrayNode}. */ - private boolean handleArrayNode(ArrayNode array, Collection collection, ObjectMapper mapper) + private boolean handleArrayNode(ArrayNode array, Collection collection, Class componentType, ObjectMapper mapper) 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!"); - Iterator value = collection.iterator(); + //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; for (JsonNode jsonNode : array) { if (!value.hasNext()) { - return nestedObjectFound; + if (componentType != null) { + collection.add(mapper.treeToValue(jsonNode, componentType)); + continue; + } } Object next = value.next(); if (ArrayNode.class.isInstance(jsonNode)) { - return handleArrayNode(array, asCollection(next), mapper); + Collection nestedCollection = asCollection(next); + Iterator iterator = nestedCollection.iterator(); + return handleArrayNode(array, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper); } if (ObjectNode.class.isInstance(jsonNode)) { @@ -256,6 +264,11 @@ public class DomainObjectReader { } } + while (value.hasNext()) { + //there are more items in the collection than contained in the json node - remove it. + collection.remove(value.next()); + } + return nestedObjectFound; } @@ -284,7 +297,9 @@ public class DomainObjectReader { if (child instanceof ObjectNode && sourceValue != null) { doMerge((ObjectNode) child, sourceValue, mapper); } else if (child instanceof ArrayNode && sourceValue != null) { - handleArrayNode((ArrayNode) child, asCollection(sourceValue), mapper); + Collection nestedCollection = asCollection(sourceValue); + Iterator iterator = nestedCollection.iterator(); + handleArrayNode((ArrayNode) child, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper); } else { source.put(entry.getKey(), mapper.treeToValue(child, sourceValue == null ? Object.class : sourceValue.getClass())); 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 a8675ab36..d8f0e70d1 100644 --- 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 @@ -15,9 +15,13 @@ */ package org.springframework.data.rest.webmvc.json; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; import java.io.ByteArrayInputStream; import java.util.ArrayList; @@ -55,11 +59,16 @@ import com.fasterxml.jackson.databind.PropertyNamingStrategy; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.base.Charsets; +import javassist.runtime.Inner; +import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; + /** * Unit tests for {@link DomainObjectReader}. * * @author Oliver Gierke * @author Craig Andrews + * @author Mathias Düsterhöft */ @RunWith(MockitoJUnitRunner.class) public class DomainObjectReaderUnitTests { @@ -373,6 +382,55 @@ public class DomainObjectReaderUnitTests { assertThat(result.inner.items.get(0).some, is("value")); } + /** + * @see DATAREST-956 + */ + @Test + public void writesArrayWithAddedItemForPut() throws Exception { + + Child inner = new Child(); + inner.items = new ArrayList(); + inner.items.add(new Item()); + + Parent source = new Parent(); + source.inner = inner; + + JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ " + + "{ \"some\" : \"value1\" }," + + "{ \"some\" : \"value2\" }," + + "{ \"some\" : \"value3\" } ] } }"); + + Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); + + assertThat(result.inner.items.size(), is(3)); + assertThat(result.inner.items.get(0).some, is("value1")); + assertThat(result.inner.items.get(1).some, is("value2")); + assertThat(result.inner.items.get(2).some, is("value3")); + } + + /** + * @see DATAREST-956 + */ + @Test + public void writesArrayWithRemovedItemForPut() throws Exception { + + Child inner = new Child(); + inner.items = new ArrayList(); + inner.items.add(new Item("test1")); + inner.items.add(new Item("test2")); + inner.items.add(new Item("test3")); + + Parent source = new Parent(); + source.inner = inner; + + JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ { \"some\" : \"value\" } ] } }"); + + Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); + + assertThat(result.inner.items.size(), is(1)); + assertThat(result.inner.items.get(0).some, is("value")); + } + @JsonAutoDetect(fieldVisibility = Visibility.ANY) static class TypeWithGenericMap { @@ -440,6 +498,8 @@ public class DomainObjectReaderUnitTests { } @JsonAutoDetect(fieldVisibility = Visibility.ANY) + @NoArgsConstructor + @AllArgsConstructor static class Item { String some; }