Skip to content

Commit

Permalink
Do not require full type reflection when listing methods/fields
Browse files Browse the repository at this point in the history 
Prior to this commit, the `RuntimeHintsAgent` and its testing
infrastructure would assume that calling `MyClass.class.getMethods()`
requires a reflection hint on the class for introspecting public/private
methods.

GraalVM does not require this, in fact this call only returns methods
that have reflection hints in the native image.

This commit refines the agent behavior for `Class.getMethods()`,
`Class.getDeclaredMethods()`, `Class.getFields()` and
`Class.getDeclaredFields()`. With this change, registering at least one
method/field for reflection is enough to match.

During the execution of Java tests, all methods and fields will be
provided, regardless of hints being registered or not. This could cause
false negatives where we're missing reflection hints on methods or
fields.
This risk is mitigated thanks to additional instrumentation on
`Method.getAnnotations()`, `Method.getParameterTypes()` and
`Method.invoke()`. If a method is found reflectively, chances are it
will be used for further reflection.

Closes gh-29091
  • Loading branch information
@bclozel
bclozel committed Sep 6, 2022
1 parent 6cce471 commit 323d190
Show file tree
Hide file tree
Showing 6 changed files with 288 additions and 54 deletions.
16 changes: 16 additions & 0 deletions integration-tests/src/test/java/org/springframework/aot/RuntimeHintsAgentTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,22 @@ private static Stream<Arguments> instrumentedReflectionMethods() {
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());
Expand Down
27 changes: 27 additions & 0 deletions spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedBridgeMethods.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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;
Expand Down Expand Up @@ -335,6 +336,32 @@ public static Object constructornewInstance(Constructor<?> constructor, Object..
* 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;
Expand Down
90 changes: 54 additions & 36 deletions spring-core-test/src/main/java/org/springframework/aot/agent/InstrumentedMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
import org.springframework.aot.hint.TypeReference;
import org.springframework.aot.hint.predicate.RuntimeHintsPredicates;

import static org.springframework.aot.hint.predicate.RuntimeHintsPredicates.reflection;
import static org.springframework.aot.hint.predicate.RuntimeHintsPredicates.resource;

/**
* Java method that is instrumented by the {@link RuntimeHintsAgent}.
*
Expand All @@ -55,7 +58,7 @@ enum InstrumentedMethod {
CLASS_FORNAME(Class.class, "forName", HintType.REFLECTION,
invocation -> {
String className = invocation.getArgument(0);
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(className));
return reflection().onType(TypeReference.of(className));
}),

/**
Expand All @@ -64,7 +67,7 @@ enum InstrumentedMethod {
CLASS_GETCLASSES(Class.class, "getClasses", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass))
return reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.DECLARED_CLASSES, MemberCategory.PUBLIC_CLASSES);
}
),
Expand All @@ -78,7 +81,7 @@ enum InstrumentedMethod {
if (constructor == null) {
return runtimeHints -> false;
}
return RuntimeHintsPredicates.reflection().onConstructor(constructor).introspect();
return reflection().onConstructor(constructor).introspect();
}
),

Expand All @@ -88,9 +91,10 @@ enum InstrumentedMethod {
CLASS_GETCONSTRUCTORS(Class.class, "getConstructors", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory(
return reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory(
MemberCategory.INTROSPECT_PUBLIC_CONSTRUCTORS, MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS,
MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyConstructor());
}
),

Expand All @@ -100,7 +104,7 @@ enum InstrumentedMethod {
CLASS_GETDECLAREDCLASSES(Class.class, "getDeclaredClasses", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_CLASSES);
return reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_CLASSES);
}
),

Expand All @@ -114,8 +118,8 @@ enum InstrumentedMethod {
return runtimeHints -> false;
}
TypeReference thisType = invocation.getInstanceTypeReference();
return RuntimeHintsPredicates.reflection().onType(thisType).withMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS)
.or(RuntimeHintsPredicates.reflection().onConstructor(constructor).introspect());
return reflection().onType(thisType).withMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS)
.or(reflection().onConstructor(constructor).introspect());
}
),

Expand All @@ -125,8 +129,9 @@ enum InstrumentedMethod {
CLASS_GETDECLAREDCONSTRUCTORS(Class.class, "getDeclaredConstructors", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
return reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyConstructor());
}),

/**
Expand All @@ -139,8 +144,8 @@ enum InstrumentedMethod {
return runtimeHints -> false;
}
TypeReference thisType = invocation.getInstanceTypeReference();
return RuntimeHintsPredicates.reflection().onType(thisType).withMemberCategory(MemberCategory.DECLARED_FIELDS)
.or(RuntimeHintsPredicates.reflection().onField(field));
return reflection().onType(thisType).withMemberCategory(MemberCategory.DECLARED_FIELDS)
.or(reflection().onField(field));
}
),

Expand All @@ -150,7 +155,8 @@ enum InstrumentedMethod {
CLASS_GETDECLAREDFIELDS(Class.class, "getDeclaredFields", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_FIELDS);
return reflection().onType(TypeReference.of(thisClass)).withMemberCategory(MemberCategory.DECLARED_FIELDS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyField());
}
),

Expand All @@ -164,9 +170,9 @@ enum InstrumentedMethod {
return runtimeHints -> false;
}
TypeReference thisType = invocation.getInstanceTypeReference();
return RuntimeHintsPredicates.reflection().onType(thisType)
return reflection().onType(thisType)
.withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS)
.or(RuntimeHintsPredicates.reflection().onMethod(method).introspect());
.or(reflection().onMethod(method).introspect());
}
),

Expand All @@ -176,8 +182,9 @@ enum InstrumentedMethod {
CLASS_GETDECLAREDMETHODS(Class.class, "getDeclaredMethods", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS);
return reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyMethod());
}
),

Expand All @@ -191,10 +198,10 @@ enum InstrumentedMethod {
return runtimeHints -> false;
}
TypeReference thisType = invocation.getInstanceTypeReference();
return RuntimeHintsPredicates.reflection().onType(thisType).withMemberCategory(MemberCategory.PUBLIC_FIELDS)
return reflection().onType(thisType).withMemberCategory(MemberCategory.PUBLIC_FIELDS)
.and(runtimeHints -> Modifier.isPublic(field.getModifiers()))
.or(RuntimeHintsPredicates.reflection().onType(thisType).withMemberCategory(MemberCategory.DECLARED_FIELDS))
.or(RuntimeHintsPredicates.reflection().onField(invocation.getReturnValue()));
.or(reflection().onType(thisType).withMemberCategory(MemberCategory.DECLARED_FIELDS))
.or(reflection().onField(invocation.getReturnValue()));
}),

/**
Expand All @@ -203,8 +210,9 @@ enum InstrumentedMethod {
CLASS_GETFIELDS(Class.class, "getFields", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.PUBLIC_FIELDS, MemberCategory.DECLARED_FIELDS);
return reflection().onType(TypeReference.of(thisClass))
.withAnyMemberCategory(MemberCategory.PUBLIC_FIELDS, MemberCategory.DECLARED_FIELDS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyField());
}
),

Expand All @@ -218,11 +226,11 @@ enum InstrumentedMethod {
return runtimeHints -> false;
}
TypeReference thisType = invocation.getInstanceTypeReference();
return RuntimeHintsPredicates.reflection().onType(thisType).withAnyMemberCategory(MemberCategory.INTROSPECT_PUBLIC_METHODS, MemberCategory.INVOKE_PUBLIC_METHODS)
return reflection().onType(thisType).withAnyMemberCategory(MemberCategory.INTROSPECT_PUBLIC_METHODS, MemberCategory.INVOKE_PUBLIC_METHODS)
.and(runtimeHints -> Modifier.isPublic(method.getModifiers()))
.or(RuntimeHintsPredicates.reflection().onType(thisType).withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS))
.or(RuntimeHintsPredicates.reflection().onMethod(method).introspect())
.or(RuntimeHintsPredicates.reflection().onMethod(method).invoke());
.or(reflection().onType(thisType).withAnyMemberCategory(MemberCategory.INTROSPECT_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_METHODS))
.or(reflection().onMethod(method).introspect())
.or(reflection().onMethod(method).invoke());
}
),

Expand All @@ -232,9 +240,10 @@ enum InstrumentedMethod {
CLASS_GETMETHODS(Class.class, "getMethods", HintType.REFLECTION,
invocation -> {
Class<?> thisClass = invocation.getInstance();
return RuntimeHintsPredicates.reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory(
return reflection().onType(TypeReference.of(thisClass)).withAnyMemberCategory(
MemberCategory.INTROSPECT_PUBLIC_METHODS, MemberCategory.INTROSPECT_DECLARED_METHODS,
MemberCategory.INVOKE_PUBLIC_METHODS, MemberCategory.INVOKE_DECLARED_METHODS);
MemberCategory.INVOKE_PUBLIC_METHODS, MemberCategory.INVOKE_DECLARED_METHODS)
.or(reflection().onType(TypeReference.of(thisClass)).withAnyMethod());
}
),

Expand All @@ -247,32 +256,41 @@ enum InstrumentedMethod {
if (klass == null) {
return runtimeHints -> false;
}
return RuntimeHintsPredicates.reflection().onType(klass);
return reflection().onType(klass);
}),

/**
* {@link Constructor#newInstance(Object...)}.
*/
CONSTRUCTOR_NEWINSTANCE(Constructor.class, "newInstance", HintType.REFLECTION,
invocation -> RuntimeHintsPredicates.reflection().onConstructor(invocation.getInstance()).invoke()),
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...)}.
*/
METHOD_INVOKE(Method.class, "invoke", HintType.REFLECTION,
invocation -> RuntimeHintsPredicates.reflection().onMethod(invocation.getInstance()).invoke()),
invocation -> reflection().onMethod(invocation.getInstance()).invoke()),

