Skip to content

Commit be7029c

Browse files
committed
Retain previous factory method in case of nested invocation with AOT
This commit harmonizes the invocation of a bean supplier with what SimpleInstantiationStrategy does. Previously, the current factory method was set to `null` once the invocation completes. This did not take into account recursive scenarios where an instance supplier triggers another instance supplier. For consistency, the thread local is removed now if we attempt to set the current method to null. SimpleInstantiationStrategy itself uses the shortcut to align the code as much as possible. Closes gh-33185
1 parent 84efba6 commit be7029c

File tree

3 files changed

+59
-14
lines changed

3 files changed

+59
-14
lines changed

spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanInstanceSupplier.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -209,12 +209,13 @@ private T invokeBeanSupplier(Executable executable, ThrowingSupplier<T> beanSupp
209209
if (!(executable instanceof Method method)) {
210210
return beanSupplier.get();
211211
}
212+
Method priorInvokedFactoryMethod = SimpleInstantiationStrategy.getCurrentlyInvokedFactoryMethod();
212213
try {
213214
SimpleInstantiationStrategy.setCurrentlyInvokedFactoryMethod(method);
214215
return beanSupplier.get();
215216
}
216217
finally {
217-
SimpleInstantiationStrategy.setCurrentlyInvokedFactoryMethod(null);
218+
SimpleInstantiationStrategy.setCurrentlyInvokedFactoryMethod(priorInvokedFactoryMethod);
218219
}
219220
}
220221

spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java

+12-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -54,12 +54,18 @@ public static Method getCurrentlyInvokedFactoryMethod() {
5454
}
5555

5656
/**
57-
* Set the factory method currently being invoked or {@code null} to reset.
57+
* Set the factory method currently being invoked or {@code null} to remove
58+
* the current value, if any.
5859
* @param method the factory method currently being invoked or {@code null}
5960
* @since 6.0
6061
*/
6162
public static void setCurrentlyInvokedFactoryMethod(@Nullable Method method) {
62-
currentlyInvokedFactoryMethod.set(method);
63+
if (method != null) {
64+
currentlyInvokedFactoryMethod.set(method);
65+
}
66+
else {
67+
currentlyInvokedFactoryMethod.remove();
68+
}
6369
}
6470

6571

@@ -133,22 +139,17 @@ public Object instantiate(RootBeanDefinition bd, @Nullable String beanName, Bean
133139
try {
134140
ReflectionUtils.makeAccessible(factoryMethod);
135141

136-
Method priorInvokedFactoryMethod = currentlyInvokedFactoryMethod.get();
142+
Method priorInvokedFactoryMethod = getCurrentlyInvokedFactoryMethod();
137143
try {
138-
currentlyInvokedFactoryMethod.set(factoryMethod);
144+
setCurrentlyInvokedFactoryMethod(factoryMethod);
139145
Object result = factoryMethod.invoke(factoryBean, args);
140146
if (result == null) {
141147
result = new NullBean();
142148
}
143149
return result;
144150
}
145151
finally {
146-
if (priorInvokedFactoryMethod != null) {
147-
currentlyInvokedFactoryMethod.set(priorInvokedFactoryMethod);
148-
}
149-
else {
150-
currentlyInvokedFactoryMethod.remove();
151-
}
152+
setCurrentlyInvokedFactoryMethod(priorInvokedFactoryMethod);
152153
}
153154
}
154155
catch (IllegalArgumentException ex) {

spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanInstanceSupplierTests.java

+44-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
import java.util.List;
2626
import java.util.Map;
2727
import java.util.Set;
28+
import java.util.concurrent.atomic.AtomicReference;
2829
import java.util.function.Consumer;
2930
import java.util.stream.Stream;
3031

@@ -51,6 +52,7 @@
5152
import org.springframework.beans.factory.support.InstanceSupplier;
5253
import org.springframework.beans.factory.support.RegisteredBean;
5354
import org.springframework.beans.factory.support.RootBeanDefinition;
55+
import org.springframework.beans.factory.support.SimpleInstantiationStrategy;
5456
import org.springframework.core.env.Environment;
5557
import org.springframework.core.io.DefaultResourceLoader;
5658
import org.springframework.core.io.ResourceLoader;
@@ -292,6 +294,33 @@ void getNestedWithNoGeneratorUsesReflection(Source source) throws Exception {
292294
assertThat(instance).isEqualTo("1");
293295
}
294296

297+
@Test // gh-33180
298+
void getWithNestedInvocationRetainsFactoryMethod() throws Exception {
299+
AtomicReference<Method> testMethodReference = new AtomicReference<>();
300+
AtomicReference<Method> anotherMethodReference = new AtomicReference<>();
301+
302+
BeanInstanceSupplier<Object> nestedInstanceSupplier = BeanInstanceSupplier
303+
.forFactoryMethod(AnotherTestStringFactory.class, "another")
304+
.withGenerator(registeredBean -> {
305+
anotherMethodReference.set(SimpleInstantiationStrategy.getCurrentlyInvokedFactoryMethod());
306+
return "Another";
307+
});
308+
RegisteredBean nestedRegisteredBean = new Source(String.class, nestedInstanceSupplier).registerBean(this.beanFactory);
309+
BeanInstanceSupplier<Object> instanceSupplier = BeanInstanceSupplier
310+
.forFactoryMethod(TestStringFactory.class, "test")
311+
.withGenerator(registeredBean -> {
312+
Object nested = nestedInstanceSupplier.get(nestedRegisteredBean);
313+
testMethodReference.set(SimpleInstantiationStrategy.getCurrentlyInvokedFactoryMethod());
314+
return "custom" + nested;
315+
});
316+
RegisteredBean registeredBean = new Source(String.class, instanceSupplier).registerBean(this.beanFactory);
317+
Object value = instanceSupplier.get(registeredBean);
318+
319+
assertThat(value).isEqualTo("customAnother");
320+
assertThat(testMethodReference.get()).isEqualTo(instanceSupplier.getFactoryMethod());
321+
assertThat(anotherMethodReference.get()).isEqualTo(nestedInstanceSupplier.getFactoryMethod());
322+
}
323+
295324
@Test
296325
void resolveArgumentsWithNoArgConstructor() {
297326
RootBeanDefinition beanDefinition = new RootBeanDefinition(
@@ -934,4 +963,18 @@ static class MethodOnInterfaceImpl implements MethodOnInterface {
934963

935964
}
936965

966+
static class TestStringFactory {
967+
968+
String test() {
969+
return "test";
970+
}
971+
}
972+
973+
static class AnotherTestStringFactory {
974+
975+
String another() {
976+
return "another";
977+
}
978+
}
979+
937980
}

0 commit comments

Comments
 (0)