Avoid implicit autowiring with Kotlin secondary ctors

Autowiring implicitely Kotlin primary constructors
when there are secondary constructors has side effects
on ConstructorResolver. It seems reasonable to
require explicit @Autowired annotation in such case.

With this commit, implicit autowiring of Kotlin
primary constructors is only performed when there
is a primary constructor defined alone or with
a default constructor (define explicitly or
generated via the kotlin-noarg compiler plugin
or via optional constructor parameters with default
values).

Issue: SPR-16022

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

@@ -284,8 +284,12 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean
 					Constructor<?> requiredConstructor = null;
 					Constructor<?> defaultConstructor = null;
 					Constructor<?> primaryConstructor = BeanUtils.findPrimaryConstructor(beanClass);
+					int nonSyntheticConstructors = 0;
 					for (Constructor<?> candidate : rawCandidates) {
-						if (primaryConstructor != null && candidate.isSynthetic()) {
+						if (!candidate.isSynthetic()) {
+							nonSyntheticConstructors++;
+						}
+						else if (primaryConstructor != null) {
 							continue;
 						}
 						AnnotationAttributes ann = findAutowiredAnnotation(candidate);
@@ -343,10 +347,11 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean
 					else if (rawCandidates.length == 1 && rawCandidates[0].getParameterCount() > 0) {
 						candidateConstructors = new Constructor<?>[] {rawCandidates[0]};
 					}
-					else if (primaryConstructor != null) {
+					else if (nonSyntheticConstructors == 2 && primaryConstructor != null && defaultConstructor != null) {
-						candidateConstructors = (defaultConstructor != null ?
+						candidateConstructors = new Constructor<?>[] {primaryConstructor, defaultConstructor};
-								new Constructor<?>[] {primaryConstructor, defaultConstructor} :
+					}
-								new Constructor<?>[] {primaryConstructor});
+					else if (nonSyntheticConstructors == 1 && primaryConstructor != null) {
+						candidateConstructors = new Constructor<?>[] {primaryConstructor};
 					}
 					else {
 						candidateConstructors = new Constructor<?>[0];

spring-beans/src/test/kotlin/org/springframework/beans/factory/annotation/KotlinAutowiredTests.kt

@@ -23,6 +23,7 @@ import org.springframework.beans.factory.support.RootBeanDefinition
 import org.springframework.tests.sample.beans.TestBean
 
 import org.junit.Assert.*
+import org.springframework.beans.factory.BeanCreationException
 import org.springframework.tests.sample.beans.Colour
 
 /**
@@ -68,23 +69,39 @@ class KotlinAutowiredTests {
 	}
 	
 	@Test  // SPR-15847
-	fun `Autowiring by primary constructor with optional parameter`() {
+	fun `Autowiring by primary constructor with mandatory and optional parameters`() {
 		val bf = DefaultListableBeanFactory()
 		val bpp = AutowiredAnnotationBeanPostProcessor()
 		bpp.setBeanFactory(bf)
 		bf.addBeanPostProcessor(bpp)
-		val bd = RootBeanDefinition(KotlinBeanWithOptionalParameter::class.java)
+		val bd = RootBeanDefinition(KotlinBeanWithMandatoryAndOptionalParameters::class.java)
 		bd.scope = RootBeanDefinition.SCOPE_PROTOTYPE
 		bf.registerBeanDefinition("bean", bd)
 		val tb = TestBean()
 		bf.registerSingleton("testBean", tb)
 
-		val kb = bf.getBean("bean", KotlinBeanWithOptionalParameter::class.java)
+		val kb = bf.getBean("bean", KotlinBeanWithMandatoryAndOptionalParameters::class.java)
 		assertSame(tb, kb.injectedFromConstructor)
 		assertEquals("foo", kb.optional)
 		assertEquals("bar", kb.initializedField)
 	}
 
+	@Test
+	fun `Autowiring by primary constructor with optional parameters`() {
+		val bf = DefaultListableBeanFactory()
+		val bpp = AutowiredAnnotationBeanPostProcessor()
+		bpp.setBeanFactory(bf)
+		bf.addBeanPostProcessor(bpp)
+		val bd = RootBeanDefinition(KotlinBeanWithOptionalParameters::class.java)
+		bd.scope = RootBeanDefinition.SCOPE_PROTOTYPE
+		bf.registerBeanDefinition("bean", bd)
+
+		val kb = bf.getBean("bean", KotlinBeanWithOptionalParameters::class.java)
+		assertNotNull(kb.optional1)
+		assertEquals("foo", kb.optional2)
+		assertEquals("bar", kb.initializedField)
+	}
+
 	@Test  // SPR-15847
 	fun `Autowiring by annotated primary constructor with optional parameter`() {
 		val bf = DefaultListableBeanFactory()
@@ -108,7 +125,7 @@ class KotlinAutowiredTests {
 		val bpp = AutowiredAnnotationBeanPostProcessor()
 		bpp.setBeanFactory(bf)
 		bf.addBeanPostProcessor(bpp)
-		val bd = RootBeanDefinition(KotlinBeanWithSecondaryConstructor::class.java)
+		val bd = RootBeanDefinition(KotlinBeanWithAutowiredSecondaryConstructor::class.java)
 		bd.scope = RootBeanDefinition.SCOPE_PROTOTYPE
 		bf.registerBeanDefinition("bean", bd)
 		val tb = TestBean()
@@ -116,7 +133,7 @@ class KotlinAutowiredTests {
 		val colour = Colour.BLUE
 		bf.registerSingleton("colour", colour)
 
-		val kb = bf.getBean("bean", KotlinBeanWithSecondaryConstructor::class.java)
+		val kb = bf.getBean("bean", KotlinBeanWithAutowiredSecondaryConstructor::class.java)
 		assertSame(tb, kb.injectedFromConstructor)
 		assertEquals("bar", kb.optional)
 		assertSame(colour, kb.injectedFromSecondaryConstructor)
@@ -152,6 +169,23 @@ class KotlinAutowiredTests {
 		assertEquals(tb, kb.testBean)
 	}
 
+	@Test(expected = BeanCreationException::class)  // SPR-16022
+	fun `No autowiring with primary and secondary non annotated constructors`() {
+		val bf = DefaultListableBeanFactory()
+		val bpp = AutowiredAnnotationBeanPostProcessor()
+		bpp.setBeanFactory(bf)
+		bf.addBeanPostProcessor(bpp)
+		val bd = RootBeanDefinition(KotlinBeanWithSecondaryConstructor::class.java)
+		bd.scope = RootBeanDefinition.SCOPE_PROTOTYPE
+		bf.registerBeanDefinition("bean", bd)
+		val tb = TestBean()
+		bf.registerSingleton("testBean", tb)
+		val colour = Colour.BLUE
+		bf.registerSingleton("colour", colour)
+
+		bf.getBean("bean", KotlinBeanWithSecondaryConstructor::class.java)
+	}
+
 
 	class KotlinBean(val injectedFromConstructor: TestBean?) {
 		
@@ -166,7 +200,7 @@ class KotlinAutowiredTests {
 		}
 	}
 
-	class KotlinBeanWithOptionalParameter(
+	class KotlinBeanWithMandatoryAndOptionalParameters(
 			val injectedFromConstructor: TestBean,
 			val optional: String = "foo"
 	) {
@@ -177,12 +211,23 @@ class KotlinAutowiredTests {
 		}
 	}
 
+	class KotlinBeanWithOptionalParameters(
+			val optional1: TestBean = TestBean(),
+			val optional2: String = "foo"
+	) {
+		var initializedField: String? = null
+
+		init {
+			initializedField = "bar"
+		}
+	}
+
 	class KotlinBeanWithOptionalParameterAndExplicitConstructor @Autowired constructor(
 			val optional: String = "foo",
 			val injectedFromConstructor: TestBean
 	)
 
-	class KotlinBeanWithSecondaryConstructor(
+	class KotlinBeanWithAutowiredSecondaryConstructor(
 			val optional: String = "foo",
 			val injectedFromConstructor: TestBean
 	) {
@@ -198,4 +243,15 @@ class KotlinAutowiredTests {
 		constructor() : this(TestBean())
 	}
 
+	class KotlinBeanWithSecondaryConstructor(
+			val optional: String = "foo",
+			val injectedFromConstructor: TestBean
+	) {
+		constructor(injectedFromSecondaryConstructor: Colour, injectedFromConstructor: TestBean, optional: String = "bar") : this(optional, injectedFromConstructor) {
+			this.injectedFromSecondaryConstructor = injectedFromSecondaryConstructor
+		}
+
+		var injectedFromSecondaryConstructor: Colour? = null
+	}
+
 }

spring-context/src/test/kotlin/org/springframework/context/annotation/Spr16022Tests.kt

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2002-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.context.annotation
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.springframework.beans.factory.BeanFactory
+import org.springframework.beans.factory.getBean
+import org.springframework.context.support.ClassPathXmlApplicationContext
+
+/**
+ * @author Sebastien Deleuze
+ */
+class Spr16022Tests {
+
+	@Test
+	fun `Register beans with multiple constructors with AnnotationConfigApplicationContext`() {
+		assert(AnnotationConfigApplicationContext(Config::class.java))
+	}
+
+	@Test
+	fun `Register beans with multiple constructors with ClassPathXmlApplicationContext`() {
+		assert(ClassPathXmlApplicationContext(CONTEXT))
+	}
+
+	private fun assert(context: BeanFactory) {
+		val bean1 = context.getBean<MultipleConstructorsTestBean>("bean1")
+		assertEquals(0, bean1.foo)
+		val bean2 = context.getBean<MultipleConstructorsTestBean>("bean2")
+		assertEquals(1, bean2.foo)
+		val bean3 = context.getBean<MultipleConstructorsTestBean>("bean3")
+		assertEquals(3, bean3.foo)
+
+	}
+
+	@Suppress("unused")
+	class MultipleConstructorsTestBean(val foo: Int) {
+		constructor(bar: String) : this(bar.length)
+		constructor(foo: Int, bar: String) : this(foo + bar.length)
+	}
+
+	@Configuration @ImportResource(CONTEXT)
+	open class Config
+}
+
+private const val CONTEXT = "org/springframework/context/annotation/multipleConstructors.xml"

spring-context/src/test/resources/org/springframework/context/annotation/multipleConstructors.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	   xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">
+
+	<bean class="org.springframework.context.annotation.Spr16022Tests.MultipleConstructorsTestBean" name="bean1">
+		<constructor-arg value="0" type="int" />
+	</bean>
+
+	<bean class="org.springframework.context.annotation.Spr16022Tests.MultipleConstructorsTestBean" name="bean2">
+		<constructor-arg value="a" type="java.lang.String" />
+	</bean>
+
+	<bean class="org.springframework.context.annotation.Spr16022Tests.MultipleConstructorsTestBean" name="bean3">
+		<constructor-arg value="2" type="int" />
+		<constructor-arg value="b" type="java.lang.String" />
+	</bean>
+
+</beans>