From a07697ef34f983b38c43054b703a38e44a5e7908 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 26 May 2015 17:35:15 +0200 Subject: [PATCH] DATAREST-556 - Fixed JsonNode handling for PUT/PATCH with a custom field naming strategy. The preparation of the JsonNode that we map onto the domain object now correctly looks up fields by their mapped names. Previously we accidentally checked the incoming JSON for fields with raw unmapped property names with caused fields invalid detected or not detected if a custom Jackson naming strategy was in place. --- .../rest/webmvc/json/DomainObjectReader.java | 79 ++++++++++++++----- .../json/DomainObjectReaderUnitTests.java | 35 +++++++- 2 files changed, 94 insertions(+), 20 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 33aff239b..ba3eec964 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 @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,10 @@ package org.springframework.data.rest.webmvc.json; import java.io.InputStream; -import java.util.Collection; -import java.util.HashSet; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import java.util.Map.Entry; -import java.util.Set; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -42,7 +41,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; /** * Component to apply an {@link ObjectNode} to an existing domain object. This is effectively a best-effort workaround - * for Jacksons inability to apply a (partial) JSON document to an existing object in a deeply nestes way. We manually + * for Jackson's inability to apply a (partial) JSON document to an existing object in a deeply nested way. We manually * detect nested objects, lookup the original value and apply the merge recursively. * * @author Oliver Gierke @@ -106,7 +105,7 @@ public class DomainObjectReader { Assert.notNull(mapper, "ObjectMapper must not be null!"); final PersistentEntity entity = entities.getPersistentEntity(target.getClass()); - final Collection properties = getJacksonProperties(entity, mapper); + final MappedProperties properties = getJacksonProperties(entity, mapper); entity.doWithProperties(new SimplePropertyHandler() { @@ -117,11 +116,13 @@ public class DomainObjectReader { @Override public void doWithPersistentProperty(PersistentProperty property) { - boolean isMappedProperty = properties.contains(property.getName()); - boolean noValueInSource = !source.has(property.getName()); + String mappedName = properties.getMappedName(property); + + boolean isMappedProperty = mappedName != null; + boolean noValueInSource = !source.has(mappedName); if (isMappedProperty && noValueInSource) { - source.putNull(property.getName()); + source.putNull(mappedName); } } }); @@ -154,7 +155,7 @@ public class DomainObjectReader { Assert.notNull(mapper, "ObjectMapper must not be null!"); PersistentEntity entity = entities.getPersistentEntity(target.getClass()); - Collection mappedProperties = getJacksonProperties(entity, mapper); + MappedProperties mappedProperties = getJacksonProperties(entity, mapper); for (Iterator> i = root.fields(); i.hasNext();) { @@ -165,15 +166,17 @@ public class DomainObjectReader { continue; } - PersistentProperty property = entity.getPersistentProperty(entry.getKey()); + String fieldName = entry.getKey(); - if (property == null || !mappedProperties.contains(property.getName())) { + if (!mappedProperties.hasPersistentPropertyForField(fieldName)) { i.remove(); continue; } if (child.isObject()) { + PersistentProperty property = mappedProperties.getPersistentProperty(fieldName); + if (associationLinks.isLinkableAssociation(property)) { continue; } @@ -192,23 +195,61 @@ public class DomainObjectReader { } /** - * Returns the names of all mapped properties for the given {@link PersistentEntity}. + * Returns the {@link MappedProperties} for the given {@link PersistentEntity}. * * @param entity must not be {@literal null}. * @param mapper must not be {@literal null}. - * @return the collection of mapped properties. + * @return */ - private Collection getJacksonProperties(PersistentEntity entity, ObjectMapper mapper) { + private MappedProperties getJacksonProperties(PersistentEntity entity, ObjectMapper mapper) { BeanDescription description = introspector.forDeserialization(mapper.getDeserializationConfig(), mapper.constructType(entity.getType()), mapper.getDeserializationConfig()); - Set properties = new HashSet(); + return new MappedProperties(entity, description); + } - for (BeanPropertyDefinition property : description.findProperties()) { - properties.add(property.getInternalName()); + /** + * Simple value object to capture a mapping of Jackson mapped field names and {@link PersistentProperty} instances. + * + * @author Oliver Gierke + */ + private static class MappedProperties { + + private final Map, String> propertyToFieldName; + private final Map> fieldNameToProperty; + + /** + * Creates a new {@link MappedProperties} instance for the given {@link PersistentEntity} and + * {@link BeanDescription}. + * + * @param entity must not be {@literal null}. + * @param description must not be {@literal null}. + */ + public MappedProperties(PersistentEntity entity, BeanDescription description) { + + this.propertyToFieldName = new HashMap, String>(); + this.fieldNameToProperty = new HashMap>(); + + for (BeanPropertyDefinition property : description.findProperties()) { + + PersistentProperty persistentProperty = entity.getPersistentProperty(property.getInternalName()); + + propertyToFieldName.put(persistentProperty, property.getName()); + fieldNameToProperty.put(property.getName(), persistentProperty); + } } - return properties; + public String getMappedName(PersistentProperty property) { + return propertyToFieldName.get(property); + } + + public boolean hasPersistentPropertyForField(String fieldName) { + return fieldNameToProperty.containsKey(fieldName); + } + + public PersistentProperty getPersistentProperty(String fieldName) { + return fieldNameToProperty.get(fieldName); + } } } 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 56753ef05..b2c0ddf4d 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 @@ -34,6 +34,7 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.PropertyNamingStrategy; import com.fasterxml.jackson.databind.node.ObjectNode; /** @@ -52,7 +53,8 @@ public class DomainObjectReaderUnitTests { public void setUp() { MongoMappingContext mappingContext = new MongoMappingContext(); - mappingContext.setInitialEntitySet(Collections.singleton(SampleUser.class)); + mappingContext.getPersistentEntity(SampleUser.class); + mappingContext.getPersistentEntity(Person.class); mappingContext.afterPropertiesSet(); PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext)); @@ -75,6 +77,23 @@ public class DomainObjectReaderUnitTests { assertThat(result.password, is("password")); } + /** + * @see DATAREST-556 + */ + @Test + public void considersMappedFieldNamesWhenApplyingNodeToDomainObject() throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + mapper.setPropertyNamingStrategy(PropertyNamingStrategy.PASCAL_CASE_TO_CAMEL_CASE); + + JsonNode node = new ObjectMapper().readTree("{\"FirstName\":\"Carter\",\"LastName\":\"Beauford\"}"); + + Person result = reader.readPut((ObjectNode) node, new Person("Dave", "Matthews"), mapper); + + assertThat(result.firstName, is("Carter")); + assertThat(result.lastName, is("Beauford")); + } + @JsonAutoDetect(fieldVisibility = Visibility.ANY) static class SampleUser { @@ -86,4 +105,18 @@ public class DomainObjectReaderUnitTests { 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; + } + } }