Pass pre-determined merged bean definition into InstanceSupplier (for inner beans)

Replaces useless protected obtainFromSupplier method with obtainInstanceFromSupplier.
Moves InstanceSupplier handling to appropriate subclass (DefaultListableBeanFactory).
BeanInstanceSupplier throws BeanInstantiationException instead of BeanCreationException.

Closes gh-29803

Author: jhoeller

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

@@ -27,9 +27,9 @@
 import java.util.stream.Collectors;
 
 import org.springframework.aot.hint.ExecutableMode;
+import org.springframework.beans.BeanInstantiationException;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.TypeConverter;
-import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.InjectionPoint;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -97,21 +97,21 @@ public final class BeanInstanceSupplier<T> extends AutowiredElementResolver impl
 	private BeanInstanceSupplier(ExecutableLookup lookup,
 			@Nullable ThrowingBiFunction<RegisteredBean, AutowiredArguments, T> generator,
 			@Nullable String[] shortcuts) {
+
 		this.lookup = lookup;
 		this.generator = generator;
 		this.shortcuts = shortcuts;
 	}
 
+
 	/**
 	 * Create a {@link BeanInstanceSupplier} that resolves
 	 * arguments for the specified bean constructor.
 	 * @param <T> the type of instance supplied
 	 * @param parameterTypes the constructor parameter types
 	 * @return a new {@link BeanInstanceSupplier} instance
 	 */
