From 2da5acc55354a246e3c08fc93b8be0ccc80854ec Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Thu, 26 Jan 2017 15:00:05 +0100
Subject: [PATCH] DATACMNS-981 - Consider merged annotations when populating
 for annotation cache.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We now consider property annotations as merged annotations when initially scanning for annotations in a property declaration. Composed annotations such as declared outside our code are captured correctly because they use an own type that is not queried by users of PersistentProperty. Own, revised annotations, using @AliasFor providing an alias for annotation values require merged annotation processing.

Previously, annotations were cached as-is without resolving @AliasFor. This caused a later lookup via findAnnotation(…) to return the cached annotation without aliasing. Because the annotation was cached, no further lookup via AnnotatedElementUtils.findMergedAnnotation(…) was attempted.

@Retention(RetentionPolicy.RUNTIME)
@Target(value = { FIELD, METHOD })
public @interface RevisedAnnnotationWithAliasFor {

	@AliasFor("value")
	String name() default "";

	@AliasFor("name")
	String value() default "";
}

public class Person {
	@RevisedAnnnotationWithAliasFor(value = "spring")
	String firstname;
}

PersistentProperty firstname = …

property.findAnnotation(…) returned previously @RevisedAnnnotationWithAliasFor(value = "spring", name = "")

now we return @RevisedAnnnotationWithAliasFor(value = "spring", name = "spring")
---
 .../AnnotationBasedPersistentProperty.java    | 44 ++++++++++---------
 ...ationBasedPersistentPropertyUnitTests.java | 37 +++++++++++++---
 2 files changed, 53 insertions(+), 28 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 6777ee5cd..bc5fd8ee8 100644
--- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
+++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2016 the original author or authors.
+ * Copyright 2011-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,9 +42,10 @@ import org.springframework.util.Assert;
 
 /**
  * Special {@link PersistentProperty} that takes annotations at a property into account.
- * 
+ *
  * @author Oliver Gierke
  * @author Christoph Strobl
+ * @author Mark Paluch
  */
 public abstract class AnnotationBasedPersistentProperty<P extends PersistentProperty<P>>
 		extends AbstractPersistentProperty<P> {
@@ -59,9 +60,8 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 
 	/**
 	 * Creates a new {@link AnnotationBasedPersistentProperty}.
-	 * 
-	 * @param field must not be {@literal null}.
-	 * @param propertyDescriptor must not be {@literal null}.
+	 *
+	 * @param property must not be {@literal null}.
 	 * @param owner must not be {@literal null}.
 	 */
 	public AnnotationBasedPersistentProperty(Property property, PersistentEntity<?, P> owner,
@@ -79,8 +79,8 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	/**
 	 * 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
+	 *
+	 * @param property
 	 * @throws MappingException in case we find an ambiguous mapping on the accessor methods
 	 */
 	private final void populateAnnotationCache(Property property) {
@@ -96,7 +96,8 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 								+ "multiple times on accessor methods of property %s in class %s!",
 						annotationType.getSimpleName(), getName(), getOwner().getType().getSimpleName());
 
-				annotationCache.put(annotationType, Optional.of(annotation));
+				annotationCache.put(annotationType,
+						Optional.of(AnnotatedElementUtils.findMergedAnnotation(it, annotationType)));
 			}
 		});
 
@@ -110,7 +111,8 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 						"Ambiguous mapping! Annotation %s configured " + "on field %s and one of its accessor methods in class %s!",
 						annotationType.getSimpleName(), it.getName(), getOwner().getType().getSimpleName());
 
-				annotationCache.put(annotationType, Optional.of(annotation));
+				annotationCache.put(annotationType,
+						Optional.of(AnnotatedElementUtils.findMergedAnnotation(it, annotationType)));
 			}
 		});
 	}
@@ -118,7 +120,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	/**
 	 * 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}.
@@ -140,7 +142,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	/**
 	 * Inspects a potentially available {@link Value} annotation at the property and returns the {@link String} value of
 	 * it.
-	 * 
+	 *
 	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#getSpelExpression()
 	 */
 	@Override
@@ -151,7 +153,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	/**
 	 * Considers plain transient fields, fields annotated with {@link Transient}, {@link Value} or {@link Autowired} as
 	 * transien.
-	 * 
+	 *
 	 * @see org.springframework.data.mapping.BasicPersistentProperty#isTransient()
 	 */
 	@Override
@@ -174,7 +176,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 		return isAnnotationPresent(Id.class);
 	}
 
-	/* 
+	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.PersistentProperty#isVersionProperty()
 	 */
@@ -190,7 +192,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 		return !isTransient() && isAnnotationPresent(Reference.class);
 	}
 
