Skip to content

Commit 6dde13f

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 (cherry picked from commit 0226580)
1 parent c057da2 commit 6dde13f

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
@@ -594,7 +594,7 @@ private void registerDependentBeans(@Nullable String beanName, Set<String> autow
594594
* Resolve the specified cached method argument or field value.
595595
*/
596596
@Nullable
597-
private Object resolvedCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
597+
private Object resolveCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
598598
if (cachedArgument instanceof DependencyDescriptor) {
599599
DependencyDescriptor descriptor = (DependencyDescriptor) cachedArgument;
600600
Assert.state(this.beanFactory != null, "No BeanFactory available");
@@ -629,10 +629,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
629629
Object value;
630630
if (this.cached) {
631631
try {
632-
value = resolvedCachedArgument(beanName, this.cachedFieldValue);
632+
value = resolveCachedArgument(beanName, this.cachedFieldValue);
633633
}
634-
catch (NoSuchBeanDefinitionException ex) {
635-
// Unexpected removal of target bean for cached argument -> re-resolve
634+
catch (BeansException ex) {
635+
// Unexpected target bean mismatch for cached argument -> re-resolve
636+
this.cached = false;
637+
logger.debug("Failed to resolve cached argument", ex);
636638
value = resolveFieldValue(field, bean, beanName);
637639
}
638640
}
@@ -661,9 +663,8 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
661663
}
662664
synchronized (this) {
663665
if (!this.cached) {
664-
Object cachedFieldValue = null;
665666
if (value != null || this.required) {
666-
cachedFieldValue = desc;
667+
Object cachedFieldValue = desc;
667668
registerDependentBeans(beanName, autowiredBeanNames);
668669
if (value != null && autowiredBeanNames.size() == 1) {
669670
String autowiredBeanName = autowiredBeanNames.iterator().next();
@@ -673,9 +674,13 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
673674
desc, autowiredBeanName, field.getType());
674675
}
675676
}
677+
this.cachedFieldValue = cachedFieldValue;
678+
this.cached = true;
679+
}
680+
else {
681+
this.cachedFieldValue = null;
682+
// cached flag remains false
676683
}
677-
this.cachedFieldValue = cachedFieldValue;
678-
this.cached = true;
679684
}
680685
}
681686
return value;
@@ -709,10 +714,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
709714
Object[] arguments;
710715
if (this.cached) {
711716
try {
712-
arguments = resolveCachedArguments(beanName);
717+
arguments = resolveCachedArguments(beanName, this.cachedMethodArguments);
713718
}
714-
catch (NoSuchBeanDefinitionException ex) {
715-
// Unexpected removal of target bean for cached argument -> re-resolve
719+
catch (BeansException ex) {
720+
// Unexpected target bean mismatch for cached argument -> re-resolve
721+
this.cached = false;
722+
logger.debug("Failed to resolve cached argument", ex);
716723
arguments = resolveMethodArguments(method, bean, beanName);
717724
}
718725
}
@@ -731,14 +738,13 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
731738
}
732739

733740
@Nullable
734-
private Object[] resolveCachedArguments(@Nullable String beanName) {
735-
Object[] cachedMethodArguments = this.cachedMethodArguments;
741+
private Object[] resolveCachedArguments(@Nullable String beanName, @Nullable Object[] cachedMethodArguments) {
736742
if (cachedMethodArguments == null) {
737743
return null;
738744
}
739745
Object[] arguments = new Object[cachedMethodArguments.length];
740746
for (int i = 0; i < arguments.length; i++) {
741-
arguments[i] = resolvedCachedArgument(beanName, cachedMethodArguments[i]);
747+
arguments[i] = resolveCachedArgument(beanName, cachedMethodArguments[i]);
742748
}
743749
return arguments;
744750
}
@@ -771,7 +777,7 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
771777
synchronized (this) {
772778
if (!this.cached) {
773779
if (arguments != null) {
774-
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, arguments.length);
780+
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, argumentCount);
775781
registerDependentBeans(beanName, autowiredBeans);
776782
if (autowiredBeans.size() == argumentCount) {
777783
Iterator<String> it = autowiredBeans.iterator();
@@ -786,11 +792,12 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
786792
}
787793
}
788794
this.cachedMethodArguments = cachedMethodArguments;
795+
this.cached = true;
789796
}
790797
else {
791798
this.cachedMethodArguments = null;
799+
// cached flag remains false
792800
}
793-
this.cached = true;
794801
}
795802
}
796803
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
@@ -154,6 +154,59 @@ public void testResourceInjectionWithNullBean() {
154154
assertThat(bean.getTestBean3()).isNull();
155155
}
156156

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

39043957

3958+
public static class SometimesNullFactoryMethods {
3959+
3960+
public static boolean active = false;
3961+
3962+
public static TestBean createTestBean() {
3963+
return (active ? new TestBean() : null);
3964+
}
3965+
3966+
public static NestedTestBean createNestedTestBean() {
3967+
return (active ? new NestedTestBean() : null);
3968+
}
3969+
}
3970+
3971+
39053972
public static class ProvidedArgumentBean {
39063973

39073974
public ProvidedArgumentBean(String[] args) {

0 commit comments

Comments
 (0)