From ee1ac227d4a70edef2895cf398808abb7e396a8b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 13 Dec 2016 09:54:26 +0100 Subject: [PATCH] DATAREST-959 - Polishing. Skip all merge logic if the source value is null. That frees all nested logic from handling with that case and us falling back to plain Jackson reading. The array handling now also opts out if the source value is not a collection or array in the first place as it means we need to let Jackson override the value with the collection given to be deserialized. Original pull request: #246. --- .../rest/webmvc/json/DomainObjectReader.java | 25 ++++++++++------ .../json/DomainObjectReaderUnitTests.java | 30 ++++++++++++++++++- 2 files changed, 45 insertions(+), 10 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 e54902ad6..a166690a1 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 @@ -23,7 +23,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; @@ -181,6 +180,10 @@ public class DomainObjectReader { PersistentPropertyAccessor accessor = entity.getPropertyAccessor(target); Object rawValue = accessor.getProperty(property); + if (rawValue == null) { + continue; + } + if (child.isArray()) { if (handleArray(child, rawValue, mapper, property.getTypeInformation())) { @@ -215,7 +218,7 @@ public class DomainObjectReader { continue; } - if (rawValue != null && property.isEntity()) { + if (property.isEntity()) { i.remove(); doMerge(objectNode, rawValue, mapper); } @@ -240,7 +243,12 @@ public class DomainObjectReader { private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation collectionType) throws Exception { - Collection collection = asCollection(source); + Collection collection = ifCollection(source); + + if (collection == null) { + return false; + } + Iterator iterator = collection.iterator(); TypeInformation componentType = iterator.hasNext() ? // ClassTypeInformation.from(iterator.next().getClass()) : // @@ -343,17 +351,16 @@ public class DomainObjectReader { } /** - * Returns the given source instance as {@link Collection}. + * Returns the given source instance as {@link Collection} or creates a new one for the given type. * * @param source can be {@literal null}. + * @param type must not be {@literal null} in case {@code source} is null. * @return */ @SuppressWarnings("unchecked") - private static Collection asCollection(Object source) { + private static Collection ifCollection(Object source) { - if (source == null) { - return new ArrayList(); - } + Assert.notNull(source, "Source instance must not be null!"); if (source instanceof Collection) { return (Collection) source; @@ -363,7 +370,7 @@ public class DomainObjectReader { return Arrays.asList((Object[]) source); } - return Collections.singleton(source); + return null; } /** 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 8df5f8b8c..fcdeb591f 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 @@ -26,10 +26,12 @@ import java.io.ByteArrayInputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.GregorianCalendar; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -394,7 +396,7 @@ public class DomainObjectReaderUnitTests { * @see DATAREST-959 */ @Test - public void writesArrayOverUndefinedValueForPut() throws Exception { + public void addsElementToPreviouslyEmptyCollection() throws Exception { Parent source = new Parent(); source.inner = new Child(); @@ -408,6 +410,31 @@ public class DomainObjectReaderUnitTests { assertThat(result.inner.items.get(0).some, is("value")); } + /** + * @see DATAREST-959 + */ + @Test + @SuppressWarnings("unchecked") + public void turnsObjectIntoCollection() throws Exception { + + Parent source = new Parent(); + source.inner = new Child(); + source.inner.object = new Item("value"); + + JsonNode node = new ObjectMapper() + .readTree("{ \"inner\" : { \"object\" : [ { \"some\" : \"value\" }, { \"some\" : \"otherValue\" } ] } }"); + + Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); + assertThat(result.inner.object, is(instanceOf(Collection.class))); + + Collection collection = (Collection) result.inner.object; + assertThat(collection.size(), is(2)); + + Iterator> iterator = (Iterator>) collection.iterator(); + assertThat(iterator.next().get("some"), is((Object) "value")); + assertThat(iterator.next().get("some"), is((Object) "otherValue")); + } + @Test public void testname() throws Exception { @@ -515,6 +542,7 @@ public class DomainObjectReaderUnitTests { @JsonAutoDetect(fieldVisibility = Visibility.ANY) static class Child { List items; + Object object; } @JsonAutoDetect(fieldVisibility = Visibility.ANY)