DATACMNS-282 - Improved caching in AnnotationBasedPersistentProperty.

AnnotationBasedPersistentProperty now caches direct annotations on construction but still tries to lookup an annotation as meta-annotation if not found in cache on later requests. Extended try/catch block in AbstractMappingContext.addPersistentEntity(…) to invalidate cache on exceptions during property creation as well.

Author: odrotbohm

src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

@@ -278,11 +278,12 @@ protected E addPersistentEntity(TypeInformation<?> typeInformation) {
 				descriptors.put(descriptor.getName(), descriptor);
 			}
 
-			ReflectionUtils.doWithFields(type, new PersistentPropertyCreator(entity, descriptors),
-					PersistentFieldFilter.INSTANCE);
-
 			try {
+
+				ReflectionUtils.doWithFields(type, new PersistentPropertyCreator(entity, descriptors),
+						PersistentFieldFilter.INSTANCE);
 				entity.verify();
+
 			} catch (MappingException e) {
 				persistentEntities.remove(typeInformation);
 				throw e;

src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java

@@ -59,9 +59,49 @@ public AnnotationBasedPersistentProperty(Field field, PropertyDescriptor propert
 			PersistentEntity<?, P> owner, SimpleTypeHolder simpleTypeHolder) {
 
 		super(field, propertyDescriptor, owner, simpleTypeHolder);
+		populateAnnotationCache(field);
 		this.value = findAnnotation(Value.class);
 	}
 
+	/**
+	 * Populates the annotation cache by eagerly accessing the annotations directly annotated to the accessors (if
+	 * available) and the backing field. Annotations override annotations found on field.
+	 * 
+	 * @param field
+	 * @throws MappingException in case we find an ambiguous mapping on the accessor methods
+	 */
+	private final void populateAnnotationCache(Field field) {
+
+		for (Method method : Arrays.asList(getGetter(), getSetter())) {
+
+			if (method == null) {
+				continue;
+			}
+
+			for (Annotation annotation : method.getAnnotations()) {
+
+				Class<? extends Annotation> annotationType = annotation.annotationType();
+
+				if (annotationCache.containsKey(annotationType)) {
+					throw new MappingException(String.format("Ambiguous mapping! Annotation %s configured "
+							+ "multiple times on accessor methods of property %s in class %s!", annotationType, getName(), getOwner()
+							.getType().getName()));
+				}
+
+				annotationCache.put(annotationType, annotation);
+			}
+		}
+
+		for (Annotation annotation : field.getAnnotations()) {
+
+			Class<? extends Annotation> annotationType = annotation.annotationType();
+
+			if (!annotationCache.containsKey(annotationType)) {
+				annotationCache.put(annotationType, annotation);
+			}
+		}
+	}
+
 	/**
 	 * Inspects a potentially available {@link Value} annotation at the property and returns the {@link String} value of
 	 * it.

src/test/java/org/springframework/data/mapping/model/AbstractAnnotationBasedPropertyUnitTests.java

@@ -23,24 +23,30 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.data.annotation.Id;
 import org.springframework.data.mapping.context.SampleMappingContext;
 import org.springframework.data.mapping.context.SamplePersistentProperty;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.test.util.ReflectionTestUtils;
 
 /**
  * @author Oliver Gierke
  */
 public class AbstractAnnotationBasedPropertyUnitTests<P extends AnnotationBasedPersistentProperty<P>> {
 
 	BasicPersistentEntity<Object, SamplePersistentProperty> entity;
+	SampleMappingContext context;
 
 	@Before
 	public void setUp() {
 
-		SampleMappingContext context = new SampleMappingContext();
+		context = new SampleMappingContext();
 		entity = context.getPersistentEntity(Sample.class);
 	}
 
@@ -72,6 +78,47 @@ public void findsMetaAnnotation() {
 		assertAnnotationPresent(Id.class, entity.getPersistentProperty("id"));
 	}
 
+	/**
+	 * @see DATACMNS-282
+	 */
+	@Test
+	public void populatesAnnotationCacheWithDirectAnnotationsOnCreation() {
+
+		SamplePersistentProperty property = entity.getPersistentProperty("meta");
+
+		// Assert direct annotations are cached on construction
+		Map<Class<? extends Annotation>, Annotation> cache = getAnnotationCache(property);
+		assertThat(cache.containsKey(MyAnnotationAsMeta.class), is(true));
+		assertThat(cache.containsKey(MyAnnotation.class), is(false));
+
+		// Assert meta annotation is found and cached
+		MyAnnotation annotation = property.findAnnotation(MyAnnotation.class);
+		assertThat(annotation, is(notNullValue()));
+		assertThat(cache.containsKey(MyAnnotation.class), is(true));
+	}
+
+	/**
+	 * @see DATACMNS-282
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void discoversAmbiguousMappingUsingDirectAnnotationsOnAccessors() {
+
+		try {
+			context.getPersistentEntity(InvalidSample.class);
+			fail("Expected MappingException!");
+		} catch (MappingException o_O) {
+			ConcurrentMap<TypeInformation<?>, ?> entities = (ConcurrentMap<TypeInformation<?>, ?>) ReflectionTestUtils
+					.getField(context, "persistentEntities");
+			assertThat(entities.containsKey(ClassTypeInformation.from(InvalidSample.class)), is(false));
+		}
+	}
+
+	@SuppressWarnings("unchecked")
+	private Map<Class<? extends Annotation>, Annotation> getAnnotationCache(SamplePersistentProperty property) {
+		return (Map<Class<? extends Annotation>, Annotation>) ReflectionTestUtils.getField(property, "annotationCache");
+	}
+
 	private <A extends Annotation> A assertAnnotationPresent(Class<A> annotationType,
 			AnnotationBasedPersistentProperty<?> property) {
 
@@ -90,6 +137,9 @@ static class Sample {
 		String getter;
 		String setter;
 
+		@MyAnnotationAsMeta
+		String meta;
+
 		@MyAnnotation("field")
 		String override;
 
@@ -109,12 +159,34 @@ public String getOverride() {
 		}
 	}
 
+	static class InvalidSample {
+
+		String meta;
+
+		@MyAnnotation
+		public String getMeta() {
+			return meta;
+		}
+
+		@MyAnnotation
+		public void setMeta(String meta) {
+			this.meta = meta;
+		}
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
 	public static @interface MyAnnotation {
 		String value() default "";
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@Target(value = { FIELD, METHOD })
+	@MyAnnotation
+	public static @interface MyAnnotationAsMeta {
+
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
 	@Id