DATAREST-575 - Fixed property lookup in MappedProperties.

Previously, MappedProperties didn't handle Jackson properties correctly, that do not expose a PersistentProperty, e.g. transient ones. That led to potential nullPointerExceptions in clients as the guarding hasPersistentPropertyForField(…) still answered true, as the backing cache contained an entry with a null value. We now skip those properties completely.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java

@@ -54,7 +54,6 @@ public class DomainObjectReader {
 
 	private final @NonNull PersistentEntities entities;
 	private final @NonNull Associations associationLinks;
-	private final @NonNull ClassIntrospector introspector = new BasicClassIntrospector();
 
 	/**
 	 * Reads the given input stream into an {@link ObjectNode} and applies that to the given existing instance.
@@ -97,7 +96,7 @@ public class DomainObjectReader {
 
 		Assert.notNull(entity, "No PersistentEntity found for ".concat(type.getName()).concat("!"));
 
-		final MappedProperties properties = getJacksonProperties(entity, mapper);
+		final MappedProperties properties = MappedProperties.fromJacksonProperties(entity, mapper);
 
 		entity.doWithProperties(new SimplePropertyHandler() {
 
@@ -156,7 +155,7 @@ public class DomainObjectReader {
 			return mapper.readerForUpdating(target).readValue(root);
 		}
 
-		MappedProperties mappedProperties = getJacksonProperties(entity, mapper);
+		MappedProperties mappedProperties = MappedProperties.fromJacksonProperties(entity, mapper);
 
 		for (Iterator<Entry<String, JsonNode>> i = root.fields(); i.hasNext();) {
 
@@ -242,27 +241,14 @@ public class DomainObjectReader {
 		}
 	}
 
-	/**
-	 * Returns the {@link MappedProperties} for the given {@link PersistentEntity}.
-	 * 
-	 * @param entity must not be {@literal null}.
-	 * @param mapper must not be {@literal null}.
-	 * @return
-	 */
-	private MappedProperties getJacksonProperties(PersistentEntity<?, ?> entity, ObjectMapper mapper) {
-
-		BeanDescription description = introspector.forDeserialization(mapper.getDeserializationConfig(),
-				mapper.constructType(entity.getType()), mapper.getDeserializationConfig());
-
-		return new MappedProperties(entity, description);
-	}
-
 	/**
 	 * Simple value object to capture a mapping of Jackson mapped field names and {@link PersistentProperty} instances.
 	 *
 	 * @author Oliver Gierke
 	 */
-	private static class MappedProperties {
+	static class MappedProperties {
+
+		private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector();
 
 		private final Map<PersistentProperty<?>, String> propertyToFieldName;
 		private final Map<String, PersistentProperty<?>> fieldNameToProperty;
@@ -270,11 +256,14 @@ public class DomainObjectReader {
 		/**
 		 * 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) {
+		private MappedProperties(PersistentEntity<?, ?> entity, BeanDescription description) {
+
+			Assert.notNull(entity, "Entity must not be null!");
+			Assert.notNull(description, "BeanDescription must not be null!");
 
 			this.propertyToFieldName = new HashMap<PersistentProperty<?>, String>();
 			this.fieldNameToProperty = new HashMap<String, PersistentProperty<?>>();
@@ -283,20 +272,58 @@ public class DomainObjectReader {
 
 				PersistentProperty<?> persistentProperty = entity.getPersistentProperty(property.getInternalName());
 
-				propertyToFieldName.put(persistentProperty, property.getName());
-				fieldNameToProperty.put(property.getName(), persistentProperty);
+				if (persistentProperty != null) {
+					propertyToFieldName.put(persistentProperty, property.getName());
+					fieldNameToProperty.put(property.getName(), persistentProperty);
+				}
 			}
 		}
 
+		/**
+		 * Creates {@link MappedProperties} for the given {@link PersistentEntity}.
+		 *
+		 * @param entity must not be {@literal null}.
+		 * @param mapper must not be {@literal null}.
+		 * @return
+		 */
+		public static MappedProperties fromJacksonProperties(PersistentEntity<?, ?> entity, ObjectMapper mapper) {
+
+			BeanDescription description = INTROSPECTOR.forDeserialization(mapper.getDeserializationConfig(),
+					mapper.constructType(entity.getType()), mapper.getDeserializationConfig());
+
+			return new MappedProperties(entity, description);
+		}
+
+		/**
+		 * @param property must not be {@literal null}
+		 * @return the mapped name for the {@link PersistentProperty}
+		 */
 		public String getMappedName(PersistentProperty<?> property) {
+
+			Assert.notNull(property, "PersistentProperty must not be null!");
+
 			return propertyToFieldName.get(property);
 		}
 
+		/**
+		 * @param fieldName must not be empty or {@literal null}.
+		 * @return {@literal true} if the field name resolves to a {@literal PersistentProperty}.
+		 */
 		public boolean hasPersistentPropertyForField(String fieldName) {
+
+			Assert.hasText(fieldName, "Field name must not be null or empty!");
+
 			return fieldNameToProperty.containsKey(fieldName);
 		}
 
+		/**
+		 * @param fieldName must not be empty or {@literal null}.
+		 * @return the {@link PersistentProperty} backing the field with the field name.
+		 */
 		public PersistentProperty<?> getPersistentProperty(String fieldName) {
+
+			Assert.hasText(fieldName, "Field name must not be null or empty!");
+
 			return fieldNameToProperty.get(fieldName);
 		}
 	}

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2016 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.json;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+import org.springframework.data.annotation.Transient;
+import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentEntity;
+import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
+import org.springframework.data.rest.webmvc.json.DomainObjectReader.MappedProperties;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+/**
+ * Unit tests for {@link MappedProperties}.
+ * 
+ * @author Oliver Gierke
+ */
+public class MappedPropertiesUnitTests {
+
+	ObjectMapper mapper = new ObjectMapper();
+	KeyValueMappingContext context = new KeyValueMappingContext();
+	KeyValuePersistentEntity<?> entity = context.getPersistentEntity(Sample.class);
+	MappedProperties properties = MappedProperties.fromJacksonProperties(entity, mapper);
+
+	/**
+	 * @see DATAREST-575
+	 */
+	@Test
+	public void doesNotExposeMappedPropertyForNonSpringDataPersistentProperty() {
+
+		assertThat(properties.hasPersistentPropertyForField("notExposedBySpringData"), is(false));
+		assertThat(properties.getPersistentProperty("notExposedBySpringData"), is(nullValue()));
+	}
+
+	/**
+	 * @see DATAREST-575
+	 */
+	@Test
+	public void doesNotExposeMappedPropertyForNonJacksonProperty() {
+
+		assertThat(properties.hasPersistentPropertyForField("notExposedByJackson"), is(false));
+		assertThat(properties.getPersistentProperty("notExposedByJackson"), is(nullValue()));
+	}
+
+	/**
+	 * @see DATAREST-575
+	 */
+	@Test
+	public void exposesProperty() {
+
+		assertThat(properties.hasPersistentPropertyForField("exposedProperty"), is(true));
+		assertThat(properties.getPersistentProperty("exposedProperty"), is(notNullValue()));
+	}
+
+	/**
+	 * @see DATAREST-575
+	 */
+	@Test
+	public void exposesRenamedPropertyByExternalName() {
+
+		assertThat(properties.hasPersistentPropertyForField("email"), is(true));
+		assertThat(properties.getPersistentProperty("email"), is(notNullValue()));
+		assertThat(properties.getMappedName(entity.getPersistentProperty("emailAddress")), is("email"));
+	}
+
+	static class Sample {
+
+		public @Transient String notExposedBySpringData;
+		public @JsonIgnore String notExposedByJackson;
+		public String exposedProperty;
+		public @JsonProperty("email") String emailAddress;
+	}
+}