-	public static <T> BeanInstanceSupplier<T> forConstructor(
-			Class<?>... parameterTypes) {
-
+	public static <T> BeanInstanceSupplier<T> forConstructor(Class<?>... parameterTypes) {
 		Assert.notNull(parameterTypes, "'parameterTypes' must not be null");
 		Assert.noNullElements(parameterTypes, "'parameterTypes' must not contain null elements");
 		return new BeanInstanceSupplier<>(new ConstructorLookup(parameterTypes), null, null);
@@ -149,11 +149,11 @@ ExecutableLookup getLookup() {
 	 * @param generator a {@link ThrowingBiFunction} that uses the
 	 * {@link RegisteredBean} and resolved {@link AutowiredArguments} to
 	 * instantiate the underlying bean
-	 * @return a new {@link BeanInstanceSupplier} instance with the specified
-	 * generator
+	 * @return a new {@link BeanInstanceSupplier} instance with the specified generator
 	 */
 	public BeanInstanceSupplier<T> withGenerator(
 			ThrowingBiFunction<RegisteredBean, AutowiredArguments, T> generator) {
+
 		Assert.notNull(generator, "'generator' must not be null");
 		return new BeanInstanceSupplier<>(this.lookup, generator, this.shortcuts);
 	}
@@ -163,11 +163,9 @@ public BeanInstanceSupplier<T> withGenerator(
 	 * {@code generator} function to instantiate the underlying bean.
 	 * @param generator a {@link ThrowingFunction} that uses the
 	 * {@link RegisteredBean} to instantiate the underlying bean
-	 * @return a new {@link BeanInstanceSupplier} instance with the specified
-	 * generator
+	 * @return a new {@link BeanInstanceSupplier} instance with the specified generator
 	 */
-	public BeanInstanceSupplier<T> withGenerator(
-			ThrowingFunction<RegisteredBean, T> generator) {
+	public BeanInstanceSupplier<T> withGenerator(ThrowingFunction<RegisteredBean, T> generator) {
 		Assert.notNull(generator, "'generator' must not be null");
 		return new BeanInstanceSupplier<>(this.lookup,
 				(registeredBean, args) -> generator.apply(registeredBean), this.shortcuts);
@@ -176,10 +174,8 @@ public BeanInstanceSupplier<T> withGenerator(
 	/**
 	 * Return a new {@link BeanInstanceSupplier} instance that uses the specified
 	 * {@code generator} supplier to instantiate the underlying bean.
-	 * @param generator a {@link ThrowingSupplier} to instantiate the underlying
-	 * bean
-	 * @return a new {@link BeanInstanceSupplier} instance with the specified
-	 * generator
+	 * @param generator a {@link ThrowingSupplier} to instantiate the underlying bean
+	 * @return a new {@link BeanInstanceSupplier} instance with the specified generator
 	 */
 	public BeanInstanceSupplier<T> withGenerator(ThrowingSupplier<T> generator) {
 		Assert.notNull(generator, "'generator' must not be null");
@@ -282,8 +278,7 @@ private MethodParameter getMethodParameter(Executable executable, int index) {
 		if (executable instanceof Method method) {
 			return new MethodParameter(method, index);
 		}
-		throw new IllegalStateException(
-				"Unsupported executable " + executable.getClass().getName());
+		throw new IllegalStateException("Unsupported executable: " + executable.getClass().getName());
 	}
 
 	private ConstructorArgumentValues resolveArgumentValues(
@@ -303,9 +298,7 @@ private ConstructorArgumentValues resolveArgumentValues(
 		return resolved;
 	}
 
-	private ValueHolder resolveArgumentValue(BeanDefinitionValueResolver resolver,
-			ValueHolder valueHolder) {
-
+	private ValueHolder resolveArgumentValue(BeanDefinitionValueResolver resolver, ValueHolder valueHolder) {
 		if (valueHolder.isConverted()) {
 			return valueHolder;
 		}
@@ -331,8 +324,7 @@ private Object resolveArgument(AbstractAutowireCapableBeanFactory beanFactory,
 		}
 		try {
 			try {
-				return beanFactory.resolveDependency(dependencyDescriptor, beanName,
-						autowiredBeans, typeConverter);
+				return beanFactory.resolveDependency(dependencyDescriptor, beanName, autowiredBeans, typeConverter);
 			}
 			catch (NoSuchBeanDefinitionException ex) {
 				if (parameterType.isArray()) {
@@ -348,47 +340,45 @@ private Object resolveArgument(AbstractAutowireCapableBeanFactory beanFactory,
 			}
 		}
 		catch (BeansException ex) {
-			throw new UnsatisfiedDependencyException(null, beanName,
-					new InjectionPoint(parameter), ex);
+			throw new UnsatisfiedDependencyException(null, beanName, new InjectionPoint(parameter), ex);
 		}
 	}
 
 	@SuppressWarnings("unchecked")
-	private T instantiate(ConfigurableBeanFactory beanFactory, Executable executable,
-			Object[] arguments) {
-
-		try {
-			if (executable instanceof Constructor<?> constructor) {
-				return (T) instantiate(constructor, arguments);
+	private T instantiate(ConfigurableBeanFactory beanFactory, Executable executable, Object[] args) {
+		if (executable instanceof Constructor<?> constructor) {
+			try {
+				return (T) instantiate(constructor, args);
 			}
-			if (executable instanceof Method method) {
-				return (T) instantiate(beanFactory, method, arguments);
+			catch (Exception ex) {
+				throw new BeanInstantiationException(constructor, ex.getMessage(), ex);
 			}
 		}
-		catch (Exception ex) {
-			throw new BeanCreationException(
-					"Unable to instantiate bean using " + executable, ex);
+		if (executable instanceof Method method) {
+			try {
+				return (T) instantiate(beanFactory, method, args);
+			}
+			catch (Exception ex) {
+				throw new BeanInstantiationException(method, ex.getMessage(), ex);
+			}
 		}
-		throw new IllegalStateException(
-				"Unsupported executable " + executable.getClass().getName());
+		throw new IllegalStateException("Unsupported executable " + executable.getClass().getName());
 	}
 
-	private Object instantiate(Constructor<?> constructor, Object[] arguments) throws Exception {
+	private Object instantiate(Constructor<?> constructor, Object[] args) throws Exception {
 		Class<?> declaringClass = constructor.getDeclaringClass();
 		if (ClassUtils.isInnerClass(declaringClass)) {
 			Object enclosingInstance = createInstance(declaringClass.getEnclosingClass());
-			arguments = ObjectUtils.addObjectToArray(arguments, enclosingInstance, 0);
+			args = ObjectUtils.addObjectToArray(args, enclosingInstance, 0);
 		}
 		ReflectionUtils.makeAccessible(constructor);
-		return constructor.newInstance(arguments);
+		return constructor.newInstance(args);
 	}
 
-	private Object instantiate(ConfigurableBeanFactory beanFactory, Method method,
-			Object[] arguments) {
-
-		ReflectionUtils.makeAccessible(method);
+	private Object instantiate(ConfigurableBeanFactory beanFactory, Method method, Object[] args) throws Exception {
 		Object target = getFactoryMethodTarget(beanFactory, method);
-		return ReflectionUtils.invokeMethod(method, target, arguments);
+		ReflectionUtils.makeAccessible(method);
+		return method.invoke(target, args);
 	}
 
 	@Nullable
@@ -416,13 +406,13 @@ private static String toCommaSeparatedNames(Class<?>... parameterTypes) {
 		return Arrays.stream(parameterTypes).map(Class::getName).collect(Collectors.joining(", "));
 	}
 
+
 	/**
 	 * Performs lookup of the {@link Executable}.
 	 */
 	static abstract class ExecutableLookup {
 
 		abstract Executable get(RegisteredBean registeredBean);
-
 	}
 
 
@@ -433,12 +423,10 @@ private static class ConstructorLookup extends ExecutableLookup {
 
 		private final Class<?>[] parameterTypes;
 
-
 		ConstructorLookup(Class<?>[] parameterTypes) {
 			this.parameterTypes = parameterTypes;
 		}
 
-
 		@Override
 		public Executable get(RegisteredBean registeredBean) {
 			Class<?> beanClass = registeredBean.getBeanClass();
@@ -456,10 +444,8 @@ public Executable get(RegisteredBean registeredBean) {
 
 		@Override
 		public String toString() {
-			return "Constructor with parameter types [%s]".formatted(
-					toCommaSeparatedNames(this.parameterTypes));
+			return "Constructor with parameter types [%s]".formatted(toCommaSeparatedNames(this.parameterTypes));
 		}
-
 	}
 
 
@@ -474,23 +460,19 @@ private static class FactoryMethodLookup extends ExecutableLookup {
 
 		private final Class<?>[] parameterTypes;
 
-
-		FactoryMethodLookup(Class<?> declaringClass, String methodName,
-				Class<?>[] parameterTypes) {
+		FactoryMethodLookup(Class<?> declaringClass, String methodName, Class<?>[] parameterTypes) {
 			this.declaringClass = declaringClass;
 			this.methodName = methodName;
 			this.parameterTypes = parameterTypes;
 		}
 
-
 		@Override
 		public Executable get(RegisteredBean registeredBean) {
 			return get();
 		}
 
 		Method get() {
-			Method method = ReflectionUtils.findMethod(this.declaringClass,
-					this.methodName, this.parameterTypes);
+			Method method = ReflectionUtils.findMethod(this.declaringClass, this.methodName, this.parameterTypes);
 			Assert.notNull(method, () -> "%s cannot be found".formatted(this));
 			return method;
 		}
@@ -501,7 +483,6 @@ public String toString() {
 					this.methodName, toCommaSeparatedNames(this.parameterTypes),
 					this.declaringClass);
 		}
-
 	}
 
 }

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

@@ -61,7 +61,6 @@
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
-import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.config.ConstructorArgumentValues;
 import org.springframework.beans.factory.config.DependencyDescriptor;
 import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessor;
@@ -1154,7 +1153,7 @@ protected BeanWrapper createBeanInstance(String beanName, RootBeanDefinition mbd
 
 		Supplier<?> instanceSupplier = mbd.getInstanceSupplier();
 		if (instanceSupplier != null) {
-			return obtainFromSupplier(instanceSupplier, beanName);
+			return obtainFromSupplier(instanceSupplier, beanName, mbd);
 		}
 
 		if (mbd.getFactoryMethodName() != null) {
@@ -1203,38 +1202,20 @@ protected BeanWrapper createBeanInstance(String beanName, RootBeanDefinition mbd
 	 * @param supplier the configured supplier
 	 * @param beanName the corresponding bean name
 	 * @return a BeanWrapper for the new instance
-	 * @since 5.0
-	 * @see #getObjectForBeanInstance
 	 */
-	protected BeanWrapper obtainFromSupplier(Supplier<?> supplier, String beanName) {
-		Object instance = obtainInstanceFromSupplier(supplier, beanName);
-		if (instance == null) {
-			instance = new NullBean();
-		}
-		BeanWrapper bw = new BeanWrapperImpl(instance);
-		initBeanWrapper(bw);
-		return bw;
-	}
-
-	@Nullable
-	private Object obtainInstanceFromSupplier(Supplier<?> supplier, String beanName) {
+	private BeanWrapper obtainFromSupplier(Supplier<?> supplier, String beanName, RootBeanDefinition mbd) {
 		String outerBean = this.currentlyCreatedBean.get();
 		this.currentlyCreatedBean.set(beanName);
+		Object instance;
+
 		try {
-			if (supplier instanceof InstanceSupplier<?> instanceSupplier) {
-				return instanceSupplier.get(RegisteredBean.of((ConfigurableListableBeanFactory) this, beanName));
-			}
-			if (supplier instanceof ThrowingSupplier<?> throwingSupplier) {
-				return throwingSupplier.getWithException();
-			}
-			return supplier.get();
+			instance = obtainInstanceFromSupplier(supplier, beanName, mbd);
 		}
 		catch (Throwable ex) {
 			if (ex instanceof BeansException beansException) {
 				throw beansException;
 			}
-			throw new BeanCreationException(beanName,
-					"Instantiation of supplied bean failed", ex);
+			throw new BeanCreationException(beanName, "Instantiation of supplied bean failed", ex);
 		}
 		finally {
 			if (outerBean != null) {
@@ -1244,6 +1225,31 @@ private Object obtainInstanceFromSupplier(Supplier<?> supplier, String beanName)
 				this.currentlyCreatedBean.remove();
 			}
 		}
+
+		if (instance == null) {
+			instance = new NullBean();
+		}
+		BeanWrapper bw = new BeanWrapperImpl(instance);
+		initBeanWrapper(bw);
+		return bw;
+	}
+
+	/**
+	 * Obtain a bean instance from the given supplier.
+	 * @param supplier the configured supplier
+	 * @param beanName the corresponding bean name
+	 * @param mbd the bean definition for the bean
+	 * @return the bean instance (possibly {@code null})
+	 * @since 6.0.7
+	 */
+	@Nullable
+	protected Object obtainInstanceFromSupplier(Supplier<?> supplier, String beanName, RootBeanDefinition mbd)
+			throws Exception {
+
+		if (supplier instanceof ThrowingSupplier<?> throwingSupplier) {
+			return throwingSupplier.getWithException();
+		}
+		return supplier.get();
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -40,6 +40,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 
 import jakarta.inject.Provider;
@@ -937,6 +938,17 @@ protected boolean isBeanEligibleForMetadataCaching(String beanName) {
 		return (this.configurationFrozen || super.isBeanEligibleForMetadataCaching(beanName));
 	}
 
+	@Override
+	@Nullable
+	protected Object obtainInstanceFromSupplier(Supplier<?> supplier, String beanName, RootBeanDefinition mbd)
+			throws Exception {
+
+		if (supplier instanceof InstanceSupplier<?> instanceSupplier) {
+			return instanceSupplier.get(RegisteredBean.of(this, beanName, mbd));
+		}
+		return super.obtainInstanceFromSupplier(supplier, beanName, mbd);
+	}
+
 	@Override
 	public void preInstantiateSingletons() throws BeansException {
 		if (logger.isTraceEnabled()) {