From 33698a764900c3d8f8bec493305bacca7eefb4cc Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Sun, 10 Aug 2014 13:10:45 +0200
Subject: [PATCH] DATACMNS-556 - Make annotation validation in
 AnnotationBasedPersistentProperty more lenient.

Annotations used on both fields and accessors are now only rejected if they're Spring Data annotations (i.e. from the org.springframework.data namespace) and the configurations of these annotations differ.
---
 .../AnnotationBasedPersistentProperty.java    | 39 ++++++++++++++-----
 ...ationBasedPersistentPropertyUnitTests.java | 24 +++++++++++-
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
index 871344c93..d3af8844b 100644
--- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
+++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
@@ -45,6 +45,8 @@ import org.springframework.util.Assert;
 public abstract class AnnotationBasedPersistentProperty<P extends PersistentProperty<P>> extends
 		AbstractPersistentProperty<P> {
 
+	private static final String SPRING_DATA_PACKAGE = "org.springframework.data";
+
 	private final Value value;
 	private final Map<Class<? extends Annotation>, Annotation> annotationCache = new HashMap<Class<? extends Annotation>, Annotation>();
 
@@ -89,11 +91,9 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 
 				Class<? extends Annotation> annotationType = annotation.annotationType();
 
-				if (annotationCache.containsKey(annotationType) && !annotationCache.get(annotationType).equals(annotation)) {
-					throw new MappingException(String.format("Ambiguous mapping! Annotation %s configured "
-							+ "multiple times on accessor methods of property %s in class %s!", annotationType.getSimpleName(),
-							getName(), getOwner().getType().getSimpleName()));
-				}
+				validateAnnotation(annotation, "Ambiguous mapping! Annotation %s configured "
+						+ "multiple times on accessor methods of property %s in class %s!", annotationType.getSimpleName(),
+						getName(), getOwner().getType().getSimpleName());
 
 				annotationCache.put(annotationType, annotation);
 			}
@@ -107,16 +107,35 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 
 			Class<? extends Annotation> annotationType = annotation.annotationType();
 
-			if (annotationCache.containsKey(annotationType)) {
-				throw new MappingException(String.format("Ambiguous mapping! Annotation %s configured "
-						+ "on field %s and one of its accessor methods in class %s!", annotationType.getSimpleName(),
-						field.getName(), getOwner().getType().getSimpleName()));
-			}
+			validateAnnotation(annotation, "Ambiguous mapping! Annotation %s configured "
+					+ "on field %s and one of its accessor methods in class %s!", annotationType.getSimpleName(),
+					field.getName(), getOwner().getType().getSimpleName());
 
 			annotationCache.put(annotationType, annotation);
 		}
 	}
 
+	/**
+	 * Verifies the given annotation candidate detected. Will be rejected if it's a Spring Data annotation and we already
+	 * found another one with a different configuration setup (i.e. other attribute values).
+	 * 
+	 * @param candidate must not be {@literal null}.
+	 * @param message must not be {@literal null}.
+	 * @param arguments must not be {@literal null}.
+	 */
+	private void validateAnnotation(Annotation candidate, String message, Object... arguments) {
+
+		Class<? extends Annotation> annotationType = candidate.annotationType();
+
+		if (!annotationType.getName().startsWith(SPRING_DATA_PACKAGE)) {
+			return;
+		}
+
+		if (annotationCache.containsKey(annotationType) && !annotationCache.get(annotationType).equals(candidate)) {
+			throw new MappingException(String.format(message, arguments));
+		}
+	}
+
 	/**
 	 * Inspects a potentially available {@link Value} annotation at the property and returns the {@link String} value of
 	 * it.
diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java
index 2dc0cea8d..e567eb20e 100644
--- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java
+++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java
@@ -25,6 +25,8 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.util.Map;
 
+import javax.annotation.Nullable;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.data.annotation.AccessType;
@@ -153,11 +155,19 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 	/**
 	 * @see DATACMNS-243
 	 */
-	@Test(expected = MappingException.class)
-	public void detectsAmbiguityCausedByFieldAnAccessorAnnotation() {
+	@Test
+	public void doesNotRejectSameAnnotationIfItsEqualOnBothFieldAndAccessor() {
 		context.getPersistentEntity(AnotherInvalidSample.class);
 	}
 
+	/**
+	 * @see DATACMNS-556
+	 */
+	@Test
+	public void doesNotRejectNonSpringDataAnnotationsUsedOnBothFieldAndAccessor() {
+		getProperty(TypeWithCustomAnnotationsOnBothFieldAndAccessor.class, "field");
+	}
+
 	@SuppressWarnings("unchecked")
 	private Map<Class<? extends Annotation>, Annotation> getAnnotationCache(SamplePersistentProperty property) {
 		return (Map<Class<? extends Annotation>, Annotation>) ReflectionTestUtils.getField(property, "annotationCache");
@@ -270,4 +280,14 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 			return lastname;
 		}
 	}
+
+	static class TypeWithCustomAnnotationsOnBothFieldAndAccessor {
+
+		@Nullable String field;
+
+		@Nullable
+		public String getField() {
+			return field;
+		}
+	}
 }