From 048394690f5fd48c5cd9ffdcd665622fa5cb3869 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 10 Oct 2013 13:01:09 +0200 Subject: [PATCH] DATACMNS-381 - Improved exception message in PropertyReferenceException. PropertyReferenceException now not only exposes the property not found but also the already resolved Property path to make it easier to understand what is causing the exception in the first place. --- .../data/mapping/PropertyPath.java | 40 ++++++++++++++----- .../mapping/PropertyReferenceException.java | 22 +++++++--- .../data/mapping/PropertyPathUnitTests.java | 16 +++++++- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/PropertyPath.java b/src/main/java/org/springframework/data/mapping/PropertyPath.java index a05a8667e..1a6548206 100644 --- a/src/main/java/org/springframework/data/mapping/PropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/PropertyPath.java @@ -18,6 +18,7 @@ package org.springframework.data.mapping; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Stack; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -63,7 +64,7 @@ public class PropertyPath implements Iterable { * @param owningType must not be {@literal null}. * @param base the {@link PropertyPath} previously found. */ - PropertyPath(String name, TypeInformation owningType, PropertyPath base) { + PropertyPath(String name, TypeInformation owningType, Stack base) { Assert.hasText(name); Assert.notNull(owningType); @@ -264,14 +265,14 @@ public class PropertyPath implements Iterable { Iterator parts = iteratorSource.iterator(); PropertyPath result = null; - PropertyPath current = null; + Stack current = new Stack(); while (parts.hasNext()) { if (result == null) { - result = create(parts.next(), type, null); - current = result; + result = create(parts.next(), type, current); + current.push(result); } else { - current = create(parts.next(), current); + current.push(create(parts.next(), current)); } } @@ -285,10 +286,12 @@ public class PropertyPath implements Iterable { * @param base * @return */ - private static PropertyPath create(String source, PropertyPath base) { + private static PropertyPath create(String source, Stack base) { - PropertyPath propertyPath = create(source, base.type, base); - base.next = propertyPath; + PropertyPath previous = base.peek(); + + PropertyPath propertyPath = create(source, previous.type, base); + previous.next = propertyPath; return propertyPath; } @@ -302,7 +305,7 @@ public class PropertyPath implements Iterable { * @param type * @return */ - private static PropertyPath create(String source, TypeInformation type, PropertyPath base) { + private static PropertyPath create(String source, TypeInformation type, Stack base) { return create(source, type, "", base); } @@ -317,7 +320,7 @@ public class PropertyPath implements Iterable { * @param addTail * @return */ - private static PropertyPath create(String source, TypeInformation type, String addTail, PropertyPath base) { + private static PropertyPath create(String source, TypeInformation type, String addTail, Stack base) { PropertyReferenceException exception = null; PropertyPath current = null; @@ -326,8 +329,14 @@ public class PropertyPath implements Iterable { current = new PropertyPath(source, type, base); + if (!base.isEmpty()) { + base.peek().next = current; + } + + base.push(current); + if (StringUtils.hasText(addTail)) { - current.next = create(addTail, current.type, current); + current.next = create(addTail, current.type, base); } return current; @@ -355,4 +364,13 @@ public class PropertyPath implements Iterable { throw exception; } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return String.format("%s.%s", owningType.getType().getSimpleName(), toDotPath()); + } } diff --git a/src/main/java/org/springframework/data/mapping/PropertyReferenceException.java b/src/main/java/org/springframework/data/mapping/PropertyReferenceException.java index 4255d7401..90ba20158 100644 --- a/src/main/java/org/springframework/data/mapping/PropertyReferenceException.java +++ b/src/main/java/org/springframework/data/mapping/PropertyReferenceException.java @@ -15,6 +15,8 @@ */ package org.springframework.data.mapping; +import java.util.Stack; + import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -26,20 +28,20 @@ import org.springframework.util.Assert; 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 static final String ERROR_TEMPLATE = "No property %s found for type %s!"; private final String propertyName; private final TypeInformation type; - private final PropertyPath base; + private final Stack 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}. + * @param base the previously calculated {@link PropertyPath}s. */ - public PropertyReferenceException(String propertyName, TypeInformation type, PropertyPath base) { + public PropertyReferenceException(String propertyName, TypeInformation type, Stack base) { Assert.hasText(propertyName); Assert.notNull(type); @@ -73,7 +75,15 @@ public class PropertyReferenceException extends RuntimeException { */ @Override public String getMessage() { - return String.format(ERROR_TEMPLATE, propertyName, type.getType().getName()); + + StringBuilder builder = new StringBuilder(String.format(ERROR_TEMPLATE, propertyName, type.getType() + .getSimpleName())); + + if (!base.isEmpty()) { + builder.append(" Traversed path: ").append(base.get(0).toString()).append("."); + } + + return builder.toString(); } /** @@ -82,6 +92,6 @@ public class PropertyReferenceException extends RuntimeException { * @return */ public PropertyPath getBaseProperty() { - return base; + return base.isEmpty() ? null : base.peek(); } } diff --git a/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java b/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java index 26dff01ef..b7c7da8c9 100644 --- a/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java @@ -36,8 +36,7 @@ import org.springframework.data.util.TypeInformation; @SuppressWarnings("unused") public class PropertyPathUnitTests { - @Rule - public ExpectedException exception = ExpectedException.none(); + @Rule public ExpectedException exception = ExpectedException.none(); @Test @SuppressWarnings("rawtypes") @@ -307,6 +306,19 @@ public class PropertyPathUnitTests { assertThat(path.next().getSegment(), is("UUID")); } + /** + * @see DATACMNS-381 + */ + public void exposesPreviouslyReferencedPathInExceptionMessage() { + + exception.expect(PropertyReferenceException.class); + exception.expectMessage("bar"); // missing variable + exception.expectMessage("String"); // type + exception.expectMessage("Bar.user.name"); // previously referenced path + + PropertyPath.from("userNameBar", Bar.class); + } + private class Foo { String userName;