From 43d39d4e8ae67b10c12c44805a310d68164b5139 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 3 Oct 2022 11:23:14 +0200 Subject: [PATCH] Enforce need for type reflection in RuntimeHintsAgent Prior to this commit, the AOT infrastructure would rely on the fact that native runtime reflection on a type would only consider methods/fields/constructors that had specific hints contributed. When listing them through the reflection API on the type, the native image would only return those for which we had hints contributed. This behavior will soon change in GraalVM and will better align with the JVM behavior: when asking for all declared methods on a type in a native image, we should get all existing methods, not just the ones registered previously in the native image. This commit aligns the behavior of the `RuntimeHintsAgent` and removes the now misleading predicates as a consequence. Closes gh-29205 --- .../aot/RuntimeHintsAgentTests.java | 16 ------- .../aot/agent/InstrumentedBridgeMethods.java | 27 ----------- .../aot/agent/InstrumentedMethod.java | 27 +++-------- .../aot/agent/InstrumentedMethodTests.java | 47 +++++-------------- .../predicate/ReflectionHintsPredicates.java | 23 --------- .../ReflectionHintsPredicatesTests.java | 36 -------------- 6 files changed, 18 insertions(+), 158 deletions(-) diff --git a/integration-tests/src/test/java/org/springframework/aot/RuntimeHintsAgentTests.java b/integration-tests/src/test/java/org/springframework/aot/RuntimeHintsAgentTests.java index e29518bae3..2309cbc01d 100644 --- a/integration-tests/src/test/java/org/springframework/aot/RuntimeHintsAgentTests.java +++ b/integration-tests/src/test/java/org/springframework/aot/RuntimeHintsAgentTests.java @@ -164,22 +164,6 @@ public class RuntimeHintsAgentTests { throw new RuntimeException(e); } }, MethodReference.of(Method.class, "invoke")), - Arguments.of((Runnable) () -> { - try { - toStringMethod.getAnnotations(); - } - catch (Exception e) { - throw new RuntimeException(e); - } - }, MethodReference.of(Method.class, "getAnnotations")), - Arguments.of((Runnable) () -> { - try { - toStringMethod.getParameterTypes(); - } - catch (Exception e) { - throw new RuntimeException(e); - } - }, MethodReference.of(Method.class, "getParameterTypes")), Arguments.of((Runnable) () -> { try { privateGreetMethod.invoke(new PrivateClass()); diff --git a/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedBridgeMethods.java b/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedBridgeMethods.java index 30c9aef2c5..df2d880c6e 100644 --- a/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedBridgeMethods.java +++ b/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedBridgeMethods.java @@ -18,7 +18,6 @@ package org.springframework.aot.agent; import java.io.IOException; import java.io.InputStream; -import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.InvocationHandler; @@ -336,32 +335,6 @@ public abstract class InstrumentedBridgeMethods { * Bridge methods for java.lang.reflect.Method */ - public static Annotation[] methodgetAnnotations(Method method) { - Annotation[] result = null; - try { - result = method.getAnnotations(); - } - finally { - RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.METHOD_GETANNOTATIONS) - .onInstance(method).returnValue(result).build(); - RecordedInvocationsPublisher.publish(invocation); - } - return result; - } - - public static Class[] methodgetParameterTypes(Method method) { - Class[] result = null; - try { - result = method.getParameterTypes(); - } - finally { - RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.METHOD_GETPARAMETERTYPES) - .onInstance(method).returnValue(result).build(); - RecordedInvocationsPublisher.publish(invocation); - } - return result; - } - public static Object methodinvoke(Method method, Object object, Object... arguments) throws InvocationTargetException, IllegalAccessException { Object result = null; boolean accessibilityChanged = false; diff --git a/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedMethod.java b/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedMethod.java index 091cfbef58..d798c10fd7 100644 --- a/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedMethod.java +++ b/spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedMethod.java @@ -93,8 +93,7 @@ enum InstrumentedMethod { Class thisClass = invocation.getInstance(); return reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory( MemberCategory.INTROSPECT_PUBLIC_CONSTRUCTORS, MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS, - MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyConstructor()); + MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS); } ), @@ -130,8 +129,7 @@ enum InstrumentedMethod { invocation -> { Class thisClass = invocation.getInstance(); return reflection().onType(TypeReference.of(thisClass)) - .withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyConstructor()); + .withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS); }), /** @@ -155,8 +153,7 @@ enum InstrumentedMethod { CLASS_GETDECLAREDFIELDS(Class.class, "getDeclaredFields", HintType.REFLECTION, invocation -> { Class thisClass = invocation.getInstance(); - return reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_FIELDS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyField()); + return reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_FIELDS); } ), @@ -183,8 +180,7 @@ enum InstrumentedMethod { invocation -> { Class thisClass = invocation.getInstance(); return reflection().onType(TypeReference.of(thisClass)) - .withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyMethod()); + .withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS); } ), @@ -211,8 +207,7 @@ enum InstrumentedMethod { invocation -> { Class thisClass = invocation.getInstance(); return reflection().onType(TypeReference.of(thisClass)) - .withAnyMemberCategory(MemberCategory.PUBLIC_FIELDS, MemberCategory.DECLARED_FIELDS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyField()); + .withAnyMemberCategory(MemberCategory.PUBLIC_FIELDS, MemberCategory.DECLARED_FIELDS); } ), @@ -242,8 +237,7 @@ enum InstrumentedMethod { Class thisClass = invocation.getInstance(); return reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory( MemberCategory.INTROSPECT_PUBLIC_METHODS, MemberCategory.INTROSPECT_DECLARED_METHODS, - MemberCategory.INVOKE_PUBLIC_METHODS, MemberCategory.INVOKE_DECLARED_METHODS) - .or(reflection().onType(TypeReference.of(thisClass)).withAnyMethod()); + MemberCategory.INVOKE_PUBLIC_METHODS, MemberCategory.INVOKE_DECLARED_METHODS); } ), @@ -265,15 +259,6 @@ enum InstrumentedMethod { CONSTRUCTOR_NEWINSTANCE(Constructor.class, "newInstance", HintType.REFLECTION, invocation -> reflection().onConstructor(invocation.getInstance()).invoke()), - /** - * {@link Method#getParameterTypes()}. - */ - METHOD_GETANNOTATIONS(Method.class, "getAnnotations", HintType.REFLECTION, - invocation -> reflection().onMethod(invocation.getInstance())), - - METHOD_GETPARAMETERTYPES(Method.class, "getParameterTypes", HintType.REFLECTION, - invocation -> reflection().onMethod(invocation.getInstance())), - /** * {@link Method#invoke(Object, Object...)}. */ diff --git a/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java b/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java index 6b989174d3..a9e4ab8520 100644 --- a/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java +++ b/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java @@ -16,7 +16,6 @@ package org.springframework.aot.agent; -import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Collections; import java.util.Comparator; @@ -193,9 +192,9 @@ class InstrumentedMethodTests { } @Test - void classGetConstructorsShouldMatchConstructorReflectionHint() throws Exception { + void classGetConstructorsShouldNotMatchConstructorReflectionHint() throws Exception { hints.reflection().registerConstructor(String.class.getConstructor(), ExecutableMode.INVOKE); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETCONSTRUCTORS, this.stringGetConstructors); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETCONSTRUCTORS, this.stringGetConstructors); } @Test @@ -249,9 +248,9 @@ class InstrumentedMethodTests { } @Test - void classGetDeclaredConstructorsShouldMatchConstructorReflectionHint() throws Exception { + void classGetDeclaredConstructorsShouldNotMatchConstructorReflectionHint() throws Exception { hints.reflection().registerConstructor(String.class.getConstructor(), ExecutableMode.INVOKE); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETDECLAREDCONSTRUCTORS, this.stringGetDeclaredConstructors); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETDECLAREDCONSTRUCTORS, this.stringGetDeclaredConstructors); } @Test @@ -349,9 +348,9 @@ class InstrumentedMethodTests { } @Test - void classGetDeclaredMethodsShouldMatchMethodReflectionHint() throws Exception { + void classGetDeclaredMethodsShouldNotMatchMethodReflectionHint() throws Exception { hints.reflection().registerMethod(String.class.getMethod("toString"), ExecutableMode.INVOKE); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETDECLAREDMETHODS, this.stringGetScaleMethod); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETDECLAREDMETHODS, this.stringGetScaleMethod); } @Test @@ -391,9 +390,9 @@ class InstrumentedMethodTests { } @Test - void classGetMethodsShouldMatchMethodReflectionHint() throws Exception { + void classGetMethodsShouldNotMatchMethodReflectionHint() throws Exception { hints.reflection().registerMethod(String.class.getMethod("toString"), ExecutableMode.INVOKE); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETMETHODS, this.stringGetMethods); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETMETHODS, this.stringGetMethods); } @Test @@ -452,28 +451,6 @@ class InstrumentedMethodTests { assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETMETHOD, this.stringGetToStringMethod); } - @Test - void methodGetAnnotationsShouldMatchIntrospectHintOnMethod() throws NoSuchMethodException { - Method toString = String.class.getMethod("toString"); - RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.METHOD_GETANNOTATIONS) - .onInstance(toString).withArguments("testString") - .returnValue(toString.getAnnotations()).build(); - hints.reflection().registerType(String.class, typeHint -> typeHint.withMethod("toString", - Collections.emptyList(), ExecutableMode.INTROSPECT)); - assertThatInvocationMatches(InstrumentedMethod.METHOD_GETANNOTATIONS, invocation); - } - - @Test - void methodGetParameterTypesShouldMatchIntrospectHintOnMethod() throws NoSuchMethodException { - Method toString = String.class.getMethod("toString"); - RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.METHOD_GETPARAMETERTYPES) - .onInstance(toString).withArguments("testString") - .returnValue(toString.getParameterTypes()).build(); - hints.reflection().registerType(String.class, typeHint -> typeHint.withMethod("toString", - Collections.emptyList(), ExecutableMode.INTROSPECT)); - assertThatInvocationMatches(InstrumentedMethod.METHOD_GETPARAMETERTYPES, invocation); - } - @Test void methodInvokeShouldMatchInvokeHintOnMethod() throws NoSuchMethodException { RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.METHOD_INVOKE) @@ -555,9 +532,9 @@ class InstrumentedMethodTests { } @Test - void classGetDeclaredFieldsShouldMatchFieldHint() throws Exception { + void classGetDeclaredFieldsShouldNotMatchFieldHint() throws Exception { hints.reflection().registerField(String.class.getDeclaredField("value")); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETDECLAREDFIELDS, this.stringGetDeclaredFields); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETDECLAREDFIELDS, this.stringGetDeclaredFields); } @Test @@ -626,9 +603,9 @@ class InstrumentedMethodTests { } @Test - void classGetFieldsShouldMatchFieldHint() throws Exception { + void classGetFieldsShouldNotMatchFieldHint() throws Exception { hints.reflection().registerField(String.class.getDeclaredField("value")); - assertThatInvocationMatches(InstrumentedMethod.CLASS_GETFIELDS, this.stringGetFields); + assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETFIELDS, this.stringGetFields); } } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicates.java b/spring-core/src/main/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicates.java index ca71c64039..2de9a41182 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicates.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicates.java @@ -243,29 +243,6 @@ public class ReflectionHintsPredicates { .anyMatch(memberCategory -> getTypeHint(hints).getMemberCategories().contains(memberCategory))); } - /** - * Refine the current predicate to only match if a hint is present for any of its constructors. - * @return the refined {@link RuntimeHints} predicate - */ - public Predicate withAnyConstructor() { - return this.and(hints -> getTypeHint(hints).constructors().findAny().isPresent()); - } - - /** - * Refine the current predicate to only match if a hint is present for any of its methods. - * @return the refined {@link RuntimeHints} predicate - */ - public Predicate withAnyMethod() { - return this.and(hints -> getTypeHint(hints).methods().findAny().isPresent()); - } - - /** - * Refine the current predicate to only match if a hint is present for any of its fields. - * @return the refined {@link RuntimeHints} predicate - */ - public Predicate withAnyField() { - return this.and(hints -> getTypeHint(hints).fields().findAny().isPresent()); - } } public abstract static class ExecutableHintPredicate implements Predicate { diff --git a/spring-core/src/test/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicatesTests.java b/spring-core/src/test/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicatesTests.java index e4ae6127de..cf6999160d 100644 --- a/spring-core/src/test/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicatesTests.java +++ b/spring-core/src/test/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicatesTests.java @@ -296,18 +296,6 @@ class ReflectionHintsPredicatesTests { assertPredicateMatches(reflection.onConstructor(privateConstructor).invoke()); } - @Test - void reflectionOnAnyConstructorDoesNotMatchTypeReflection() { - runtimeHints.reflection().registerType(SampleClass.class); - assertPredicateDoesNotMatch(reflection.onType(SampleClass.class).withAnyConstructor()); - } - - @Test - void reflectionOnAnyConstructorMatchesConstructorReflection() { - runtimeHints.reflection().registerConstructor(publicConstructor, ExecutableMode.INVOKE); - assertPredicateMatches(reflection.onType(SampleClass.class).withAnyConstructor()); - } - } @Nested @@ -457,18 +445,6 @@ class ReflectionHintsPredicatesTests { assertPredicateMatches(reflection.onMethod(SampleClass.class, "privateMethod").invoke()); } - @Test - void reflectionOnAnyMethodDoesNotMatchTypeReflection() { - runtimeHints.reflection().registerType(SampleClass.class); - assertPredicateDoesNotMatch(reflection.onType(SampleClass.class).withAnyMethod()); - } - - @Test - void reflectionOnAnyMethodMatchesMethodReflection() { - runtimeHints.reflection().registerMethod(publicMethod, ExecutableMode.INVOKE); - assertPredicateMatches(reflection.onType(SampleClass.class).withAnyMethod()); - } - } @Nested @@ -527,18 +503,6 @@ class ReflectionHintsPredicatesTests { assertPredicateMatches(reflection.onField(SampleClass.class, "privateField")); } - @Test - void reflectionOnAnyFieldDoesNotMatchTypeReflection() { - runtimeHints.reflection().registerType(SampleClass.class); - assertPredicateDoesNotMatch(reflection.onType(SampleClass.class).withAnyField()); - } - - @Test - void reflectionOnAnyFieldMatchesFieldReflection() { - runtimeHints.reflection().registerField(publicField); - assertPredicateMatches(reflection.onType(SampleClass.class).withAnyField()); - } - } private void assertPredicateMatches(Predicate predicate) {