Enforce non-null value from getBean and at injection points

Bean-derived null values may still get passed into bean properties and injection points but only if those are declared as non-required. Note that getBean will never return null; a manual bean.equals(null) / "null".equals(bean.toString()) check identifies expected null values now.  This will only ever happen with custom FactoryBeans or factory methods returning null - and since all common cases are handled by autowiring or bean property values in bean definitions, there should be no need to ever manually check for such a null value received from getBean.

Issue: SPR-15829

spring-context/src/main/java/org/springframework/cache/support/NullValue.java

@@ -18,6 +18,8 @@ package org.springframework.cache.support;
 
 import java.io.Serializable;
 
+import org.springframework.lang.Nullable;
+
 /**
  * Simple serializable class that serves as a {@code null} replacement
  * for cache stores which otherwise do not support {@code null} values.
@@ -46,4 +48,20 @@ public final class NullValue implements Serializable {
 		return INSTANCE;
 	}
 
+
+	@Override
+	public boolean equals(@Nullable Object obj) {
+		return (this == obj || obj == null);
+	}
+
+	@Override
+	public int hashCode() {
+		return NullValue.class.hashCode();
+	}
+
+	@Override
+	public String toString() {
+		return "null";
+	}
+
 }

spring-context/src/main/java/org/springframework/context/support/SimpleThreadScope.java

@@ -72,7 +72,6 @@ public class SimpleThreadScope implements Scope {
 		Object scopedObject = scope.get(name);
 		if (scopedObject == null) {
 			scopedObject = objectFactory.getObject();
-			Assert.state(scopedObject != null, "Scoped object resolved to null");
 			scope.put(name, scopedObject);
 		}
 		return scopedObject;

spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java

@@ -508,7 +508,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	//---------------------------------------------------------------------
 
 	/**
-	 * Registers the defined beans with the {@link MBeanServer}.
+	 * Register the defined beans with the {@link MBeanServer}.
 	 * <p>Each bean is exposed to the {@code MBeanServer} via a
 	 * {@code ModelMBean}. The actual implemetation of the
 	 * {@code ModelMBean} interface used depends on the implementation of
@@ -566,7 +566,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	}
 
 	/**
-	 * Registers an individual bean with the {@link #setServer MBeanServer}.
+	 * Register an individual bean with the {@link #setServer MBeanServer}.
 	 * <p>This method is responsible for deciding <strong>how</strong> a bean
 	 * should be exposed to the {@code MBeanServer}. Specifically, if the
 	 * supplied {@code mapValue} is the name of a bean that is configured
@@ -579,15 +579,13 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	 * @param mapValue the value configured for this bean in the beans map;
 	 * may be either the {@code String} name of a bean, or the bean itself
 	 * @param beanKey the key associated with this bean in the beans map
-	 * @return the {@code ObjectName} under which the resource was registered,
-	 * or {@code null} if the actual resource was {@code null} as well
+	 * @return the {@code ObjectName} under which the resource was registered
 	 * @throws MBeanExportException if the export failed
 	 * @see #setBeans
 	 * @see #registerBeanInstance
 	 * @see #registerLazyInit
 	 */
