From c3c5eaf9145f99c6de3318b9dc986799a1c6e957 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 11 May 2023 16:00:21 +0200 Subject: [PATCH] Align AOT contributions for injection with the runtime behavior Bean post processors that use InjectionMetadata checks if a property value for the element it is about to inject is set and skip it, so that the property value is used. Previously, the AOT contribution for the same behavior did not check if a matching property value is set and therefore override the user-defined value. This commit introduces an additional method that filters the injected element list so that only the elements that should be processed are defined. Closes gh-30476 --- .../AutowiredAnnotationBeanPostProcessor.java | 9 +++--- .../factory/annotation/InjectionMetadata.java | 26 ++++++++++++++-- ...nBeanRegistrationAotContributionTests.java | 30 +++++++++++++++++-- ...ersistenceAnnotationBeanPostProcessor.java | 3 +- ...BeanPostProcessorAotContributionTests.java | 25 ++++++++++++++-- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java index 5d9ca7a3b30..6163667f9a8 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java @@ -286,7 +286,8 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA String beanName = registeredBean.getBeanName(); RootBeanDefinition beanDefinition = registeredBean.getMergedBeanDefinition(); InjectionMetadata metadata = findInjectionMetadata(beanName, beanClass, beanDefinition); - Collection autowiredElements = getAutowiredElements(metadata); + Collection autowiredElements = getAutowiredElements(metadata, + registeredBean.getMergedBeanDefinition().getPropertyValues()); if (!ObjectUtils.isEmpty(autowiredElements)) { return new AotContribution(beanClass, autowiredElements, getAutowireCandidateResolver()); } @@ -295,8 +296,8 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA @SuppressWarnings({ "rawtypes", "unchecked" }) - private Collection getAutowiredElements(InjectionMetadata metadata) { - return (Collection) metadata.getInjectedElements(); + private Collection getAutowiredElements(InjectionMetadata metadata, PropertyValues propertyValues) { + return (Collection) metadata.getInjectedElements(propertyValues); } @Nullable @@ -752,7 +753,7 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA @Override protected void inject(Object bean, @Nullable String beanName, @Nullable PropertyValues pvs) throws Throwable { - if (checkPropertySkipping(pvs)) { + if (!shouldInject(pvs)) { return; } Method method = (Method) this.member; diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java index dbb2fc8ad03..219891e5452 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java @@ -97,6 +97,19 @@ public class InjectionMetadata { return Collections.unmodifiableCollection(this.injectedElements); } + /** + * Return the {@link InjectedElement elements} to inject based on the + * specified {@link PropertyValues}. If a property is already defined + * for an {@link InjectedElement}, it is excluded. + * @param pvs the property values to consider + * @return the elements to inject + * @since 6.0.10 + */ + public Collection getInjectedElements(@Nullable PropertyValues pvs) { + return this.injectedElements.stream() + .filter(candidate -> candidate.shouldInject(pvs)).toList(); + } + /** * Determine whether this metadata instance needs to be refreshed. * @param clazz the current target class @@ -230,21 +243,28 @@ public class InjectionMetadata { } } + protected boolean shouldInject(@Nullable PropertyValues pvs) { + if (this.isField) { + return true; + } + return !checkPropertySkipping(pvs); + } + /** * Either this or {@link #getResourceToInject} needs to be overridden. */ protected void inject(Object target, @Nullable String requestingBeanName, @Nullable PropertyValues pvs) throws Throwable { + if (!shouldInject(pvs)) { + return; + } if (this.isField) { Field field = (Field) this.member; ReflectionUtils.makeAccessible(field); field.set(target, getResourceToInject(target, requestingBeanName)); } else { - if (checkPropertySkipping(pvs)) { - return; - } try { Method method = (Method) this.member; ReflectionUtils.makeAccessible(method); diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java index 81773b39737..d2c089199a8 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java @@ -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. @@ -65,11 +65,15 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { private final DefaultListableBeanFactory beanFactory; + private final AutowiredAnnotationBeanPostProcessor beanPostProcessor; + AutowiredAnnotationBeanRegistrationAotContributionTests() { this.generationContext = new TestGenerationContext(); this.beanRegistrationCode = new MockBeanRegistrationCode(this.generationContext); this.beanFactory = new DefaultListableBeanFactory(); + this.beanPostProcessor = new AutowiredAnnotationBeanPostProcessor(); + this.beanPostProcessor.setBeanFactory(this.beanFactory); } @@ -185,10 +189,19 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { }); } + @Test + void contributeWhenMethodInjectionHasMatchingPropertyValue() { + RootBeanDefinition beanDefinition = new RootBeanDefinition(InjectionBean.class); + beanDefinition.getPropertyValues().addPropertyValue("counter", 42); + this.beanFactory.registerBeanDefinition("test", beanDefinition); + BeanRegistrationAotContribution contribution = this.beanPostProcessor + .processAheadOfTime(RegisteredBean.of(this.beanFactory, "test")); + assertThat(contribution).isNull(); + } + private RegisteredBean getAndApplyContribution(Class beanClass) { RegisteredBean registeredBean = registerBean(beanClass); - BeanRegistrationAotContribution contribution = new AutowiredAnnotationBeanPostProcessor() - .processAheadOfTime(registeredBean); + BeanRegistrationAotContribution contribution = this.beanPostProcessor.processAheadOfTime(registeredBean); assertThat(contribution).isNotNull(); contribution.applyTo(this.generationContext, this.beanRegistrationCode); return registeredBean; @@ -229,4 +242,15 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { result.accept(compiled.getInstance(BiFunction.class), compiled)); } + static class InjectionBean { + + private Integer counter; + + @Autowired + public void setCounter(Integer counter) { + this.counter = counter; + } + + } + } diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java index 944b5513c7e..4621c933a2d 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java @@ -359,7 +359,8 @@ public class PersistenceAnnotationBeanPostProcessor implements InstantiationAwar String beanName = registeredBean.getBeanName(); RootBeanDefinition beanDefinition = registeredBean.getMergedBeanDefinition(); InjectionMetadata metadata = findInjectionMetadata(beanDefinition, beanClass, beanName); - Collection injectedElements = metadata.getInjectedElements(); + Collection injectedElements = metadata.getInjectedElements( + registeredBean.getMergedBeanDefinition().getPropertyValues()); if (!CollectionUtils.isEmpty(injectedElements)) { return new AotContribution(beanClass, injectedElements); } diff --git a/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessorAotContributionTests.java b/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessorAotContributionTests.java index 8e0dbfe12ab..cb31b8bf014 100644 --- a/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessorAotContributionTests.java +++ b/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessorAotContributionTests.java @@ -43,6 +43,7 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.test.tools.CompileWithForkedClassLoader; import org.springframework.core.test.tools.Compiled; import org.springframework.core.test.tools.TestCompiler; +import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -67,6 +68,20 @@ class PersistenceAnnotationBeanPostProcessorAotContributionTests { this.generationContext = new TestGenerationContext(); } + @Test + void processAheadOfTimeWhenPersistenceUnitOnFieldAndPropertyValueSet() { + RegisteredBean registeredBean = registerBean(DefaultPersistenceUnitField.class); + registeredBean.getMergedBeanDefinition().getPropertyValues().add("emf", "myEntityManagerFactory"); + assertThat(processAheadOfTime(registeredBean)).isNotNull(); // Field not handled by property values + } + + @Test + void processAheadOfTimeWhenPersistenceUnitOnMethodAndPropertyValueSet() { + RegisteredBean registeredBean = registerBean(DefaultPersistenceUnitMethod.class); + registeredBean.getMergedBeanDefinition().getPropertyValues().add("emf", "myEntityManagerFactory"); + assertThat(processAheadOfTime(registeredBean)).isNull(); + } + @Test void processAheadOfTimeWhenPersistenceUnitOnPublicField() { RegisteredBean registeredBean = registerBean(DefaultPersistenceUnitField.class); @@ -192,9 +207,7 @@ class PersistenceAnnotationBeanPostProcessorAotContributionTests { private void testCompile(RegisteredBean registeredBean, BiConsumer, Compiled> result) { - PersistenceAnnotationBeanPostProcessor postProcessor = new PersistenceAnnotationBeanPostProcessor(); - BeanRegistrationAotContribution contribution = postProcessor - .processAheadOfTime(registeredBean); + BeanRegistrationAotContribution contribution = processAheadOfTime(registeredBean); BeanRegistrationCode beanRegistrationCode = mock(); contribution.applyTo(generationContext, beanRegistrationCode); generationContext.writeGeneratedContent(); @@ -202,6 +215,12 @@ class PersistenceAnnotationBeanPostProcessorAotContributionTests { .compile(compiled -> result.accept(new Invoker(compiled), compiled)); } + @Nullable + private BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) { + PersistenceAnnotationBeanPostProcessor postProcessor = new PersistenceAnnotationBeanPostProcessor(); + return postProcessor.processAheadOfTime(registeredBean); + } + static class Invoker implements BiConsumer { private Compiled compiled;