MBeanExporter silently ignores null beans

Issue: SPR-15031
(cherry picked from commit 1e58c80)

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

@@ -83,7 +83,7 @@ import org.springframework.util.ObjectUtils;
  * via the {@link #setListeners(MBeanExporterListener[]) listeners} property, allowing
  * application code to be notified of MBean registration and unregistration events.
  *
- * <p>This exporter is compatible with MBeans and MXBeans on Java 6 and above.
+ * <p>This exporter is compatible with MBeans as well as MXBeans.
  *
  * @author Rob Harrop
  * @author Juergen Hoeller
@@ -466,7 +466,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 				objectName = JmxUtils.appendIdentityToObjectName(objectName, managedResource);
 			}
 		}
-		catch (Exception ex) {
+		catch (Throwable ex) {
 			throw new MBeanExportException("Unable to generate ObjectName for MBean [" + managedResource + "]", ex);
 		}
 		registerManagedResource(managedResource, objectName);
@@ -570,7 +570,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 	 * should be exposed to the {@code MBeanServer}. Specifically, if the
 	 * supplied {@code mapValue} is the name of a bean that is configured
 	 * for lazy initialization, then a proxy to the resource is registered with
-	 * the {@code MBeanServer} so that the the lazy load behavior is
+	 * the {@code MBeanServer} so that the lazy load behavior is
 	 * honored. If the bean is already an MBean then it will be registered
 	 * directly with the {@code MBeanServer} without any intervention. For
 	 * all other beans or bean names, the resource itself is registered with
@@ -578,7 +578,8 @@ 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
+	 * @return the {@code ObjectName} under which the resource was registered,
+	 * or {@code null} if the actual resource was {@code null} as well
 	 * @throws MBeanExportException if the export failed
 	 * @see #setBeans
 	 * @see #registerBeanInstance
@@ -599,12 +600,14 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 				}
 				else {
 					Object bean = this.beanFactory.getBean(beanName);
-					ObjectName objectName = registerBeanInstance(bean, beanKey);
-					replaceNotificationListenerBeanNameKeysIfNecessary(beanName, objectName);
-					return objectName;
+					if (bean != null) {
+						ObjectName objectName = registerBeanInstance(bean, beanKey);
+						replaceNotificationListenerBeanNameKeysIfNecessary(beanName, objectName);
+						return objectName;
+					}
 				}
 			}
