From 999b5d4462f979b8dda2b0cb975ae939eacd1c61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= <snicoll@vmware.com>
Date: Tue, 24 Oct 2023 13:25:06 +0200
Subject: [PATCH] Better error reporting when instance does not match factory
 method

This commit improves the handling of IllegalArgumentException in
SimpleInstantiationStrategy. Previously, only arguments mismatch were
handled but the exception can also be thrown if the factory instance
does not match the target method.

Closes gh-28897
---
 .../support/SimpleInstantiationStrategy.java  |  7 +++
 .../SimpleInstantiationStrategyTests.java     | 46 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java
index 9efbc3b9b8..b3c509f36d 100644
--- a/spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java
+++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java
@@ -36,6 +36,7 @@ import org.springframework.util.StringUtils;
  *
  * @author Rod Johnson
  * @author Juergen Hoeller
+ * @author Stephane Nicoll
  * @since 1.1
  */
 public class SimpleInstantiationStrategy implements InstantiationStrategy {
@@ -152,6 +153,12 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
 			}
 		}
 		catch (IllegalArgumentException ex) {
+			if (factoryBean != null
+					&& !factoryMethod.getDeclaringClass().isAssignableFrom(factoryBean.getClass())) {
+				throw new BeanInstantiationException(factoryMethod,
+						"Illegal factory instance for factory method '" + factoryMethod.getName() + "'; " +
+						"instance: " + factoryBean.getClass().getName(), ex);
+			}
 			throw new BeanInstantiationException(factoryMethod,
 					"Illegal arguments to factory method '" + factoryMethod.getName() + "'; " +
 					"args: " + StringUtils.arrayToCommaDelimitedString(args), ex);
diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/support/SimpleInstantiationStrategyTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/support/SimpleInstantiationStrategyTests.java
index f0c9683188..439560361c 100644
--- a/spring-beans/src/test/java/org/springframework/beans/factory/support/SimpleInstantiationStrategyTests.java
+++ b/spring-beans/src/test/java/org/springframework/beans/factory/support/SimpleInstantiationStrategyTests.java
@@ -53,6 +53,14 @@ class SimpleInstantiationStrategyTests {
 		assertThat(simpleBean).isEqualTo("Test42");
 	}
 
+	@Test
+	void instantiateWitSubClassFactoryArgs() {
+		RootBeanDefinition bd = new RootBeanDefinition(String.class);
+		Object simpleBean = instantiate(bd, new ExtendedSampleFactory(),
+				method(SampleFactory.class, "beanWithTwoArgs"), "Test", 42);
+		assertThat(simpleBean).isEqualTo("42Test");
+	}
+
 	@Test
 	void instantiateWithNullValueReturnsNullBean() {
 		RootBeanDefinition bd = new RootBeanDefinition(String.class);
@@ -71,6 +79,28 @@ class SimpleInstantiationStrategyTests {
 				.withMessageContaining("args: 42,Test");
 	}
 
+	@Test
+	void instantiateWithTargetTypeMismatch() {
+		RootBeanDefinition bd = new RootBeanDefinition(String.class);
+		assertThatExceptionOfType(BeanInstantiationException.class).isThrownBy(() -> instantiate(
+						bd, new AnotherFactory(),
+						method(SampleFactory.class, "beanWithTwoArgs"), "Test", 42))
+				.withMessageContaining("Illegal factory instance for factory method 'beanWithTwoArgs'")
+				.withMessageContaining("instance: " + AnotherFactory.class.getName())
+				.withMessageNotContaining("args: Test,42");
+	}
+
+	@Test
+	void instantiateWithTargetTypeNotAssignable() {
+		RootBeanDefinition bd = new RootBeanDefinition(String.class);
+		assertThatExceptionOfType(BeanInstantiationException.class).isThrownBy(() -> instantiate(
+						bd, new SampleFactory(),
+						method(ExtendedSampleFactory.class, "beanWithTwoArgs"), "Test", 42))
+				.withMessageContaining("Illegal factory instance for factory method 'beanWithTwoArgs'")
+				.withMessageContaining("instance: " + SampleFactory.class.getName())
+				.withMessageNotContaining("args: Test,42");
+	}
+
 	@Test
 	void instantiateWithException() {
 		RootBeanDefinition bd = new RootBeanDefinition(String.class);
@@ -115,4 +145,20 @@ class SimpleInstantiationStrategyTests {
 		}
 
 	}
+
+	static class ExtendedSampleFactory extends SampleFactory {
+
+		@Override
+		String beanWithTwoArgs(String first, Integer second) {
+			return second + first;
+		}
+	}
+
+	static class AnotherFactory {
+
+		String beanWithTwoArgs(String first, Integer second) {
+			return second + first;
+		}
+
+	}
 }