Skip to content

Commit

Permalink
Fix hints and predicates for Field reflective access
Browse files Browse the repository at this point in the history
This commit revisits the arrangement for Field hints after changes made
in gh-34239.

Closes gh-34294
  • Loading branch information
bclozel committed Jan 21, 2025
1 parent 3302bc4 commit f85752a
Show file tree
Hide file tree
Showing 21 changed files with 106 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public AspectJAdvisorContribution(Class<?> beanClass) {

@Override
public void applyTo(GenerationContext generationContext, BeanRegistrationCode beanRegistrationCode) {
generationContext.getRuntimeHints().reflection().registerType(this.beanClass, MemberCategory.INVOKE_DECLARED_FIELDS);
generationContext.getRuntimeHints().reflection().registerType(this.beanClass, MemberCategory.ACCESS_DECLARED_FIELDS);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class AspectJAdvisorBeanRegistrationAotProcessorTests {
@Test
void shouldProcessAspectJClass() {
process(AspectJClass.class);
assertThat(reflection().onType(AspectJClass.class).withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS))
assertThat(reflection().onType(AspectJClass.class).withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS))
.accepts(this.runtimeHints);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,10 @@ private void registerReflectionHints(RootBeanDefinition beanDefinition, Method w
// ReflectionUtils#findField searches recursively in the type hierarchy
Class<?> searchType = beanDefinition.getTargetType();
while (searchType != null && searchType != writeMethod.getDeclaringClass()) {
this.hints.reflection().registerType(searchType, MemberCategory.INVOKE_DECLARED_FIELDS);
this.hints.reflection().registerType(searchType, MemberCategory.ACCESS_DECLARED_FIELDS);
searchType = searchType.getSuperclass();
}
this.hints.reflection().registerType(writeMethod.getDeclaringClass(), MemberCategory.INVOKE_DECLARED_FIELDS);
this.hints.reflection().registerType(writeMethod.getDeclaringClass(), MemberCategory.ACCESS_DECLARED_FIELDS);
}

private void addQualifiers(CodeBlock.Builder code, RootBeanDefinition beanDefinition) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ private void assertHasMethodInvokeHints(Class<?> beanType, String... methodNames

private void assertHasDeclaredFieldsHint(Class<?> beanType) {
assertThat(RuntimeHintsPredicates.reflection()
.onType(beanType).withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS))
.onType(beanType).withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS))
.accepts(this.generationContext.getRuntimeHints());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public AotContribution(Collection<Class<?>> validatedClasses,
public void applyTo(GenerationContext generationContext, BeanRegistrationCode beanRegistrationCode) {
ReflectionHints hints = generationContext.getRuntimeHints().reflection();
for (Class<?> validatedClass : this.validatedClasses) {
hints.registerType(validatedClass, MemberCategory.INVOKE_DECLARED_FIELDS);
hints.registerType(validatedClass, MemberCategory.ACCESS_DECLARED_FIELDS);
}
for (Class<? extends ConstraintValidator<?, ?>> constraintValidatorClass : this.constraintValidatorClasses) {
hints.registerType(constraintValidatorClass, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ void refreshForAotRegisterHintsForCglibProxy() {
TypeReference cglibType = TypeReference.of(CglibConfiguration.class.getName() + "$$SpringCGLIB$$0");
assertThat(RuntimeHintsPredicates.reflection().onType(cglibType)
.withMemberCategories(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS,
MemberCategory.INVOKE_DECLARED_METHODS, MemberCategory.INVOKE_DECLARED_FIELDS))
MemberCategory.INVOKE_DECLARED_METHODS, MemberCategory.ACCESS_DECLARED_FIELDS))
.accepts(runtimeHints);
assertThat(RuntimeHintsPredicates.reflection().onType(CglibConfiguration.class)
.withMemberCategories(MemberCategory.INVOKE_PUBLIC_METHODS, MemberCategory.INVOKE_DECLARED_METHODS))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void shouldProcessMethodParameterLevelConstraint() {
process(MethodParameterLevelConstraint.class);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(2);
assertThat(RuntimeHintsPredicates.reflection().onType(MethodParameterLevelConstraint.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(ExistsValidator.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)).accepts(this.generationContext.getRuntimeHints());
}
Expand All @@ -89,7 +89,7 @@ void shouldProcessConstructorParameterLevelConstraint() {
process(ConstructorParameterLevelConstraint.class);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(2);
assertThat(RuntimeHintsPredicates.reflection().onType(ConstructorParameterLevelConstraint.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(ExistsValidator.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)).accepts(this.generationContext.getRuntimeHints());
}
Expand All @@ -99,7 +99,7 @@ void shouldProcessPropertyLevelConstraint() {
process(PropertyLevelConstraint.class);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(2);
assertThat(RuntimeHintsPredicates.reflection().onType(PropertyLevelConstraint.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(ExistsValidator.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)).accepts(this.generationContext.getRuntimeHints());
}
Expand All @@ -109,7 +109,7 @@ void shouldProcessGenericTypeLevelConstraint() {
process(GenericTypeLevelConstraint.class);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(2);
assertThat(RuntimeHintsPredicates.reflection().onType(GenericTypeLevelConstraint.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(PatternValidator.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)).accepts(this.generationContext.getRuntimeHints());
}
Expand All @@ -119,9 +119,9 @@ void shouldProcessTransitiveGenericTypeLevelConstraint() {
process(TransitiveGenericTypeLevelConstraint.class);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(3);
assertThat(RuntimeHintsPredicates.reflection().onType(TransitiveGenericTypeLevelConstraint.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(Exclude.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
assertThat(RuntimeHintsPredicates.reflection().onType(PatternValidator.class)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS)).accepts(this.generationContext.getRuntimeHints());
}
Expand All @@ -132,7 +132,7 @@ void shouldProcessRecursiveGenericsWithoutInfiniteRecursion(Class<?> beanClass)
process(beanClass);
assertThat(this.generationContext.getRuntimeHints().reflection().typeHints()).hasSize(1);
assertThat(RuntimeHintsPredicates.reflection().onType(beanClass)
.withMemberCategory(MemberCategory.INVOKE_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
.withMemberCategory(MemberCategory.ACCESS_DECLARED_FIELDS)).accepts(this.generationContext.getRuntimeHints());
}

@Test // gh-33940
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,13 @@ enum InstrumentedMethod {
* {@link Field#get(Object)}.
*/
FIELD_GET(Field.class, "get", HintType.REFLECTION,
invocation -> reflection().onFieldInvocation(invocation.getInstance())),
invocation -> reflection().onFieldAccess(invocation.getInstance())),

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


/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private void registerReflectionHints(ReflectionHints hints, Set<Type> seen, Type
typeHint.withMembers(MemberCategory.INVOKE_PUBLIC_CONSTRUCTORS,
MemberCategory.INVOKE_PUBLIC_METHODS);
}
typeHint.withMembers(MemberCategory.INVOKE_DECLARED_FIELDS,
typeHint.withMembers(MemberCategory.ACCESS_DECLARED_FIELDS,
MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
for (Method method : clazz.getMethods()) {
String methodName = method.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public enum MemberCategory {

/**
* A category that represents reflective field access on public {@linkplain Field fields}.
* @deprecated in favor of @link #INVOKE_PUBLIC_FIELDS} with similar semantics.
* @deprecated in favor of {@link #ACCESS_PUBLIC_FIELDS} with similar semantics.
* @see Field#get(Object)
* @see Field#set(Object, Object)
*/
Expand All @@ -44,7 +44,7 @@ public enum MemberCategory {
* A category that represents reflective field access on
* {@linkplain Class#getDeclaredFields() declared fields}: all fields defined by the
* class but not inherited fields.
* @deprecated in favor of @link #INVOKE_DECLARED_FIELDS} with similar semantics.
* @deprecated in favor of {@link #ACCESS_DECLARED_FIELDS} with similar semantics.
* @see Class#getDeclaredFields()
* @see Field#get(Object)
* @see Field#set(Object, Object)
Expand All @@ -53,20 +53,23 @@ public enum MemberCategory {
DECLARED_FIELDS,

/**
* A category that represents getting/setting values on public {@linkplain Field fields}.
* A category that represents reflective field access on public {@linkplain Field fields}..
* @see Field#get(Object)
* @see Field#set(Object, Object)
* @since 7.0
*/
INVOKE_PUBLIC_FIELDS,
ACCESS_PUBLIC_FIELDS,

/**
* A category that represents getting/setting values on declared {@linkplain Field fields}.
* A category that represents reflective field access on
* {@linkplain Class#getDeclaredFields() declared fields}: all fields defined by the
* class but not inherited fields.
* @see Class#getDeclaredFields()
* @see Field#get(Object)
* @see Field#set(Object, Object)
* @since 7.0
*/
INVOKE_DECLARED_FIELDS,
ACCESS_DECLARED_FIELDS,

/**
* A category that defines public {@linkplain Constructor constructors} can
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,69 +212,55 @@ else if (methods.size() > 1) {
}

/**
* Return a predicate that checks whether a reflection hint is registered for the field that matches the given selector.
* Return a predicate that checks whether a reflective field access hint is registered for the field.
* This looks up a field on the given type with the expected name, if present.
* By default, unsafe or write access is not considered.
* <p>The returned type exposes additional methods that refine the predicate behavior.
* @param type the type holding the field
* @param fieldName the field name
* @return the {@link RuntimeHints} predicate
* @throws IllegalArgumentException if a field cannot be found with the given name.
* @deprecated since 7.0 in favor of {@link #onFieldInvocation(Class, String)}
* or {@link #onType(Class)}.
* @deprecated since 7.0 in favor of {@link #onFieldAccess(Class, String)} with similar semantics.
*/
@Deprecated(since = "7.0", forRemoval = true)
public FieldHintPredicate onField(Class<?> type, String fieldName) {
Assert.notNull(type, "'type' must not be null");
Assert.hasText(fieldName, "'fieldName' must not be empty");
Field field = ReflectionUtils.findField(type, fieldName);
if (field == null) {
throw new IllegalArgumentException("No field named '%s' on class %s".formatted(fieldName, type.getName()));
}
return new FieldHintPredicate(field);
public Predicate<RuntimeHints> onField(Class<?> type, String fieldName) {
return onFieldAccess(type, fieldName);
}

/**
* Return a predicate that checks whether an invocation hint is registered for the field that matches the given selector.
* Return a predicate that checks whether a reflective field access hint is registered for the field.
* This looks up a field on the given type with the expected name, if present.
* @param type the type holding the field
* @param fieldName the field name
* @return the {@link RuntimeHints} predicate
* @throws IllegalArgumentException if a field cannot be found with the given name.
* @since 7.0
*/
public Predicate<RuntimeHints> onFieldInvocation(Class<?> type, String fieldName) {
public Predicate<RuntimeHints> onFieldAccess(Class<?> type, String fieldName) {
Assert.notNull(type, "'type' must not be null");
Assert.hasText(fieldName, "'fieldName' must not be empty");
Field field = ReflectionUtils.findField(type, fieldName);
if (field == null) {
throw new IllegalArgumentException("No field named '%s' on class %s".formatted(fieldName, type.getName()));
}
return new FieldHintPredicate(field).invocation();
return new FieldHintPredicate(field);
}

/**
* Return a predicate that checks whether a reflection hint is registered for the field that matches the given selector.
* Return a predicate that checks whether a reflective field access hint is registered for the field.
* This looks up a field on the given type with the expected name, if present.
* By default, unsafe or write access is not considered.
* <p>The returned type exposes additional methods that refine the predicate behavior.
* @param className the name of the class holding the field
* @param fieldName the field name
* @return the {@link RuntimeHints} predicate
* @throws ClassNotFoundException if the class cannot be resolved.
* @throws IllegalArgumentException if a field cannot be found with the given name.
* @deprecated since 7.0 in favor of {@link #onFieldInvocation(String, String)}
* or {@link #onType(Class)}.
* @deprecated since 7.0 in favor of {@link #onFieldAccess(String, String)} with similar semantics.
*/
@Deprecated(since = "7.0", forRemoval = true)
public FieldHintPredicate onField(String className, String fieldName) throws ClassNotFoundException {
Assert.hasText(className, "'className' must not be empty");
Assert.hasText(fieldName, "'fieldName' must not be empty");
return onField(Class.forName(className), fieldName);
public Predicate<RuntimeHints> onField(String className, String fieldName) throws ClassNotFoundException {
return onFieldAccess(className, fieldName);
}

/**
* Return a predicate that checks whether an invocation hint is registered for the field that matches the given selector.
* Return a predicate that checks whether an invocation hint is registered for the field.
* This looks up a field on the given type with the expected name, if present.
* @param className the name of the class holding the field
* @param fieldName the field name
Expand All @@ -283,23 +269,21 @@ public FieldHintPredicate onField(String className, String fieldName) throws Cla
* @throws IllegalArgumentException if a field cannot be found with the given name.
* @since 7.0
*/
public Predicate<RuntimeHints> onFieldInvocation(String className, String fieldName) throws ClassNotFoundException {
public Predicate<RuntimeHints> onFieldAccess(String className, String fieldName) throws ClassNotFoundException {
Assert.hasText(className, "'className' must not be empty");
Assert.hasText(fieldName, "'fieldName' must not be empty");
return onField(Class.forName(className), fieldName).invocation();
return onFieldAccess(Class.forName(className), fieldName);
}

/**
* Return a predicate that checks whether a reflective field access hint is registered for the given field.
* @param field the field
* @return the {@link RuntimeHints} predicate
* @deprecated since 7.0 in favor of {@link #onFieldInvocation(Field)}
* or {@link #onType(Class)}.
* @deprecated since 7.0 in favor of {@link #onFieldAccess(Field)} with similar semantics.
*/
@Deprecated(since = "7.0", forRemoval = true)
public FieldHintPredicate onField(Field field) {
Assert.notNull(field, "'field' must not be null");
return new FieldHintPredicate(field);
public Predicate<RuntimeHints> onField(Field field) {
return onFieldAccess(field);
}

/**
Expand All @@ -308,9 +292,9 @@ public FieldHintPredicate onField(Field field) {
* @return the {@link RuntimeHints} predicate
* @since 7.0
*/
public Predicate<RuntimeHints> onFieldInvocation(Field field) {
public Predicate<RuntimeHints> onFieldAccess(Field field) {
Assert.notNull(field, "'field' must not be null");
return new FieldHintPredicate(field).invocation();
return new FieldHintPredicate(field);
}


Expand Down Expand Up @@ -494,39 +478,34 @@ public static class FieldHintPredicate implements Predicate<RuntimeHints> {

private final Field field;

private @Nullable ExecutableMode executableMode;

FieldHintPredicate(Field field) {
this.field = field;
}

/**
* Refine the current predicate to only match if an invocation hint is registered for this field.
* @return the refined {@link RuntimeHints} predicate
* @since 7.0
*/
public FieldHintPredicate invocation() {
this.executableMode = ExecutableMode.INVOKE;
return this;
}

@Override
public boolean test(RuntimeHints runtimeHints) {
TypeHint typeHint = runtimeHints.reflection().getTypeHint(this.field.getDeclaringClass());
if (typeHint != null) {
if (this.executableMode == ExecutableMode.INVOKE) {
if (Modifier.isPublic(this.field.getModifiers())) {
return typeHint.getMemberCategories().contains(MemberCategory.INVOKE_PUBLIC_FIELDS);
}
else {
return typeHint.getMemberCategories().contains(MemberCategory.INVOKE_DECLARED_FIELDS);
}
}
else {
return true;
}
if (typeHint == null) {
return false;
}
return memberCategoryMatch(typeHint) || exactMatch(typeHint);
}

@SuppressWarnings("removal")
private boolean memberCategoryMatch(TypeHint typeHint) {
if (Modifier.isPublic(this.field.getModifiers())) {
return typeHint.getMemberCategories().contains(MemberCategory.ACCESS_PUBLIC_FIELDS)
|| typeHint.getMemberCategories().contains(MemberCategory.PUBLIC_FIELDS);
}
else {
return typeHint.getMemberCategories().contains(MemberCategory.ACCESS_DECLARED_FIELDS)
|| typeHint.getMemberCategories().contains(MemberCategory.DECLARED_FIELDS);
}
return false;
}

private boolean exactMatch(TypeHint typeHint) {
return typeHint.fields().anyMatch(fieldHint ->
this.field.getName().equals(fieldHint.getName()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public abstract class ClassHintUtils {
private static final Consumer<TypeHint.Builder> asClassBasedProxy = hint ->
hint.withMembers(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS,
MemberCategory.INVOKE_DECLARED_METHODS,
MemberCategory.INVOKE_DECLARED_FIELDS);
MemberCategory.ACCESS_DECLARED_FIELDS);

private static final Consumer<TypeHint.Builder> asProxiedUserClass = hint ->
hint.withMembers(MemberCategory.INVOKE_PUBLIC_METHODS,
Expand Down
Loading

0 comments on commit f85752a

Please sign in to comment.