/**
* {@link Field#get(Object)}.
*/
FIELD_GET(Field.class, "get", HintType.REFLECTION,
invocation -> RuntimeHintsPredicates.reflection().onField(invocation.getInstance())),
invocation -> reflection().onField(invocation.getInstance())),

/**
* {@link Field#set(Object, Object)}.
*/
FIELD_SET(Field.class, "set", HintType.REFLECTION,
invocation -> RuntimeHintsPredicates.reflection().onField(invocation.getInstance()).withWriteMode()),
invocation -> reflection().onField(invocation.getInstance()).withWriteMode()),


/*
Expand All @@ -285,7 +303,7 @@ enum InstrumentedMethod {
RESOURCEBUNDLE_GETBUNDLE(ResourceBundle.class, "getBundle", HintType.RESOURCE_BUNDLE,
invocation -> {
String bundleName = invocation.getArgument(0);
return RuntimeHintsPredicates.resource().forBundle(bundleName);
return resource().forBundle(bundleName);
}),

/*
Expand All @@ -299,7 +317,7 @@ enum InstrumentedMethod {
invocation -> {
Class<?> thisClass = invocation.getInstance();
String resourceName = invocation.getArgument(0);
return RuntimeHintsPredicates.resource().forResource(TypeReference.of(thisClass), resourceName);
return resource().forResource(TypeReference.of(thisClass), resourceName);
}),

/**
Expand All @@ -315,7 +333,7 @@ enum InstrumentedMethod {
CLASSLOADER_GETRESOURCE(ClassLoader.class, "getResource", HintType.RESOURCE_PATTERN,
invocation -> {
String resourceName = invocation.getArgument(0);
return RuntimeHintsPredicates.resource().forResource(resourceName);
return resource().forResource(resourceName);
}),

/**
Expand Down
Loading

0 comments on commit 323d190

Please sign in to comment.