DATAREST-801 - Improved nested value handling in ValidationErrors.

We now correctly handle nested values by manually traversing the potentially nested property path, creating a PropertyAccessor for each nesting step considering the property access settings defined in the mapping metadata.

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/ValidationErrors.java

@@ -15,12 +15,20 @@
  */
 package org.springframework.data.rest.core;
 
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
+
 import org.springframework.beans.BeansException;
 import org.springframework.beans.ConfigurablePropertyAccessor;
 import org.springframework.beans.DirectFieldAccessor;
+import org.springframework.beans.NotReadablePropertyException;
+import org.springframework.beans.PropertyAccessorFactory;
+import org.springframework.beans.PropertyAccessorUtils;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
+import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.util.Assert;
 import org.springframework.validation.AbstractPropertyBindingResult;
 import org.springframework.validation.Errors;
@@ -36,8 +44,8 @@ public class ValidationErrors extends AbstractPropertyBindingResult {
 
 	private static final long serialVersionUID = 8141826537389141361L;
 
-	private final PersistentPropertyAccessor accessor;
-	private PersistentEntity<?, ?> entity;
+	private final Object source;
+	private final PersistentEntities entities;
 
 	/**
 	 * Creates a new {@link ValidationErrors} instance for the given source object and {@link PersistentEntity}.
@@ -45,17 +53,15 @@ public class ValidationErrors extends AbstractPropertyBindingResult {
 	 * @param source the source object to gather validation errors on, must not be {@literal null}.
 	 * @param entity the {@link PersistentEntity} for the given source instance, must not be {@literal null}.
 	 */
-	public ValidationErrors(Object source, PersistentEntity<?, ?> entity) {
+	public ValidationErrors(Object source, PersistentEntities entities) {
 
 		super(source.getClass().getSimpleName());
 
 		Assert.notNull(source, "Entity must not be null!");
-		Assert.notNull(entity, "PersistentEntity must not be null!");
-		Assert.isTrue(entity.getType().isInstance(source),
-				"Given source object is not of type of the given PersistentEntity");
+		Assert.notNull(entities, "PersistentEntities must not be null!");
 
-		this.entity = entity;
-		this.accessor = entity.getPropertyAccessor(source);
+		this.entities = entities;
+		this.source = source;
 	}
 
 	/* 
@@ -69,32 +75,39 @@ public class ValidationErrors extends AbstractPropertyBindingResult {
 
 			@Override
 			public Object getPropertyValue(String propertyName) throws BeansException {
-				PersistentProperty<?> property = entity.getPersistentProperty(propertyName);
-				return property == null ? null : accessor.getProperty(property);
+
+				Collection<String> segments = Arrays.asList(propertyName.split("\\."));
+				Iterator<String> iterator = segments.iterator();
+				Object value = source;
+
+				do {
+
+					String segment = iterator.next();
+					PersistentEntity<?, ?> entity = entities.getPersistentEntity(value.getClass());
+					PersistentProperty<?> property = entity.getPersistentProperty(PropertyAccessorUtils.getPropertyName(segment));
+
+					if (property == null) {
+						throw new NotReadablePropertyException(source.getClass(), propertyName);
+					}
+
+					ConfigurablePropertyAccessor accessor = property.usePropertyAccess()
+							? PropertyAccessorFactory.forBeanPropertyAccess(value)
+							: PropertyAccessorFactory.forDirectFieldAccess(value);
+					value = accessor.getPropertyValue(segment);
+
+				} while (iterator.hasNext());
+
+				return value;
 			}
 		};
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.validation.AbstractBindingResult#getFieldValue(java.lang.String)
-	 */
-	@Override
-	public Object getFieldValue(String field) {
-
-		if (field.contains(".")) {
-			return super.getFieldValue(field);
-		}
-
-		return accessor.getProperty(entity.getPersistentProperty(field));
-	}
-
 	/* 
 	 * (non-Javadoc)
 	 * @see org.springframework.validation.AbstractBindingResult#getTarget()
 	 */
 	@Override
 	public Object getTarget() {
-		return accessor.getBean();
+		return source;
 	}
 }

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/event/ValidatingRepositoryEventListener.java

@@ -23,14 +23,12 @@ import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.ObjectFactory;
-import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.rest.core.RepositoryConstraintViolationException;
 import org.springframework.data.rest.core.ValidationErrors;
 import org.springframework.util.Assert;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
-import org.springframework.validation.DirectFieldBindingResult;
 import org.springframework.validation.Errors;
 import org.springframework.validation.ValidationUtils;
 import org.springframework.validation.Validator;
@@ -167,18 +165,13 @@ public class ValidatingRepositoryEventListener extends AbstractRepositoryEventLi
 			return null;
 		}
 
