From 9c55d22f782a86219fb263b58c77f024cc01e45f Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 19 Dec 2016 16:02:59 +0100 Subject: [PATCH] MBeanExporter silently ignores null beans Issue: SPR-15031 --- .../jmx/export/MBeanExporter.java | 26 +++--- .../jmx/export/MBeanExporterTests.java | 87 ++++++++++++++++--- 2 files changed, 88 insertions(+), 25 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java index c58632be43..4385854cb9 100644 --- a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java +++ b/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. * - *

This exporter is compatible with MBeans and MXBeans on Java 6 and above. + *

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); @@ -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 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); } diff --git a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java index 59ac629589..e8e1224e13 100644 --- a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java +++ b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java @@ -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,7 +70,6 @@ import static org.junit.Assert.*; * @author Sam Brannen * @author Stephane Nicoll */ -@SuppressWarnings("deprecation") public class MBeanExporterTests extends AbstractMBeanServerTests { @Rule @@ -235,7 +236,6 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { assertListener(listener2); } - @Test public void testExportJdkProxy() throws Exception { JmxTestBean bean = new JmxTestBean(); @@ -541,10 +541,7 @@ public 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 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); @@ -608,10 +602,7 @@ public 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 @@ -667,6 +658,37 @@ public 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()); } @@ -799,4 +821,41 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } } + + public interface SomethingMBean {} + + public static class Something implements SomethingMBean {} + + + public static class ProperSomethingFactoryBean implements FactoryBean { + + @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 { + + @Override public Something getObject() { + return null; + } + + @Override public Class getObjectType() { + return Something.class; + } + + @Override public boolean isSingleton() { + return true; + } + } + } -- GitLab