root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Honor contract of @Repeatable in AnnotationUtils 

Prior to this commit, the implementation of getRepeatableAnnotation()
in Spring's AnnotationUtils complied neither with the contract of
getAnnotationsByType() nor with the contract of
getDeclaredAnnotationsByType() as defined in AnnotatedElement in Java 8.

Specifically, unexpected results can be encountered when using Spring's
support for @Repeatable annotations: either annotations show up in the
returned set in the wrong order, or annotations are returned in the set
that should not even be found based on the semantics of @Repeatable.

This commit remedies this problem by deprecating the existing
getRepeatableAnnotation() methods and replacing them with new
getRepeatableAnnotations() and getDeclaredRepeatableAnnotations()
methods that comply with the contracts of Java's getAnnotationsByType()
and getDeclaredAnnotationsByType(), respectively.

Issue: SPR-13068
This commit is contained in:
Sam Brannen 2015-06-20 01:21:39 +02:00
parent 6c530b7bfb
commit fb83e83e78
4 changed files with 251 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java
View File

@ -224,7 +224,7 @@ public class ScheduledAnnotationBeanPostProcessor implements BeanPostProcessor,
@Override
public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
for (Scheduled scheduled :
AnnotationUtils.getRepeatableAnnotation(method, Schedules.class, Scheduled.class)) {
AnnotationUtils.getRepeatableAnnotations(method, Schedules.class, Scheduled.class)) {
processScheduled(scheduled, method, bean);
annotatedMethods.add(method);
}

146
spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java
View File