-			else {
+			else if (mapValue != null) {
 				// Plain bean instance -> register it directly.
 				if (this.beanFactory != null) {
 					Map<String, ?> beansOfSameType =
@@ -621,10 +624,11 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 				return registerBeanInstance(mapValue, beanKey);
 			}
 		}
-		catch (Exception ex) {
+		catch (Throwable ex) {
 			throw new UnableToRegisterMBeanException(
 					"Unable to register MBean [" + mapValue + "] with key '" + beanKey + "'", ex);
 		}
+		return null;
 	}
 
 	/**
@@ -816,7 +820,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 			mbean.setManagedResource(managedResource, MR_TYPE_OBJECT_REFERENCE);
 			return mbean;
 		}
-		catch (Exception ex) {
+		catch (Throwable ex) {
 			throw new MBeanExportException("Could not create ModelMBean for managed resource [" +
 					managedResource + "] with key '" + beanKey + "'", ex);
 		}
@@ -984,7 +988,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 						}
 					}
 				}
-				catch (Exception ex) {
+				catch (Throwable ex) {
 					throw new MBeanExportException("Unable to register NotificationListener", ex);
 				}
 			}
@@ -1004,7 +1008,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
 					this.server.removeNotificationListener(mappedObjectName, bean.getNotificationListener(),
 							bean.getNotificationFilter(), bean.getHandback());
 				}
-				catch (Exception ex) {
+				catch (Throwable ex) {
 					if (logger.isDebugEnabled()) {
 						logger.debug("Unable to unregister NotificationListener", ex);
 					}

spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -39,8 +39,10 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
+import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.jmx.AbstractMBeanServerTests;
@@ -68,8 +70,7 @@ import static org.junit.Assert.*;
  * @author Sam Brannen
  * @author Stephane Nicoll
  */
-@SuppressWarnings("deprecation")
-public final class MBeanExporterTests extends AbstractMBeanServerTests {
+public class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	@Rule
 	public final ExpectedException thrown = ExpectedException.none();
@@ -79,7 +80,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	@Test
 	public void testRegisterNullNotificationListenerType() throws Exception {
-		Map<String, NotificationListener> listeners = new HashMap<String, NotificationListener>();
+		Map<String, NotificationListener> listeners = new HashMap<>();
 		// put null in as a value...
 		listeners.put("*", null);
 		MBeanExporter exporter = new MBeanExporter();
@@ -90,7 +91,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	@Test
 	public void testRegisterNotificationListenerForNonExistentMBean() throws Exception {
-		Map<String, NotificationListener> listeners = new HashMap<String, NotificationListener>();
+		Map<String, NotificationListener> listeners = new HashMap<>();
 		NotificationListener dummyListener = new NotificationListener() {
 			@Override
 			public void handleNotification(Notification notification, Object handback) {
@@ -130,7 +131,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	@Test
 	public void testUserCreatedMBeanRegWithDynamicMBean() throws Exception {
-		Map<String, Object> map = new HashMap<String, Object>();
+		Map<String, Object> map = new HashMap<>();
 		map.put("spring:name=dynBean", new TestDynamicMBean());
 
 		InvokeDetectAssembler asm = new InvokeDetectAssembler();
@@ -235,7 +236,6 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		assertListener(listener2);
 	}
 
-
 	@Test
 	public void testExportJdkProxy() throws Exception {
 		JmxTestBean bean = new JmxTestBean();
@@ -249,7 +249,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		IJmxTestBean proxy = (IJmxTestBean) factory.getProxy();
 		String name = "bean:mmm=whatever";
 
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put(name, proxy);
 
 		MBeanExporter exporter = new MBeanExporter();
@@ -268,7 +268,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		SelfNamingTestBean testBean = new SelfNamingTestBean();
 		testBean.setObjectName(objectName);
 
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put("foo", testBean);
 
 		MBeanExporter exporter = new MBeanExporter();
@@ -295,14 +295,14 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 		String objectName2 = "spring:test=equalBean";
 
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put(objectName.toString(), springRegistered);
 		beans.put(objectName2, springRegistered);
 
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setServer(server);
 		exporter.setBeans(beans);
-		exporter.setRegistrationBehavior(MBeanExporter.REGISTRATION_IGNORE_EXISTING);
+		exporter.setRegistrationPolicy(RegistrationPolicy.IGNORE_EXISTING);
 
 		start(exporter);
 
@@ -327,7 +327,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		Person springRegistered = new Person();
 		springRegistered.setName("Sally Greenwood");
 
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put(objectName.toString(), springRegistered);
 
 		MBeanExporter exporter = new MBeanExporter();
@@ -353,7 +353,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		bean.setName(name);
 		ObjectName objectName = ObjectNameManager.getInstance("spring:type=Test");
 
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put(objectName.toString(), bean);
 
 		MBeanExporter exporter = new MBeanExporter();
@@ -383,7 +383,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		factory.registerSingleton(exportedBeanName, new TestBean());
 
 		MBeanExporter exporter = new MBeanExporter();
-		Map<String, Object> beansToExport = new HashMap<String, Object>();
+		Map<String, Object> beansToExport = new HashMap<>();
 		beansToExport.put(OBJECT_NAME, exportedBeanName);
 		exporter.setBeans(beansToExport);
 		exporter.setServer(getServer());
@@ -470,7 +470,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setServer(getServer());
-		Map<String, Object> beansToExport = new HashMap<String, Object>();
+		Map<String, Object> beansToExport = new HashMap<>();
 		beansToExport.put(OBJECT_NAME, OBJECT_NAME);
 		exporter.setBeans(beansToExport);
 		exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(OBJECT_NAME));
@@ -526,7 +526,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 	@Test
 	public void testNotRunningInBeanFactoryAndPassedBeanNameToExport() throws Exception {
 		MBeanExporter exporter = new MBeanExporter();
-		Map<String, Object> beans = new HashMap<String, Object>();
+		Map<String, Object> beans = new HashMap<>();
 		beans.put(OBJECT_NAME, "beanName");
 		exporter.setBeans(beans);
 		thrown.expect(MBeanExportException.class);
@@ -541,10 +541,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		start(exporter);
 	}
 
-	/**
-	 * SPR-2158
-	 */
-	@Test
+	@Test  // SPR-2158
 	public void testMBeanIsNotUnregisteredSpuriouslyIfSomeExternalProcessHasUnregisteredMBean() throws Exception {
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setBeans(getBeanMap());
@@ -561,10 +558,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 				listener.getUnregistered().size());
 	}
 
-	/**
-	 * SPR-3302
-	 */
-	@Test
+	@Test  // SPR-3302
 	public void testBeanNameCanBeUsedInNotificationListenersMap() throws Exception {
 		String beanName = "charlesDexterWard";
 		BeanDefinitionBuilder testBean = BeanDefinitionBuilder.rootBeanDefinition(JmxTestBean.class);
@@ -576,7 +570,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setServer(getServer());
-		Map<String, Object> beansToExport = new HashMap<String, Object>();
+		Map<String, Object> beansToExport = new HashMap<>();
 		beansToExport.put("test:what=ever", testBeanInstance);
 		exporter.setBeans(beansToExport);
 		exporter.setBeanFactory(factory);
@@ -598,7 +592,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setServer(getServer());
-		Map<String, Object> beansToExport = new HashMap<String, Object>();
+		Map<String, Object> beansToExport = new HashMap<>();
 		beansToExport.put("test:what=ever", testBeanInstance);
 		exporter.setBeans(beansToExport);
 		exporter.setBeanFactory(factory);
@@ -608,10 +602,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		start(exporter);
 	}
 
-	/*
-	 * SPR-3625
-	 */
-	@Test
+	@Test  // SPR-3625
 	public void testMBeanIsUnregisteredForRuntimeExceptionDuringInitialization() throws Exception {
 		BeanDefinitionBuilder builder1 = BeanDefinitionBuilder.rootBeanDefinition(Person.class);
 		BeanDefinitionBuilder builder2 = BeanDefinitionBuilder
@@ -626,7 +617,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 		MBeanExporter exporter = new MBeanExporter();
 		exporter.setServer(getServer());
-		Map<String, Object> beansToExport = new HashMap<String, Object>();
+		Map<String, Object> beansToExport = new HashMap<>();
 		beansToExport.put(objectName1, objectName1);
 		beansToExport.put(objectName2, objectName2);
 		exporter.setBeans(beansToExport);
@@ -667,12 +658,43 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 				ObjectNameManager.getInstance(secondBeanName));
 	}
 
+	@Test
+	public void testRegisterFactoryBean() throws MalformedObjectNameException {
+		DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
+		factory.registerBeanDefinition("spring:type=FactoryBean", new RootBeanDefinition(ProperSomethingFactoryBean.class));
+
+		MBeanExporter exporter = new MBeanExporter();
+		exporter.setServer(getServer());
+		exporter.setBeanFactory(factory);
+		exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ALL);
+
+		start(exporter);
+		assertIsRegistered("Non-null FactoryBean object registered",
+				ObjectNameManager.getInstance("spring:type=FactoryBean"));
+	}
+
+	@Test
+	public void testIgnoreNullObjectFromFactoryBean() throws MalformedObjectNameException {
+		DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
+		factory.registerBeanDefinition("spring:type=FactoryBean", new RootBeanDefinition(NullSomethingFactoryBean.class));
+
+		MBeanExporter exporter = new MBeanExporter();
+		exporter.setServer(getServer());
+		exporter.setBeanFactory(factory);
+		exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ALL);
+
+		start(exporter);
+		assertIsNotRegistered("Null FactoryBean object not registered",
+				ObjectNameManager.getInstance("spring:type=FactoryBean"));
+	}
+
+
 	private ConfigurableApplicationContext load(String context) {
 		return new ClassPathXmlApplicationContext(context, getClass());
 	}
 
 	private Map<String, Object> getBeanMap() {
-		Map<String, Object> map = new HashMap<String, Object>();
+		Map<String, Object> map = new HashMap<>();
 		map.put(OBJECT_NAME, new JmxTestBean());
 		return map;
 	}
@@ -700,9 +722,9 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	private static class MockMBeanExporterListener implements MBeanExporterListener {
 
-		private List<ObjectName> registered = new ArrayList<ObjectName>();
+		private List<ObjectName> registered = new ArrayList<>();
 
-		private List<ObjectName> unregistered = new ArrayList<ObjectName>();
+		private List<ObjectName> unregistered = new ArrayList<>();
 
 		@Override
 		public void mbeanRegistered(ObjectName objectName) {
@@ -762,7 +784,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 
 	public static final class StubNotificationListener implements NotificationListener {
 
-		private List<Notification> notifications = new ArrayList<Notification>();
+		private List<Notification> notifications = new ArrayList<>();
 
 		@Override
 		public void handleNotification(Notification notification, Object handback) {
@@ -799,4 +821,41 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
 		}
 	}
 
+
+	public interface SomethingMBean {}
+
+	public static class Something implements SomethingMBean {}
+
+
+	public static class ProperSomethingFactoryBean implements FactoryBean<Something> {
+
+		@Override public Something getObject() {
+			return new Something();
+		}
+
+		@Override public Class<?> getObjectType() {
+			return Something.class;
+		}
+
+		@Override public boolean isSingleton() {
+			return true;
+		}
+	}
+
+
+	public static class NullSomethingFactoryBean implements FactoryBean<Something> {
+
+		@Override public Something getObject() {
+			return null;
+		}
+
+		@Override public Class<?> getObjectType() {
+			return Something.class;
+		}
+
+		@Override public boolean isSingleton() {
+			return true;
+		}
+	}
+
 }