-	@Nullable
-	protected ObjectName registerBeanNameOrInstance(@Nullable Object mapValue, String beanKey) throws MBeanExportException {
+	protected ObjectName registerBeanNameOrInstance(Object mapValue, String beanKey) throws MBeanExportException {
 		try {
 			if (mapValue instanceof String) {
 				// Bean name pointing to a potentially lazy-init bean in the factory.
@@ -607,7 +605,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 					return objectName;
 				}
 			}
-			else if (mapValue != null) {
+			else {
 				// Plain bean instance -> register it directly.
 				if (this.beanFactory != null) {
 					Map<String, ?> beansOfSameType =
@@ -623,9 +621,6 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 				}
 				return registerBeanInstance(mapValue, beanKey);
 			}
-			else {
-				return null;
-			}
 		}
 		catch (Throwable ex) {
 			throw new UnableToRegisterMBeanException(
@@ -634,14 +629,14 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	}
 
 	/**
-	 * Replaces any bean names used as keys in the {@code NotificationListener}
+	 * Replace any bean names used as keys in the {@code NotificationListener}
 	 * mappings with their corresponding {@code ObjectName} values.
 	 * @param beanName the name of the bean to be registered
 	 * @param objectName the {@code ObjectName} under which the bean will be registered
 	 * with the {@code MBeanServer}
 	 */
-	private void replaceNotificationListenerBeanNameKeysIfNecessary(String beanName, @Nullable ObjectName objectName) {
-		if (objectName != null && this.notificationListeners != null) {
+	private void replaceNotificationListenerBeanNameKeysIfNecessary(String beanName, ObjectName objectName) {
+		if (this.notificationListeners != null) {
 			for (NotificationListenerBean notificationListener : this.notificationListeners) {
 				notificationListener.replaceObjectName(beanName, objectName);
 			}
@@ -656,12 +651,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	 * @return the {@code ObjectName} under which the bean was registered
 	 * with the {@code MBeanServer}
 	 */
-	@Nullable
-	private ObjectName registerBeanInstance(@Nullable Object bean, String beanKey) throws JMException {
-		if (bean == null) {
-			return null;
-		}
-
+	private ObjectName registerBeanInstance(Object bean, String beanKey) throws JMException {
 		ObjectName objectName = getObjectName(bean, beanKey);
 		Object mbeanToExpose = null;
 		if (isMBean(bean.getClass())) {
@@ -695,7 +685,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	}
 
 	/**
-	 * Registers beans that are configured for lazy initialization with the
+	 * Register beans that are configured for lazy initialization with the
 	 * {@code MBeanServer} indirectly through a proxy.
 	 * @param beanName the name of the bean in the {@code BeanFactory}
 	 * @param beanKey the key associated with this bean in the beans map
@@ -890,7 +880,13 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 					Class<?> beanClass = this.beanFactory.getType(beanName);
 					if (beanClass != null && callback.include(beanClass, beanName)) {
 						boolean lazyInit = isBeanDefinitionLazyInit(this.beanFactory, beanName);
-						Object beanInstance = (!lazyInit ? this.beanFactory.getBean(beanName) : null);
+						Object beanInstance = null;
+						if (!lazyInit) {
+							beanInstance = this.beanFactory.getBean(beanName);
+							if (!beanClass.isInstance(beanInstance)) {
+								continue;
+							}
+						}
 						if (!ScopedProxyUtils.isScopedTarget(beanName) && !beans.containsValue(beanName) &&
 								(beanInstance == null ||
 										!CollectionUtils.containsInstance(beans.values(), beanInstance))) {

spring-context/src/test/java/org/springframework/cache/interceptor/CacheResolverCustomizationTests.java

@@ -58,14 +58,14 @@ public class CacheResolverCustomizationTests {
 
 
 	@Before
-	public void setUp() {
+	public void setup() {
 		ApplicationContext context = new AnnotationConfigApplicationContext(Config.class);
 		this.cacheManager = context.getBean("cacheManager", CacheManager.class);
 		this.anotherCacheManager = context.getBean("anotherCacheManager", CacheManager.class);
-
 		this.simpleService = context.getBean(SimpleService.class);
 	}
 
+
 	@Test
 	public void noCustomization() {
 		Cache cache = this.cacheManager.getCache("default");
@@ -162,12 +162,6 @@ public class CacheResolverCustomizationTests {
 			return CacheTestUtils.createSimpleCacheManager("default", "primary", "secondary");
 		}
 
-		@Override
-		@Bean
-		public KeyGenerator keyGenerator() {
-			return null;
-		}
-
 		@Bean
 		public CacheManager anotherCacheManager() {
 			return CacheTestUtils.createSimpleCacheManager("default", "primary", "secondary");

spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * 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.
@@ -114,7 +114,7 @@ public class CommonAnnotationBeanPostProcessorTests {
 		rbd.setFactoryMethodName("create");
 		bf.registerBeanDefinition("bean", rbd);
 
-		assertNull(bf.getBean("bean"));
+		assertEquals("null", bf.getBean("bean").toString());
 		bf.destroySingletons();
 	}
 

spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * 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.
@@ -30,13 +30,6 @@ import static org.junit.Assert.*;
  */
 public class GenericApplicationContextTests {
 
-	@Test
-	public void nullBeanRegistration() {
-		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
-		bf.registerSingleton("nullBean", null);
-		new GenericApplicationContext(bf).refresh();
-	}
-
 	@Test
 	public void getBeanForClass() {
 		GenericApplicationContext ac = new GenericApplicationContext();