Skip to content

Commit

Permalink
Refresh cached value after unexpected mismatch (e.g. null vs non-null)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jhoeller
jhoeller committed Jul 4, 2023
1 parent 3ef1b7d commit 0226580
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 16 deletions.
39 changes: 23 additions & 16 deletions ...va/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java
View file
Expand Up @@ -638,7 +638,7 @@ private void registerDependentBeans(@Nullable String beanName, Set<String> autow
* Resolve the specified cached method argument or field value.
*/
@Nullable
private Object resolvedCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
private Object resolveCachedArgument(@Nullable String beanName, @Nullable Object cachedArgument) {
if (cachedArgument instanceof DependencyDescriptor descriptor) {
Assert.state(this.beanFactory != null, "No BeanFactory available");
return this.beanFactory.resolveDependency(descriptor, beanName, null, null);
Expand Down Expand Up @@ -683,10 +683,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
Object value;
if (this.cached) {
try {
value = resolvedCachedArgument(beanName, this.cachedFieldValue);
value = resolveCachedArgument(beanName, this.cachedFieldValue);
}
catch (NoSuchBeanDefinitionException ex) {
// Unexpected removal of target bean for cached argument -> re-resolve
catch (BeansException ex) {
// Unexpected target bean mismatch for cached argument -> re-resolve
this.cached = false;
logger.debug("Failed to resolve cached argument", ex);
value = resolveFieldValue(field, bean, beanName);
}
}
Expand Down Expand Up @@ -715,9 +717,8 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
}
synchronized (this) {
if (!this.cached) {
Object cachedFieldValue = null;
if (value != null || this.required) {
cachedFieldValue = desc;
Object cachedFieldValue = desc;
registerDependentBeans(beanName, autowiredBeanNames);
if (value != null && autowiredBeanNames.size() == 1) {
String autowiredBeanName = autowiredBeanNames.iterator().next();
Expand All @@ -727,9 +728,13 @@ private Object resolveFieldValue(Field field, Object bean, @Nullable String bean
desc, autowiredBeanName, field.getType());
}
}
this.cachedFieldValue = cachedFieldValue;
this.cached = true;
}
else {
this.cachedFieldValue = null;
// cached flag remains false
}
this.cachedFieldValue = cachedFieldValue;
this.cached = true;
}
}
return value;
Expand Down Expand Up @@ -760,10 +765,12 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
Object[] arguments;
if (this.cached) {
try {
arguments = resolveCachedArguments(beanName);
arguments = resolveCachedArguments(beanName, this.cachedMethodArguments);
}
catch (NoSuchBeanDefinitionException ex) {
// Unexpected removal of target bean for cached argument -> re-resolve
catch (BeansException ex) {
// Unexpected target bean mismatch for cached argument -> re-resolve
this.cached = false;
logger.debug("Failed to resolve cached argument", ex);
arguments = resolveMethodArguments(method, bean, beanName);
}
}
Expand All @@ -782,14 +789,13 @@ protected void inject(Object bean, @Nullable String beanName, @Nullable Property
}

@Nullable
private Object[] resolveCachedArguments(@Nullable String beanName) {
Object[] cachedMethodArguments = this.cachedMethodArguments;
private Object[] resolveCachedArguments(@Nullable String beanName, @Nullable Object[] cachedMethodArguments) {
if (cachedMethodArguments == null) {
return null;
}
Object[] arguments = new Object[cachedMethodArguments.length];
for (int i = 0; i < arguments.length; i++) {
arguments[i] = resolvedCachedArgument(beanName, cachedMethodArguments[i]);
arguments[i] = resolveCachedArgument(beanName, cachedMethodArguments[i]);
}
return arguments;
}
Expand Down Expand Up @@ -822,7 +828,7 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
synchronized (this) {
if (!this.cached) {
if (arguments != null) {
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, arguments.length);
DependencyDescriptor[] cachedMethodArguments = Arrays.copyOf(descriptors, argumentCount);
registerDependentBeans(beanName, autowiredBeans);
if (autowiredBeans.size() == argumentCount) {
Iterator<String> it = autowiredBeans.iterator();
Expand All @@ -837,11 +843,12 @@ private Object[] resolveMethodArguments(Method method, Object bean, @Nullable St
}
}
this.cachedMethodArguments = cachedMethodArguments;
this.cached = true;
}
else {
this.cachedMethodArguments = null;
// cached flag remains false
}
this.cached = true;
}
}
return arguments;
Expand Down
67 changes: 67 additions & 0 deletions ...g/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java
View file
Expand Up @@ -151,6 +151,59 @@ void resourceInjectionWithNullBean() {
assertThat(bean.getTestBean3()).isNull();
}

@Test
void resourceInjectionWithSometimesNullBean() {
RootBeanDefinition bd = new RootBeanDefinition(OptionalResourceInjectionBean.class);
bd.setScope(BeanDefinition.SCOPE_PROTOTYPE);
bf.registerBeanDefinition("annotatedBean", bd);
RootBeanDefinition tb = new RootBeanDefinition(SometimesNullFactoryMethods.class);
tb.setFactoryMethodName("createTestBean");
tb.setScope(BeanDefinition.SCOPE_PROTOTYPE);
bf.registerBeanDefinition("testBean", tb);

SometimesNullFactoryMethods.active = false;
OptionalResourceInjectionBean bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNull();
assertThat(bean.getTestBean2()).isNull();
assertThat(bean.getTestBean3()).isNull();

SometimesNullFactoryMethods.active = true;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNotNull();
assertThat(bean.getTestBean2()).isNotNull();
assertThat(bean.getTestBean3()).isNotNull();

SometimesNullFactoryMethods.active = false;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNull();
assertThat(bean.getTestBean2()).isNull();
assertThat(bean.getTestBean3()).isNull();

SometimesNullFactoryMethods.active = false;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNull();
assertThat(bean.getTestBean2()).isNull();
assertThat(bean.getTestBean3()).isNull();

SometimesNullFactoryMethods.active = true;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNotNull();
assertThat(bean.getTestBean2()).isNotNull();
assertThat(bean.getTestBean3()).isNotNull();

SometimesNullFactoryMethods.active = true;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNotNull();
assertThat(bean.getTestBean2()).isNotNull();
assertThat(bean.getTestBean3()).isNotNull();

SometimesNullFactoryMethods.active = false;
bean = (OptionalResourceInjectionBean) bf.getBean("annotatedBean");
assertThat(bean.getTestBean()).isNull();
assertThat(bean.getTestBean2()).isNull();
assertThat(bean.getTestBean3()).isNull();
}

@Test
void extendedResourceInjection() {
RootBeanDefinition bd = new RootBeanDefinition(TypedExtendedResourceInjectionBean.class);
Expand Down Expand Up @@ -3881,6 +3934,20 @@ public static NestedTestBean createNestedTestBean() {
}


public static class SometimesNullFactoryMethods {

public static boolean active = false;

public static TestBean createTestBean() {
return (active ? new TestBean() : null);
}

public static NestedTestBean createNestedTestBean() {
return (active ? new NestedTestBean() : null);
}
}


public static class ProvidedArgumentBean {

public ProvidedArgumentBean(String[] args) {
Expand Down

0 comments on commit 0226580

Please sign in to comment.