From 0c5c3103c66ef8d1a23f9db5c6bcec2ef09c6989 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 19 Jul 2018 16:51:13 +0200 Subject: [PATCH] ReflectiveMethodExecutor skips interface search (plus related polishing) --- .../GenericApplicationListenerAdapter.java | 16 +++++------ .../org/springframework/util/ClassUtils.java | 12 ++++---- .../expression/spel/ast/MethodReference.java | 9 +++--- .../support/ReflectiveMethodExecutor.java | 28 +++++++++++-------- .../spel/CachedMethodExecutorTests.java | 17 +++-------- 5 files changed, 39 insertions(+), 43 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java b/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java index 32f99063893..6984744c3ac 100644 --- a/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java +++ b/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -86,17 +86,11 @@ public class GenericApplicationListenerAdapter implements GenericApplicationList return (this.delegate instanceof Ordered ? ((Ordered) this.delegate).getOrder() : Ordered.LOWEST_PRECEDENCE); } - @Nullable - static ResolvableType resolveDeclaredEventType(Class listenerType) { - ResolvableType resolvableType = ResolvableType.forClass(listenerType).as(ApplicationListener.class); - return (resolvableType.hasGenerics() ? resolvableType.getGeneric() : null); - } @Nullable private static ResolvableType resolveDeclaredEventType(ApplicationListener listener) { ResolvableType declaredEventType = resolveDeclaredEventType(listener.getClass()); - if (declaredEventType == null || declaredEventType.isAssignableFrom( - ResolvableType.forClass(ApplicationEvent.class))) { + if (declaredEventType == null || declaredEventType.isAssignableFrom(ApplicationEvent.class)) { Class targetClass = AopUtils.getTargetClass(listener); if (targetClass != listener.getClass()) { declaredEventType = resolveDeclaredEventType(targetClass); @@ -105,4 +99,10 @@ public class GenericApplicationListenerAdapter implements GenericApplicationList return declaredEventType; } + @Nullable + static ResolvableType resolveDeclaredEventType(Class listenerType) { + ResolvableType resolvableType = ResolvableType.forClass(listenerType).as(ApplicationListener.class); + return (resolvableType.hasGenerics() ? resolvableType.getGeneric() : null); + } + } diff --git a/spring-core/src/main/java/org/springframework/util/ClassUtils.java b/spring-core/src/main/java/org/springframework/util/ClassUtils.java index c3bd9c9d512..f274ef24fbc 100644 --- a/spring-core/src/main/java/org/springframework/util/ClassUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ClassUtils.java @@ -227,7 +227,7 @@ public abstract class ClassUtils { * @param name the name of the Class * @param classLoader the class loader to use * (may be {@code null}, which indicates the default class loader) - * @return Class instance for the supplied name + * @return a class instance for the supplied name * @throws ClassNotFoundException if the class was not found * @throws LinkageError if the class file could not be loaded * @see Class#forName(String, boolean, ClassLoader) @@ -298,7 +298,7 @@ public abstract class ClassUtils { * @param className the name of the Class * @param classLoader the class loader to use * (may be {@code null}, which indicates the default class loader) - * @return Class instance for the supplied name + * @return a class instance for the supplied name * @throws IllegalArgumentException if the class name was not resolvable * (that is, the class could not be found or the class file could not be loaded) * @see #forName(String, ClassLoader) @@ -868,7 +868,7 @@ public abstract class ClassUtils { public static Class getUserClass(Class clazz) { if (clazz.getName().contains(CGLIB_CLASS_SEPARATOR)) { Class superclass = clazz.getSuperclass(); - if (superclass != null && Object.class != superclass) { + if (superclass != null && superclass != Object.class) { return superclass; } } @@ -1227,10 +1227,10 @@ public abstract class ClassUtils { * access (e.g. calls to {@code Class#getDeclaredMethods} etc, this implementation * will fall back to returning the originally provided method. * @param method the method to be invoked, which may come from an interface - * @param targetClass the target class for the current invocation. - * May be {@code null} or may not even implement the method. + * @param targetClass the target class for the current invocation + * (may be {@code null} or may not even implement the method) * @return the specific target method, or the original method if the - * {@code targetClass} doesn't implement it or is {@code null} + * {@code targetClass} does not implement it */ public static Method getMostSpecificMethod(Method method, @Nullable Class targetClass) { if (targetClass != null && targetClass != method.getDeclaringClass() && isOverridable(method, targetClass)) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index b6a7505ffc6..9dd732e535e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -182,8 +182,7 @@ public class MethodReference extends SpelNodeImpl { @Nullable TypeDescriptor target, List argumentTypes) { List methodResolvers = evaluationContext.getMethodResolvers(); - if (methodResolvers.size() != 1 || - !(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) { + if (methodResolvers.size() != 1 || !(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) { // Not a default ReflectiveMethodResolver - don't know whether caching is valid return null; } @@ -249,7 +248,7 @@ public class MethodReference extends SpelNodeImpl { Method method = ((ReflectiveMethodExecutor) executorToCheck.get()).getMethod(); String descriptor = CodeFlow.toDescriptor(method.getReturnType()); if (this.nullSafe && CodeFlow.isPrimitive(descriptor)) { - originalPrimitiveExitTypeDescriptor = descriptor; + this.originalPrimitiveExitTypeDescriptor = descriptor; this.exitTypeDescriptor = CodeFlow.toBoxedDescriptor(descriptor); } else { @@ -301,7 +300,7 @@ public class MethodReference extends SpelNodeImpl { return true; } - + @Override public void generateCode(MethodVisitor mv, CodeFlow cf) { CachedMethodExecutor executorToCheck = this.cachedExecutor; @@ -332,7 +331,7 @@ public class MethodReference extends SpelNodeImpl { // Something on the stack when nothing is needed mv.visitInsn(POP); } - + if (CodeFlow.isPrimitive(descriptor)) { CodeFlow.insertBoxIfNecessary(mv, descriptor.charAt(0)); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java index b125535ce6e..ec7be523c3d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -29,6 +29,8 @@ import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; /** + * {@link MethodExecutor} that works via reflection. + * * @author Andy Clement * @author Juergen Hoeller * @since 3.0 @@ -48,6 +50,10 @@ public class ReflectiveMethodExecutor implements MethodExecutor { private boolean argumentConversionOccurred = false; + /** + * Create a new executor for the given method. + * @param method the method to invoke + */ public ReflectiveMethodExecutor(Method method) { this.method = method; if (method.isVarArgs()) { @@ -60,6 +66,9 @@ public class ReflectiveMethodExecutor implements MethodExecutor { } + /** + * Return the original method that this executor has been configured for. + */ public Method getMethod() { return this.method; } @@ -68,21 +77,22 @@ public class ReflectiveMethodExecutor implements MethodExecutor { * Find the first public class in the methods declaring class hierarchy that declares this method. * Sometimes the reflective method discovery logic finds a suitable method that can easily be * called via reflection but cannot be called from generated code when compiling the expression - * because of visibility restrictions. For example if a non public class overrides toString(), this - * helper method will walk up the type hierarchy to find the first public type that declares the - * method (if there is one!). For toString() it may walk as far as Object. + * because of visibility restrictions. For example if a non-public class overrides toString(), + * this helper method will walk up the type hierarchy to find the first public type that declares + * the method (if there is one!). For toString() it may walk as far as Object. */ @Nullable public Class getPublicDeclaringClass() { if (!this.computedPublicDeclaringClass) { - this.publicDeclaringClass = discoverPublicClass(this.method, this.method.getDeclaringClass()); + this.publicDeclaringClass = + discoverPublicDeclaringClass(this.method, this.method.getDeclaringClass()); this.computedPublicDeclaringClass = true; } return this.publicDeclaringClass; } @Nullable - private Class discoverPublicClass(Method method, Class clazz) { + private Class discoverPublicDeclaringClass(Method method, Class clazz) { if (Modifier.isPublic(clazz.getModifiers())) { try { clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); @@ -92,12 +102,8 @@ public class ReflectiveMethodExecutor implements MethodExecutor { // Continue below... } } - Class[] ifcs = clazz.getInterfaces(); - for (Class ifc: ifcs) { - discoverPublicClass(method, ifc); - } if (clazz.getSuperclass() != null) { - return discoverPublicClass(method, clazz.getSuperclass()); + return discoverPublicDeclaringClass(method, clazz.getSuperclass()); } return null; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java index 060e67e5c18..2aaf0cb171a 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2018 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. @@ -16,7 +16,6 @@ package org.springframework.expression.spel; -import org.junit.Before; import org.junit.Test; import org.springframework.expression.Expression; @@ -36,17 +35,11 @@ public class CachedMethodExecutorTests { private final ExpressionParser parser = new SpelExpressionParser(); - private StandardEvaluationContext context; - - - @Before - public void setUp() throws Exception { - this.context = new StandardEvaluationContext(new RootObject()); - } + private final StandardEvaluationContext context = new StandardEvaluationContext(new RootObject()); @Test - public void testCachedExecutionForParameters() throws Exception { + public void testCachedExecutionForParameters() { Expression expression = this.parser.parseExpression("echo(#var)"); assertMethodExecution(expression, 42, "int: 42"); @@ -56,7 +49,7 @@ public class CachedMethodExecutorTests { } @Test - public void testCachedExecutionForTarget() throws Exception { + public void testCachedExecutionForTarget() { Expression expression = this.parser.parseExpression("#var.echo(42)"); assertMethodExecution(expression, new RootObject(), "int: 42"); @@ -76,7 +69,6 @@ public class CachedMethodExecutorTests { public String echo(String value) { return "String: " + value; } - } public static class RootObject extends BaseObject { @@ -84,7 +76,6 @@ public class CachedMethodExecutorTests { public String echo(int value) { return "int: " + value; } - } }