Polish ProtectedAccess options

This commit improves how protected access analysis operates. Rather than
providing a static boolean, a function callback for the member to
analyse is used. This permits to change the decision whether reflection
can be used, or if the return type is assigned.

Both of those are already applicable, with InjectionGenerator relying
on reflection for private fields, and DefaultBeanInstanceGenerator
assigning the bean instance if additional contributors are present.

This commit also moves the logic of computing the options where the code
is actually generated.

See gh-28030
This commit is contained in:
Stephane Nicoll
2022-02-15 13:15:20 +01:00
parent 4782d4c080
commit 5bbc7dbce2
5 changed files with 243 additions and 106 deletions

View File

@@ -39,19 +39,21 @@ import org.springframework.util.ClassUtils;
*/
class DefaultBeanInstanceGenerator {
private static final Options BEAN_INSTANCE_OPTIONS = new Options(false, true);
private final Executable instanceCreator;
private final List<BeanInstanceContributor> contributors;
private final InjectionGenerator injectionGenerator;
private final Options beanInstanceOptions;
DefaultBeanInstanceGenerator(Executable instanceCreator, List<BeanInstanceContributor> contributors) {
this.instanceCreator = instanceCreator;
this.contributors = List.copyOf(contributors);
this.injectionGenerator = new InjectionGenerator();
this.beanInstanceOptions = Options.defaults().useReflection(member -> false)
.assignReturnType(member -> !this.contributors.isEmpty()).build();
}
/**
@@ -62,7 +64,7 @@ class DefaultBeanInstanceGenerator {
*/
public CodeContribution generateBeanInstance(RuntimeHints runtimeHints) {
DefaultCodeContribution contribution = new DefaultCodeContribution(runtimeHints);
contribution.protectedAccess().analyze(this.instanceCreator, BEAN_INSTANCE_OPTIONS);
contribution.protectedAccess().analyze(this.instanceCreator, this.beanInstanceOptions);
if (this.instanceCreator instanceof Constructor<?> constructor) {
writeBeanInstantiation(contribution, constructor);
}

View File

@@ -28,6 +28,8 @@ import java.util.Arrays;
import java.util.List;
import java.util.function.Consumer;
import org.springframework.aot.generator.ProtectedAccess;
import org.springframework.aot.generator.ProtectedAccess.Options;
import org.springframework.beans.factory.generator.config.BeanDefinitionRegistrar.BeanInstanceContext;
import org.springframework.javapoet.CodeBlock;
import org.springframework.javapoet.CodeBlock.Builder;
@@ -51,6 +53,12 @@ import org.springframework.util.ReflectionUtils;
*/
public class InjectionGenerator {
private static final Options FIELD_INJECTION_OPTIONS = Options.defaults()
.useReflection(member -> Modifier.isPrivate(member.getModifiers())).build();
private static final Options METHOD_INJECTION_OPTIONS = Options.defaults()
.useReflection(member -> false).build();
private final BeanParameterGenerator parameterGenerator = new BeanParameterGenerator();
@@ -77,7 +85,8 @@ public class InjectionGenerator {
* in the specified {@link Member}.
* @param member the field or method to inject
* @param required whether the value is required
* @return a statement that injects a value to the specified membmer
* @return a statement that injects a value to the specified member
* @see #getProtectedAccessInjectionOptions(Member)
*/
public CodeBlock writeInjection(Member member, boolean required) {
if (member instanceof Method method) {
@@ -89,6 +98,23 @@ public class InjectionGenerator {
throw new IllegalArgumentException("Could not handle member " + member);
}
/**
* Return the {@link Options} to use if protected access analysis is
* required for the specified {@link Member}.
* @param member the field or method to handle
* @return the options to use to analyse protected access
* @see ProtectedAccess
*/
public Options getProtectedAccessInjectionOptions(Member member) {
if (member instanceof Method) {
return METHOD_INJECTION_OPTIONS;
}
if (member instanceof Field) {
return FIELD_INJECTION_OPTIONS;
}
throw new IllegalArgumentException("Could not handle member " + member);
}
private CodeBlock write(Constructor<?> creator) {
Builder code = CodeBlock.builder();
Class<?> declaringType = ClassUtils.getUserClass(creator.getDeclaringClass());

View File

@@ -24,6 +24,8 @@ import java.lang.reflect.Method;
import org.junit.jupiter.api.Test;
import org.springframework.aot.generator.ProtectedAccess;
import org.springframework.aot.generator.ProtectedAccess.Options;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.generator.InjectionGeneratorTests.SimpleConstructorBean.InnerClass;
import org.springframework.javapoet.support.CodeSnippet;
@@ -40,6 +42,8 @@ import static org.mockito.Mockito.mock;
*/
class InjectionGeneratorTests {
private final ProtectedAccess protectedAccess = new ProtectedAccess();
@Test
void writeInstantiationForConstructorWithNoArgUseShortcut() {
Constructor<?> constructor = SimpleBean.class.getDeclaredConstructors()[0];
@@ -162,6 +166,42 @@ class InjectionGeneratorTests {
})""");
}
@Test
void getProtectedAccessInjectionOptionsForUnsupportedMember() {
assertThatIllegalArgumentException().isThrownBy(() ->
getProtectedAccessInjectionOptions(mock(Member.class)));
}
@Test
void getProtectedAccessInjectionOptionsForPackagePublicField() {
analyzeProtectedAccess(field(SampleBean.class, "enabled"));
assertThat(this.protectedAccess.isAccessible("com.example")).isTrue();
}
@Test
void getProtectedAccessInjectionOptionsForPackageProtectedField() {
analyzeProtectedAccess(field(SampleBean.class, "counter"));
assertPrivilegedAccess(SampleBean.class);
}
@Test
void getProtectedAccessInjectionOptionsForPrivateField() {
analyzeProtectedAccess(field(SampleBean.class, "source"));
assertThat(this.protectedAccess.isAccessible("com.example")).isTrue();
}
@Test
void getProtectedAccessInjectionOptionsForPublicMethod() {
analyzeProtectedAccess(method(SampleBean.class, "setEnabled", Boolean.class));
assertThat(this.protectedAccess.isAccessible("com.example")).isTrue();
}
@Test
void getProtectedAccessInjectionOptionsForPackageProtectedMethod() {
analyzeProtectedAccess(method(SampleBean.class, "sourceAndCounter", String.class, Integer.class));
assertPrivilegedAccess(SampleBean.class);
}
private Method method(Class<?> type, String name, Class<?>... parameterTypes) {
Method method = ReflectionUtils.findMethod(type, name, parameterTypes);
@@ -183,14 +223,34 @@ class InjectionGeneratorTests {
return CodeSnippet.process(code -> code.add(new InjectionGenerator().writeInjection(member, required)));
}
private void analyzeProtectedAccess(Member member) {
this.protectedAccess.analyze(member, getProtectedAccessInjectionOptions(member));
}
private Options getProtectedAccessInjectionOptions(Member member) {
return new InjectionGenerator().getProtectedAccessInjectionOptions(member);
}
private void assertPrivilegedAccess(Class<?> target) {
assertThat(this.protectedAccess.isAccessible("com.example")).isFalse();
assertThat(this.protectedAccess.getPrivilegedPackageName("com.example")).isEqualTo(target.getPackageName());
assertThat(this.protectedAccess.isAccessible(target.getPackageName())).isTrue();
}
@SuppressWarnings("unused")
static class SampleBean {
public static class SampleBean {
public Boolean enabled;
private String source;
Integer counter;
public void setEnabled(Boolean enabled) {
}
void sourceAndCounter(String source, Integer counter) {
}