From 6d688e196d8f4c41284087e5346da29770349b0c Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 18 Oct 2022 12:50:54 +0200 Subject: [PATCH] Restore support of list of inner bean definitions This commit restores the support of multiple bean definitions being specified in a `List` as a property value or constructor argument. Rather than handling inner bean definitions externally, there are now supported by BeanDefinitionPropertiesCodeGenerator, and list of such type is handled transparently. Closes gh-29075 --- .../aot/BeanDefinitionMethodGenerator.java | 12 ++--- .../BeanDefinitionMethodGeneratorFactory.java | 29 ++++++++--- ...BeanDefinitionPropertiesCodeGenerator.java | 52 +++++++++++++------ ...nDefinitionPropertyValueCodeGenerator.java | 42 +++++++++------ .../aot/BeanRegistrationsAotProcessor.java | 2 +- ...DefinitionMethodGeneratorFactoryTests.java | 17 +++--- .../BeanDefinitionMethodGeneratorTests.java | 32 ++++++++++++ ...nitionPropertyValueCodeGeneratorTests.java | 10 ++-- 8 files changed, 136 insertions(+), 60 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGenerator.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGenerator.java index a6c518e120..4ef2debf2d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGenerator.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGenerator.java @@ -57,7 +57,7 @@ class BeanDefinitionMethodGenerator { private final Executable constructorOrFactoryMethod; @Nullable - private final String innerBeanPropertyName; + private final String currentPropertyName; private final List aotContributions; @@ -66,18 +66,18 @@ class BeanDefinitionMethodGenerator { * Create a new {@link BeanDefinitionMethodGenerator} instance. * @param methodGeneratorFactory the method generator factory * @param registeredBean the registered bean - * @param innerBeanPropertyName the inner bean property name + * @param currentPropertyName the current property name * @param aotContributions the AOT contributions */ BeanDefinitionMethodGenerator( BeanDefinitionMethodGeneratorFactory methodGeneratorFactory, - RegisteredBean registeredBean, @Nullable String innerBeanPropertyName, + RegisteredBean registeredBean, @Nullable String currentPropertyName, List aotContributions) { this.methodGeneratorFactory = methodGeneratorFactory; this.registeredBean = registeredBean; this.constructorOrFactoryMethod = registeredBean.resolveConstructorOrFactoryMethod(); - this.innerBeanPropertyName = innerBeanPropertyName; + this.currentPropertyName = currentPropertyName; this.aotContributions = aotContributions; } @@ -188,8 +188,8 @@ class BeanDefinitionMethodGenerator { } private String getName() { - if (this.innerBeanPropertyName != null) { - return this.innerBeanPropertyName; + if (this.currentPropertyName != null) { + return this.currentPropertyName; } if (!this.registeredBean.isGeneratedBeanName()) { return getSimpleBeanName(this.registeredBean.getBeanName()); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactory.java index ba2e8dfcfc..73f34e71ab 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactory.java @@ -35,9 +35,10 @@ import org.springframework.util.ObjectUtils; * {@link RegisteredBean}. * * @author Phillip Webb + * @author Stephane Nicoll * @since 6.0 * @see BeanDefinitionMethodGenerator - * @see #getBeanDefinitionMethodGenerator(RegisteredBean, String) + * @see #getBeanDefinitionMethodGenerator(RegisteredBean) */ class BeanDefinitionMethodGeneratorFactory { @@ -79,18 +80,19 @@ class BeanDefinitionMethodGeneratorFactory { /** * Return a {@link BeanDefinitionMethodGenerator} for the given - * {@link RegisteredBean} or {@code null} if the registered bean is excluded - * by a {@link BeanRegistrationExcludeFilter}. The resulting + * {@link RegisteredBean} defined with the specified property name, or + * {@code null} if the registered bean is excluded by a + * {@link BeanRegistrationExcludeFilter}. The resulting * {@link BeanDefinitionMethodGenerator} will include all * {@link BeanRegistrationAotProcessor} provided contributions. * @param registeredBean the registered bean - * @param innerBeanPropertyName the inner bean property name or {@code null} + * @param currentPropertyName the property name that this bean belongs to * @return a new {@link BeanDefinitionMethodGenerator} instance or * {@code null} */ @Nullable BeanDefinitionMethodGenerator getBeanDefinitionMethodGenerator( - RegisteredBean registeredBean, @Nullable String innerBeanPropertyName) { + RegisteredBean registeredBean, @Nullable String currentPropertyName) { if (isExcluded(registeredBean)) { return null; @@ -98,7 +100,22 @@ class BeanDefinitionMethodGeneratorFactory { List contributions = getAotContributions( registeredBean); return new BeanDefinitionMethodGenerator(this, registeredBean, - innerBeanPropertyName, contributions); + currentPropertyName, contributions); + } + + /** + * Return a {@link BeanDefinitionMethodGenerator} for the given + * {@link RegisteredBean} or {@code null} if the registered bean is excluded + * by a {@link BeanRegistrationExcludeFilter}. The resulting + * {@link BeanDefinitionMethodGenerator} will include all + * {@link BeanRegistrationAotProcessor} provided contributions. + * @param registeredBean the registered bean + * @return a new {@link BeanDefinitionMethodGenerator} instance or + * {@code null} + */ + @Nullable + BeanDefinitionMethodGenerator getBeanDefinitionMethodGenerator(RegisteredBean registeredBean) { + return getBeanDefinitionMethodGenerator(registeredBean, null); } private boolean isExcluded(RegisteredBean registeredBean) { diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java index ec13bb3a53..f4dc48e2ed 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java @@ -18,6 +18,7 @@ package org.springframework.beans.factory.aot; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -81,8 +82,6 @@ class BeanDefinitionPropertiesCodeGenerator { private final Predicate attributeFilter; - private final BiFunction customValueCodeGenerator; - private final BeanDefinitionPropertyValueCodeGenerator valueCodeGenerator; @@ -92,9 +91,8 @@ class BeanDefinitionPropertiesCodeGenerator { this.hints = hints; this.attributeFilter = attributeFilter; - this.customValueCodeGenerator = customValueCodeGenerator; - this.valueCodeGenerator = new BeanDefinitionPropertyValueCodeGenerator( - generatedMethods); + this.valueCodeGenerator = new BeanDefinitionPropertyValueCodeGenerator(generatedMethods, + (object, type) -> customValueCodeGenerator.apply(PropertyNamesStack.peek(), object)); } @@ -150,12 +148,7 @@ class BeanDefinitionPropertiesCodeGenerator { .getConstructorArgumentValues().getIndexedArgumentValues(); if (!argumentValues.isEmpty()) { argumentValues.forEach((index, valueHolder) -> { - String name = valueHolder.getName(); - Object value = valueHolder.getValue(); - CodeBlock valueCode = this.customValueCodeGenerator.apply(name, value); - if (valueCode == null) { - valueCode = this.valueCodeGenerator.generateCode(value); - } + CodeBlock valueCode = generateValue(valueHolder.getName(), valueHolder.getValue()); code.addStatement( "$L.getConstructorArgumentValues().addIndexedArgumentValue($L, $L)", BEAN_DEFINITION_VARIABLE, index, valueCode); @@ -170,11 +163,7 @@ class BeanDefinitionPropertiesCodeGenerator { if (!propertyValues.isEmpty()) { for (PropertyValue propertyValue : propertyValues) { String name = propertyValue.getName(); - Object value = propertyValue.getValue(); - CodeBlock valueCode = this.customValueCodeGenerator.apply(name, value); - if (valueCode == null) { - valueCode = this.valueCodeGenerator.generateCode(value); - } + CodeBlock valueCode = generateValue(name, propertyValue.getValue()); code.addStatement("$L.getPropertyValues().addPropertyValue($S, $L)", BEAN_DEFINITION_VARIABLE, propertyValue.getName(), valueCode); } @@ -191,6 +180,16 @@ class BeanDefinitionPropertiesCodeGenerator { } } + private CodeBlock generateValue(@Nullable String name, @Nullable Object value) { + try { + PropertyNamesStack.push(name); + return this.valueCodeGenerator.generateCode(value); + } + finally { + PropertyNamesStack.pop(); + } + } + private Class getInfrastructureType(RootBeanDefinition beanDefinition) { if (beanDefinition.hasBeanClass()) { Class beanClass = beanDefinition.getBeanClass(); @@ -282,4 +281,25 @@ class BeanDefinitionPropertiesCodeGenerator { } } + static class PropertyNamesStack { + + private static final ThreadLocal> threadLocal = ThreadLocal.withInitial(ArrayDeque::new); + + static void push(@Nullable String name) { + String valueToSet = (name != null) ? name : ""; + threadLocal.get().push(valueToSet); + } + + static void pop() { + threadLocal.get().pop(); + } + + @Nullable + static String peek() { + String value = threadLocal.get().peek(); + return ("".equals(value) ? null : value); + } + + } + } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGenerator.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGenerator.java index 234938d599..f291ffd187 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGenerator.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGenerator.java @@ -17,6 +17,7 @@ package org.springframework.beans.factory.aot; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -29,6 +30,7 @@ import java.util.Map.Entry; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; +import java.util.function.BiFunction; import java.util.stream.Stream; import org.springframework.aot.generate.GeneratedMethod; @@ -61,26 +63,32 @@ class BeanDefinitionPropertyValueCodeGenerator { private final GeneratedMethods generatedMethods; - private final List delegates = List.of( - new PrimitiveDelegate(), - new StringDelegate(), - new CharsetDelegate(), - new EnumDelegate(), - new ClassDelegate(), - new ResolvableTypeDelegate(), - new ArrayDelegate(), - new ManagedListDelegate(), - new ManagedSetDelegate(), - new ManagedMapDelegate(), - new ListDelegate(), - new SetDelegate(), - new MapDelegate(), - new BeanReferenceDelegate() - ); + private final List delegates; - BeanDefinitionPropertyValueCodeGenerator(GeneratedMethods generatedMethods) { + BeanDefinitionPropertyValueCodeGenerator(GeneratedMethods generatedMethods, + @Nullable BiFunction customValueGenerator) { this.generatedMethods = generatedMethods; + this.delegates = new ArrayList<>(); + if (customValueGenerator != null) { + this.delegates.add(customValueGenerator::apply); + } + this.delegates.addAll(List.of( + new PrimitiveDelegate(), + new StringDelegate(), + new CharsetDelegate(), + new EnumDelegate(), + new ClassDelegate(), + new ResolvableTypeDelegate(), + new ArrayDelegate(), + new ManagedListDelegate(), + new ManagedSetDelegate(), + new ManagedMapDelegate(), + new ListDelegate(), + new SetDelegate(), + new MapDelegate(), + new BeanReferenceDelegate() + )); } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanRegistrationsAotProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanRegistrationsAotProcessor.java index 9388d79919..189d50e75d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanRegistrationsAotProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanRegistrationsAotProcessor.java @@ -39,7 +39,7 @@ class BeanRegistrationsAotProcessor implements BeanFactoryInitializationAotProce for (String beanName : beanFactory.getBeanDefinitionNames()) { RegisteredBean registeredBean = RegisteredBean.of(beanFactory, beanName); BeanDefinitionMethodGenerator beanDefinitionMethodGenerator = beanDefinitionMethodGeneratorFactory - .getBeanDefinitionMethodGenerator(registeredBean, null); + .getBeanDefinitionMethodGenerator(registeredBean); if (beanDefinitionMethodGenerator != null) { registrations.put(beanName, beanDefinitionMethodGenerator); } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactoryTests.java index 8c0ce75413..82e4a1184d 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorFactoryTests.java @@ -66,8 +66,7 @@ class BeanDefinitionMethodGeneratorFactoryTests { RegisteredBean registeredBean = registerTestBean(beanFactory); BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean, - null)).isNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean)).isNull(); } @Test @@ -79,8 +78,7 @@ class BeanDefinitionMethodGeneratorFactoryTests { new MockBeanRegistrationExcludeFilter(true, 0)); BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean, - null)).isNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean)).isNull(); } @Test @@ -100,8 +98,7 @@ class BeanDefinitionMethodGeneratorFactoryTests { RegisteredBean registeredBean = registerTestBean(beanFactory); BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean, - null)).isNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean)).isNull(); assertThat(filter1.wasCalled()).isTrue(); assertThat(filter2.wasCalled()).isTrue(); assertThat(filter3.wasCalled()).isTrue(); @@ -127,7 +124,7 @@ class BeanDefinitionMethodGeneratorFactoryTests { BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); BeanDefinitionMethodGenerator methodGenerator = methodGeneratorFactory - .getBeanDefinitionMethodGenerator(registeredBean, null); + .getBeanDefinitionMethodGenerator(registeredBean); assertThat(methodGenerator).extracting("aotContributions").asList() .containsExactly(beanContribution, loaderContribution); } @@ -144,8 +141,8 @@ class BeanDefinitionMethodGeneratorFactoryTests { RegisteredBean registeredBean2 = RegisteredBean.of(beanFactory, "test2"); BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean1, null)).isNull(); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean2, null)).isNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean1)).isNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean2)).isNull(); } @Test @@ -157,7 +154,7 @@ class BeanDefinitionMethodGeneratorFactoryTests { RegisteredBean registeredBean1 = RegisteredBean.of(beanFactory, "test"); BeanDefinitionMethodGeneratorFactory methodGeneratorFactory = new BeanDefinitionMethodGeneratorFactory( AotServices.factoriesAndBeans(springFactoriesLoader, beanFactory)); - assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean1, null)).isNotNull(); + assertThat(methodGeneratorFactory.getBeanDefinitionMethodGenerator(registeredBean1)).isNotNull(); } private RegisteredBean registerTestBean(DefaultListableBeanFactory beanFactory) { diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorTests.java index 1715fc740a..d90f7534e3 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorTests.java @@ -38,6 +38,7 @@ import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueH import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.InstanceSupplier; +import org.springframework.beans.factory.support.ManagedList; import org.springframework.beans.factory.support.RegisteredBean; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.testfixture.beans.AnnotatedBean; @@ -380,6 +381,37 @@ class BeanDefinitionMethodGeneratorTests { }); } + @SuppressWarnings("unchecked") + @Test + void generateBeanDefinitionMethodWhenHasListOfInnerBeansPropertyValueGeneratesMethod() { + RootBeanDefinition firstInnerBeanDefinition = (RootBeanDefinition) BeanDefinitionBuilder + .rootBeanDefinition(TestBean.class).addPropertyValue("name", "one") + .getBeanDefinition(); + RootBeanDefinition secondInnerBeanDefinition = (RootBeanDefinition) BeanDefinitionBuilder + .rootBeanDefinition(TestBean.class).addPropertyValue("name", "two") + .getBeanDefinition(); + ManagedList list = new ManagedList<>(); + list.add(firstInnerBeanDefinition); + list.add(secondInnerBeanDefinition); + RootBeanDefinition beanDefinition = new RootBeanDefinition(TestBean.class); + beanDefinition.getPropertyValues().add("someList", list); + RegisteredBean registeredBean = registerBean(beanDefinition); + BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator( + this.methodGeneratorFactory, registeredBean, null, + Collections.emptyList()); + MethodReference method = generator.generateBeanDefinitionMethod( + this.generationContext, this.beanRegistrationsCode); + compile(method, (actual, compiled) -> { + ManagedList actualPropertyValue = (ManagedList) actual + .getPropertyValues().get("someList"); + assertThat(actualPropertyValue).isNotNull().hasSize(2); + assertThat(actualPropertyValue.get(0).getPropertyValues().get("name")).isEqualTo("one"); + assertThat(actualPropertyValue.get(1).getPropertyValues().get("name")).isEqualTo("two"); + assertThat(compiled.getSourceFileFromPackage(TestBean.class.getPackageName())) + .contains("getSomeListBeanDefinition()", "getSomeListBeanDefinition1()"); + }); + } + @Test void generateBeanDefinitionMethodWhenHasInnerBeanConstructorValueGeneratesMethod() { RootBeanDefinition innerBeanDefinition = (RootBeanDefinition) BeanDefinitionBuilder diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGeneratorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGeneratorTests.java index 31140cad15..ca3c512841 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGeneratorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertyValueCodeGeneratorTests.java @@ -64,12 +64,15 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException */ class BeanDefinitionPropertyValueCodeGeneratorTests { + private static BeanDefinitionPropertyValueCodeGenerator createPropertyValuesCodeGenerator(GeneratedClass generatedClass) { + return new BeanDefinitionPropertyValueCodeGenerator(generatedClass.getMethods(), null); + } + private void compile(Object value, BiConsumer result) { TestGenerationContext generationContext = new TestGenerationContext(); DeferredTypeBuilder typeBuilder = new DeferredTypeBuilder(); GeneratedClass generatedClass = generationContext.getGeneratedClasses().addForFeature("TestCode", typeBuilder); - CodeBlock generatedCode = new BeanDefinitionPropertyValueCodeGenerator( - generatedClass.getMethods()).generateCode(value); + CodeBlock generatedCode = createPropertyValuesCodeGenerator(generatedClass).generateCode(value); typeBuilder.set(type -> { type.addModifiers(Modifier.PUBLIC); type.addSuperinterface( @@ -544,8 +547,7 @@ class BeanDefinitionPropertyValueCodeGeneratorTests { TestGenerationContext context = new TestGenerationContext(); GeneratedClass generatedClass = context.getGeneratedClasses() .addForFeature("Test", type -> {}); - new BeanDefinitionPropertyValueCodeGenerator(generatedClass.getMethods()) - .generateCode(value); + createPropertyValuesCodeGenerator(generatedClass).generateCode(value); } record SampleValue(String name) {}