Skip to content

Commit

Permalink
Avoid reflection for annotation attribute method invocations
Browse files Browse the repository at this point in the history
As a follow up to 332b25b, this commit consistently avoids the use of
reflection for annotation attribute method invocations.

See gh-29301
Closes gh-29448
  • Loading branch information
sbrannen committed Nov 8, 2022
1 parent e5878ab commit f4b3333
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 41 deletions.
Expand Up @@ -32,7 +32,6 @@
import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.MirrorSet;
import org.springframework.lang.Nullable;
import org.springframework.util.ObjectUtils;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -241,7 +240,7 @@ private void processAliases(int attributeIndex, List<Method> aliases) {
mapping.claimedAliases.addAll(aliases);
if (mapping.annotation != null) {
int[] resolvedMirrors = mapping.mirrorSets.resolve(null,
mapping.annotation, ReflectionUtils::invokeMethod);
mapping.annotation, AnnotationUtils::invokeAnnotationMethod);
for (int i = 0; i < mapping.attributes.size(); i++) {
if (aliases.contains(mapping.attributes.get(i))) {
this.annotationValueMappings[attributeIndex] = resolvedMirrors[i];
Expand Down Expand Up @@ -505,7 +504,7 @@ Object getMappedAnnotationValue(int attributeIndex, boolean metaAnnotationsOnly)
if (source == this && metaAnnotationsOnly) {
return null;
}
return ReflectionUtils.invokeMethod(source.attributes.get(mappedIndex), source.annotation);
return AnnotationUtils.invokeAnnotationMethod(source.attributes.get(mappedIndex), source.annotation);
}

/**
Expand Down Expand Up @@ -594,7 +593,7 @@ private static boolean areEquivalent(Annotation annotation, @Nullable Object ext
AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType());
for (int i = 0; i < attributes.size(); i++) {
Method attribute = attributes.get(i);
Object value1 = ReflectionUtils.invokeMethod(attribute, annotation);
Object value1 = AnnotationUtils.invokeAnnotationMethod(attribute, annotation);
Object value2;
if (extractedValue instanceof TypeMappedAnnotation) {
value2 = ((TypeMappedAnnotation<?>) extractedValue).getValue(attribute.getName()).orElse(null);
Expand Down
Expand Up @@ -19,9 +19,10 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Array;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -1052,23 +1053,42 @@ public static Object getValue(@Nullable Annotation annotation, @Nullable String
}
try {
Method method = annotation.annotationType().getDeclaredMethod(attributeName);
ReflectionUtils.makeAccessible(method);
return method.invoke(annotation);
return invokeAnnotationMethod(method, annotation);
}
catch (NoSuchMethodException ex) {
return null;
}
catch (InvocationTargetException ex) {
rethrowAnnotationConfigurationException(ex.getTargetException());
throw new IllegalStateException("Could not obtain value for annotation attribute '" +
attributeName + "' in " + annotation, ex);
}
catch (Throwable ex) {
rethrowAnnotationConfigurationException(ex);
handleIntrospectionFailure(annotation.getClass(), ex);
return null;
}
}

/**
* Invoke the supplied annotation attribute {@link Method} on the supplied
* {@link Annotation}.
* <p>An attempt will first be made to invoke the method via the annotation's
* {@link InvocationHandler} (if the annotation instance is a JDK dynamic proxy).
* If that fails, an attempt will be made to invoke the method via reflection.
* @param method the method to invoke
* @param annotation the annotation on which to invoke the method
* @return the value returned from the method invocation
* @since 5.3.24
*/
static Object invokeAnnotationMethod(Method method, Object annotation) {
if (Proxy.isProxyClass(annotation.getClass())) {
try {
InvocationHandler handler = Proxy.getInvocationHandler(annotation);
return handler.invoke(annotation, method, null);
}
catch (Throwable ex) {
// ignore and fall back to reflection below
}
}
return ReflectionUtils.invokeMethod(method, annotation);
}

/**
* If the supplied throwable is an {@link AnnotationConfigurationException},
* it will be cast to an {@code AnnotationConfigurationException} and thrown,
Expand Down
Expand Up @@ -109,7 +109,7 @@ boolean isValid(Annotation annotation) {
for (int i = 0; i < size(); i++) {
if (canThrowTypeNotPresentException(i)) {
try {
get(i).invoke(annotation);
AnnotationUtils.invokeAnnotationMethod(get(i), annotation);
}
catch (Throwable ex) {
return false;
Expand All @@ -134,7 +134,7 @@ void validate(Annotation annotation) {
for (int i = 0; i < size(); i++) {
if (canThrowTypeNotPresentException(i)) {
try {
get(i).invoke(annotation);
AnnotationUtils.invokeAnnotationMethod(get(i), annotation);
}
catch (Throwable ex) {
throw new IllegalStateException("Could not obtain annotation attribute value for " +
Expand Down
Expand Up @@ -18,16 +18,13 @@

import java.lang.annotation.Annotation;
import java.lang.annotation.Repeatable;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Map;

import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ConcurrentReferenceHashMap;
import org.springframework.util.ObjectUtils;
import org.springframework.util.ReflectionUtils;

/**
* Strategy used to determine annotations that act as containers for other
Expand Down Expand Up @@ -133,19 +130,6 @@ public static RepeatableContainers none() {
return NoRepeatableContainers.INSTANCE;
}

private static Object invokeAnnotationMethod(Annotation annotation, Method method) {
if (Proxy.isProxyClass(annotation.getClass())) {
try {
InvocationHandler handler = Proxy.getInvocationHandler(annotation);
return handler.invoke(annotation, method, null);
}
catch (Throwable ex) {
// ignore and fall back to reflection below
}
}
return ReflectionUtils.invokeMethod(method, annotation);
}


/**
* Standard {@link RepeatableContainers} implementation that searches using
Expand All @@ -168,7 +152,7 @@ private static class StandardRepeatableContainers extends RepeatableContainers {
Annotation[] findRepeatedAnnotations(Annotation annotation) {
Method method = getRepeatedAnnotationsMethod(annotation.annotationType());
if (method != null) {
return (Annotation[]) invokeAnnotationMethod(annotation, method);
return (Annotation[]) AnnotationUtils.invokeAnnotationMethod(method, annotation);
}
return super.findRepeatedAnnotations(annotation);
}
Expand Down Expand Up @@ -255,7 +239,7 @@ private Class<? extends Annotation> deduceContainer(Class<? extends Annotation>
@Nullable
Annotation[] findRepeatedAnnotations(Annotation annotation) {
if (this.container.isAssignableFrom(annotation.annotationType())) {
return (Annotation[]) invokeAnnotationMethod(annotation, this.valueMethod);
return (Annotation[]) AnnotationUtils.invokeAnnotationMethod(this.valueMethod, annotation);
}
return super.findRepeatedAnnotations(annotation);
}
Expand Down
Expand Up @@ -111,7 +111,7 @@ private boolean annotationEquals(Object other) {
for (int i = 0; i < this.attributes.size(); i++) {
Method attribute = this.attributes.get(i);
Object thisValue = getAttributeValue(attribute);
Object otherValue = ReflectionUtils.invokeMethod(attribute, other);
Object otherValue = AnnotationUtils.invokeAnnotationMethod(attribute, other);
if (!ObjectUtils.nullSafeEquals(thisValue, otherValue)) {
return false;
}
Expand Down
Expand Up @@ -33,7 +33,6 @@
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;

/**
* {@link MergedAnnotation} that adapts attributes from a root annotation by
Expand All @@ -43,9 +42,9 @@
* {@code BiFunction}. This allows various different annotation models to be
* supported by the same class. For example, the attributes source might be an
* actual {@link Annotation} instance where methods on the annotation instance
* are {@linkplain ReflectionUtils#invokeMethod(Method, Object) invoked} to extract
* values. Equally, the source could be a simple {@link Map} with values
* extracted using {@link Map#get(Object)}.
* are {@linkplain AnnotationUtils#invokeAnnotationMethod(Method, Object) invoked}
* to extract values. Similarly, the source could be a simple {@link Map} with
* values extracted using {@link Map#get(Object)}.
*
* <p>Extracted root attribute values must be compatible with the attribute
* return type, namely:
Expand Down Expand Up @@ -434,7 +433,7 @@ private Object getValueFromMetaAnnotation(int attributeIndex, boolean forMirrorR
}
if (value == null) {
Method attribute = this.mapping.getAttributes().get(attributeIndex);
value = ReflectionUtils.invokeMethod(attribute, this.mapping.getAnnotation());
value = AnnotationUtils.invokeAnnotationMethod(attribute, this.mapping.getAnnotation());
}
return value;
}
Expand Down Expand Up @@ -555,7 +554,7 @@ private MergedAnnotation<?> adaptToMergedAnnotation(Object value, Class<? extend

private ValueExtractor getValueExtractor(Object value) {
if (value instanceof Annotation) {
return ReflectionUtils::invokeMethod;
return AnnotationUtils::invokeAnnotationMethod;
}
if (value instanceof Map) {
return TypeMappedAnnotation::extractFromMap;
Expand Down Expand Up @@ -615,7 +614,8 @@ private ClassLoader getClassLoader() {
static <A extends Annotation> MergedAnnotation<A> 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), null, source, annotation, ReflectionUtils::invokeMethod, 0);
return new TypeMappedAnnotation<>(
mappings.get(0), null, source, annotation, AnnotationUtils::invokeAnnotationMethod, 0);
}

static <A extends Annotation> MergedAnnotation<A> of(
Expand Down Expand Up @@ -649,7 +649,7 @@ static <A extends Annotation> TypeMappedAnnotation<A> createIfPossible(
int aggregateIndex, IntrospectionFailureLogger logger) {

return createIfPossible(mapping, source, annotation,
ReflectionUtils::invokeMethod, aggregateIndex, logger);
AnnotationUtils::invokeAnnotationMethod, aggregateIndex, logger);
}

@Nullable
Expand Down

0 comments on commit f4b3333

Please sign in to comment.