Refined throwing of BeanCreationExceptions (and reflection exceptions)

Issue: SPR-14883

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

@@ -243,23 +243,28 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean
 
 		// Let's check for lookup methods here..
 		if (!this.lookupMethodsChecked.contains(beanName)) {
-			ReflectionUtils.doWithMethods(beanClass, new ReflectionUtils.MethodCallback() {
-				@Override
-				public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
-					Lookup lookup = method.getAnnotation(Lookup.class);
-					if (lookup != null) {
-						LookupOverride override = new LookupOverride(method, lookup.value());
-						try {
-							RootBeanDefinition mbd = (RootBeanDefinition) beanFactory.getMergedBeanDefinition(beanName);
-							mbd.getMethodOverrides().addOverride(override);
-						}
-						catch (NoSuchBeanDefinitionException ex) {
-							throw new BeanCreationException(beanName,
-									"Cannot apply @Lookup to beans without corresponding bean definition");
+			try {
+				ReflectionUtils.doWithMethods(beanClass, new ReflectionUtils.MethodCallback() {
+					@Override
+					public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
+						Lookup lookup = method.getAnnotation(Lookup.class);
+						if (lookup != null) {
+							LookupOverride override = new LookupOverride(method, lookup.value());
+							try {
+								RootBeanDefinition mbd = (RootBeanDefinition) beanFactory.getMergedBeanDefinition(beanName);
+								mbd.getMethodOverrides().addOverride(override);
+							}
+							catch (NoSuchBeanDefinitionException ex) {
+								throw new BeanCreationException(beanName,
+										"Cannot apply @Lookup to beans without corresponding bean definition");
+							}
 						}
 					}
-				}
-			});
+				});
+			}
+			catch (IllegalStateException ex) {
+				throw new BeanCreationException(beanName, "Lookup method resolution failed", ex);
+			}
 			this.lookupMethodsChecked.add(beanName);
 		}
 

spring-beans/src/main/java/org/springframework/beans/factory/config/InstantiationAwareBeanPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -103,7 +103,6 @@ public interface InstantiationAwareBeanPostProcessor extends BeanPostProcessor {
 	 * @see org.springframework.beans.MutablePropertyValues
 	 */
 	PropertyValues postProcessPropertyValues(
-			PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName)
-			throws BeansException;
+			PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName) throws BeansException;
 
 }