-	/* 
+	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#isWritable()
 	 */
@@ -203,7 +205,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	 * Returns the annotation found for the current {@link AnnotationBasedPersistentProperty}. Will prefer getters or
 	 * setters annotations over ones found at the backing field as the former can be used to reconfigure the metadata in
 	 * subclasses.
-	 * 
+	 *
 	 * @param annotationType must not be {@literal null}.
 	 * @return
 	 */
@@ -223,7 +225,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 						.findFirst());
 	}
 
-	/* 
+	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.PersistentProperty#findPropertyOrOwnerAnnotation(java.lang.Class)
 	 */
@@ -236,7 +238,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 
 	/**
 	 * Puts the given annotation into the local cache and returns it.
-	 * 
+	 *
 	 * @param annotation
 	 * @return
 	 */
@@ -251,7 +253,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 
 	/**
 	 * Returns whether the property carries the an annotation of the given type.
-	 * 
+	 *
 	 * @param annotationType the annotation type to look up.
 	 * @return
 	 */
@@ -259,7 +261,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 		return findAnnotation(annotationType).isPresent();
 	}
 
-	/* 
+	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#usePropertyAccess()
 	 */
@@ -268,7 +270,7 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 		return super.usePropertyAccess() || usePropertyAccess;
 	}
 
-	/* 
+	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#toString()
 	 */
@@ -288,6 +290,6 @@ public abstract class AnnotationBasedPersistentProperty<P extends PersistentProp
 	}
 
 	private Stream<Optional<? extends AnnotatedElement>> getAccessors() {
-		return Arrays.<Optional<? extends AnnotatedElement>>asList(getGetter(), getSetter(), getField()).stream();
+		return Arrays.<Optional<? extends AnnotatedElement>> asList(getGetter(), getSetter(), getField()).stream();
 	}
 }
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 cb13345ec..a350f6af0 100755
--- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java
+++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java
@@ -29,6 +29,7 @@ import javax.annotation.Nullable;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.data.annotation.AccessType;
@@ -42,7 +43,7 @@ import org.springframework.test.util.ReflectionTestUtils;
 
 /**
  * Unit tests for {@link AnnotationBasedPersistentProperty}.
- * 
+ *
  * @author Oliver Gierke
  * @author Christoph Strobl
  */
@@ -223,6 +224,17 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 		});
 	}
 
+	@Test // DATACMNS-867
+	public void revisedAnnotationWithAliasShouldHaveSynthesizedAttributeValues() {
+
+		assertThat(entity.getPersistentProperty("setter")).hasValueSatisfying(property -> {
+			assertThat(property.findAnnotation(RevisedAnnnotationWithAliasFor.class)).hasValueSatisfying(annotation -> {
+				assertThat(annotation.name()).isEqualTo("my-value");
+				assertThat(annotation.value()).isEqualTo("my-value");
+			});
+		});
+	}
+
 	@SuppressWarnings("unchecked")
 	private Map<Class<? extends Annotation>, Annotation> getAnnotationCache(SamplePersistentProperty property) {
 		return (Map<Class<? extends Annotation>, Annotation>) ReflectionTestUtils.getField(property, "annotationCache");
@@ -248,7 +260,7 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 
 		@MyAnnotation String field;
 		String getter;
-		String setter;
+		@RevisedAnnnotationWithAliasFor(value = "my-value") String setter;
 		String doubleMapping;
 
 		@MyAnnotationAsMeta String meta;
@@ -310,30 +322,41 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
-	public static @interface MyAnnotation {
+	public @interface MyAnnotation {
 		String value() default "";
 	}
 
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD })
 	@MyAnnotation
-	public static @interface MyAnnotationAsMeta {
+	public @interface MyAnnotationAsMeta {
 
 	}
 
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD })
 	@MyAnnotation
-	public static @interface MyComposedAnnotationUsingAliasFor {
+	public @interface MyComposedAnnotationUsingAliasFor {
 
 		@AliasFor(annotation = MyAnnotation.class, attribute = "value")
 		String name() default "spring";
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@Target(value = { FIELD, METHOD })
+	@interface RevisedAnnnotationWithAliasFor {
+
+		@AliasFor("value")
+		String name() default "";
+
+		@AliasFor("name")
+		String value() default "";
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
 	@Id
-	public static @interface MyId {
+	public @interface MyId {
 	}
 
 	static class FieldAccess {
@@ -380,7 +403,7 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 	@ReadOnlyProperty
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(FIELD)
-	static @interface CustomReadOnly {
+	@interface CustomReadOnly {
 
 	}
 }