From 1ebdeb95892cf6821992f1678e93c6a8cb6c0bef Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Wed, 29 Nov 2017 10:28:09 +0100
Subject: [PATCH] DATAREST-1163 - ValidationErrors now properly looks up null
 values.

The field value lookup in ValidationErrors previously threw a NotReadablePropertyException in case a property value was null as we incorrectly piped the null value into an Optional in turn. We now eagerly reject the property if we can't find a PersistentProperty in the metamodel to avoid this.
---
 .../data/rest/core/ValidationErrors.java      | 42 +++++++++----------
 .../rest/core/ValidationErrorsUnitTests.java  |  9 ++++
 2 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/ValidationErrors.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/ValidationErrors.java
index 0fafbe2e7..c5ce7625d 100644
--- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/ValidationErrors.java
+++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/ValidationErrors.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -18,7 +18,6 @@ package org.springframework.data.rest.core;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.Optional;
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.ConfigurablePropertyAccessor;
@@ -83,17 +82,30 @@ public class ValidationErrors extends AbstractPropertyBindingResult {
 
 				do {
 
-					String segment = iterator.next();
-
-					Optional<? extends PersistentProperty<?>> property = entities.getPersistentEntity(value.getClass())//
-							.map(it -> it.getPersistentProperty(PropertyAccessorUtils.getPropertyName(segment)));
-
-					value = getValue(value, property, segment, propertyName);
+					value = lookupValueOn(value, iterator.next());
 
 				} while (iterator.hasNext());
 
 				return value;
 			}
+
+			/**
+			 * @param value the original value, must not be {@literal null}..
+			 * @param segment the property segment to look up, must not be {@literal null} or empty.
+			 * @return
+			 */
+			private Object lookupValueOn(Object value, String segment) {
+
+				PersistentProperty<?> property = entities.getPersistentEntity(value.getClass()) //
+						.map(it -> it.getPersistentProperty(PropertyAccessorUtils.getPropertyName(segment))) //
+						.orElseThrow(() -> new NotReadablePropertyException(value.getClass(), segment));
+
+				ConfigurablePropertyAccessor accessor = property.usePropertyAccess() //
+						? PropertyAccessorFactory.forBeanPropertyAccess(value) //
+						: PropertyAccessorFactory.forDirectFieldAccess(value);
+
+				return accessor.getPropertyValue(segment);
+			}
 		};
 	}
 
@@ -105,18 +117,4 @@ public class ValidationErrors extends AbstractPropertyBindingResult {
 	public Object getTarget() {
 		return source;
 	}
-
-	private static Object getValue(Object source, Optional<? extends PersistentProperty<?>> property, String segment,
-			String name) {
-
-		return property.map(it -> {
-
-			ConfigurablePropertyAccessor accessor = it.usePropertyAccess()
-					? PropertyAccessorFactory.forBeanPropertyAccess(source)
-					: PropertyAccessorFactory.forDirectFieldAccess(source);
-
-			return accessor.getPropertyValue(segment);
-
-		}).orElseThrow(() -> new NotReadablePropertyException(source.getClass(), name));
-	}
 }
diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/ValidationErrorsUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/ValidationErrorsUnitTests.java
index 352aaf421..7e6a07c40 100755
--- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/ValidationErrorsUnitTests.java
+++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/ValidationErrorsUnitTests.java
@@ -63,6 +63,14 @@ public class ValidationErrorsUnitTests {
 		expectedErrorBehavior(new ValidationErrors(new Foo(), entities));
 	}
 
+	@Test // DATAREST-1163
+	public void returnsNullForPropertyValue() {
+
+		ValidationErrors errors = new ValidationErrors(new Foo(), entities);
+
+		assertThat(errors.getFieldValue("bar")).isNull();
+	}
+
 	private static void expectedErrorBehavior(Errors errors) {
 
 		assertThat(errors.getFieldValue("bars")).isNotNull();
@@ -79,6 +87,7 @@ public class ValidationErrorsUnitTests {
 
 	static class Foo {
 		List<Bar> bars = Collections.singletonList(new Bar());
+		Bar bar = null;
 	}
 
 	static class Bar {