Skip to content

Commit

Permalink
Add AOT support for generic constructor argument values
Browse files Browse the repository at this point in the history 
This commit improves compatibility with the core container when running
in AOT mode by adding support for generic constructor argument values.

Previously, these were ignored altogether. We now have code generation
support for them as well as resolution that is similar to what
AbstractAutowiredCapableBeanFactory does in a regular runtime.

Closes spring-projectsgh-31420
  • Loading branch information
@snicoll
snicoll committed Oct 12, 2023
1 parent 1e7fc60 commit 4b5d038
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 41 deletions.
32 changes: 28 additions & 4 deletions ...ain/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand All @@ -43,6 +44,7 @@
import org.springframework.beans.factory.FactoryBean;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.beans.factory.config.ConstructorArgumentValues;
import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder;
import org.springframework.beans.factory.support.AbstractBeanDefinition;
import org.springframework.beans.factory.support.AutowireCandidateQualifier;
Expand Down Expand Up @@ -168,16 +170,38 @@ private void addInitDestroyHint(Class<?> beanUserClass, String methodName) {
}

private void addConstructorArgumentValues(CodeBlock.Builder code, BeanDefinition beanDefinition) {
Map<Integer, ValueHolder> argumentValues =
beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues();
if (!argumentValues.isEmpty()) {
argumentValues.forEach((index, valueHolder) -> {
ConstructorArgumentValues constructorValues = beanDefinition.getConstructorArgumentValues();
Map<Integer, ValueHolder> indexedValues = constructorValues.getIndexedArgumentValues();
if (!indexedValues.isEmpty()) {
indexedValues.forEach((index, valueHolder) -> {
CodeBlock valueCode = generateValue(valueHolder.getName(), valueHolder.getValue());
code.addStatement(
"$L.getConstructorArgumentValues().addIndexedArgumentValue($L, $L)",
BEAN_DEFINITION_VARIABLE, index, valueCode);
});
}
List<ValueHolder> genericValues = constructorValues.getGenericArgumentValues();
if (!genericValues.isEmpty()) {
genericValues.forEach(valueHolder -> {
String valueName = valueHolder.getName();
CodeBlock valueCode = generateValue(valueName, valueHolder.getValue());
if (valueName != null) {
CodeBlock valueTypeCode = this.valueCodeGenerator.generateCode(valueHolder.getType());
code.addStatement(
"$L.getConstructorArgumentValues().addGenericArgumentValue(new $T($L, $L, $S))",
BEAN_DEFINITION_VARIABLE, ValueHolder.class, valueCode, valueTypeCode, valueName);
}
else if (valueHolder.getType() != null) {
code.addStatement("$L.getConstructorArgumentValues().addGenericArgumentValue($L, $S)",
BEAN_DEFINITION_VARIABLE, valueCode, valueHolder.getType());

}
else {
code.addStatement("$L.getConstructorArgumentValues().addGenericArgumentValue($L)",
BEAN_DEFINITION_VARIABLE, valueCode);
}
});
}
}

private void addPropertyValues(CodeBlock.Builder code, RootBeanDefinition beanDefinition) {
Expand Down
27 changes: 19 additions & 8 deletions spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanInstanceSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import java.lang.reflect.Executable;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Parameter;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -248,7 +250,7 @@ private AutowiredArguments resolveArguments(RegisteredBean registeredBean, Execu
Assert.isTrue(this.shortcuts == null || this.shortcuts.length == resolved.length,
() -> "'shortcuts' must contain " + resolved.length + " elements");

ConstructorArgumentValues argumentValues = resolveArgumentValues(registeredBean);
ValueHolder[] argumentValues = resolveArgumentValues(registeredBean, executable);
Set<String> autowiredBeanNames = new LinkedHashSet<>(resolved.length * 2);
for (int i = startIndex; i < parameterCount; i++) {
MethodParameter parameter = getMethodParameter(executable, i);
Expand All @@ -257,7 +259,7 @@ private AutowiredArguments resolveArguments(RegisteredBean registeredBean, Execu
if (shortcut != null) {
descriptor = new ShortcutDependencyDescriptor(descriptor, shortcut);
}
ValueHolder argumentValue = argumentValues.getIndexedArgumentValue(i, null);
ValueHolder argumentValue = argumentValues[i];
resolved[i - startIndex] = resolveArgument(registeredBean, descriptor, argumentValue, autowiredBeanNames);
}
registerDependentBeans(registeredBean.getBeanFactory(), registeredBean.getBeanName(), autowiredBeanNames);
Expand All @@ -275,18 +277,27 @@ private MethodParameter getMethodParameter(Executable executable, int index) {
throw new IllegalStateException("Unsupported executable: " + executable.getClass().getName());
}

private ConstructorArgumentValues resolveArgumentValues(RegisteredBean registeredBean) {
ConstructorArgumentValues resolved = new ConstructorArgumentValues();
private ValueHolder[] resolveArgumentValues(RegisteredBean registeredBean, Executable executable) {
Parameter[] parameters = executable.getParameters();
ValueHolder[] resolved = new ValueHolder[parameters.length];
RootBeanDefinition beanDefinition = registeredBean.getMergedBeanDefinition();
if (beanDefinition.hasConstructorArgumentValues() &&
registeredBean.getBeanFactory() instanceof AbstractAutowireCapableBeanFactory beanFactory) {
BeanDefinitionValueResolver valueResolver = new BeanDefinitionValueResolver(
beanFactory, registeredBean.getBeanName(), beanDefinition, beanFactory.getTypeConverter());
ConstructorArgumentValues values = beanDefinition.getConstructorArgumentValues();
values.getIndexedArgumentValues().forEach((index, valueHolder) -> {
ValueHolder resolvedValue = resolveArgumentValue(valueResolver, valueHolder);
resolved.addIndexedArgumentValue(index, resolvedValue);
});
Set<ValueHolder> usedValueHolders = new HashSet<>(parameters.length);
for (int i = 0; i < parameters.length; i++) {
Class<?> parameterType = parameters[i].getType();
String parameterName = (parameters[i].isNamePresent() ? parameters[i].getName() : null);
ValueHolder valueHolder = values.getArgumentValue(
i, parameterType, parameterName, usedValueHolders);
if (valueHolder != null) {
ValueHolder resolvedValue = resolveArgumentValue(valueResolver, valueHolder);
resolved[i] = resolvedValue;
usedValueHolders.add(valueHolder);
}
}
}
return resolved;
}
Expand Down
27 changes: 26 additions & 1 deletion ...ava/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void setRoleWhenOther() {
}

@Test
void constructorArgumentValuesWhenValues() {
void constructorArgumentValuesWhenIndexedValues() {
this.beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(0, String.class);
this.beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1, "test");
this.beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(2, 123);
Expand All @@ -231,6 +231,31 @@ void constructorArgumentValuesWhenValues() {
});
}

@Test
void constructorArgumentValuesWhenGenericValuesWithName() {
this.beanDefinition.getConstructorArgumentValues().addGenericArgumentValue(String.class);
this.beanDefinition.getConstructorArgumentValues().addGenericArgumentValue(2, Long.class.getName());
this.beanDefinition.getConstructorArgumentValues().addGenericArgumentValue(
new ValueHolder("value", null, "param1"));
this.beanDefinition.getConstructorArgumentValues().addGenericArgumentValue
(new ValueHolder("another", CharSequence.class.getName(), "param2"));
compile((actual, compiled) -> {
List<ValueHolder> values = actual.getConstructorArgumentValues().getGenericArgumentValues();
assertThat(values.get(0)).satisfies(assertValueHolder(String.class, null, null));
assertThat(values.get(1)).satisfies(assertValueHolder(2, Long.class, null));
assertThat(values.get(2)).satisfies(assertValueHolder("value", null, "param1"));
assertThat(values.get(3)).satisfies(assertValueHolder("another", CharSequence.class, "param2"));
});
}

private Consumer<ValueHolder> assertValueHolder(Object value, @Nullable Class<?> type, @Nullable String name) {
return valueHolder -> {
assertThat(valueHolder.getValue()).isEqualTo(value);
assertThat(valueHolder.getType()).isEqualTo((type != null ? type.getName() : null));
assertThat(valueHolder.getName()).isEqualTo(name);
};
}

@Test
void propertyValuesWhenValues() {
this.beanDefinition.setTargetType(PropertyValuesBean.class);
Expand Down
62 changes: 57 additions & 5 deletions ...-beans/src/test/java/org/springframework/beans/factory/aot/BeanInstanceSupplierTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ void resolveArgumentsWithMixedArgsConstructorWithUserBeanReference(Source source
}

@Test
void resolveArgumentsWithUserValueWithTypeConversionRequired() {
void resolveIndexedArgumentsWithUserValueWithTypeConversionRequired() {
Source source = new Source(CharDependency.class,
BeanInstanceSupplier.forConstructor(char.class));
RegisteredBean registerBean = source.registerBean(this.beanFactory,
Expand All @@ -503,8 +503,24 @@ void resolveArgumentsWithUserValueWithTypeConversionRequired() {
assertThat(arguments.getObject(0)).isInstanceOf(Character.class).isEqualTo('\\');
}

@Test
void resolveGenericArgumentsWithUserValueWithTypeConversionRequired() {
Source source = new Source(CharDependency.class,
BeanInstanceSupplier.forConstructor(char.class));
RegisteredBean registerBean = source.registerBean(this.beanFactory,
beanDefinition -> {
beanDefinition
.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_CONSTRUCTOR);
beanDefinition.getConstructorArgumentValues()
.addGenericArgumentValue("\\", char.class.getName());
});
AutowiredArguments arguments = source.getResolver().resolveArguments(registerBean);
assertThat(arguments.toArray()).hasSize(1);
assertThat(arguments.getObject(0)).isInstanceOf(Character.class).isEqualTo('\\');
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveArgumentsWithUserValueWithBeanReference(Source source) {
void resolveIndexedArgumentsWithUserValueWithBeanReference(Source source) {
this.beanFactory.registerSingleton("stringBean", "string");
RegisteredBean registerBean = source.registerBean(this.beanFactory,
beanDefinition -> beanDefinition.getConstructorArgumentValues()
Expand All @@ -516,7 +532,18 @@ void resolveArgumentsWithUserValueWithBeanReference(Source source) {
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveArgumentsWithUserValueWithBeanDefinition(Source source) {
void resolveGenericArgumentsWithUserValueWithBeanReference(Source source) {
this.beanFactory.registerSingleton("stringBean", "string");
RegisteredBean registerBean = source.registerBean(this.beanFactory,
beanDefinition -> beanDefinition.getConstructorArgumentValues()
.addGenericArgumentValue(new RuntimeBeanReference("stringBean")));
AutowiredArguments arguments = source.getResolver().resolveArguments(registerBean);
assertThat(arguments.toArray()).hasSize(1);
assertThat(arguments.getObject(0)).isEqualTo("string");
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveIndexedArgumentsWithUserValueWithBeanDefinition(Source source) {
AbstractBeanDefinition userValue = BeanDefinitionBuilder
.rootBeanDefinition(String.class, () -> "string").getBeanDefinition();
RegisteredBean registerBean = source.registerBean(this.beanFactory,
Expand All @@ -528,11 +555,23 @@ void resolveArgumentsWithUserValueWithBeanDefinition(Source source) {
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveArgumentsWithUserValueThatIsAlreadyResolved(Source source) {
void resolveGenericArgumentsWithUserValueWithBeanDefinition(Source source) {
AbstractBeanDefinition userValue = BeanDefinitionBuilder
.rootBeanDefinition(String.class, () -> "string").getBeanDefinition();
RegisteredBean registerBean = source.registerBean(this.beanFactory,
beanDefinition -> beanDefinition.getConstructorArgumentValues()
.addGenericArgumentValue(userValue));
AutowiredArguments arguments = source.getResolver().resolveArguments(registerBean);
assertThat(arguments.toArray()).hasSize(1);
assertThat(arguments.getObject(0)).isEqualTo("string");
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveIndexedArgumentsWithUserValueThatIsAlreadyResolved(Source source) {
RegisteredBean registerBean = source.registerBean(this.beanFactory);
BeanDefinition mergedBeanDefinition = this.beanFactory
.getMergedBeanDefinition("testBean");
ValueHolder valueHolder = new ValueHolder('a');
ValueHolder valueHolder = new ValueHolder("a");
valueHolder.setConvertedValue("this is an a");
mergedBeanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(0,
valueHolder);
Expand All @@ -541,6 +580,19 @@ void resolveArgumentsWithUserValueThatIsAlreadyResolved(Source source) {
assertThat(arguments.getObject(0)).isEqualTo("this is an a");
}

@ParameterizedResolverTest(Sources.SINGLE_ARG)
void resolveGenericArgumentsWithUserValueThatIsAlreadyResolved(Source source) {
RegisteredBean registerBean = source.registerBean(this.beanFactory);
BeanDefinition mergedBeanDefinition = this.beanFactory
.getMergedBeanDefinition("testBean");
ValueHolder valueHolder = new ValueHolder("a");
valueHolder.setConvertedValue("this is an a");
mergedBeanDefinition.getConstructorArgumentValues().addGenericArgumentValue(valueHolder);
AutowiredArguments arguments = source.getResolver().resolveArguments(registerBean);
assertThat(arguments.toArray()).hasSize(1);
assertThat(arguments.getObject(0)).isEqualTo("this is an a");
}

@Test
void resolveArgumentsWhenUsingShortcutsInjectsDirectly() {
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory() {
Expand Down
35 changes: 17 additions & 18 deletions .../src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,26 +479,25 @@ private void postProcessRootBeanDefinition(List<MergedBeanDefinitionPostProcesso
BeanDefinitionValueResolver valueResolver = new BeanDefinitionValueResolver(this.beanFactory, beanName, bd);
postProcessors.forEach(postProcessor -> postProcessor.postProcessMergedBeanDefinition(bd, beanType, beanName));
for (PropertyValue propertyValue : bd.getPropertyValues().getPropertyValueList()) {
Object value = propertyValue.getValue();
if (value instanceof AbstractBeanDefinition innerBd) {
Class<?> innerBeanType = resolveBeanType(innerBd);
resolveInnerBeanDefinition(valueResolver, innerBd, (innerBeanName, innerBeanDefinition)
-> postProcessRootBeanDefinition(postProcessors, innerBeanName, innerBeanType, innerBeanDefinition));
}
if (value instanceof TypedStringValue typedStringValue) {
resolveTypeStringValue(typedStringValue);
}
postProcessValue(postProcessors, valueResolver, propertyValue.getValue());
}
for (ValueHolder valueHolder : bd.getConstructorArgumentValues().getIndexedArgumentValues().values()) {
Object value = valueHolder.getValue();
if (value instanceof AbstractBeanDefinition innerBd) {
Class<?> innerBeanType = resolveBeanType(innerBd);
resolveInnerBeanDefinition(valueResolver, innerBd, (innerBeanName, innerBeanDefinition)
-> postProcessRootBeanDefinition(postProcessors, innerBeanName, innerBeanType, innerBeanDefinition));
}
if (value instanceof TypedStringValue typedStringValue) {
resolveTypeStringValue(typedStringValue);
}
postProcessValue(postProcessors, valueResolver, valueHolder.getValue());
}
for (ValueHolder valueHolder : bd.getConstructorArgumentValues().getGenericArgumentValues()) {
postProcessValue(postProcessors, valueResolver, valueHolder.getValue());
}
}

private void postProcessValue(List<MergedBeanDefinitionPostProcessor> postProcessors,
BeanDefinitionValueResolver valueResolver, @Nullable Object value) {
if (value instanceof AbstractBeanDefinition innerBd) {
Class<?> innerBeanType = resolveBeanType(innerBd);
resolveInnerBeanDefinition(valueResolver, innerBd, (innerBeanName, innerBeanDefinition)
-> postProcessRootBeanDefinition(postProcessors, innerBeanName, innerBeanType, innerBeanDefinition));
}
if (value instanceof TypedStringValue typedStringValue) {
resolveTypeStringValue(typedStringValue);
}
}

Expand Down
4 changes: 3 additions & 1 deletion ...xt/src/test/java/org/springframework/context/aot/ApplicationContextAotGeneratorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,10 @@ void processAheadOfTimeWhenHasTypedStringValue() {
assertThat(employee.getName()).isEqualTo("John Smith");
assertThat(employee.getAge()).isEqualTo(42);
assertThat(employee.getCompany()).isEqualTo("Acme Widgets, Inc.");
assertThat(freshApplicationContext.getBean("pet", Pet.class)
assertThat(freshApplicationContext.getBean("petIndexed", Pet.class)
.getName()).isEqualTo("Fido");
assertThat(freshApplicationContext.getBean("petGeneric", Pet.class)
.getName()).isEqualTo("Dofi");
});
}

Expand Down

0 comments on commit 4b5d038

Please sign in to comment.