Skip to content

Commit

Permalink
Fix RuntimeHintsPredicates matching rules
Browse files Browse the repository at this point in the history 
Prior to this commit, the `RuntimeHintsPredicates` would assume that
registering introspection or invocation hints for "all declared methods"
on a type would also include "all public methods". This is not true, as
the Java reflection API itself behaves differently.
`getDeclaredMethods()` does not return a superset of `getMethods()`, as
the latter can return inherited methods, but not the former.
Same reasoning applies to fields.

This commit fixes the hints predicates to only match if the correct hint
has been registered.

Fixes gh-31224
  • Loading branch information
@bclozel
bclozel committed Sep 15, 2023
1 parent 8f13031 commit 2270498
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 33 deletions.
12 changes: 6 additions & 6 deletions spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,15 +408,15 @@ void classGetMethodShouldMatchInvokePublicMethodsHint() {
}

@Test
void classGetMethodShouldMatchIntrospectDeclaredMethodsHint() {
void classGetMethodShouldNotMatchIntrospectDeclaredMethodsHint() {
hints.reflection().registerType(String.class, MemberCategory.INTROSPECT_DECLARED_METHODS);
assertThatInvocationMatches(InstrumentedMethod.CLASS_GETMETHOD, this.stringGetToStringMethod);
assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETMETHOD, this.stringGetToStringMethod);
}

@Test
void classGetMethodShouldMatchInvokeDeclaredMethodsHint() {
void classGetMethodShouldNotMatchInvokeDeclaredMethodsHint() {
hints.reflection().registerType(String.class, MemberCategory.INVOKE_DECLARED_METHODS);
assertThatInvocationMatches(InstrumentedMethod.CLASS_GETMETHOD, this.stringGetToStringMethod);
assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETMETHOD, this.stringGetToStringMethod);
}

@Test
Expand Down Expand Up @@ -544,9 +544,9 @@ void classGetFieldShouldMatchPublicFieldsHint() {
}

@Test
void classGetFieldShouldMatchDeclaredFieldsHint() {
void classGetFieldShouldNotMatchDeclaredFieldsHint() {
hints.reflection().registerType(PublicField.class, MemberCategory.DECLARED_FIELDS);
assertThatInvocationMatches(InstrumentedMethod.CLASS_GETFIELD, this.getPublicField);
assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETFIELD, this.getPublicField);
}

@Test
Expand Down
38 changes: 20 additions & 18 deletions ...-core/src/main/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicates.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -274,19 +274,6 @@ public ExecutableHintPredicate<T> invoke() {
return this;
}

@Override
public boolean test(RuntimeHints runtimeHints) {
return (new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass()))
.withAnyMemberCategory(getPublicMemberCategories())
.and(hints -> Modifier.isPublic(this.executable.getModifiers())))
.or(new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass())).withAnyMemberCategory(getDeclaredMemberCategories()))
.or(exactMatch()).test(runtimeHints);
}

abstract MemberCategory[] getPublicMemberCategories();

abstract MemberCategory[] getDeclaredMemberCategories();

abstract Predicate<RuntimeHints> exactMatch();

