From 535270137478391d86c92bb9ce191d19fdbcb387 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 650c5196f..17efb3ebd 100644
--- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
+++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
@@ -47,6 +47,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>();
 
@@ -91,11 +93,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);
 			}
@@ -109,16 +109,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 b3c74c712..fb8988f94 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;
@@ -156,8 +158,8 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 	/**
 	 * @see DATACMNS-243
 	 */
-	@Test(expected = MappingException.class)
-	public void detectsAmbiguityCausedByFieldAnAccessorAnnotation() {
+	@Test
+	public void doesNotRejectSameAnnotationIfItsEqualOnBothFieldAndAccessor() {
 		context.getPersistentEntity(AnotherInvalidSample.class);
 	}
 
@@ -193,6 +195,14 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 		assertThat(getProperty(ClassWithReadOnlyProperties.class, "customReadOnlyProperty").isWritable(), is(false));
 	}
 
+	/**
+	 * @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");
@@ -317,6 +327,16 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 		@CustomReadOnly String customReadOnlyProperty;
 	}
 
+	static class TypeWithCustomAnnotationsOnBothFieldAndAccessor {
+
+		@Nullable String field;
+
+		@Nullable
+		public String getField() {
+			return field;
+		}
+	}
+
 	@ReadOnlyProperty
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(FIELD)