From 8c26717bdcb6c07688fa757782a1597b642f827a 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 (cherry picked from commit e9def51) --- .../jmx/export/MBeanExporter.java | 28 ++++---- .../jmx/export/MBeanExporterTests.java | 72 ++++++++++++++++++- 2 files changed, 87 insertions(+), 13 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 d448614fcee5..5c9ed67755f3 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 @@ -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. @@ -446,7 +446,7 @@ public ObjectName registerManagedResource(Object managedResource) throws MBeanEx 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); @@ -548,7 +548,7 @@ protected boolean isBeanDefinitionLazyInit(ListableBeanFactory beanFactory, Stri * 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 @@ -556,7 +556,8 @@ protected boolean isBeanDefinitionLazyInit(ListableBeanFactory beanFactory, Stri * @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 @@ -577,12 +578,14 @@ protected ObjectName registerBeanNameOrInstance(Object mapValue, String beanKey) } 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 = @@ -599,10 +602,11 @@ protected ObjectName registerBeanNameOrInstance(Object mapValue, String beanKey) return registerBeanInstance(mapValue, beanKey); } } - catch (Exception ex) { + catch (Throwable ex) { throw new UnableToRegisterMBeanException( "Unable to register MBean [" + mapValue + "] with key '" + beanKey + "'", ex); } + return null; } /** @@ -794,7 +798,7 @@ protected ModelMBean createAndConfigureMBean(Object managedResource, String bean 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); } @@ -960,7 +964,7 @@ private void registerNotificationListeners() throws MBeanExportException { } } } - catch (Exception ex) { + catch (Throwable ex) { throw new MBeanExportException("Unable to register NotificationListener", ex); } } @@ -980,7 +984,7 @@ private void unregisterNotificationListeners() { 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 ba123ff93198..f7ac16cf8e23 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. @@ -35,8 +35,10 @@ import org.junit.Test; 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.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.core.io.ClassPathResource; import org.springframework.jmx.AbstractMBeanServerTests; @@ -677,6 +679,37 @@ public void testMBeanIsUnregisteredForRuntimeExceptionDuringInitialization() thr ObjectNameManager.getInstance(objectName2)); } + @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); + exporter.afterPropertiesSet(); + + 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); + exporter.afterPropertiesSet(); + + assertIsNotRegistered("Null FactoryBean object not registered", + ObjectNameManager.getInstance("spring:type=FactoryBean")); + } + + private Map getBeanMap() { Map map = new HashMap(); map.put(OBJECT_NAME, new JmxTestBean()); @@ -805,4 +838,41 @@ public boolean includeBean(Class beanClass, String beanName) { } } + + public interface SomethingMBean {} + + public static class Something implements SomethingMBean {} + + + public static class ProperSomethingFactoryBean implements FactoryBean { + + public Something getObject() { + return new Something(); + } + + public Class getObjectType() { + return Something.class; + } + + public boolean isSingleton() { + return true; + } + } + + + public static class NullSomethingFactoryBean implements FactoryBean { + + public Something getObject() { + return null; + } + + public Class getObjectType() { + return Something.class; + } + + public boolean isSingleton() { + return true; + } + } + }