From 8e1af29bf2d4a8a8203fc2a5556eb4a89483300a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 24 Jan 2017 16:09:53 +0100 Subject: [PATCH] DATAREST-977 - Fixed reading of complex enums on collection expansion for PATCH. When merging collections on PATCH we now don't use the first collection item's type for all elements but inspect the values for each existing element found. When it comes to appending elements to the collection, wen now just stick to the declared component type as type hint for reading the provided value. --- .../rest/webmvc/json/DomainObjectReader.java | 46 +++++++++++++------ .../json/DomainObjectReaderUnitTests.java | 46 ++++++++++++++++++- 2 files changed, 78 insertions(+), 14 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 91f3104e3..8935f415a 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 @@ -340,12 +340,7 @@ public class DomainObjectReader { return false; } - Iterator iterator = collection.iterator(); - TypeInformation componentType = iterator.hasNext() ? // - ClassTypeInformation.from(iterator.next().getClass()) : // - collectionType.getComponentType(); - - return handleArrayNode((ArrayNode) node, collection, mapper, componentType); + return handleArrayNode((ArrayNode) node, collection, mapper, collectionType.getComponentType()); } /** @@ -373,8 +368,7 @@ public class DomainObjectReader { if (!value.hasNext()) { - Class type = componentType == null ? Object.class : componentType.getType(); - collection.add(mapper.treeToValue(jsonNode, type)); + collection.add(mapper.treeToValue(jsonNode, getTypeToMap(null, componentType).getType())); continue; } @@ -382,7 +376,7 @@ public class DomainObjectReader { Object next = value.next(); if (ArrayNode.class.isInstance(jsonNode)) { - return handleArray(jsonNode, next, mapper, componentType); + return handleArray(jsonNode, next, mapper, getTypeToMap(value, componentType)); } if (ObjectNode.class.isInstance(jsonNode)) { @@ -417,7 +411,7 @@ public class DomainObjectReader { Iterator> fields = node.fields(); Class keyType = typeOrObject(type.getComponentType()); - Class valueType = typeOrObject(type.getMapValueType()); + TypeInformation valueType = type.getMapValueType(); while (fields.hasNext()) { @@ -427,6 +421,7 @@ public class DomainObjectReader { Object mappedKey = mapper.readValue(quote(key), keyType); Object sourceValue = source.get(mappedKey); + TypeInformation typeToMap = getTypeToMap(sourceValue, valueType); if (value instanceof ObjectNode && sourceValue != null) { @@ -434,12 +429,11 @@ public class DomainObjectReader { } else if (value instanceof ArrayNode && sourceValue != null) { - handleArray(value, sourceValue, mapper, type); + handleArray(value, sourceValue, mapper, getTypeToMap(sourceValue, typeToMap)); } else { - Class typeToRead = sourceValue != null ? sourceValue.getClass() : valueType; - source.put(mappedKey, mapper.treeToValue(value, typeToRead)); + source.put(mappedKey, mapper.treeToValue(value, typeToMap.getType())); } fields.remove(); @@ -554,4 +548,30 @@ public class DomainObjectReader { private static Class typeOrObject(TypeInformation type) { return type == null ? Object.class : type.getType(); } + + /** + * Returns the type to read for the given value and default type. The type will be defaulted to {@link Object} if + * missing. If the given value's type is different from the given default (i.e. more concrete) the value's type will + * be used. + * + * @param value can be {@literal null}. + * @param type can be {@literal null}. + * @return + */ + private static TypeInformation getTypeToMap(Object value, TypeInformation type) { + + if (type == null) { + type = ClassTypeInformation.OBJECT; + } + + if (value == null) { + return type; + } + + if (Enum.class.isInstance(value)) { + return ClassTypeInformation.from(((Enum) value).getDeclaringClass()); + } + + return value.getClass().equals(type.getType()) ? type : ClassTypeInformation.from(value.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 329f39851..03b09400f 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,7 +15,7 @@ */ package org.springframework.data.rest.webmvc.json; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; @@ -93,6 +93,7 @@ public class DomainObjectReaderUnitTests { mappingContext.getPersistentEntity(Parent.class); mappingContext.getPersistentEntity(Product.class); mappingContext.getPersistentEntity(TransientReadOnlyProperty.class); + mappingContext.getPersistentEntity(CollectionOfEnumWithMethods.class); mappingContext.afterPropertiesSet(); PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext)); @@ -451,6 +452,20 @@ public class DomainObjectReaderUnitTests { reader.readPut((ObjectNode) node, new TransientReadOnlyProperty(), mapper); } + @Test // DATAREST-977 + public void readsCollectionOfComplexEnum() throws Exception { + + CollectionOfEnumWithMethods sample = new CollectionOfEnumWithMethods(); + sample.enums.add(SampleEnum.FIRST); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode node = mapper.readTree("{ \"enums\" : [ \"SECOND\", \"FIRST\" ] }"); + + CollectionOfEnumWithMethods result = reader.merge((ObjectNode) node, sample, mapper); + + assertThat(result.enums, contains(SampleEnum.SECOND, SampleEnum.FIRST)); + } + @SuppressWarnings("unchecked") private static T as(Object source, Class type) { @@ -585,4 +600,33 @@ public class DomainObjectReaderUnitTests { public void setName(String name) {} } + + // DATAREST-977 + + interface EnumInterface { + String getFoo(); + } + + static enum SampleEnum implements EnumInterface { + + FIRST { + + @Override + public String getFoo() { + return "first"; + } + + }, + SECOND { + + public String getFoo() { + return "second"; + } + }; + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class CollectionOfEnumWithMethods { + List enums = new ArrayList(); + } }