DATACMNS-172 - Improved handling of invalid PropertyPaths.

Introduced PropertyReferenceException to capture the invalid property and the base PropertyPath a property expression could be resolved into. Introduced getLeafProperty() to be able to access the leaf property of a property path being created.

spring-data-commons-core/src/main/java/org/springframework/data/mapping/PropertyPath.java

@@ -24,6 +24,7 @@ import java.util.regex.Pattern;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -35,7 +36,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
 
 	private static final String DELIMITERS = "_\\.";
 	private static final Pattern SPLITTER = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", DELIMITERS));
-	private static final String ERROR_TEMPLATE = "No property %s found for type %s";
 
 	private final TypeInformation<?> owningType;
 	private final String name;
@@ -52,7 +52,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 */
 	PropertyPath(String name, Class<?> owningType) {
 
-		this(name, ClassTypeInformation.from(owningType));
+		this(name, ClassTypeInformation.from(owningType), null);
 	}
 
 	/**
@@ -60,8 +60,9 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * 
 	 * @param name must not be {@literal null} or empty.
 	 * @param owningType must not be {@literal null}.
+	 * @param base the {@link PropertyPath} previously found.
 	 */
-	PropertyPath(String name, TypeInformation<?> owningType) {
+	PropertyPath(String name, TypeInformation<?> owningType, PropertyPath base) {
 
 		Assert.hasText(name);
 		Assert.notNull(owningType);
@@ -70,7 +71,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 		TypeInformation<?> type = owningType.getProperty(propertyName);
 
 		if (type == null) {
-			throw new IllegalArgumentException(String.format(ERROR_TEMPLATE, propertyName, owningType.getType()));
+			throw new PropertyReferenceException(propertyName, owningType, base);
 		}
 
 		this.owningType = owningType;
@@ -79,23 +80,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
 		this.name = propertyName;
 	}
 
-	/**
-	 * Creates a {@link PropertyPath} with the given name inside the given owning type and tries to resolve the other
-	 * {@link String} to create nested properties.
-	 * 
-	 * @param name must not be {@literal null} or empty.
-	 * @param owningType must not be {@literal null}.
-	 * @param toTraverse
-	 */
-	PropertyPath(String name, TypeInformation<?> owningType, String toTraverse) {
-
-		this(name, owningType);
-
-		if (StringUtils.hasText(toTraverse)) {
-			this.next = from(toTraverse, type);
-		}
-	}
-
 	/**
 	 * Returns the owning type of the {@link PropertyPath}.
 	 * 
@@ -111,10 +95,25 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * @return the name will never be {@literal null}.
 	 */
 	public String getSegment() {
-
 		return name;
 	}
 
+	/**
+	 * Returns the leaf property of the {@link PropertyPath}.
+	 * 
+	 * @return will never be {@literal null}.
+	 */
+	public PropertyPath getLeafProperty() {
+
+		PropertyPath result = this;
+
+		while (result.hasNext()) {
+			result = result.next();
+		}
+
+		return result;
+	}
+
 	/**
 	 * Returns the type of the property will return the plain resolved type for simple properties, the component type for
 	 * any {@link Iterable} or the value type of a {@link java.util.Map} if the property is one.
@@ -122,7 +121,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * @return
 	 */
 	public Class<?> getType() {
-
 		return this.type.getType();
 	}
 
@@ -189,7 +187,8 @@ public class PropertyPath implements Iterable<PropertyPath> {
 
 		PropertyPath that = (PropertyPath) obj;
 
-		return this.name.equals(that.name) && this.type.equals(that.type);
+		return this.name.equals(that.name) && this.type.equals(that.type)
+				&& ObjectUtils.nullSafeEquals(this.next, that.next);
 	}
 
 	/*
@@ -199,7 +198,13 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	@Override
 	public int hashCode() {
 
-		return name.hashCode() + type.hashCode();
+		int result = 17;
+
+		result += 31 * name.hashCode();
+		result += 31 * type.hashCode();
+		result += 31 * (next == null ? 0 : next.hashCode());
+
+		return result;
 	}
 
 	/* 
@@ -262,7 +267,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 
 		while (parts.hasNext()) {
 			if (result == null) {
-				result = create(parts.next(), type);
+				result = create(parts.next(), type, null);
 				current = result;
 			} else {
 				current = create(parts.next(), current);
@@ -281,7 +286,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 */
 	private static PropertyPath create(String source, PropertyPath base) {
 
-		PropertyPath propertyPath = create(source, base.type);
+		PropertyPath propertyPath = create(source, base.type, base);
 		base.next = propertyPath;
 		return propertyPath;
 	}
@@ -296,9 +301,9 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * @param type
 	 * @return
 	 */
-	private static PropertyPath create(String source, TypeInformation<?> type) {
+	private static PropertyPath create(String source, TypeInformation<?> type, PropertyPath base) {
 
-		return create(source, type, "");
+		return create(source, type, "", base);
 	}
 
 	/**
@@ -311,13 +316,27 @@ public class PropertyPath implements Iterable<PropertyPath> {
 	 * @param addTail
 	 * @return
 	 */
-	private static PropertyPath create(String source, TypeInformation<?> type, String addTail) {
+	private static PropertyPath create(String source, TypeInformation<?> type, String addTail, PropertyPath base) {
 
-		IllegalArgumentException exception = null;
+		PropertyReferenceException exception = null;
+		PropertyPath current = null;
 
 		try {
-			return new PropertyPath(source, type, addTail);
-		} catch (IllegalArgumentException e) {
+
+			current = new PropertyPath(source, type, base);
+
+			if (StringUtils.hasText(addTail)) {
+				current.next = create(addTail, current.type, current);
+			}
+
+			return current;
+
+		} catch (PropertyReferenceException e) {
+
+			if (current != null) {
+				throw e;
+			}
+
 			exception = e;
 		}
 
@@ -330,7 +349,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
 			String head = source.substring(0, position);
 			String tail = source.substring(position);
 
-			return create(head, type, tail + addTail);
+			return create(head, type, tail + addTail, base);
 		}
 
 		throw exception;

spring-data-commons-core/src/main/java/org/springframework/data/mapping/PropertyReferenceException.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2012 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 org.springframework.data.util.TypeInformation;
+import org.springframework.util.Assert;
+
+/**
+ * Exception being thrown when creating {@link PropertyPath} instances.
+ * 
+ * @author Oliver Gierke
+ */
+public class PropertyReferenceException extends RuntimeException {
+
+	private static final long serialVersionUID = -5254424051438976570L;
+	private static final String ERROR_TEMPLATE = "No property %s found for type %s";
+
+	private final String propertyName;
+	private final TypeInformation<?> type;
+	private final PropertyPath base;
+
+	/**
+	 * 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 base the base {@link PropertyPath}.
+	 */
+	public PropertyReferenceException(String propertyName, TypeInformation<?> type, PropertyPath base) {
+
+		Assert.hasText(propertyName);
+		Assert.notNull(type);
+
+		this.propertyName = propertyName;
+		this.type = type;
+		this.base = base;
+	}
+
+	/**
+	 * Returns the name of the property not found.
+	 * 
+	 * @return will not be {@literal null} or empty.
+	 */
+	public String getPropertyName() {
+		return propertyName;
+	}
+
+	/**
+	 * Returns the type the property could not be found on.
+	 * 
+	 * @return the type
+	 */
+	public TypeInformation<?> getType() {
+		return type;
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see java.lang.Throwable#getMessage()
+	 */
+	@Override
+	public String getMessage() {
+		return String.format(ERROR_TEMPLATE, propertyName, type.getType().getName());
+	}
+
+	/**
+	 * Returns the {@link PropertyPath} which could be resolved so far.
+	 * 
+	 * @return
+	 */
+	public PropertyPath getBaseProperty() {
+		return base;
+	}
+}

spring-data-commons-core/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java

@@ -22,7 +22,11 @@ import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
+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 PropertyPath}.
@@ -30,14 +34,19 @@ import org.junit.Test;
  * @author Oliver Gierke
  */
 @SuppressWarnings("unused")
-public class PropertyUnitTests {
+public class PropertyPathUnitTests {
+
+	@Rule
+	public ExpectedException exception = ExpectedException.none();
 
 	@Test
+	@SuppressWarnings("rawtypes")
 	public void parsesSimplePropertyCorrectly() throws Exception {
 
 		PropertyPath reference = PropertyPath.from("userName", Foo.class);
 		assertThat(reference.hasNext(), is(false));
 		assertThat(reference.toDotPath(), is("userName"));
+		assertThat(reference.getOwningType(), is((TypeInformation) ClassTypeInformation.from(Foo.class)));
 	}
 
 	@Test
@@ -105,22 +114,37 @@ public class PropertyUnitTests {
 		assertThat(reference.next(), is(new PropertyPath("name", FooBar.class)));
 	}
 
-	@Test(expected = IllegalArgumentException.class)
+	@Test
 	public void handlesInvalidCollectionCompountTypeProperl() {
 
-		PropertyPath.from("usersMame", Bar.class);
+		try {
+			PropertyPath.from("usersMame", Bar.class);
+			fail("Expected PropertyReferenceException!");
+		} catch (PropertyReferenceException e) {
+			assertThat(e.getPropertyName(), is("mame"));
+			assertThat(e.getBaseProperty(), is(PropertyPath.from("users", Bar.class)));
+		}
 	}
 
-	@Test(expected = IllegalArgumentException.class)
-	public void handlesInvalidMapValueTypeProperl() {
+	@Test
+	public void handlesInvalidMapValueTypeProperly() {
 
-		PropertyPath.from("userMapMame", Bar.class);
+		try {
+			PropertyPath.from("userMapMame", Bar.class);
+			fail();
+		} catch (PropertyReferenceException e) {
+			assertThat(e.getPropertyName(), is("mame"));
+			assertThat(e.getBaseProperty(), is(PropertyPath.from("userMap", Bar.class)));
+		}
 	}
 
 	@Test
 	public void findsNested() {
 
 		PropertyPath from = PropertyPath.from("barUserName", Sample.class);
+
+		assertThat(from, is(notNullValue()));
+		assertThat(from.getLeafProperty(), is(PropertyPath.from("name", FooBar.class)));
 	}
 
 	/**
@@ -139,7 +163,12 @@ public class PropertyUnitTests {
 
 	@Test
 	public void supportsDotNotationAsWell() {
-		PropertyPath.from("bar.userMap.name", Sample.class);
+
+		PropertyPath propertyPath = PropertyPath.from("bar.userMap.name", Sample.class);
+
+		assertThat(propertyPath, is(notNullValue()));
+		assertThat(propertyPath.getSegment(), is("bar"));
+		assertThat(propertyPath.getLeafProperty(), is(PropertyPath.from("name", FooBar.class)));
 	}
 
 	@Test
@@ -174,7 +203,7 @@ public class PropertyUnitTests {
 		try {
 			PropertyPath.from("_id", Foo.class);
 			fail();
-		} catch (IllegalArgumentException e) {
+		} catch (PropertyReferenceException e) {
 			assertThat(e.getMessage(), containsString("property _id"));
 		}
 	}
@@ -187,7 +216,7 @@ public class PropertyUnitTests {
 		try {
 			PropertyPath.from("_foo_id", Sample2.class);
 			fail();
-		} catch (IllegalArgumentException e) {
+		} catch (PropertyReferenceException e) {
 			assertThat(e.getMessage(), containsString("property id"));
 		}
 	}
@@ -200,7 +229,7 @@ public class PropertyUnitTests {
 		try {
 			PropertyPath.from("_foo__id", Sample2.class);
 			fail();
-		} catch (IllegalArgumentException e) {
+		} catch (PropertyReferenceException e) {
 			assertThat(e.getMessage(), containsString("property _id"));
 		}
 	}
@@ -208,11 +237,50 @@ public class PropertyUnitTests {
 	/**
 	 * @see DATACMNS 158
 	 */
-	@Test(expected = IllegalArgumentException.class)
+	@Test(expected = PropertyReferenceException.class)
 	public void rejectsInvalidPathsContainingDigits() {
 		PropertyPath.from("PropertyThatWillFail4Sure", Foo.class);
 	}
 
+	@Test
+	public void rejectsInvalidProperty() {
+
+		try {
+			PropertyPath.from("bar", Foo.class);
+			fail();
+		} catch (PropertyReferenceException e) {
+			assertThat(e.getBaseProperty(), is(nullValue()));
+		}
+	}
+
+	@Test
+	public void samePathsEqual() {
+
+		PropertyPath left = PropertyPath.from("user.name", Bar.class);
+		PropertyPath right = PropertyPath.from("user.name", Bar.class);
+
+		PropertyPath shortPath = PropertyPath.from("user", Bar.class);
+
+		assertThat(left, is(right));
+		assertThat(right, is(left));
+		assertThat(left, is(not(shortPath)));
+		assertThat(shortPath, is(not(left)));
+
+		assertThat(left, is(not(new Object())));
+	}
+
+	@Test
+	public void hashCodeTests() {
+
+		PropertyPath left = PropertyPath.from("user.name", Bar.class);
+		PropertyPath right = PropertyPath.from("user.name", Bar.class);
+
+		PropertyPath shortPath = PropertyPath.from("user", Bar.class);
+
+		assertThat(left.hashCode(), is(right.hashCode()));
+		assertThat(left.hashCode(), is(not(shortPath.hashCode())));
+	}
+
 	private class Foo {
 
 		String userName;