/**
Expand All @@ -309,6 +296,14 @@ public static class ConstructorHintPredicate extends ExecutableHintPredicate<Con
}

@Override
public boolean test(RuntimeHints runtimeHints) {
return (new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass()))
.withAnyMemberCategory(getPublicMemberCategories())
.and(hints -> Modifier.isPublic(this.executable.getModifiers())))
.or(new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass())).withAnyMemberCategory(getDeclaredMemberCategories()))
.or(exactMatch()).test(runtimeHints);
}

MemberCategory[] getPublicMemberCategories() {
if (this.executableMode == ExecutableMode.INTROSPECT) {
return new MemberCategory[] { MemberCategory.INTROSPECT_PUBLIC_CONSTRUCTORS,
Expand All @@ -317,7 +312,6 @@ MemberCategory[] getPublicMemberCategories() {
return new MemberCategory[] { MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS };
}

@Override
MemberCategory[] getDeclaredMemberCategories() {
if (this.executableMode == ExecutableMode.INTROSPECT) {
return new MemberCategory[] { MemberCategory.INTROSPECT_DECLARED_CONSTRUCTORS,
Expand All @@ -344,6 +338,16 @@ public static class MethodHintPredicate extends ExecutableHintPredicate<Method>
}

@Override
public boolean test(RuntimeHints runtimeHints) {
return (new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass()))
.withAnyMemberCategory(getPublicMemberCategories())
.and(hints -> Modifier.isPublic(this.executable.getModifiers())))
.or(new TypeHintPredicate(TypeReference.of(this.executable.getDeclaringClass()))
.withAnyMemberCategory(getDeclaredMemberCategories())
.and(hints -> !Modifier.isPublic(this.executable.getModifiers())))
.or(exactMatch()).test(runtimeHints);
}

MemberCategory[] getPublicMemberCategories() {
if (this.executableMode == ExecutableMode.INTROSPECT) {
return new MemberCategory[] { MemberCategory.INTROSPECT_PUBLIC_METHODS,
Expand All @@ -352,7 +356,6 @@ MemberCategory[] getPublicMemberCategories() {
return new MemberCategory[] { MemberCategory.INVOKE_PUBLIC_METHODS };
}

@Override
MemberCategory[] getDeclaredMemberCategories() {

if (this.executableMode == ExecutableMode.INTROSPECT) {
Expand Down Expand Up @@ -392,8 +395,7 @@ public boolean test(RuntimeHints runtimeHints) {

private boolean memberCategoryMatch(TypeHint typeHint) {
if (Modifier.isPublic(this.field.getModifiers())) {
return typeHint.getMemberCategories().contains(MemberCategory.PUBLIC_FIELDS) ||
typeHint.getMemberCategories().contains(MemberCategory.DECLARED_FIELDS);
return typeHint.getMemberCategories().contains(MemberCategory.PUBLIC_FIELDS);
}
else {
return typeHint.getMemberCategories().contains(MemberCategory.DECLARED_FIELDS);
Expand Down
18 changes: 9 additions & 9 deletions .../src/test/java/org/springframework/aot/hint/predicate/ReflectionHintsPredicatesTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -329,15 +329,15 @@ void methodIntrospectionMatchesInvokePublicMethods() {
}

@Test
void methodIntrospectionMatchesIntrospectDeclaredMethods() {
void methodIntrospectionDoesNotMatchIntrospectDeclaredMethods() {
runtimeHints.reflection().registerType(SampleClass.class, MemberCategory.INTROSPECT_DECLARED_METHODS);
assertPredicateMatches(reflection.onMethod(SampleClass.class, "publicMethod").introspect());
assertPredicateDoesNotMatch(reflection.onMethod(SampleClass.class, "publicMethod").introspect());
}

@Test
void methodIntrospectionMatchesInvokeDeclaredMethods() {
void methodIntrospectionDoesNotMatchInvokeDeclaredMethods() {
runtimeHints.reflection().registerType(SampleClass.class, MemberCategory.INVOKE_DECLARED_METHODS);
assertPredicateMatches(reflection.onMethod(SampleClass.class, "publicMethod").introspect());
assertPredicateDoesNotMatch(reflection.onMethod(SampleClass.class, "publicMethod").introspect());
}

@Test
Expand Down Expand Up @@ -373,9 +373,9 @@ void methodInvocationDoesNotMatchIntrospectDeclaredMethods() {
}

@Test
void methodInvocationMatchesInvokeDeclaredMethods() {
void methodInvocationDoesNotMatchInvokeDeclaredMethods() {
runtimeHints.reflection().registerType(SampleClass.class, MemberCategory.INVOKE_DECLARED_METHODS);
assertPredicateMatches(reflection.onMethod(SampleClass.class, "publicMethod").invoke());
assertPredicateDoesNotMatch(reflection.onMethod(SampleClass.class, "publicMethod").invoke());
}

@Test
Expand Down Expand Up @@ -482,9 +482,9 @@ void fieldReflectionMatchesPublicFieldsHint() {
}

@Test
void fieldReflectionMatchesDeclaredFieldsHint() {
void fieldReflectionDoesNotMatchDeclaredFieldsHint() {
runtimeHints.reflection().registerType(SampleClass.class, MemberCategory.DECLARED_FIELDS);
assertPredicateMatches(reflection.onField(SampleClass.class, "publicField"));
assertPredicateDoesNotMatch(reflection.onField(SampleClass.class, "publicField"));
}

@Test
Expand Down

0 comments on commit 2270498

Please sign in to comment.