Skip to content

Commit 7985ab3

Browse files
sdeleuzesdeleuze
committed
Throw an exception for suspending factory methods
Suspending factory methods are not supported, and can have side effects, so it is better to fail explicitly for such use case. Closes gh-32719
1 parent a02861f commit 7985ab3

File tree

9 files changed

+200
-9
lines changed

9 files changed

+200
-9
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public CodeBlock generateCode(RegisteredBean registeredBean, InstantiationDescri
142142
if (constructorOrFactoryMethod instanceof Constructor<?> constructor) {
143143
return generateCodeForConstructor(registeredBean, constructor);
144144
}
145-
if (constructorOrFactoryMethod instanceof Method method) {
145+
if (constructorOrFactoryMethod instanceof Method method && !KotlinDetector.isSuspendingFunction(method)) {
146146
return generateCodeForFactoryMethod(registeredBean, method, instantiationDescriptor.targetClass());
147147
}
148148
throw new IllegalStateException(

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.springframework.beans.factory.config.RuntimeBeanReference;
6464
import org.springframework.beans.factory.config.TypedStringValue;
6565
import org.springframework.core.CollectionFactory;
66+
import org.springframework.core.KotlinDetector;
6667
import org.springframework.core.MethodParameter;
6768
import org.springframework.core.NamedThreadLocal;
6869
import org.springframework.core.ParameterNameDiscoverer;
@@ -623,6 +624,11 @@ else if (void.class == factoryMethodToUse.getReturnType()) {
623624
"Invalid factory method '" + mbd.getFactoryMethodName() + "' on class [" +
624625
factoryClass.getName() + "]: needs to have a non-void return type!");
625626
}
627+
else if (KotlinDetector.isKotlinPresent() && KotlinDetector.isSuspendingFunction(factoryMethodToUse)) {
628+
throw new BeanCreationException(mbd.getResourceDescription(), beanName,
629+
"Invalid factory method '" + mbd.getFactoryMethodName() + "' on class [" +
630+
factoryClass.getName() + "]: suspending functions are not supported!");
631+
}
626632
else if (ambiguousFactoryMethods != null) {
627633
throw new BeanCreationException(mbd.getResourceDescription(), beanName,
628634
"Ambiguous factory method matches found on class [" + factoryClass.getName() + "] " +

‎spring-beans/src/test/kotlin/org/springframework/beans/factory/aot/InstanceSupplierCodeGeneratorKotlinTests.kt

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ import org.junit.jupiter.api.Test
2222
import org.springframework.aot.hint.*
2323
import org.springframework.aot.test.generate.TestGenerationContext
2424
import org.springframework.beans.factory.config.BeanDefinition
25-
import org.springframework.beans.factory.support.DefaultListableBeanFactory
26-
import org.springframework.beans.factory.support.InstanceSupplier
27-
import org.springframework.beans.factory.support.RegisteredBean
28-
import org.springframework.beans.factory.support.RootBeanDefinition
25+
import org.springframework.beans.factory.support.*
26+
import org.springframework.beans.testfixture.beans.KotlinConfiguration
2927
import org.springframework.beans.testfixture.beans.KotlinTestBean
3028
import org.springframework.beans.testfixture.beans.KotlinTestBeanWithOptionalParameter
3129
import org.springframework.beans.testfixture.beans.factory.aot.DeferredTypeBuilder
@@ -47,6 +45,8 @@ class InstanceSupplierCodeGeneratorKotlinTests {
4745

4846
private val generationContext = TestGenerationContext()
4947

48+
private val beanFactory = DefaultListableBeanFactory()
49+
5050
@Test
5151
fun generateWhenHasDefaultConstructor() {
5252
val beanDefinition: BeanDefinition = RootBeanDefinition(KotlinTestBean::class.java)
@@ -74,6 +74,39 @@ class InstanceSupplierCodeGeneratorKotlinTests {
7474
.satisfies(hasMemberCategory(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS))
7575
}
7676

77+
@Test
78+
fun generateWhenHasFactoryMethodWithNoArg() {
79+
val beanDefinition = BeanDefinitionBuilder
80+
.rootBeanDefinition(String::class.java)
81+
.setFactoryMethodOnBean("stringBean", "config").beanDefinition
82+
this.beanFactory.registerBeanDefinition("config", BeanDefinitionBuilder
83+
.genericBeanDefinition(KotlinConfiguration::class.java).beanDefinition
84+
)
85+
compile(beanFactory, beanDefinition) { instanceSupplier, compiled ->
86+
val bean = getBean<String>(beanFactory, beanDefinition, instanceSupplier)
87+
Assertions.assertThat(bean).isInstanceOf(String::class.java)
88+
Assertions.assertThat(bean).isEqualTo("Hello")
89+
Assertions.assertThat(compiled.sourceFile).contains(
90+
"getBeanFactory().getBean(KotlinConfiguration.class).stringBean()"
91+
)
92+
}
93+
Assertions.assertThat<TypeHint?>(getReflectionHints().getTypeHint(KotlinConfiguration::class.java))
94+
.satisfies(hasMethodWithMode(ExecutableMode.INTROSPECT))
95+
}
96+
97+
@Test
98+
fun generateWhenHasSuspendingFactoryMethod() {
99+
val beanDefinition = BeanDefinitionBuilder
100+
.rootBeanDefinition(String::class.java)
101+
.setFactoryMethodOnBean("suspendingStringBean", "config").beanDefinition
102+
this.beanFactory.registerBeanDefinition("config", BeanDefinitionBuilder
103+
.genericBeanDefinition(KotlinConfiguration::class.java).beanDefinition
104+
)
105+
Assertions.assertThatIllegalStateException().isThrownBy {
106+
compile(beanFactory, beanDefinition) { _, _ -> }
107+
}
108+
}
109+
77110
private fun getReflectionHints(): ReflectionHints {
78111
return generationContext.runtimeHints.reflection()
79112
}
@@ -96,6 +129,12 @@ class InstanceSupplierCodeGeneratorKotlinTests {
96129
}
97130
}
98131

132+
private fun hasMethodWithMode(mode: ExecutableMode): ThrowingConsumer<TypeHint> {
133+
return ThrowingConsumer { hint: TypeHint ->
134+
Assertions.assertThat(hint.methods()).anySatisfy(hasMode(mode))
135+
}
136+
}
137+
99138
@Suppress("UNCHECKED_CAST")
100139
private fun <T> getBean(beanFactory: DefaultListableBeanFactory, beanDefinition: BeanDefinition,
101140
instanceSupplier: InstanceSupplier<*>): T {

‎spring-beans/src/test/kotlin/org/springframework/beans/factory/support/ConstructorResolverKotlinTests.kt

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.beans.factory.support
18+
19+
import org.assertj.core.api.Assertions
20+
import org.junit.jupiter.api.Test
21+
import org.springframework.beans.BeanWrapper
22+
import org.springframework.beans.factory.BeanCreationException
23+
import org.springframework.beans.factory.config.BeanDefinition
24+
import org.springframework.beans.testfixture.beans.factory.generator.factory.KotlinFactory
25+
26+
class ConstructorResolverKotlinTests {
27+
28+
@Test
29+
fun instantiateBeanInstanceWithBeanClassAndFactoryMethodName() {
30+
val beanFactory = DefaultListableBeanFactory()
31+
val beanDefinition: BeanDefinition = BeanDefinitionBuilder
32+
.rootBeanDefinition(KotlinFactory::class.java).setFactoryMethod("create")
33+
.beanDefinition
34+
val beanWrapper = instantiate(beanFactory, beanDefinition)
35+
Assertions.assertThat(beanWrapper.wrappedInstance).isEqualTo("test")
36+
}
37+
38+
@Test
39+
fun instantiateBeanInstanceWithBeanClassAndSuspendingFactoryMethodName() {
40+
val beanFactory = DefaultListableBeanFactory()
41+
val beanDefinition: BeanDefinition = BeanDefinitionBuilder
42+
.rootBeanDefinition(KotlinFactory::class.java).setFactoryMethod("suspendingCreate")
43+
.beanDefinition
44+
Assertions.assertThatThrownBy { instantiate(beanFactory, beanDefinition, null) }
45+
.isInstanceOf(BeanCreationException::class.java)
46+
.hasMessageContaining("suspending functions are not supported")
47+
48+
}
49+
50+
private fun instantiate(beanFactory: DefaultListableBeanFactory, beanDefinition: BeanDefinition,
51+
vararg explicitArgs: Any?): BeanWrapper {
52+
return ConstructorResolver(beanFactory)
53+
.instantiateUsingFactoryMethod("testBean", (beanDefinition as RootBeanDefinition), explicitArgs)
54+
}
55+
}

‎spring-beans/src/testFixtures/kotlin/org/springframework/beans/testfixture/beans/KotlinConfiguration.kt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.beans.testfixture.beans
18+
19+
class KotlinConfiguration {
20+
21+
fun stringBean(): String {
22+
return "Hello"
23+
}
24+
25+
suspend fun suspendingStringBean(): String {
26+
return "Hello"
27+
}
28+
}

‎spring-beans/src/testFixtures/kotlin/org/springframework/beans/testfixture/beans/factory/generator/factory/KotlinFactory.kt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.beans.testfixture.beans.factory.generator.factory
18+
19+
class KotlinFactory {
20+
21+
companion object {
22+
23+
@JvmStatic
24+
fun create() = "test"
25+
26+
@JvmStatic
27+
suspend fun suspendingCreate() = "test"
28+
}
29+
}

‎spring-core/src/main/java/org/springframework/core/MethodParameter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ public String getParameterName() {
720720
else if (this.executable instanceof Constructor<?> constructor) {
721721
parameterNames = discoverer.getParameterNames(constructor);
722722
}
723-
if (parameterNames != null) {
723+
if (parameterNames != null && this.parameterIndex < parameterNames.length) {
724724
this.parameterName = parameterNames[this.parameterIndex];
725725
}
726726
this.parameterNameDiscoverer = null;

‎spring-core/src/test/kotlin/org/springframework/core/AbstractReflectionParameterNameDiscovererKotlinTests.kt

Lines changed: 14 additions & 1 deletion
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.
@@ -20,6 +20,7 @@ import org.assertj.core.api.Assertions.assertThat
2020
import org.junit.jupiter.api.Test
2121

2222
import org.springframework.util.ReflectionUtils
23+
import kotlin.coroutines.Continuation
2324

2425
/**
2526
* Abstract tests for Kotlin [ParameterNameDiscoverer] aware implementations.
@@ -46,6 +47,14 @@ abstract class AbstractReflectionParameterNameDiscovererKotlinTests(protected va
4647
assertThat(actualMethodParams).contains("message")
4748
}
4849

50+
@Test
51+
fun getParameterNamesOnSuspendingFunction() {
52+
val method = ReflectionUtils.findMethod(CoroutinesMessageService::class.java, "sendMessage",
53+
String::class.java, Continuation::class.java)!!
54+
val actualMethodParams = parameterNameDiscoverer.getParameterNames(method)
55+
assertThat(actualMethodParams).containsExactly("message")
56+
}
57+
4958
@Test
5059
fun getParameterNamesOnExtensionMethod() {
5160
val method = ReflectionUtils.findMethod(UtilityClass::class.java, "identity", String::class.java)!!
@@ -65,4 +74,8 @@ abstract class AbstractReflectionParameterNameDiscovererKotlinTests(protected va
6574
fun String.identity() = this
6675
}
6776

77+
class CoroutinesMessageService {
78+
suspend fun sendMessage(message: String) = message
79+
}
80+
6881
}

‎spring-core/src/test/kotlin/org/springframework/core/MethodParameterKotlinTests.kt

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,27 @@ class MethodParameterKotlinTests {
114114
assertThat(returnGenericParameterType("suspendFun8")).isEqualTo(Object::class.java)
115115
}
116116

117+
@Test
118+
fun `Parameter name for regular function`() {
119+
val methodParameter = returnMethodParameter("nullable", 0)
120+
methodParameter.initParameterNameDiscovery(KotlinReflectionParameterNameDiscoverer())
121+
assertThat(methodParameter.getParameterName()).isEqualTo("nullable")
122+
}
123+
124+
@Test
125+
fun `Parameter name for suspending function`() {
126+
val methodParameter = returnMethodParameter("suspendFun", 0)
127+
methodParameter.initParameterNameDiscovery(KotlinReflectionParameterNameDiscoverer())
128+
assertThat(methodParameter.getParameterName()).isEqualTo("p1")
129+
}
130+
131+
@Test
132+
fun `Continuation parameter name for suspending function`() {
133+
val methodParameter = returnMethodParameter("suspendFun", 1)
134+
methodParameter.initParameterNameDiscovery(KotlinReflectionParameterNameDiscoverer())
135+
assertThat(methodParameter.getParameterName()).isNull()
136+
}
137+
117138
@Test
118139
fun `Continuation parameter is optional`() {
119140
val method = this::class.java.getDeclaredMethod("suspendFun", String::class.java, Continuation::class.java)
@@ -126,8 +147,8 @@ class MethodParameterKotlinTests {
126147
private fun returnGenericParameterTypeName(funName: String) = returnGenericParameterType(funName).typeName
127148
private fun returnGenericParameterTypeBoundName(funName: String) = (returnGenericParameterType(funName) as TypeVariable<*>).bounds[0].typeName
128149

129-
private fun returnMethodParameter(funName: String) =
130-
MethodParameter(this::class.declaredFunctions.first { it.name == funName }.javaMethod!!, -1)
150+
private fun returnMethodParameter(funName: String, parameterIndex: Int = -1) =
151+
MethodParameter(this::class.declaredFunctions.first { it.name == funName }.javaMethod!!, parameterIndex)
131152

132153
@Suppress("unused_parameter")
133154
fun nullable(nullable: String?): Int? = 42

0 commit comments

Comments
 (0)