-		Class<?> domainType = entity.getClass();
-		PersistentEntities persistentEntities = persistentEntitiesFactory.getObject();
-		PersistentEntity<?, ?> persistentEntity = persistentEntities.getPersistentEntity(domainType);
+		Errors errors = new ValidationErrors(entity, persistentEntitiesFactory.getObject());
 
-		Errors errors = persistentEntity == null ? new DirectFieldBindingResult(entity, domainType.getSimpleName())
-				: new ValidationErrors(entity, persistentEntity);
+		for (Validator validator : getValidatorsForEvent(event)) {
 
-		for (Validator v : getValidatorsForEvent(event)) {
-
-			if (v.supports(domainType)) {
-				LOGGER.debug("{}: {} with {}", event, entity, v);
-				ValidationUtils.invokeValidator(v, entity, errors);
+			if (validator.supports(entity.getClass())) {
+				LOGGER.debug("{}: {} with {}", event, entity, validator);
+				ValidationUtils.invokeValidator(validator, entity, errors);
 			}
 		}
 

spring-data-rest-core/src/test/java/org/springframework/data/rest/core/ValidationErrorsUnitTests.java

@@ -18,11 +18,16 @@ package org.springframework.data.rest.core;
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
-import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
+import org.junit.Before;
 import org.junit.Test;
+import org.springframework.beans.NotReadablePropertyException;
 import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
+import org.springframework.data.mapping.context.PersistentEntities;
+import org.springframework.validation.Errors;
 
 /**
  * Unit tests for {@link ValidationErrors}.
@@ -31,7 +36,16 @@ import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingCon
  */
 public class ValidationErrorsUnitTests {
 
-	KeyValueMappingContext context = new KeyValueMappingContext();
+	PersistentEntities entities;
+
+	@Before
+	public void setUp() {
+
+		KeyValueMappingContext context = new KeyValueMappingContext();
+		context.getPersistentEntity(Foo.class);
+
+		this.entities = new PersistentEntities(Arrays.asList(context));
+	}
 
 	/**
 	 * @see DATAREST-798
@@ -39,7 +53,7 @@ public class ValidationErrorsUnitTests {
 	@Test
 	public void exposesNestedViolationsCorrectly() {
 
-		ValidationErrors errors = new ValidationErrors(new Foo(), context.getPersistentEntity(Foo.class));
+		ValidationErrors errors = new ValidationErrors(new Foo(), entities);
 
 		errors.pushNestedPath("bars[0]");
 		errors.rejectValue("field", "asdf");
@@ -48,11 +62,33 @@ public class ValidationErrorsUnitTests {
 		assertThat(errors.getFieldError().getField(), is("bars[0].field"));
 	}
 
+	/**
+	 * @see DATAREST-801
+	 */
+	@Test
+	public void getsTheNestedFieldsValue() {
+		expectedErrorBehavior(new ValidationErrors(new Foo(), entities));
+	}
+
+	private static void expectedErrorBehavior(Errors errors) {
+
+		assertThat(errors.getFieldValue("bars"), is(notNullValue()));
+
+		errors.pushNestedPath("bars[0]");
+
+		try {
+			errors.getFieldValue("bars");
+			fail("Expected NotReadablePropertyException!");
+		} catch (NotReadablePropertyException e) {}
+
+		assertThat(errors.getFieldValue("field"), is((Object) "Hello"));
+	}
+
 	static class Foo {
-		List<Bar> bars = new ArrayList<Bar>();
+		List<Bar> bars = Collections.singletonList(new Bar());
 	}
 
 	static class Bar {
-		String field;
+		String field = "Hello";
 	}
 }