8004912: Repeating annotations - getAnnotationsByType(Class<T>) is not working as expected for few inheritance scenarios
8019420: Repeatable non-inheritable annotation types are mishandled by Core Reflection
Reviewed-by: jfranck
--- a/jdk/src/share/classes/java/lang/Class.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/java/lang/Class.java Tue Oct 22 12:35:27 2013 +0200
@@ -3314,7 +3314,10 @@
public <A extends Annotation> A[] getAnnotationsByType(Class<A> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(annotationData().annotations, annotationClass);
+ AnnotationData annotationData = annotationData();
+ return AnnotationSupport.getAssociatedAnnotations(annotationData.declaredAnnotations,
+ annotationData.annotations,
+ annotationClass);
}
/**
@@ -3344,7 +3347,8 @@
public <A extends Annotation> A[] getDeclaredAnnotationsByType(Class<A> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(annotationData().declaredAnnotations, annotationClass);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(annotationData().declaredAnnotations,
+ annotationClass);
}
/**
--- a/jdk/src/share/classes/java/lang/reflect/Executable.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/java/lang/reflect/Executable.java Tue Oct 22 12:35:27 2013 +0200
@@ -527,7 +527,7 @@
public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(declaredAnnotations(), annotationClass);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(declaredAnnotations(), annotationClass);
}
/**
--- a/jdk/src/share/classes/java/lang/reflect/Field.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/java/lang/reflect/Field.java Tue Oct 22 12:35:27 2013 +0200
@@ -1123,7 +1123,7 @@
public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(declaredAnnotations(), annotationClass);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(declaredAnnotations(), annotationClass);
}
/**
--- a/jdk/src/share/classes/java/lang/reflect/Parameter.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/java/lang/reflect/Parameter.java Tue Oct 22 12:35:27 2013 +0200
@@ -295,7 +295,7 @@
public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(declaredAnnotations(), annotationClass);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(declaredAnnotations(), annotationClass);
}
/**
--- a/jdk/src/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java Tue Oct 22 12:35:27 2013 +0200
@@ -166,7 +166,7 @@
@Override
public <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotation) {
- return AnnotationSupport.getMultipleAnnotations(annotations, annotation);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(annotations, annotation);
}
// AnnotatedType
--- a/jdk/src/share/classes/sun/reflect/annotation/AnnotationSupport.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/sun/reflect/annotation/AnnotationSupport.java Tue Oct 22 12:35:27 2013 +0200
@@ -28,109 +28,224 @@
import java.lang.annotation.*;
import java.lang.reflect.*;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
public final class AnnotationSupport {
+
/**
- * Finds and returns all annotation of the type indicated by
- * {@code annotationClass} from the {@code Map} {@code
- * annotationMap}. Looks into containers of the {@code
- * annotationClass} (as specified by an the {@code
- * annotationClass} type being meta-annotated with an {@code
- * Repeatable} annotation).
+ * Finds and returns all annotations in {@code annotations} matching
+ * the given {@code annoClass}.
+ *
+ * Apart from annotations directly present in {@code annotations} this
+ * method searches for annotations inside containers i.e. indirectly
+ * present annotations.
+ *
+ * The order of the elements in the array returned depends on the iteration
+ * order of the provided map. Specifically, the directly present annotations
+ * come before the indirectly present annotations if and only if the
+ * directly present annotations come before the indirectly present
+ * annotations in the map.
*
- * @param annotationMap the {@code Map} used to store annotations indexed by their type
- * @param annotationClass the type of annotation to search for
+ * @param annotations the {@code Map} in which to search for annotations
+ * @param annoClass the type of annotation to search for
+ * @param includeNonInheritedContainees if false, the annoClass must be
+ * inheritable for the containers to be searched
*
- * @return an array of instances of {@code annotationClass} or an empty array if none were found
+ * @return an array of instances of {@code annoClass} or an empty
+ * array if none were found
*/
- public static <A extends Annotation> A[] getMultipleAnnotations(
- final Map<Class<? extends Annotation>, Annotation> annotationMap,
- final Class<A> annotationClass) {
- final List<A> res = new ArrayList<A>();
+ private static <A extends Annotation> A[] getDirectlyAndIndirectlyPresent(
+ Map<Class<? extends Annotation>, Annotation> annotations,
+ Class<A> annoClass,
+ boolean includeNonInheritedContainees) {
+
+ List<A> result = new ArrayList<A>();
@SuppressWarnings("unchecked")
- final A candidate = (A)annotationMap.get(annotationClass);
- if (candidate != null) {
- res.add(candidate);
+ A direct = (A) annotations.get(annoClass);
+ if (direct != null)
+ result.add(direct);
+
+ if (includeNonInheritedContainees ||
+ AnnotationType.getInstance(annoClass).isInherited()) {
+ A[] indirect = getIndirectlyPresent(annotations, annoClass);
+
+ if (indirect != null) {
+
+ boolean indirectFirst = direct == null ||
+ containerBeforeContainee(annotations, annoClass);
+
+ result.addAll((indirectFirst ? 0 : 1), Arrays.asList(indirect));
+ }
}
- final Class<? extends Annotation> containerClass = getContainer(annotationClass);
- if (containerClass != null) {
- res.addAll(unpackAll(annotationMap.get(containerClass), annotationClass));
- }
+ @SuppressWarnings("unchecked")
+ A[] arr = (A[]) Array.newInstance(annoClass, result.size());
+ return result.toArray(arr);
+ }
+
+
+ /**
+ * Equivalent to calling {@code getDirectlyAndIndirectlyPresentAnnotations(
+ * annotations, annoClass, true)}.
+ */
+ public static <A extends Annotation> A[] getDirectlyAndIndirectlyPresent(
+ Map<Class<? extends Annotation>, Annotation> annotations,
+ Class<A> annoClass) {
+
+ return getDirectlyAndIndirectlyPresent(annotations, annoClass, true);
+ }
+
- @SuppressWarnings("unchecked") // should be safe annotationClass is a token for A
- final A[] emptyTemplateArray = (A[])Array.newInstance(annotationClass, 0);
- return res.isEmpty() ? emptyTemplateArray : res.toArray(emptyTemplateArray);
+ /**
+ * Finds and returns all annotations matching the given {@code annoClass}
+ * indirectly present in {@code annotations}.
+ *
+ * @param annotations annotations to search indexed by their types
+ * @param annoClass the type of annotation to search for
+ *
+ * @return an array of instances of {@code annoClass} or an empty array if no
+ * indirectly present annotations were found
+ */
+ private static <A extends Annotation> A[] getIndirectlyPresent(
+ Map<Class<? extends Annotation>, Annotation> annotations,
+ Class<A> annoClass) {
+
+ Repeatable repeatable = annoClass.getDeclaredAnnotation(Repeatable.class);
+ if (repeatable == null)
+ return null; // Not repeatable -> no indirectly present annotations
+
+ Class<? extends Annotation> containerClass = repeatable.value();
+
+ Annotation container = annotations.get(containerClass);
+ if (container == null)
+ return null;
+
+ // Unpack container
+ A[] valueArray = getValueArray(container);
+ checkTypes(valueArray, container, annoClass);
+
+ return valueArray;
}
- /** Helper to get the container, or null if none, of an annotation. */
- private static <A extends Annotation> Class<? extends Annotation> getContainer(Class<A> annotationClass) {
- Repeatable containingAnnotation = annotationClass.getDeclaredAnnotation(Repeatable.class);
- return (containingAnnotation == null) ? null : containingAnnotation.value();
+
+ /**
+ * Figures out if conatiner class comes before containee class among the
+ * keys of the given map.
+ *
+ * @return true if container class is found before containee class when
+ * iterating over annotations.keySet().
+ */
+ private static <A extends Annotation> boolean containerBeforeContainee(
+ Map<Class<? extends Annotation>, Annotation> annotations,
+ Class<A> annoClass) {
+
+ Class<? extends Annotation> containerClass =
+ annoClass.getDeclaredAnnotation(Repeatable.class).value();
+
+ for (Class<? extends Annotation> c : annotations.keySet()) {
+ if (c == containerClass) return true;
+ if (c == annoClass) return false;
+ }
+
+ // Neither containee nor container present
+ return false;
}
- /** Reflectively look up and get the returned array from the the
- * invocation of the value() element on an instance of an
- * Annotation.
+
+ /**
+ * Finds and returns all associated annotations matching the given class.
+ *
+ * The order of the elements in the array returned depends on the iteration
+ * order of the provided maps. Specifically, the directly present annotations
+ * come before the indirectly present annotations if and only if the
+ * directly present annotations come before the indirectly present
+ * annotations in the relevant map.
+ *
+ * @param declaredAnnotations the declared annotations indexed by their types
+ * @param allAnnotations declared and inherited annotations indexed by their types
+ * @param annoClass the type of annotation to search for
+ *
+ * @return an array of instances of {@code annoClass} or an empty array if none were found.
*/
- private static <A extends Annotation> A[] getValueArray(Annotation containerInstance) {
+ public static <A extends Annotation> A[] getAssociatedAnnotations(
+ Map<Class<? extends Annotation>, Annotation> declaredAnnotations,
+ Map<Class<? extends Annotation>, Annotation> allAnnotations,
+ Class<A> annoClass) {
+
+ // Search declared
+ A[] result = getDirectlyAndIndirectlyPresent(declaredAnnotations, annoClass);
+
+ // Search inherited
+ if (result.length == 0)
+ result = getDirectlyAndIndirectlyPresent(allAnnotations, annoClass, false);
+
+ return result;
+ }
+
+
+ /* Reflectively invoke the values-method of the given annotation
+ * (container), cast it to an array of annotations and return the result.
+ */
+ private static <A extends Annotation> A[] getValueArray(Annotation container) {
try {
- // the spec tells us the container must have an array-valued
- // value element. Get the AnnotationType, get the "value" element
- // and invoke it to get the contents.
+ // According to JLS the container must have an array-valued value
+ // method. Get the AnnotationType, get the "value" method and invoke
+ // it to get the content.
- Class<? extends Annotation> containerClass = containerInstance.annotationType();
+ Class<? extends Annotation> containerClass = container.annotationType();
AnnotationType annoType = AnnotationType.getInstance(containerClass);
if (annoType == null)
- throw new AnnotationFormatError(containerInstance + " is an invalid container for repeating annotations");
+ throw invalidContainerException(container, null);
Method m = annoType.members().get("value");
if (m == null)
- throw new AnnotationFormatError(containerInstance +
- " is an invalid container for repeating annotations");
+ throw invalidContainerException(container, null);
+
m.setAccessible(true);
- @SuppressWarnings("unchecked") // not provably safe, but we catch the ClassCastException
- A[] a = (A[])m.invoke(containerInstance); // this will erase to (Annotation[]) but we
- // do a runtime cast on the return-value
- // in the methods that call this method
- return a;
- } catch (IllegalAccessException | // couldnt loosen security
- IllegalArgumentException | // parameters doesn't match
+ // This will erase to (Annotation[]) but we do a runtime cast on the
+ // return-value in the method that call this method.
+ @SuppressWarnings("unchecked")
+ A[] values = (A[]) m.invoke(container);
+
+ return values;
+
+ } catch (IllegalAccessException | // couldn't loosen security
+ IllegalArgumentException | // parameters doesn't match
InvocationTargetException | // the value method threw an exception
- ClassCastException e) { // well, a cast failed ...
- throw new AnnotationFormatError(
- containerInstance + " is an invalid container for repeating annotations",
- e);
+ ClassCastException e) {
+
+ throw invalidContainerException(container, e);
+
}
}
- /* Sanity check type of and return a list of all the annotation
- * instances of type {@code annotationClass} from {@code
- * containerInstance}.
- */
- private static <A extends Annotation> List<A> unpackAll(Annotation containerInstance,
- Class<A> annotationClass) {
- if (containerInstance == null) {
- return Collections.emptyList(); // container not present
- }
+
+ private static AnnotationFormatError invalidContainerException(Annotation anno,
+ Throwable cause) {
+ return new AnnotationFormatError(
+ anno + " is an invalid container for repeating annotations",
+ cause);
+ }
+
- try {
- A[] a = getValueArray(containerInstance);
- List<A> l = new ArrayList<>(a.length);
- for (int i = 0; i < a.length; i++)
- l.add(annotationClass.cast(a[i]));
- return l;
- } catch (ClassCastException |
- NullPointerException e) {
- throw new AnnotationFormatError(
- String.format("%s is an invalid container for repeating annotations of type: %s",
- containerInstance, annotationClass),
- e);
+ /* Sanity check type of all the annotation instances of type {@code annoClass}
+ * from {@code container}.
+ */
+ private static <A extends Annotation> void checkTypes(A[] annotations,
+ Annotation container,
+ Class<A> annoClass) {
+ for (A a : annotations) {
+ if (!annoClass.isInstance(a)) {
+ throw new AnnotationFormatError(
+ String.format("%s is an invalid container for " +
+ "repeating annotations of type: %s",
+ container, annoClass));
+ }
}
}
}
--- a/jdk/src/share/classes/sun/reflect/generics/reflectiveObjects/TypeVariableImpl.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/src/share/classes/sun/reflect/generics/reflectiveObjects/TypeVariableImpl.java Tue Oct 22 12:35:27 2013 +0200
@@ -198,7 +198,7 @@
@Override
public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
Objects.requireNonNull(annotationClass);
- return AnnotationSupport.getMultipleAnnotations(mapAnnotations(getAnnotations()), annotationClass);
+ return AnnotationSupport.getDirectlyAndIndirectlyPresent(mapAnnotations(getAnnotations()), annotationClass);
}
@Override
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/annotation/repeatingAnnotations/NonInheritableContainee.java Tue Oct 22 12:35:27 2013 +0200
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8019420
+ * @summary Repeatable non-inheritable annotation types are mishandled by Core Reflection
+ */
+
+import java.util.*;
+import java.lang.annotation.*;
+import java.lang.reflect.*;
+
+public class NonInheritableContainee {
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Repeatable(InheritedAnnotationContainer.class)
+ @interface NonInheritedAnnotationRepeated {
+ String name();
+ }
+
+ @Inherited
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface InheritedAnnotationContainer {
+ NonInheritedAnnotationRepeated[] value();
+ }
+
+ @NonInheritedAnnotationRepeated(name="A")
+ @NonInheritedAnnotationRepeated(name="B")
+ class Parent {}
+ class Sample extends Parent {}
+
+
+ public static void main(String[] args) {
+
+ Annotation[] anns = Sample.class.getAnnotationsByType(
+ NonInheritedAnnotationRepeated.class);
+
+ if (anns.length != 0)
+ throw new RuntimeException("Non-@Inherited containees should not " +
+ "be inherited even though its container is @Inherited.");
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/annotation/repeatingAnnotations/OrderUnitTest.java Tue Oct 22 12:35:27 2013 +0200
@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8004912
+ * @summary Unit test for order of annotations returned by get[Declared]AnnotationsByType.
+ *
+ * @run main OrderUnitTest
+ */
+
+import java.lang.annotation.*;
+import java.lang.reflect.*;
+
+public class OrderUnitTest {
+
+ public static void main(String[] args) {
+ testOrder(Case1.class);
+ testOrder(Case2.class);
+ }
+
+ private static void testOrder(AnnotatedElement e) {
+ Annotation[] decl = e.getDeclaredAnnotations();
+ Foo[] declByType = e.getDeclaredAnnotationsByType(Foo.class);
+
+ if (decl[0] instanceof Foo != declByType[0].isDirect() ||
+ decl[1] instanceof Foo != declByType[1].isDirect()) {
+ throw new RuntimeException("Order of directly / indirectly present " +
+ "annotations from getDeclaredAnnotationsByType does not " +
+ "match order from getDeclaredAnnotations.");
+ }
+ }
+}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface FooContainer {
+ Foo[] value();
+}
+
+@Retention(RetentionPolicy.RUNTIME)
+@Repeatable(FooContainer.class)
+@interface Foo {
+ boolean isDirect();
+}
+
+
+@Foo(isDirect = true) @FooContainer({@Foo(isDirect = false)})
+class Case1 {
+}
+
+
+@FooContainer({@Foo(isDirect = false)}) @Foo(isDirect = true)
+class Case2 {
+}
--- a/jdk/test/java/lang/annotation/repeatingAnnotations/RepeatedUnitTest.java Tue Oct 22 10:57:40 2013 +0200
+++ b/jdk/test/java/lang/annotation/repeatingAnnotations/RepeatedUnitTest.java Tue Oct 22 12:35:27 2013 +0200
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 7154390 8005712 8007278
+ * @bug 7154390 8005712 8007278 8004912
* @summary Unit test for repeated annotation reflection
*
* @compile RepeatedUnitTest.java subpackage/package-info.java subpackage/Container.java subpackage/Containee.java subpackage/NonRepeated.java subpackage/InheritedContainee.java subpackage/InheritedContainer.java subpackage/InheritedNonRepeated.java
@@ -51,6 +51,12 @@
inheritedMe3();
inheritedMe4();
+ inheritedMe5(); // ContainerOnSuperSingleOnSub
+ inheritedMe6(); // RepeatableOnSuperSingleOnSub
+ inheritedMe7(); // SingleAnnoOnSuperContainerOnSub
+ inheritedMe8(); // SingleOnSuperRepeatableOnSub
+
+
// CONSTRUCTOR
checkMultiplier(Me1.class.getConstructor(new Class[0]), 10);
@@ -159,6 +165,30 @@
check(e.getAnnotationsByType(NonRepeated.class)[0].value() == 1000);
}
+ static void inheritedMe5() {
+ AnnotatedElement e = Me5.class;
+ check(2 == e.getAnnotations().length);
+ check(1 == countAnnotation(e, InheritedContainee.class));
+ }
+
+ static void inheritedMe6() {
+ AnnotatedElement e = Me6.class;
+ check(2 == e.getAnnotations().length);
+ check(1 == countAnnotation(e, InheritedContainee.class));
+ }
+
+ static void inheritedMe7() {
+ AnnotatedElement e = Me7.class;
+ check(2 == e.getAnnotations().length);
+ check(2 == countAnnotation(e, InheritedContainee.class));
+ }
+
+ static void inheritedMe8() {
+ AnnotatedElement e = Me8.class;
+ check(2 == e.getAnnotations().length);
+ check(2 == countAnnotation(e, InheritedContainee.class));
+ }
+
static void checkMultiplier(AnnotatedElement e, int m) {
// Basic sanity of non-repeating getAnnotation(Class)
check(e.getAnnotation(NonRepeated.class).value() == 5 * m);
@@ -252,3 +282,31 @@
@InheritedContainee(1000) @InheritedContainee(2000) @InheritedContainee(3000) @InheritedContainee(4000)
@Containee(1000) @Containee(2000) @Containee(3000) @Containee(4000)
class Me4 extends Father {}
+
+
+@InheritedContainer({@InheritedContainee(1), @InheritedContainee(2)})
+class SuperOf5 {}
+
+@InheritedContainee(3)
+class Me5 extends SuperOf5{}
+
+
+@InheritedContainee(1) @InheritedContainee(2)
+class SuperOf6 {}
+
+@InheritedContainee(3)
+class Me6 extends SuperOf6 {}
+
+
+@InheritedContainee(1)
+class SuperOf7 {}
+
+@InheritedContainer({@InheritedContainee(2), @InheritedContainee(3)})
+class Me7 extends SuperOf7 {}
+
+
+@InheritedContainee(1)
+class SuperOf8 {}
+
+@InheritedContainee(2) @InheritedContainee(3)
+class Me8 extends SuperOf8 {}