From 0bbf0aec99e23de346c6b4ed9e34e4af05ef2204 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 11 Feb 2013 15:56:14 +0100 Subject: [PATCH] DATACMNS-282 - Improved caching in AnnotationBasedPersistentProperty. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../context/AbstractMappingContext.java | 7 +- .../AnnotationBasedPersistentProperty.java | 40 ++++++++++ ...tractAnnotationBasedPropertyUnitTests.java | 74 ++++++++++++++++++- 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index 68d5bfb9e..97128572f 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -278,11 +278,12 @@ public abstract class AbstractMappingContext 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 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 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. diff --git a/src/test/java/org/springframework/data/mapping/model/AbstractAnnotationBasedPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AbstractAnnotationBasedPropertyUnitTests.java index 3df79a599..1d5627e91 100644 --- a/src/test/java/org/springframework/data/mapping/model/AbstractAnnotationBasedPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AbstractAnnotationBasedPropertyUnitTests.java @@ -23,12 +23,17 @@ import java.lang.annotation.Annotation; 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 @@ -36,11 +41,12 @@ import org.springframework.data.mapping.context.SamplePersistentProperty; public class AbstractAnnotationBasedPropertyUnitTests

> { BasicPersistentEntity entity; + SampleMappingContext context; @Before public void setUp() { - SampleMappingContext context = new SampleMappingContext(); + context = new SampleMappingContext(); entity = context.getPersistentEntity(Sample.class); } @@ -72,6 +78,47 @@ public class AbstractAnnotationBasedPropertyUnitTests

, 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, ?> entities = (ConcurrentMap, ?>) ReflectionTestUtils + .getField(context, "persistentEntities"); + assertThat(entities.containsKey(ClassTypeInformation.from(InvalidSample.class)), is(false)); + } + } + + @SuppressWarnings("unchecked") + private Map, Annotation> getAnnotationCache(SamplePersistentProperty property) { + return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); + } + private A assertAnnotationPresent(Class annotationType, AnnotationBasedPersistentProperty property) { @@ -90,6 +137,9 @@ public class AbstractAnnotationBasedPropertyUnitTests