@ -61,8 +61,9 @@ import org.springframework.util.StringUtils;
* inheritance hierarchy of the given method ({@link #findAnnotation(Method, Class)}).
*
* <h3>Terminology</h3>
* The terms <em>directly present</em> and <em>present</em> have the same
* meanings as defined in the class-level Javadoc for {@link AnnotatedElement}.
* The terms <em>directly present</em>, <em>indirectly present</em>, and
* <em>present</em> have the same meanings as defined in the class-level
* Javadoc for {@link AnnotatedElement} (in Java 8).
*
* <p>An annotation is <em>meta-present</em> on an element if the annotation
* is declared as a meta-annotation on some other annotation which is
@ -248,53 +249,125 @@ public abstract class AnnotationUtils {
}
/**
* Get the <em>repeatable</em> {@link Annotation}s of {@code annotationType}
* from the supplied {@link Method}, where such annotations are either
* <em>present</em> or <em>meta-present</em> on the method.
* <p>Handles both single annotations and annotations nested within a
* <em>containing annotation</em>.
* <p>Correctly handles bridge {@link Method Methods} generated by the compiler.
* <p>Meta-annotations will be searched if the annotation is not
* <em>present</em> on the supplied method.
* @param method the method to look for annotations on; never {@code null}
* @param containerAnnotationType the type of the container that holds the
* annotations; may be {@code null} if a container is not supported
* @param annotationType the annotation type to look for
* @return the annotations found or an empty set; never {@code null}
* Delegates to {@link #getRepeatableAnnotations}.
* @since 4.0
* @see org.springframework.core.BridgeMethodResolver#findBridgedMethod(Method)
* @see java.lang.annotation.Repeatable
* @deprecated As of Spring Framework 4.2, use {@link #getRepeatableAnnotations}
* or {@link #getDeclaredRepeatableAnnotations} instead.
*/
@Deprecated
public static <A extends Annotation> Set<A> getRepeatableAnnotation(Method method,
Class<? extends Annotation> containerAnnotationType, Class<A> annotationType) {
Method resolvedMethod = BridgeMethodResolver.findBridgedMethod(method);
return getRepeatableAnnotation((AnnotatedElement) resolvedMethod, containerAnnotationType, annotationType);
return getRepeatableAnnotations(method, containerAnnotationType, annotationType);
}
/**
* Get the <em>repeatable</em> {@link Annotation}s of {@code annotationType}
* from the supplied {@link AnnotatedElement}, where such annotations are
* either <em>present</em> or <em>meta-present</em> on the element.
* Delegates to {@link #getRepeatableAnnotations}.
* @since 4.0
* @deprecated As of Spring Framework 4.2, use {@link #getRepeatableAnnotations}
* or {@link #getDeclaredRepeatableAnnotations} instead.
*/
@Deprecated
public static <A extends Annotation> Set<A> getRepeatableAnnotation(AnnotatedElement annotatedElement,
Class<? extends Annotation> containerAnnotationType, Class<A> annotationType) {
return getRepeatableAnnotations(annotatedElement, containerAnnotationType, annotationType);
}
/**
* Get the <em>repeatable</em> {@linkplain Annotation annotations} of
* {@code annotationType} from the supplied {@link AnnotatedElement}, where
* such annotations are either <em>present</em> or <em>meta-present</em>
* on the element.
* <p>This method mimics the functionality of Java 8's
* {@link java.lang.reflect.AnnotatedElement#getAnnotationsByType(Class)}
* with additional support for meta-annotations.
* <p>Handles both single annotations and annotations nested within a
* <em>containing annotation</em>.
* <em>container annotation</em>.
* <p>Correctly handles <em>bridge methods</em> generated by the
* compiler if the supplied element is a {@link Method}.
* <p>Meta-annotations will be searched if the annotation is not
* <em>present</em> on the supplied element.
* @param annotatedElement the element to look for annotations on; never {@code null}
* @param containerAnnotationType the type of the container that holds the
* annotations; may be {@code null} if a container is not supported
* @param annotationType the annotation type to look for
* @param containerAnnotationType the type of the container that holds
* the annotations; may be {@code null} if a container is not supported
* @param annotationType the annotation type to look for; never {@code null}
* @return the annotations found or an empty set; never {@code null}
* @since 4.0
* @since 4.2
* @see #getDeclaredRepeatableAnnotations(AnnotatedElement, Class, Class)
* @see org.springframework.core.BridgeMethodResolver#findBridgedMethod
* @see java.lang.annotation.Repeatable
* @see java.lang.reflect.AnnotatedElement#getAnnotationsByType
*/
public static <A extends Annotation> Set<A> getRepeatableAnnotation(AnnotatedElement annotatedElement,
public static <A extends Annotation> Set<A> getRepeatableAnnotations(AnnotatedElement annotatedElement,
Class<? extends Annotation> containerAnnotationType, Class<A> annotationType) {
Set<A> annotations = getDeclaredRepeatableAnnotations(annotatedElement, containerAnnotationType, annotationType);
if (!annotations.isEmpty()) {
return annotations;
}
return getRepeatableAnnotations(annotatedElement, containerAnnotationType, annotationType, false);
}
/**
* Get the declared <em>repeatable</em> {@linkplain Annotation annotations}
* of {@code annotationType} from the supplied {@link AnnotatedElement},
* where such annotations are either <em>directly present</em>,
* <em>indirectly present</em>, or <em>meta-present</em> on the element.
* <p>This method mimics the functionality of Java 8's
* {@link java.lang.reflect.AnnotatedElement#getDeclaredAnnotationsByType(Class)}
* with additional support for meta-annotations.
* <p>Handles both single annotations and annotations nested within a
* <em>container annotation</em>.
* <p>Correctly handles <em>bridge methods</em> generated by the
* compiler if the supplied element is a {@link Method}.
* <p>Meta-annotations will be searched if the annotation is not
* <em>present</em> on the supplied element.
* @param annotatedElement the element to look for annotations on; never {@code null}
* @param containerAnnotationType the type of the container that holds
* the annotations; may be {@code null} if a container is not supported
* @param annotationType the annotation type to look for; never {@code null}
* @return the annotations found or an empty set; never {@code null}
* @since 4.2
* @see #getRepeatableAnnotations(AnnotatedElement, Class, Class)
* @see org.springframework.core.BridgeMethodResolver#findBridgedMethod
* @see java.lang.annotation.Repeatable
* @see java.lang.reflect.AnnotatedElement#getDeclaredAnnotationsByType
*/
public static <A extends Annotation> Set<A> getDeclaredRepeatableAnnotations(AnnotatedElement annotatedElement,
Class<? extends Annotation> containerAnnotationType, Class<A> annotationType) {
return getRepeatableAnnotations(annotatedElement, containerAnnotationType, annotationType, true);
}
/**
* Perform the actual work for {@link #getRepeatableAnnotations(AnnotatedElement, Class, Class)}
* and {@link #getDeclaredRepeatableAnnotations(AnnotatedElement, Class, Class)}.
* <p>Correctly handles <em>bridge methods</em> generated by the
* compiler if the supplied element is a {@link Method}.
* <p>Meta-annotations will be searched if the annotation is not
* <em>present</em> on the supplied element.
*
* @param annotatedElement the element to look for annotations on; never {@code null}
* @param containerAnnotationType the type of the container that holds
* the annotations; may be {@code null} if a container is not supported
* @param annotationType the annotation type to look for; never {@code null}
* @param declaredMode {@code true} if only declared annotations (i.e.,
* directly or indirectly present) should be considered.
* @return the annotations found or an empty set; never {@code null}
* @since 4.2
* @see org.springframework.core.BridgeMethodResolver#findBridgedMethod
* @see java.lang.annotation.Repeatable
*/
private static <A extends Annotation> Set<A> getRepeatableAnnotations(AnnotatedElement annotatedElement,
Class<? extends Annotation> containerAnnotationType, Class<A> annotationType, boolean declaredMode) {
Assert.notNull(annotatedElement, "annotatedElement must not be null");
Assert.notNull(annotationType, "annotationType must not be null");
try {
if (annotatedElement.getAnnotations().length > 0) {
return new AnnotationCollector<A>(containerAnnotationType, annotationType).getResult(annotatedElement);
if (annotatedElement instanceof Method) {
annotatedElement = BridgeMethodResolver.findBridgedMethod((Method) annotatedElement);
}
return new AnnotationCollector<A>(containerAnnotationType, annotationType, declaredMode).getResult(annotatedElement);
}
catch (Exception ex) {
handleIntrospectionFailure(annotatedElement, ex);
@ -1596,16 +1669,20 @@ public abstract class AnnotationUtils {
private final Class<A> annotationType;
private final boolean declaredMode;
private final Set<AnnotatedElement> visited = new HashSet<AnnotatedElement>();
private final Set<A> result = new LinkedHashSet<A>();
public AnnotationCollector(Class<? extends Annotation> containerAnnotationType, Class<A> annotationType) {
AnnotationCollector(Class<? extends Annotation> containerAnnotationType, Class<A> annotationType, boolean declaredMode) {
this.containerAnnotationType = containerAnnotationType;
this.annotationType = annotationType;
this.declaredMode = declaredMode;
}
public Set<A> getResult(AnnotatedElement element) {
Set<A> getResult(AnnotatedElement element) {
process(element);
return Collections.unmodifiableSet(this.result);
}
@ -1614,7 +1691,8 @@ public abstract class AnnotationUtils {
private void process(AnnotatedElement element) {
if (this.visited.add(element)) {
try {
for (Annotation ann : element.getAnnotations()) {
Annotation[] annotations = (declaredMode ? element.getDeclaredAnnotations() : element.getAnnotations());
for (Annotation ann : annotations) {
Class<? extends Annotation> currentAnnotationType = ann.annotationType();
if (ObjectUtils.nullSafeEquals(this.annotationType, currentAnnotationType)) {
this.result.add(synthesizeAnnotation((A) ann, element));

157
spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java
View File

@ -493,40 +493,130 @@ public class AnnotationUtilsTests {
@Test
public void findRepeatableAnnotationOnComposedAnnotation() {
Repeatable repeatable = findAnnotation(MyRepeatableMeta.class, Repeatable.class);
Repeatable repeatable = findAnnotation(MyRepeatableMeta1.class, Repeatable.class);
assertNotNull(repeatable);
assertEquals(MyRepeatableContainer.class, repeatable.value());
}
@Test
public void getRepeatableFromMethod() throws Exception {
public void getRepeatableAnnotationsDeclaredOnMethod() throws Exception {
Method method = InterfaceWithRepeated.class.getMethod("foo");
Set<MyRepeatable> annotations = getRepeatableAnnotation(method, MyRepeatableContainer.class, MyRepeatable.class);
Set<MyRepeatable> annotations = getRepeatableAnnotations(method, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(annotations);
List<String> values = annotations.stream().map(MyRepeatable::value).collect(toList());
assertThat(values, equalTo(Arrays.asList("a", "b", "c", "meta")));
assertThat(values, is(Arrays.asList("A", "B", "C", "meta1")));
}
@Test
public void getRepeatableWithMissingAttributeAliasDeclaration() throws Exception {
public void getRepeatableAnnotationsDeclaredOnClassWithMissingAttributeAliasDeclaration() throws Exception {
exception.expect(AnnotationConfigurationException.class);
exception.expectMessage(containsString("Attribute [value] in"));
exception.expectMessage(containsString(BrokenContextConfig.class.getName()));
exception.expectMessage(containsString("must be declared as an @AliasFor [locations]"));
getRepeatableAnnotation(BrokenConfigHierarchyTestCase.class, BrokenHierarchy.class, BrokenContextConfig.class);
getRepeatableAnnotations(BrokenConfigHierarchyTestCase.class, BrokenHierarchy.class, BrokenContextConfig.class);
}
@Test
public void getRepeatableWithAttributeAliases() throws Exception {
Set<ContextConfig> annotations = getRepeatableAnnotation(ConfigHierarchyTestCase.class, Hierarchy.class,
ContextConfig.class);
public void getRepeatableAnnotationsDeclaredOnClassWithAttributeAliases() throws Exception {
final List<String> expectedLocations = Arrays.asList("A", "B");
Set<ContextConfig> annotations = getRepeatableAnnotations(ConfigHierarchyTestCase.class, Hierarchy.class, ContextConfig.class);
assertNotNull(annotations);
List<String> locations = annotations.stream().map(ContextConfig::locations).collect(toList());
assertThat(locations, equalTo(Arrays.asList("A", "B")));
assertThat(locations, is(expectedLocations));
List<String> values = annotations.stream().map(ContextConfig::value).collect(toList());
assertThat(values, equalTo(Arrays.asList("A", "B")));
assertThat(values, is(expectedLocations));
}
@Test
public void getRepeatableAnnotationsDeclaredOnClass() {
final List<String> expectedValuesJava = Arrays.asList("A", "B", "C");
final List<String> expectedValuesSpring = Arrays.asList("A", "B", "C", "meta1");
// Java 8
MyRepeatable[] array = MyRepeatableClass.class.getAnnotationsByType(MyRepeatable.class);
assertNotNull(array);
List<String> values = stream(array).map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesJava));
// Spring
Set<MyRepeatable> set = getRepeatableAnnotations(MyRepeatableClass.class, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(set);
values = set.stream().map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesSpring));
}
@Test
public void getRepeatableAnnotationsDeclaredOnSuperclass() {
final Class<?> clazz = SubMyRepeatableClass.class;
final List<String> expectedValuesJava = Arrays.asList("A", "B", "C");
final List<String> expectedValuesSpring = Arrays.asList("A", "B", "C", "meta1");
// Java 8
MyRepeatable[] array = clazz.getAnnotationsByType(MyRepeatable.class);
assertNotNull(array);
List<String> values = stream(array).map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesJava));
// Spring
Set<MyRepeatable> set = getRepeatableAnnotations(clazz, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(set);
values = set.stream().map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesSpring));
}
@Test
public void getRepeatableAnnotationsDeclaredOnClassAndSuperclass() {
final Class<?> clazz = SubMyRepeatableWithAdditionalLocalDeclarationsClass.class;
final List<String> expectedValuesJava = Arrays.asList("X", "Y", "Z");
final List<String> expectedValuesSpring = Arrays.asList("X", "Y", "Z", "meta2");
// Java 8
MyRepeatable[] array = clazz.getAnnotationsByType(MyRepeatable.class);
assertNotNull(array);
List<String> values = stream(array).map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesJava));
// Spring
Set<MyRepeatable> set = getRepeatableAnnotations(clazz, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(set);
values = set.stream().map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesSpring));
}
@Test
public void getDeclaredRepeatableAnnotationsDeclaredOnClass() {
final List<String> expectedValuesJava = Arrays.asList("A", "B", "C");
final List<String> expectedValuesSpring = Arrays.asList("A", "B", "C", "meta1");
// Java 8
MyRepeatable[] array = MyRepeatableClass.class.getDeclaredAnnotationsByType(MyRepeatable.class);
assertNotNull(array);
List<String> values = stream(array).map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesJava));
// Spring
Set<MyRepeatable> set = getDeclaredRepeatableAnnotations(MyRepeatableClass.class, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(set);
values = set.stream().map(MyRepeatable::value).collect(toList());
assertThat(values, is(expectedValuesSpring));
}
@Test
public void getDeclaredRepeatableAnnotationsDeclaredOnSuperclass() {
final Class<?> clazz = SubMyRepeatableClass.class;
// Java 8
MyRepeatable[] array = clazz.getDeclaredAnnotationsByType(MyRepeatable.class);
assertNotNull(array);
assertThat(array.length, is(0));
// Spring
Set<MyRepeatable> set = getDeclaredRepeatableAnnotations(clazz, MyRepeatableContainer.class, MyRepeatable.class);
assertNotNull(set);
assertThat(set.size(), is(0));
}
@Test
@ -915,6 +1005,8 @@ public class AnnotationUtilsTests {
@Test
public void synthesizeAnnotationWithAttributeAliasesInNestedAnnotations() throws Exception {
List<String> expectedLocations = Arrays.asList("A", "B");
Hierarchy hierarchy = ConfigHierarchyTestCase.class.getAnnotation(Hierarchy.class);
assertNotNull(hierarchy);
Hierarchy synthesizedHierarchy = synthesizeAnnotation(hierarchy);
@ -927,14 +1019,16 @@ public class AnnotationUtilsTests {
Arrays.stream(configs).allMatch(c -> c instanceof SynthesizedAnnotation));
List<String> locations = Arrays.stream(configs).map(ContextConfig::locations).collect(toList());
assertThat(locations, equalTo(Arrays.asList("A", "B")));
assertThat(locations, is(expectedLocations));
List<String> values = Arrays.stream(configs).map(ContextConfig::value).collect(toList());
assertThat(values, equalTo(Arrays.asList("A", "B")));
assertThat(values, is(expectedLocations));
}
@Test
public void synthesizeAnnotationWithArrayOfAnnotations() throws Exception {
List<String> expectedLocations = Arrays.asList("A", "B");
Hierarchy hierarchy = ConfigHierarchyTestCase.class.getAnnotation(Hierarchy.class);
assertNotNull(hierarchy);
Hierarchy synthesizedHierarchy = synthesizeAnnotation(hierarchy);
@ -945,7 +1039,7 @@ public class AnnotationUtilsTests {
ContextConfig[] configs = synthesizedHierarchy.value();
List<String> locations = Arrays.stream(configs).map(ContextConfig::locations).collect(toList());
assertThat(locations, equalTo(Arrays.asList("A", "B")));
assertThat(locations, is(expectedLocations));
// Alter array returned from synthesized annotation
configs[0] = contextConfig;
@ -953,7 +1047,7 @@ public class AnnotationUtilsTests {
// Re-retrieve the array from the synthesized annotation
configs = synthesizedHierarchy.value();
List<String> values = Arrays.stream(configs).map(ContextConfig::value).collect(toList());
assertThat(values, equalTo(Arrays.asList("A", "B")));
assertThat(values, is(expectedLocations));
}
@Test
@ -1231,18 +1325,39 @@ public class AnnotationUtilsTests {
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@MyRepeatable("meta")
@interface MyRepeatableMeta {
@MyRepeatable("meta1")
@interface MyRepeatableMeta1 {
}
public interface InterfaceWithRepeated {
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@MyRepeatable("meta2")
@interface MyRepeatableMeta2 {
}
@MyRepeatable("a")
@MyRepeatableContainer({ @MyRepeatable("b"), @MyRepeatable("c") })
@MyRepeatableMeta
interface InterfaceWithRepeated {
@MyRepeatable("A")
@MyRepeatableContainer({ @MyRepeatable("B"), @MyRepeatable("C") })
@MyRepeatableMeta1
void foo();
}
@MyRepeatable("A")
@MyRepeatableContainer({ @MyRepeatable("B"), @MyRepeatable("C") })
@MyRepeatableMeta1
static class MyRepeatableClass {
}
static class SubMyRepeatableClass extends MyRepeatableClass {
}
@MyRepeatable("X")
@MyRepeatableContainer({ @MyRepeatable("Y"), @MyRepeatable("Z") })
@MyRepeatableMeta2
static class SubMyRepeatableWithAdditionalLocalDeclarationsClass extends MyRepeatableClass {
}
enum RequestMethod {
GET, POST
}

4
spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java
View File

@ -123,10 +123,10 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
private void executeSqlScripts(TestContext testContext, ExecutionPhase executionPhase) throws Exception {
boolean classLevel = false;
Set<Sql> sqlAnnotations = AnnotationUtils.getRepeatableAnnotation(testContext.getTestMethod(), SqlGroup.class,
Set<Sql> sqlAnnotations = AnnotationUtils.getRepeatableAnnotations(testContext.getTestMethod(), SqlGroup.class,
Sql.class);
if (sqlAnnotations.isEmpty()) {
sqlAnnotations = AnnotationUtils.getRepeatableAnnotation(testContext.getTestClass(), SqlGroup.class,
sqlAnnotations = AnnotationUtils.getRepeatableAnnotations(testContext.getTestClass(), SqlGroup.class,
Sql.class);
if (!sqlAnnotations.isEmpty()) {
classLevel = true;