From c0df12d9fa25e9c50fc52b52a1254cea8e836500 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 8 Dec 2016 09:44:36 +0100 Subject: [PATCH] DATAREST-956 - Polishing. Some tiny refactorings in DomainObjectReader. We're now using TypeInformation instead of Class to preserve more generics information when it comes to deeper nesting. Moved some code around in the unit tests. Original pull request: #245. --- .../rest/webmvc/json/DomainObjectReader.java | 74 +++++++++---- .../json/DomainObjectReaderUnitTests.java | 100 +++++++++--------- 2 files changed, 102 insertions(+), 72 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 054340bf4..ce86de0d0 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 @@ -33,6 +33,8 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.SimplePropertyHandler; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.rest.webmvc.mapping.Associations; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.util.Assert; @@ -177,9 +179,7 @@ public class DomainObjectReader { if (child.isArray()) { - boolean nestedObjectFound = handleArrayNode((ArrayNode) child, asCollection(rawValue), property.getComponentType(), mapper); - - if (nestedObjectFound) { + if (handleArray(child, rawValue, mapper, property.getTypeInformation())) { i.remove(); } @@ -201,7 +201,7 @@ public class DomainObjectReader { continue; } - doMergeNestedMap((Map) rawValue, objectNode, mapper); + doMergeNestedMap((Map) rawValue, objectNode, mapper, property.getTypeInformation()); // Remove potentially emptied Map as values have been handled recursively if (!objectNode.fieldNames().hasNext()) { @@ -221,40 +221,65 @@ public class DomainObjectReader { return mapper.readerForUpdating(target).readValue(root); } + /** + * 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, + * falling back to component type lookup on the given type. + * + * @param node must not be {@literal null}. + * @param source must not be {@literal null}. + * @param mapper must not be {@literal null}. + * @param collectionType must not be {@literal null}. + * @return + * @throws Exception + */ + private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation collectionType) + throws Exception { + + Collection collection = asCollection(source); + Iterator iterator = collection.iterator(); + TypeInformation componentType = iterator.hasNext() ? // + ClassTypeInformation.from(iterator.next().getClass()) : // + collectionType.getComponentType(); + + return handleArrayNode((ArrayNode) node, collection, mapper, componentType); + } + /** * Applies the diff handling to {@link ArrayNode}s, potentially recursing into nested ones. * * @param array the source {@link ArrayNode}m, must not be {@literal null}. * @param collection the actual collection values, must not be {@literal null}. - * @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}. + * @param mapper the {@link ObjectMapper} to use, must not be {@literal null}. + * @param componentType the item type of the collection, can be {@literal null}. + * @return whether an object merge has been applied to the {@link ArrayNode}. */ - private boolean handleArrayNode(ArrayNode array, Collection collection, Class componentType, ObjectMapper mapper) - throws Exception { + private boolean handleArrayNode(ArrayNode array, Collection collection, ObjectMapper mapper, + TypeInformation componentType) 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!"); - //we need an iterator for the original collection - we might modify it but we want to keep iterating over the original collection + // 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()) { - if (componentType != null) { - collection.add(mapper.treeToValue(jsonNode, componentType)); - continue; - } + + Class type = componentType == null ? Object.class : componentType.getType(); + collection.add(mapper.treeToValue(jsonNode, type)); + + continue; } Object next = value.next(); if (ArrayNode.class.isInstance(jsonNode)) { - Collection nestedCollection = asCollection(next); - Iterator iterator = nestedCollection.iterator(); - return handleArrayNode(array, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper); + return handleArray(jsonNode, next, mapper, componentType); } if (ObjectNode.class.isInstance(jsonNode)) { @@ -264,8 +289,8 @@ public class DomainObjectReader { } } + // there are more items in the collection than contained in the JSON node - remove it. while (value.hasNext()) { - //there are more items in the collection than contained in the json node - remove it. collection.remove(value.next()); } @@ -280,7 +305,8 @@ public class DomainObjectReader { * @param mapper must not be {@literal null}. * @throws Exception */ - private void doMergeNestedMap(Map source, ObjectNode node, ObjectMapper mapper) throws Exception { + private void doMergeNestedMap(Map source, ObjectNode node, ObjectMapper mapper, + TypeInformation type) throws Exception { if (source == null) { return; @@ -295,15 +321,17 @@ public class DomainObjectReader { Object sourceValue = source.get(entry.getKey()); if (child instanceof ObjectNode && sourceValue != null) { + doMerge((ObjectNode) child, sourceValue, mapper); + } else if (child instanceof ArrayNode && sourceValue != null) { - Collection nestedCollection = asCollection(sourceValue); - Iterator iterator = nestedCollection.iterator(); - handleArrayNode((ArrayNode) child, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper); + + handleArray(child, sourceValue, mapper, type); + } else { - Class target = sourceValue == null ? Object.class : sourceValue.getClass(); - source.put(entry.getKey(), mapper.treeToValue(child, target)); + Class valueType = sourceValue == null ? Object.class : sourceValue.getClass(); + source.put(entry.getKey(), mapper.treeToValue(child, valueType)); } fields.remove(); 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 d8f0e70d1..1aab3ca3c 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,13 +15,12 @@ */ package org.springframework.data.rest.webmvc.json; -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 static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; import java.io.ByteArrayInputStream; import java.util.ArrayList; @@ -59,10 +58,6 @@ 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}. * @@ -310,40 +305,6 @@ public class DomainObjectReaderUnitTests { assertThat(result.inner, is(sameInstance(inner))); } - @SuppressWarnings("unchecked") - private static T as(Object source, Class type) { - - assertThat(source, is(instanceOf(type))); - return (T) source; - } - - @JsonAutoDetect(fieldVisibility = Visibility.ANY) - static class SampleUser { - - String name; - @JsonIgnore String password; - Map relatedUsers; - - public SampleUser(String name, String password) { - this.name = name; - this.password = password; - } - } - - /** - * @see DATAREST-556 - */ - @JsonAutoDetect(fieldVisibility = Visibility.ANY) - static class Person { - - String firstName, lastName; - - public Person(String firstName, String lastName) { - this.firstName = firstName; - this.lastName = lastName; - } - } - /** * @see DATAREST-937 */ @@ -395,10 +356,8 @@ public class DomainObjectReaderUnitTests { Parent source = new Parent(); source.inner = inner; - JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ " + - "{ \"some\" : \"value1\" }," + - "{ \"some\" : \"value2\" }," + - "{ \"some\" : \"value3\" } ] } }"); + JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ " + "{ \"some\" : \"value1\" }," + + "{ \"some\" : \"value2\" }," + "{ \"some\" : \"value3\" } ] } }"); Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); @@ -431,6 +390,49 @@ public class DomainObjectReaderUnitTests { assertThat(result.inner.items.get(0).some, is("value")); } + @Test + public void testname() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + JsonNode node = mapper.readTree("\"asd\""); + + assertThat(mapper.treeToValue(node, Object.class), is((Object) "asd")); + } + + @SuppressWarnings("unchecked") + private static T as(Object source, Class type) { + + assertThat(source, is(instanceOf(type))); + return (T) source; + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class SampleUser { + + String name; + @JsonIgnore String password; + Map relatedUsers; + + public SampleUser(String name, String password) { + this.name = name; + this.password = password; + } + } + + /** + * @see DATAREST-556 + */ + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Person { + + String firstName, lastName; + + public Person(String firstName, String lastName) { + this.firstName = firstName; + this.lastName = lastName; + } + } + @JsonAutoDetect(fieldVisibility = Visibility.ANY) static class TypeWithGenericMap {