DATACMNS-801 - Make sure PropertyReferenceException does not expose duplicate potential matches.

We now use a Set to collect potential matches from fields and accessors.

Polished assertions and JavaDoc in PropertyPath along the way.

Author: odrotbohm

src/main/java/org/springframework/data/mapping/PropertyPath.java

@@ -16,6 +16,7 @@
 package org.springframework.data.mapping;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Stack;
@@ -53,7 +54,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * @param owningType must not be {@literal null}.
 	 */
 	PropertyPath(String name, Class<?> owningType) {
-		this(name, ClassTypeInformation.from(owningType), null);
+		this(name, ClassTypeInformation.from(owningType), Collections.<PropertyPath> emptyList());
 	}
 
 	/**
@@ -65,8 +66,9 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 */
 	PropertyPath(String name, TypeInformation<?> owningType, List<PropertyPath> base) {
 
-		Assert.hasText(name);
-		Assert.notNull(owningType);
+		Assert.hasText(name, "Name must not be null or empty!");
+		Assert.notNull(owningType, "Owning type must not be null!");
+		Assert.notNull(base, "Perviously found properties must not be null!");
 
 		String propertyName = name.matches(ALL_UPPERCASE) ? name : StringUtils.uncapitalize(name);
 		TypeInformation<?> propertyType = owningType.getProperty(propertyName);

src/main/java/org/springframework/data/mapping/PropertyReferenceException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2015 the original author or authors.
+ * Copyright 2012-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.
@@ -15,9 +15,11 @@
  */
 package org.springframework.data.mapping;
 
-import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.springframework.beans.PropertyMatches;
 import org.springframework.data.util.TypeInformation;
@@ -38,20 +40,21 @@ public class PropertyReferenceException extends RuntimeException {
 	private final String propertyName;
 	private final TypeInformation<?> type;
 	private final List<PropertyPath> alreadyResolvedPath;
-	private final List<String> propertyMatches;
+	private final Set<String> propertyMatches;
 
 	/**
 	 * Creates a new {@link PropertyReferenceException}.
 	 * 
-	 * @param propertyName the name of the property not found on the given type.
-	 * @param type the type the property could not be found on.
-	 * @param alreadyResolvedPah the previously calculated {@link PropertyPath}s.
+	 * @param propertyName the name of the property not found on the given type, must not be {@literal null} or empty.
+	 * @param type the type the property could not be found on, must not be {@literal null}.
+	 * @param alreadyResolvedPah the previously calculated {@link PropertyPath}s, must not be {@literal null}.
 	 */
 	public PropertyReferenceException(String propertyName, TypeInformation<?> type,
 			List<PropertyPath> alreadyResolvedPah) {
 
-		Assert.hasText(propertyName);
-		Assert.notNull(type);
+		Assert.hasText(propertyName, "Property name must not be null!");
+		Assert.notNull(type, "Type must not be null!");
+		Assert.notNull(alreadyResolvedPah, "Already resolved paths must not be null!");
 
 		this.propertyName = propertyName;
 		this.type = type;
@@ -71,12 +74,21 @@ public String getPropertyName() {
 	/**
 	 * Returns the type the property could not be found on.
 	 * 
-	 * @return the type
+	 * @return will never be {@literal null}.
 	 */
 	public TypeInformation<?> getType() {
 		return type;
 	}
 
+	/**
+	 * Returns the properties that the invalid property might have been meant to be referred to.
+	 * 
+	 * @return will never be {@literal null}.
+	 */
+	Collection<String> getPropertyMatches() {
+		return propertyMatches;
+	}
+
 	/* 
 	 * (non-Javadoc)
 	 * @see java.lang.Throwable#getMessage()
@@ -114,7 +126,7 @@ public PropertyPath getBaseProperty() {
 	 * Returns whether the given {@link PropertyReferenceException} has a deeper resolution depth (i.e. a longer path of
 	 * already resolved properties) than the current exception.
 	 * 
-	 * @param exception
+	 * @param exception must not be {@literal null}.
 	 * @return
 	 */
 	public boolean hasDeeperResolutionDepthThan(PropertyReferenceException exception) {
@@ -128,9 +140,9 @@ public boolean hasDeeperResolutionDepthThan(PropertyReferenceException exception
 	 * @param type must not be {@literal null}.
 	 * @return
 	 */
-	private static List<String> detectPotentialMatches(String propertyName, Class<?> type) {
+	private static Set<String> detectPotentialMatches(String propertyName, Class<?> type) {
 
-		List<String> result = new ArrayList<String>();
+		Set<String> result = new HashSet<String>();
 		result.addAll(Arrays.asList(PropertyMatches.forField(propertyName, type).getPossibleMatches()));
 		result.addAll(Arrays.asList(PropertyMatches.forProperty(propertyName, type).getPossibleMatches()));
 

src/test/java/org/springframework/data/mapping/PropertyReferenceExceptionUnitTests.java

@@ -0,0 +1,96 @@
+/*
+ * 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.mapping;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
+
+/**
+ * Unit tests for {@link PropertyReferenceException}
+ *
+ * @author Oliver Gierke
+ */
+public class PropertyReferenceExceptionUnitTests {
+
+	static final TypeInformation<Sample> TYPE_INFO = ClassTypeInformation.from(Sample.class);
+	static final List<PropertyPath> NO_PATHS = Collections.emptyList();
+
+	public @Rule ExpectedException exception = ExpectedException.none();
+
+	@Test
+	public void rejectsNullPropertyName() {
+
+		exception.expect(IllegalArgumentException.class);
+		exception.expectMessage("name");
+
+		new PropertyReferenceException(null, TYPE_INFO, NO_PATHS);
+	}
+
+	@Test
+	public void rejectsNullType() {
+
+		exception.expect(IllegalArgumentException.class);
+		exception.expectMessage("Type");
+
+		new PropertyReferenceException("nme", null, NO_PATHS);
+	}
+
+	@Test
+	public void rejectsNullPaths() {
+
+		exception.expect(IllegalArgumentException.class);
+		exception.expectMessage("paths");
+
+		new PropertyReferenceException("nme", TYPE_INFO, null);
+	}
+
+	/**
+	 * @see DATACMNS-801
+	 */
+	@Test
+	public void exposesPotentialMatch() {
+
+		PropertyReferenceException exception = new PropertyReferenceException("nme", TYPE_INFO, NO_PATHS);
+
+		Collection<String> matches = exception.getPropertyMatches();
+
+		assertThat(matches, hasSize(1));
+		assertThat(matches, hasItem("name"));
+	}
+
+	static class Sample {
+
+		String name;
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+}