Skip to content

Commit 0226580

Browse files
committed
Refresh cached value after unexpected mismatch (e.g. null vs non-null)
In addition to the previously addressed removal of bean definitions, this is able to deal with prototype factory methods returning non-null after null or also null after non-null. Stale cached values are getting refreshed rather than bypassed. Closes gh-30794
1 parent 3ef1b7d commit 0226580

File tree

2 files changed

+90
-16
lines changed

2 files changed

+90
-16
lines changed

spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ private void registerDependentBeans(@Nullable String beanName, Set<String> autow
638638
* Resolve the specified cached method argument or field value.
639639
*/
640640
@Nullable
641-
private Object resolvedCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
641+
private Object resolveCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
642642
if (cachedArgument instanceof DependencyDescriptor descriptor) {
643643
Assert.state(this.beanFactory != null, "No BeanFactory available");
644644
return this.beanFactory.resolveDependency(descriptor, beanName, null, null);
@@ -683,10 +683,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
683683
Object value;
684684
if (this.cached) {
685685
try {
686-
value = resolvedCachedArgument(beanName, this.cachedFieldValue);
686+
value = resolveCachedArgument(beanName, this.cachedFieldValue);
687687
}
688-
catch (NoSuchBeanDefinitionException ex) {
689-
// Unexpected removal of target bean for cached argument -> re-resolve
688+
catch (BeansException ex) {
689+
// Unexpected target bean mismatch for cached argument -> re-resolve
690+
this.cached = false;
691+
logger.debug("Failed to resolve cached argument", ex);
690692
value = resolveFieldValue(field, bean, beanName);
691693
}
692694
}
@@ -715,9 +717,8 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
715717
}
716718
synchronized (this) {
717719
if (!this.cached) {
718-
Object cachedFieldValue = null;
719720
if (value != null || this.required) {
720-
cachedFieldValue = desc;
721+
Object cachedFieldValue = desc;
721722
registerDependentBeans(beanName, autowiredBeanNames);
722723
if (value != null && autowiredBeanNames.size() == 1) {
723724
String autowiredBeanName = autowiredBeanNames.iterator().next();
@@ -727,9 +728,13 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
727728
desc, autowiredBeanName, field.getType());
728729
}
729730
}
731+
this.cachedFieldValue = cachedFieldValue;
732+
this.cached = true;
733+
}
734+
else {
735+
this.cachedFieldValue = null;
736+
// cached flag remains false
730737
}
731-
this.cachedFieldValue = cachedFieldValue;
732-
this.cached = true;
733738
}
734739
}
735740
return value;
@@ -760,10 +765,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
760765
Object[] arguments;
761766
if (this.cached) {
762767
try {
763-
arguments = resolveCachedArguments(beanName);
768+
arguments = resolveCachedArguments(beanName, this.cachedMethodArguments);
764769
}
765-
catch (NoSuchBeanDefinitionException ex) {
766-
// Unexpected removal of target bean for cached argument -> re-resolve
770+
catch (BeansException ex) {
771+
// Unexpected target bean mismatch for cached argument -> re-resolve
772+
this.cached = false;
773+
logger.debug("Failed to resolve cached argument", ex);
767774
arguments = resolveMethodArguments(method, bean, beanName);
768775
}
769776
}
@@ -782,14 +789,13 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
782789
}
783790

784791
@Nullable
785-
private Object[] resolveCachedArguments(@Nullable String beanName) {
786-
Object[] cachedMethodArguments = this.cachedMethodArguments;
792+
private Object[] resolveCachedArguments(@Nullable String beanName, @Nullable Object[] cachedMethodArguments) {
787793
if (cachedMethodArguments == null) {
788794
return null;
789795
}
790796
Object[] arguments = new Object[cachedMethodArguments.length];
791797
for (int i = 0; i < arguments.length; i++) {
792-
arguments[i] = resolvedCachedArgument(beanName, cachedMethodArguments[i]);
798+
arguments[i] = resolveCachedArgument(beanName, cachedMethodArguments[i]);
793799
}
794800
return arguments;
795801
}
@@ -822,7 +828,7 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
822828
synchronized (this) {
823829
if (!this.cached) {
824830
if (arguments != null) {
825-
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, arguments.length);
831+
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, argumentCount);
826832
registerDependentBeans(beanName, autowiredBeans);
827833
if (autowiredBeans.size() == argumentCount) {
828834
Iterator<String> it = autowiredBeans.iterator();
@@ -837,11 +843,12 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
837843
}
838844
}
839845
this.cachedMethodArguments = cachedMethodArguments;
846+
this.cached = true;
840847
}
841848
else {
842849
this.cachedMethodArguments = null;
850+
// cached flag remains false
843851
}
844-
this.cached = true;
845852
}
846853
}
847854
return arguments;

spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,59 @@ void resourceInjectionWithNullBean() {
151151
assertThat(bean.getTestBean3()).isNull();
152152
}
153153

154+
@Test
155+
void resourceInjectionWithSometimesNullBean() {
156+
RootBeanDefinition bd = new RootBeanDefinition(OptionalResourceInjectionBean.class);
157+
bd.setScope(BeanDefinition.SCOPE_PROTOTYPE);
158+
bf.registerBeanDefinition("annotatedBean", bd);
159+
RootBeanDefinition tb = new RootBeanDefinition(SometimesNullFactoryMethods.class);
160+
tb.setFactoryMethodName("createTestBean");
161+
tb.setScope(BeanDefinition.SCOPE_PROTOTYPE);
162+
bf.registerBeanDefinition("testBean", tb);
163+
164+
SometimesNullFactoryMethods.active = false;
165+
OptionalResourceInjectionBean bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
166+
assertThat(bean.getTestBean()).isNull();
167+
assertThat(bean.getTestBean2()).isNull();
168+
assertThat(bean.getTestBean3()).isNull();
169+
170+
SometimesNullFactoryMethods.active = true;
171+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
172+
assertThat(bean.getTestBean()).isNotNull();
173+
assertThat(bean.getTestBean2()).isNotNull();
174+
assertThat(bean.getTestBean3()).isNotNull();
175+
176+
SometimesNullFactoryMethods.active = false;
177+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
178+
assertThat(bean.getTestBean()).isNull();
179+
assertThat(bean.getTestBean2()).isNull();
180+
assertThat(bean.getTestBean3()).isNull();
181+
182+
SometimesNullFactoryMethods.active = false;
183+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
184+
assertThat(bean.getTestBean()).isNull();
185+
assertThat(bean.getTestBean2()).isNull();
186+
assertThat(bean.getTestBean3()).isNull();
187+
188+
SometimesNullFactoryMethods.active = true;
189+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
190+
assertThat(bean.getTestBean()).isNotNull();
191+
assertThat(bean.getTestBean2()).isNotNull();
192+
assertThat(bean.getTestBean3()).isNotNull();
193+
194+
SometimesNullFactoryMethods.active = true;
195+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
196+
assertThat(bean.getTestBean()).isNotNull();
197+
assertThat(bean.getTestBean2()).isNotNull();
198+
assertThat(bean.getTestBean3()).isNotNull();
199+
200+
SometimesNullFactoryMethods.active = false;
201+
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
202+
assertThat(bean.getTestBean()).isNull();
203+
assertThat(bean.getTestBean2()).isNull();
204+
assertThat(bean.getTestBean3()).isNull();
205+
}
206+
154207
@Test
155208
void extendedResourceInjection() {
156209
RootBeanDefinition bd = new RootBeanDefinition(TypedExtendedResourceInjectionBean.class);
@@ -3881,6 +3934,20 @@ public static NestedTestBean createNestedTestBean() {
38813934
}
38823935

38833936

3937+
public static class SometimesNullFactoryMethods {
3938+
3939+
public static boolean active = false;
3940+
3941+
public static TestBean createTestBean() {
3942+
return (active ? new TestBean() : null);
3943+
}
3944+
3945+
public static NestedTestBean createNestedTestBean() {
3946+
return (active ? new NestedTestBean() : null);
3947+
}
3948+
}
3949+
3950+
38843951
public static class ProvidedArgumentBean {
38853952

38863953
public ProvidedArgumentBean(String[] args) {

0 commit comments

Comments
 (0)