spring-beans/src/main/java/org/springframework/beans/factory/config/Scope.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -68,6 +68,7 @@ public interface Scope {
 	 * @param objectFactory the {@link ObjectFactory} to use to create the scoped
 	 * object if it is not present in the underlying storage mechanism
 	 * @return the desired object (never {@code null})
+	 * @throws IllegalStateException if the underlying scope is not currently active
 	 */
 	Object get(String name, ObjectFactory<?> objectFactory);
 
@@ -84,6 +85,7 @@ public interface Scope {
 	 * removing an object.
 	 * @param name the name of the object to remove
 	 * @return the removed object, or {@code null} if no object was present
+	 * @throws IllegalStateException if the underlying scope is not currently active
 	 * @see #registerDestructionCallback
 	 */
 	Object remove(String name);
@@ -112,6 +114,7 @@ public interface Scope {
 	 * so it can safely be executed without an enclosing try-catch block.
 	 * Furthermore, the Runnable will usually be serializable, provided
 	 * that its target object is serializable as well.
+	 * @throws IllegalStateException if the underlying scope is not currently active
 	 * @see org.springframework.beans.factory.DisposableBean
 	 * @see org.springframework.beans.factory.support.AbstractBeanDefinition#getDestroyMethodName()
 	 * @see DestructionAwareBeanPostProcessor
@@ -123,6 +126,7 @@ public interface Scope {
 	 * E.g. the HttpServletRequest object for key "request".
 	 * @param key the contextual key
 	 * @return the corresponding object, or {@code null} if none found
+	 * @throws IllegalStateException if the underlying scope is not currently active
 	 */
 	Object resolveContextualObject(String key);
 
@@ -139,6 +143,7 @@ public interface Scope {
 	 * underlying storage mechanism has no obvious candidate for such an ID.
 	 * @return the conversation ID, or {@code null} if there is no
 	 * conversation ID for the current scope
+	 * @throws IllegalStateException if the underlying scope is not currently active
 	 */
 	String getConversationId();
 

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

@@ -479,21 +479,11 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 					"BeanPostProcessor before instantiation of bean failed", ex);
 		}
 
-		try {
-			Object beanInstance = doCreateBean(beanName, mbdToUse, args);
-			if (logger.isDebugEnabled()) {
-				logger.debug("Finished creating instance of bean '" + beanName + "'");
-			}
-			return beanInstance;
-		}
-		catch (BeanCreationException ex) {
-			// A previously detected exception with proper bean creation context already...
-			throw ex;
-		}
-		catch (Throwable ex) {
-			throw new BeanCreationException(
-					mbdToUse.getResourceDescription(), beanName, "Unexpected exception during bean creation", ex);
+		Object beanInstance = doCreateBean(beanName, mbdToUse, args);
+		if (logger.isDebugEnabled()) {
+			logger.debug("Finished creating instance of bean '" + beanName + "'");
 		}
+		return beanInstance;
 	}
 
 	/**
@@ -532,7 +522,7 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 				}
 				catch (Throwable ex) {
 					throw new BeanCreationException(mbd.getResourceDescription(), beanName,
-							"Post-processing failed of bean type [" + beanType + "] failed", ex);
+							"Post-processing of merged bean definition failed", ex);
 				}
 				mbd.postProcessed = true;
 			}
@@ -948,12 +938,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 	 * @param mbd the merged bean definition for the bean
 	 * @param beanType the actual type of the managed bean instance
 	 * @param beanName the name of the bean
-	 * @throws BeansException if any post-processing failed
 	 * @see MergedBeanDefinitionPostProcessor#postProcessMergedBeanDefinition
 	 */
-	protected void applyMergedBeanDefinitionPostProcessors(RootBeanDefinition mbd, Class<?> beanType, String beanName)
-			throws BeansException {
-
+	protected void applyMergedBeanDefinitionPostProcessors(RootBeanDefinition mbd, Class<?> beanType, String beanName) {
 		for (BeanPostProcessor bp : getBeanPostProcessors()) {
 			if (bp instanceof MergedBeanDefinitionPostProcessor) {
 				MergedBeanDefinitionPostProcessor bdp = (MergedBeanDefinitionPostProcessor) bp;
@@ -996,12 +983,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 	 * @param beanClass the class of the bean to be instantiated
 	 * @param beanName the name of the bean
 	 * @return the bean object to use instead of a default instance of the target bean, or {@code null}
-	 * @throws BeansException if any post-processing failed
 	 * @see InstantiationAwareBeanPostProcessor#postProcessBeforeInstantiation
 	 */
-	protected Object applyBeanPostProcessorsBeforeInstantiation(Class<?> beanClass, String beanName)
-			throws BeansException {
-
+	protected Object applyBeanPostProcessorsBeforeInstantiation(Class<?> beanClass, String beanName) {
 		for (BeanPostProcessor bp : getBeanPostProcessors()) {
 			if (bp instanceof InstantiationAwareBeanPostProcessor) {
 				InstantiationAwareBeanPostProcessor ibp = (InstantiationAwareBeanPostProcessor) bp;

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

@@ -214,7 +214,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
 			if (singletonObject == null) {
 				if (this.singletonsCurrentlyInDestruction) {
 					throw new BeanCreationNotAllowedException(beanName,
-							"Singleton bean creation not allowed while the singletons of this factory are in destruction " +
+							"Singleton bean creation not allowed while singletons of this factory are in destruction " +
 							"(Do not request a bean from a BeanFactory in a destroy method implementation!)");
 				}
 				if (logger.isDebugEnabled()) {

spring-core/src/main/java/org/springframework/util/ReflectionUtils.java

@@ -483,7 +483,6 @@ public abstract class ReflectionUtils {
 	 * @param clazz the class to introspect
 	 * @param mc the callback to invoke for each method
 	 * @since 4.2
-	 * @throws IllegalStateException if introspection fails
 	 * @see #doWithMethods
 	 */
 	public static void doWithLocalMethods(Class<?> clazz, MethodCallback mc) {
@@ -505,7 +504,6 @@ public abstract class ReflectionUtils {
 	 * twice, unless excluded by a {@link MethodFilter}.
 	 * @param clazz the class to introspect
 	 * @param mc the callback to invoke for each method
-	 * @throws IllegalStateException if introspection fails
 	 * @see #doWithMethods(Class, MethodCallback, MethodFilter)
 	 */
 	public static void doWithMethods(Class<?> clazz, MethodCallback mc) {
@@ -520,7 +518,6 @@ public abstract class ReflectionUtils {
 	 * @param clazz the class to introspect
 	 * @param mc the callback to invoke for each method
 	 * @param mf the filter that determines the methods to apply the callback to
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static void doWithMethods(Class<?> clazz, MethodCallback mc, MethodFilter mf) {
 		// Keep backing up the inheritance hierarchy.
@@ -550,7 +547,6 @@ public abstract class ReflectionUtils {
 	 * Get all declared methods on the leaf class and all superclasses.
 	 * Leaf class methods are included first.
 	 * @param leafClass the class to introspect
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static Method[] getAllDeclaredMethods(Class<?> leafClass) {
 		final List<Method> methods = new ArrayList<Method>(32);
@@ -568,7 +564,6 @@ public abstract class ReflectionUtils {
 	 * Leaf class methods are included first and while traversing the superclass hierarchy
 	 * any methods found with signatures matching a method already included are filtered out.
 	 * @param leafClass the class to introspect
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static Method[] getUniqueDeclaredMethods(Class<?> leafClass) {
 		final List<Method> methods = new ArrayList<Method>(32);
@@ -609,33 +604,27 @@ public abstract class ReflectionUtils {
 	 * interfaces, since those are effectively to be treated just like declared methods.
 	 * @param clazz the class to introspect
 	 * @return the cached array of methods
-	 * @throws IllegalStateException if introspection fails
 	 * @see Class#getDeclaredMethods()
 	 */
 	private static Method[] getDeclaredMethods(Class<?> clazz) {
+		Assert.notNull(clazz, "Class must not be null");
 		Method[] result = declaredMethodsCache.get(clazz);
 		if (result == null) {
-			try {
-				Method[] declaredMethods = clazz.getDeclaredMethods();
-				List<Method> defaultMethods = findConcreteMethodsOnInterfaces(clazz);
-				if (defaultMethods != null) {
-					result = new Method[declaredMethods.length + defaultMethods.size()];
-					System.arraycopy(declaredMethods, 0, result, 0, declaredMethods.length);
-					int index = declaredMethods.length;
-					for (Method defaultMethod : defaultMethods) {
-						result[index] = defaultMethod;
-						index++;
-					}
+			Method[] declaredMethods = clazz.getDeclaredMethods();
+			List<Method> defaultMethods = findConcreteMethodsOnInterfaces(clazz);
+			if (defaultMethods != null) {
+				result = new Method[declaredMethods.length + defaultMethods.size()];
+				System.arraycopy(declaredMethods, 0, result, 0, declaredMethods.length);
+				int index = declaredMethods.length;
+				for (Method defaultMethod : defaultMethods) {
+					result[index] = defaultMethod;
+					index++;
 				}
-				else {
-					result = declaredMethods;
-				}
-				declaredMethodsCache.put(clazz, (result.length == 0 ? NO_METHODS : result));
 			}
-			catch (Throwable ex) {
-				throw new IllegalStateException("Failed to introspect Class [" + clazz +
-						"] from ClassLoader [" + clazz.getClassLoader() + "]", ex);
+			else {
+				result = declaredMethods;
 			}
+			declaredMethodsCache.put(clazz, (result.length == 0 ? NO_METHODS : result));
 		}
 		return result;
 	}
@@ -661,7 +650,6 @@ public abstract class ReflectionUtils {
 	 * @param clazz the target class to analyze
 	 * @param fc the callback to invoke for each field
 	 * @since 4.2
-	 * @throws IllegalStateException if introspection fails
 	 * @see #doWithFields
 	 */
 	public static void doWithLocalFields(Class<?> clazz, FieldCallback fc) {
@@ -680,7 +668,6 @@ public abstract class ReflectionUtils {
 	 * class hierarchy to get all declared fields.
 	 * @param clazz the target class to analyze
 	 * @param fc the callback to invoke for each field
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static void doWithFields(Class<?> clazz, FieldCallback fc) {
 		doWithFields(clazz, fc, null);
@@ -692,7 +679,6 @@ public abstract class ReflectionUtils {
 	 * @param clazz the target class to analyze
 	 * @param fc the callback to invoke for each field
 	 * @param ff the filter that determines the fields to apply the callback to
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static void doWithFields(Class<?> clazz, FieldCallback fc, FieldFilter ff) {
 		// Keep backing up the inheritance hierarchy.
@@ -720,20 +706,14 @@ public abstract class ReflectionUtils {
 	 * in order to avoid the JVM's SecurityManager check and defensive array copying.
 	 * @param clazz the class to introspect
 	 * @return the cached array of fields
-	 * @throws IllegalStateException if introspection fails
 	 * @see Class#getDeclaredFields()
 	 */
 	private static Field[] getDeclaredFields(Class<?> clazz) {
+		Assert.notNull(clazz, "Class must not be null");
 		Field[] result = declaredFieldsCache.get(clazz);
 		if (result == null) {
-			try {
-				result = clazz.getDeclaredFields();
-				declaredFieldsCache.put(clazz, (result.length == 0 ? NO_FIELDS : result));
-			}
-			catch (Throwable ex) {
-				throw new IllegalStateException("Failed to introspect Class [" + clazz +
-						"] from ClassLoader [" + clazz.getClassLoader() + "]", ex);
-			}
+			result = clazz.getDeclaredFields();
+			declaredFieldsCache.put(clazz, (result.length == 0 ? NO_FIELDS : result));
 		}
 		return result;
 	}
@@ -742,7 +722,6 @@ public abstract class ReflectionUtils {
 	 * Given the source object and the destination, which must be the same class
 	 * or a subclass, copy all fields, including inherited fields. Designed to
 	 * work on objects with public no-arg constructors.
-	 * @throws IllegalStateException if introspection fails
 	 */
 	public static void shallowCopyFieldState(final Object src, final Object dest) {
 		if (src == null) {