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 6fc0013ebcf..bbb6ec2fbf7 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); @@ -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 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 718d03e865d..e8e1224e135 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 @@ -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 listeners = new HashMap(); + Map 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 listeners = new HashMap(); + Map 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 map = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beansToExport = new HashMap(); + Map 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 beansToExport = new HashMap(); + Map 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 beans = new HashMap(); + Map 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 beansToExport = new HashMap(); + Map 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 beansToExport = new HashMap(); + Map 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 beansToExport = new HashMap(); + Map 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 getBeanMap() { - Map map = new HashMap(); + Map 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 registered = new ArrayList(); + private List registered = new ArrayList<>(); - private List unregistered = new ArrayList(); + private List 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 notifications = new ArrayList(); + private List 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 { + + @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; + } + } + }