From daec3531b3a386074484c3a1ccf82c79b769e96f Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 6 May 2019 13:09:23 -0700 Subject: [PATCH] Support String->Class adaptation in MergedAnnotation Update TypeMappedAnnotation so that Strings can be used to represent Class attribute values. This will allow ASM annotation readers to present a `MergedAnnotation` instance without necessarily having the actual class values on the classpath. When the underlying value is a String, any calls to `getValue(name, String.class)` or `asMap(Adapt.CLASS_TO_STRING)` will simply return the original String. Calls that need the actual Class result (such as `getClass`) will use `Class.forName` and may throw a `ClassNotFoundException` at that point. This commit also allows an empty Object[] to be used to represent any empty primitive array. See gh-22884 --- .../core/annotation/MergedAnnotation.java | 21 ++- .../core/annotation/TypeMappedAnnotation.java | 173 ++++++++++++++---- .../annotation/TypeMappedAnnotationTests.java | 107 +++++++++++ 3 files changed, 265 insertions(+), 36 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java index cdc3c58a21..ef350a231e 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java @@ -562,7 +562,26 @@ public interface MergedAnnotation { static MergedAnnotation of( @Nullable AnnotatedElement source, Class annotationType, @Nullable Map attributes) { - return TypeMappedAnnotation.of(source, annotationType, attributes); + return of(null, source, annotationType, attributes); + } + + /** + * Create a new {@link MergedAnnotation} instance of the specified + * annotation type with attributes values supplied by a map. + * @param classLoader the class loader used to resolve class attributes + * @param source the source for the annotation. This source is used only for + * information and logging. It does not need to actually contain + * the specified annotations and it will not be searched. + * @param annotationType the annotation type + * @param attributes the annotation attributes or {@code null} if just default + * values should be used + * @return a {@link MergedAnnotation} instance for the annotation and attributes + */ + static MergedAnnotation of( + @Nullable ClassLoader classLoader, @Nullable Object source, + Class annotationType, @Nullable Map attributes) { + + return TypeMappedAnnotation.of(classLoader, source, annotationType, attributes); } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java index da348b3416..40e7bd356d 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java @@ -18,8 +18,11 @@ package org.springframework.core.annotation; import java.lang.annotation.Annotation; import java.lang.reflect.Array; +import java.lang.reflect.Member; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -68,8 +71,28 @@ import org.springframework.util.ReflectionUtils; */ final class TypeMappedAnnotation extends AbstractMergedAnnotation { + private static final Object[] EMPTY_OBJECT_ARRAY = {}; + + private static final Map, Object> EMPTY_ARRAYS; + static { + Map, Object> emptyArrays = new HashMap<>(); + emptyArrays.put(String.class, new String[] {}); + emptyArrays.put(boolean.class, new boolean[] {}); + emptyArrays.put(byte.class, new byte[] {}); + emptyArrays.put(char.class, new char[] {}); + emptyArrays.put(double.class, new double[] {}); + emptyArrays.put(float.class, new float[] {}); + emptyArrays.put(int.class, new int[] {}); + emptyArrays.put(long.class, new long[] {}); + emptyArrays.put(short.class, new short[] {}); + EMPTY_ARRAYS = Collections.unmodifiableMap(emptyArrays); + } + private final AnnotationTypeMapping mapping; + @Nullable + private final ClassLoader classLoader; + @Nullable private final Object source; @@ -93,21 +116,23 @@ final class TypeMappedAnnotation extends AbstractMergedAnn private String string; - private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable Object source, - @Nullable Object rootAttributes, BiFunction valueExtractor, - int aggregateIndex) { + private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable ClassLoader classLoader, + @Nullable Object source, @Nullable Object rootAttributes, + BiFunction valueExtractor, int aggregateIndex) { - this(mapping, source, rootAttributes, valueExtractor, aggregateIndex, null); + this(mapping, classLoader, source, rootAttributes, valueExtractor, aggregateIndex, null); } - private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable Object source, - @Nullable Object rootAttributes, BiFunction valueExtractor, - int aggregateIndex, @Nullable int[] resolvedRootMirrors) { + private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable ClassLoader classLoader, + @Nullable Object source, @Nullable Object rootAttributes, + BiFunction valueExtractor, int aggregateIndex, + @Nullable int[] resolvedRootMirrors) { + this.mapping = mapping; + this.classLoader = classLoader; this.source = source; this.rootAttributes = rootAttributes; this.valueExtractor = valueExtractor; - this.mapping = mapping; this.aggregateIndex = aggregateIndex; this.useMergedValues = true; this.attributeFilter = null; @@ -117,11 +142,13 @@ final class TypeMappedAnnotation extends AbstractMergedAnn mapping.getMirrorSets().resolve(source, this, this::getValueForMirrorResolution)); } - private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable Object source, - @Nullable Object rootAnnotation, BiFunction valueExtractor, - int aggregateIndex, boolean useMergedValues, @Nullable Predicate attributeFilter, + private TypeMappedAnnotation(AnnotationTypeMapping mapping, @Nullable ClassLoader classLoader, + @Nullable Object source, @Nullable Object rootAnnotation, + BiFunction valueExtractor, int aggregateIndex, + boolean useMergedValues, @Nullable Predicate attributeFilter, int[] resolvedRootMirrors, int[] resolvedMirrors) { + this.classLoader = classLoader; this.source = source; this.rootAttributes = rootAnnotation; this.valueExtractor = valueExtractor; @@ -173,7 +200,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn if (parentMapping == null) { return null; } - return new TypeMappedAnnotation<>(parentMapping, this.source, this.rootAttributes, + return new TypeMappedAnnotation<>(parentMapping, this.classLoader, this.source, this.rootAttributes, this.valueExtractor, this.aggregateIndex, this.resolvedRootMirrors); } @@ -183,7 +210,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return this; } AnnotationTypeMapping rootMapping = this.mapping.getRoot(); - return new TypeMappedAnnotation<>(rootMapping, this.source, this.rootAttributes, + return new TypeMappedAnnotation<>(rootMapping, this.classLoader, this.source, this.rootAttributes, this.valueExtractor, this.aggregateIndex, this.resolvedRootMirrors); } @@ -204,7 +231,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn Assert.notNull(type, "Type must not be null"); Assert.isAssignable(type, attribute.getReturnType(), () -> "Attribute " + attributeName + " type mismatch:"); - return (MergedAnnotation) getRequiredValue(attributeIndex); + return (MergedAnnotation) getRequiredValue(attributeIndex, attributeName); } @Override @@ -218,7 +245,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn Assert.notNull(type, "Type must not be null"); Assert.notNull(componentType, () -> "Attribute " + attributeName + " is not an array"); Assert.isAssignable(type, componentType, () -> "Attribute " + attributeName + " component type mismatch:"); - return (MergedAnnotation[]) getRequiredValue(attributeIndex); + return (MergedAnnotation[]) getRequiredValue(attributeIndex, attributeName); } @Override @@ -236,14 +263,14 @@ final class TypeMappedAnnotation extends AbstractMergedAnn if (this.attributeFilter != null) { predicate = this.attributeFilter.and(predicate); } - return new TypeMappedAnnotation<>(this.mapping, this.source, this.rootAttributes, + return new TypeMappedAnnotation<>(this.mapping, this.classLoader, this.source, this.rootAttributes, this.valueExtractor, this.aggregateIndex, this.useMergedValues, predicate, this.resolvedRootMirrors, this.resolvedMirrors); } @Override public MergedAnnotation withNonMergedAttributes() { - return new TypeMappedAnnotation<>(this.mapping, this.source, this.rootAttributes, + return new TypeMappedAnnotation<>(this.mapping, this.classLoader, this.source, this.rootAttributes, this.valueExtractor, this.aggregateIndex, false, this.attributeFilter, this.resolvedRootMirrors, this.resolvedMirrors); } @@ -359,10 +386,11 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return (attributeIndex != -1 ? getValue(attributeIndex, type) : null); } - private final Object getRequiredValue(int attributeIndex) { + private final Object getRequiredValue(int attributeIndex, String attributeName) { Object value = getValue(attributeIndex, Object.class); if (value == null) { - throw new NoSuchElementException("No element at attribute index " + attributeIndex); + throw new NoSuchElementException("No element at attribute index " + + attributeIndex + " for name " + attributeName); } return value; } @@ -400,7 +428,8 @@ final class TypeMappedAnnotation extends AbstractMergedAnn } if (mapping.getDepth() == 0) { Method attribute = mapping.getAttributes().get(attributeIndex); - return this.valueExtractor.apply(attribute, this.rootAttributes); + Object result = this.valueExtractor.apply(attribute, this.rootAttributes); + return (result != null) ? result : attribute.getDefaultValue(); } return getValueFromMetaAnnotation(attributeIndex, forMirrorResolution); } @@ -430,12 +459,13 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return null; } value = adaptForAttribute(attribute, value); - if (type == Object.class) { - type = (Class) getDefaultAdaptType(attribute); - } - else if (value instanceof Class && type == String.class) { + type = getAdaptType(attribute, type); + if (value instanceof Class && type == String.class) { value = ((Class) value).getName(); } + else if (value instanceof String && type == Class.class) { + value = ClassUtils.resolveClassName((String) value, getClassLoader()); + } else if (value instanceof Class[] && type == String[].class) { Class[] classes = (Class[]) value; String[] names = new String[classes.length]; @@ -444,6 +474,14 @@ final class TypeMappedAnnotation extends AbstractMergedAnn } value = names; } + else if (value instanceof String[] && type == Class[].class) { + String[] names = (String[]) value; + Class[] classes = new Class[names.length]; + for (int i = 0; i < names.length; i++) { + classes[i] = ClassUtils.resolveClassName(names[i], getClassLoader()); + } + value = classes; + } else if (value instanceof MergedAnnotation && type.isAnnotation()) { MergedAnnotation annotation = (MergedAnnotation) value; value = annotation.synthesize(); @@ -484,9 +522,14 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return result; } if ((attributeType == Class.class && value instanceof String) || - (attributeType == Class[].class && value instanceof String[])) { + (attributeType == Class[].class && value instanceof String[]) || + (attributeType == String.class && value instanceof Class) || + (attributeType == String[].class && value instanceof Class[])) { return value; } + if(attributeType.isArray() && isEmptyObjectArray(value)) { + return emptyArray(attributeType.getComponentType()); + } if (!attributeType.isInstance(value)) { throw new IllegalStateException("Attribute '" + attribute.getName() + "' in annotation " + getType().getName() + " should be compatible with " + @@ -496,9 +539,24 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return value; } + private boolean isEmptyObjectArray(Object value) { + return value instanceof Object[] && Arrays.equals((Object[]) value, EMPTY_OBJECT_ARRAY); + } + + private Object emptyArray(Class componentType) { + Object result = EMPTY_ARRAYS.get(componentType); + if (result == null) { + result = Array.newInstance(componentType, 0); + } + return result; + } + private MergedAnnotation adaptToMergedAnnotation(Object value, Class annotationType) { + if (value instanceof MergedAnnotation) { + return (MergedAnnotation) value; + } AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0); - return new TypeMappedAnnotation<>(mapping, this.source, value, getValueExtractor(value), this.aggregateIndex); + return new TypeMappedAnnotation<>(mapping, null, this.source, value, getValueExtractor(value), this.aggregateIndex); } private BiFunction getValueExtractor(Object value) { @@ -511,15 +569,19 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return this.valueExtractor; } - private Class getDefaultAdaptType(Method attribute) { + @SuppressWarnings("unchecked") + private Class getAdaptType(Method attribute, Class type) { + if (type != Object.class) { + return type; + } Class attributeType = attribute.getReturnType(); if (attributeType.isAnnotation()) { - return MergedAnnotation.class; + return (Class) MergedAnnotation.class; } if (attributeType.isArray() && attributeType.getComponentType().isAnnotation()) { - return MergedAnnotation[].class; + return (Class) MergedAnnotation[].class; } - return ClassUtils.resolvePrimitiveIfNecessary(attributeType); + return (Class) ClassUtils.resolvePrimitiveIfNecessary(attributeType); } private int getAttributeIndex(String attributeName, boolean required) { @@ -540,20 +602,51 @@ final class TypeMappedAnnotation extends AbstractMergedAnn return false; } + @Nullable + private ClassLoader getClassLoader() { + if (this.classLoader != null) { + return this.classLoader; + } + if (this.source != null) { + if (this.source instanceof Class) { + return ((Class) source).getClassLoader(); + } + if (this.source instanceof Member) { + ((Member) this.source).getDeclaringClass().getClassLoader(); + } + } + return null; + } + static MergedAnnotation from(@Nullable Object source, A annotation) { Assert.notNull(annotation, "Annotation must not be null"); AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(annotation.annotationType()); - return new TypeMappedAnnotation<>(mappings.get(0), source, annotation, ReflectionUtils::invokeMethod, 0); + return new TypeMappedAnnotation<>(mappings.get(0), null, source, annotation, ReflectionUtils::invokeMethod, 0); } - static MergedAnnotation of(@Nullable Object source, + static MergedAnnotation of( + @Nullable ClassLoader classLoader, @Nullable Object source, Class annotationType, @Nullable Map attributes) { Assert.notNull(annotationType, "Annotation type must not be null"); AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(annotationType); return new TypeMappedAnnotation<>( - mappings.get(0), source, attributes, TypeMappedAnnotation::extractFromMap, 0); + mappings.get(0), classLoader, source, attributes, TypeMappedAnnotation::extractFromMap, 0); + } + + @Nullable + static TypeMappedAnnotation createIfPossible( + AnnotationTypeMapping mapping, MergedAnnotation annotation, IntrospectionFailureLogger logger) { + if (annotation instanceof TypeMappedAnnotation) { + TypeMappedAnnotation typeMappedAnnotation = (TypeMappedAnnotation) annotation; + return createIfPossible(mapping, typeMappedAnnotation.source, + typeMappedAnnotation.rootAttributes, + typeMappedAnnotation.valueExtractor, + typeMappedAnnotation.aggregateIndex, logger); + } + return createIfPossible(mapping, annotation.getSource(), annotation.synthesize(), + annotation.getAggregateIndex(), logger); } @Nullable @@ -561,9 +654,19 @@ final class TypeMappedAnnotation extends AbstractMergedAnn AnnotationTypeMapping mapping, @Nullable Object source, Annotation annotation, int aggregateIndex, IntrospectionFailureLogger logger) { + return createIfPossible(mapping, source, annotation, + ReflectionUtils::invokeMethod, aggregateIndex, logger); + } + + @Nullable + static TypeMappedAnnotation createIfPossible( + AnnotationTypeMapping mapping, @Nullable Object source, Object rootAttribute, + BiFunction valueExtractor, + int aggregateIndex, IntrospectionFailureLogger logger) { + try { - return new TypeMappedAnnotation<>(mapping, source, annotation, - ReflectionUtils::invokeMethod, aggregateIndex); + return new TypeMappedAnnotation<>(mapping, null, source, rootAttribute, + valueExtractor, aggregateIndex); } catch (Exception ex) { if (ex instanceof AnnotationConfigurationException) { diff --git a/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java b/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java index f079ee7826..d76d6fb206 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java @@ -16,13 +16,18 @@ package org.springframework.core.annotation; +import java.io.InputStream; import java.lang.annotation.Annotation; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.junit.Test; import static org.assertj.core.api.Assertions.*; +import static org.junit.Assert.*; /** * Tests for {@link TypeMappedAnnotation}. See also @@ -65,6 +70,54 @@ public class TypeMappedAnnotationTests { assertThat(annotation.getString("convention")).isEqualTo("convention"); } + @Test + public void adaptFromEmptyArrayToAnyComponentType() { + AttributeMethods methods = AttributeMethods.forAnnotationType(ArrayTypes.class); + Map attributes = new HashMap<>(); + for (int i = 0; i < methods.size(); i++) { + attributes.put(methods.get(i).getName(), new Object[] {}); + } + MergedAnnotation annotation = TypeMappedAnnotation.of(null, null, + ArrayTypes.class, attributes); + assertThat(annotation.getValue("stringValue")).contains(new String[] {}); + assertThat(annotation.getValue("byteValue")).contains(new byte[] {}); + assertThat(annotation.getValue("shortValue")).contains(new short[] {}); + assertThat(annotation.getValue("intValue")).contains(new int[] {}); + assertThat(annotation.getValue("longValue")).contains(new long[] {}); + assertThat(annotation.getValue("booleanValue")).contains(new boolean[] {}); + assertThat(annotation.getValue("charValue")).contains(new char[] {}); + assertThat(annotation.getValue("doubleValue")).contains(new double[] {}); + assertThat(annotation.getValue("floatValue")).contains(new float[] {}); + assertThat(annotation.getValue("classValue")).contains(new Class[] {}); + assertThat(annotation.getValue("annotationValue")).contains(new MergedAnnotation[] {}); + assertThat(annotation.getValue("enumValue")).contains(new ExampleEnum[] {}); + } + + @Test + public void adaptFromNestedMergedAnnotation() { + MergedAnnotation nested = MergedAnnotation.of(Nested.class); + MergedAnnotation annotation = TypeMappedAnnotation.of(null, null, + NestedContainer.class, Collections.singletonMap("value", nested)); + assertThat(annotation.getAnnotation("value", Nested.class)).isSameAs(nested); + } + + @Test + public void adaptFromStringToClass() { + MergedAnnotation annotation = TypeMappedAnnotation.of(null, null, + ClassAttributes.class, + Collections.singletonMap("classValue", InputStream.class.getName())); + assertThat(annotation.getString("classValue")).isEqualTo(InputStream.class.getName()); + assertThat(annotation.getClass("classValue")).isEqualTo(InputStream.class); + } + + @Test + public void adaptFromStringArrayToClassArray() { + MergedAnnotation annotation = TypeMappedAnnotation.of(null, null, ClassAttributes.class, + Collections.singletonMap("classArrayValue", new String[] { InputStream.class.getName() })); + assertThat(annotation.getStringArray("classArrayValue")).containsExactly(InputStream.class.getName()); + assertThat(annotation.getClassArray("classArrayValue")).containsExactly(InputStream.class); + } + private TypeMappedAnnotation getTypeMappedAnnotation( Class source, Class annotationType) { return getTypeMappedAnnotation(source, annotationType, annotationType); @@ -160,4 +213,58 @@ public class TypeMappedAnnotationTests { } + @Retention(RetentionPolicy.RUNTIME) + static @interface ArrayTypes { + + String[] stringValue(); + + byte[] byteValue(); + + short[] shortValue(); + + int[] intValue(); + + long[] longValue(); + + boolean[] booleanValue(); + + char[] charValue(); + + double[] doubleValue(); + + float[] floatValue(); + + Class[] classValue(); + + ExplicitMirror[] annotationValue(); + + ExampleEnum[] enumValue(); + + } + + enum ExampleEnum {ONE,TWO,THREE} + + @Retention(RetentionPolicy.RUNTIME) + static @interface NestedContainer { + + Nested value(); + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface Nested { + + String value() default ""; + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface ClassAttributes { + + Class classValue(); + + Class[] classArrayValue(); + + } + }