MBeanExporter should not implement SmartLifecycle but rather receive a ContextRefreshedEvent-like callback

This commit removes the immediate package dependency cycle between the context and jmx packages. A specific callback arrangement will follow in time for 4.1 RC1; at this point, it's temporarily back to registration kicked off by afterPropertiesSet again.

Issue: SPR-8045
This commit is contained in:
Juergen Hoeller 2014-07-07 21:45:40 +02:00
parent b559f15a00
commit 1115374188
4 changed files with 89 additions and 199 deletions

View File

@ -50,7 +50,6 @@ import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.context.SmartLifecycle;
import org.springframework.core.Constants;
import org.springframework.jmx.export.assembler.AutodetectCapableMBeanInfoAssembler;
import org.springframework.jmx.export.assembler.MBeanInfoAssembler;
@ -99,7 +98,7 @@ import org.springframework.util.ObjectUtils;
* @see MBeanExporterListener
*/
public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExportOperations,
BeanClassLoaderAware, BeanFactoryAware, InitializingBean, DisposableBean, SmartLifecycle {
BeanClassLoaderAware, BeanFactoryAware, InitializingBean, DisposableBean {
/**
* Autodetection mode indicating that no autodetection should be used.
@ -154,6 +153,12 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
/** The strategy to use for creating ObjectNames for an object */
private ObjectNamingStrategy namingStrategy = new KeyNamingStrategy();
/** Indicates whether Spring should modify generated ObjectNames */
private boolean ensureUniqueRuntimeObjectNames = true;
/** Indicates whether Spring should expose the managed resource ClassLoader in the MBean */
private boolean exposeManagedResourceClassLoader = true;
/** A set of bean names that should be excluded from autodetection */
private Set<String> excludedBeans;
@ -167,28 +172,12 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
private final Map<NotificationListenerBean, ObjectName[]> registeredNotificationListeners =
new LinkedHashMap<NotificationListenerBean, ObjectName[]>();
/** Indicates whether Spring should modify generated ObjectNames */
private boolean ensureUniqueRuntimeObjectNames = true;
/** Indicates whether Spring should expose the managed resource ClassLoader in the MBean */
private boolean exposeManagedResourceClassLoader = true;
/** Indicate whether to auto-startup within the container-managed lifecycle */
private boolean autoStartup = true;
/** Indicate the phase to use within the container-managed lifecycle */
private int phase = Integer.MAX_VALUE;
/** Stores the ClassLoader to use for generating lazy-init proxies */
private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
/** Stores the BeanFactory for use in autodetection process */
private ListableBeanFactory beanFactory;
private boolean running = false;
private final Object lifecycleMonitor = new Object();
/**
* Supply a {@code Map} of beans to be registered with the JMX
@ -295,6 +284,31 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
this.namingStrategy = namingStrategy;
}
/**
* Indicates whether Spring should ensure that {@link ObjectName ObjectNames}
* generated by the configured {@link ObjectNamingStrategy} for
* runtime-registered MBeans ({@link #registerManagedResource}) should get
* modified: to ensure uniqueness for every instance of a managed {@code Class}.
* <p>The default value is {@code true}.
* @see #registerManagedResource
* @see JmxUtils#appendIdentityToObjectName(javax.management.ObjectName, Object)
*/
public void setEnsureUniqueRuntimeObjectNames(boolean ensureUniqueRuntimeObjectNames) {
this.ensureUniqueRuntimeObjectNames = ensureUniqueRuntimeObjectNames;
}
/**
* Indicates whether or not the managed resource should be exposed on the
* {@link Thread#getContextClassLoader() thread context ClassLoader} before
* allowing any invocations on the MBean to occur.
* <p>The default value is {@code true}, exposing a {@link SpringModelMBean}
* which performs thread context ClassLoader management. Switch this flag off to
* expose a standard JMX {@link javax.management.modelmbean.RequiredModelMBean}.
*/
public void setExposeManagedResourceClassLoader(boolean exposeManagedResourceClassLoader) {
this.exposeManagedResourceClassLoader = exposeManagedResourceClassLoader;
}
/**
* Set the list of names for beans that should be excluded from autodetection.
*/
@ -358,61 +372,6 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
notificationListeners.toArray(new NotificationListenerBean[notificationListeners.size()]);
}
/**
* Indicates whether Spring should ensure that {@link ObjectName ObjectNames}
* generated by the configured {@link ObjectNamingStrategy} for
* runtime-registered MBeans ({@link #registerManagedResource}) should get
* modified: to ensure uniqueness for every instance of a managed {@code Class}.
* <p>The default value is {@code true}.
* @see #registerManagedResource
* @see JmxUtils#appendIdentityToObjectName(javax.management.ObjectName, Object)
*/
public void setEnsureUniqueRuntimeObjectNames(boolean ensureUniqueRuntimeObjectNames) {
this.ensureUniqueRuntimeObjectNames = ensureUniqueRuntimeObjectNames;
}
/**
* Indicates whether or not the managed resource should be exposed on the
* {@link Thread#getContextClassLoader() thread context ClassLoader} before
* allowing any invocations on the MBean to occur.
* <p>The default value is {@code true}, exposing a {@link SpringModelMBean}
* which performs thread context ClassLoader management. Switch this flag off to
* expose a standard JMX {@link javax.management.modelmbean.RequiredModelMBean}.
*/
public void setExposeManagedResourceClassLoader(boolean exposeManagedResourceClassLoader) {
this.exposeManagedResourceClassLoader = exposeManagedResourceClassLoader;
}
/**
* Set whether to automatically export MBeans after initialization.
* <p>Default is "true"; set this to "false" to allow for manual startup
* through the {@link #start()} method.
*/
public void setAutoStartup(boolean autoStartup) {
this.autoStartup = autoStartup;
}
@Override
public boolean isAutoStartup() {
return this.autoStartup;
}
/**
* Specify the phase in which the MBeans should be exported to the
* JMX domain. The startup order proceeds from lowest to highest, and
* the shutdown order is the reverse of that. By default this value
* is {@code Integer.MAX_VALUE} meaning that MBeans are exported
* as late as possible and removed from the domain as soon as possible.
*/
public void setPhase(int phase) {
this.phase = phase;
}
@Override
public int getPhase() {
return this.phase;
}
@Override
public void setBeanClassLoader(ClassLoader classLoader) {
this.beanClassLoader = classLoader;
@ -436,6 +395,11 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
}
}
//---------------------------------------------------------------------
// Lifecycle in bean factory: automatically register/unregister beans
//---------------------------------------------------------------------
@Override
public void afterPropertiesSet() {
// If no server was provided then try to find one. This is useful in an environment
@ -443,62 +407,36 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
if (this.server == null) {
this.server = JmxUtils.locateMBeanServer();
}
register(); // TODO: to be replaced with some ContextRefreshedEvent-like callback
}
//---------------------------------------------------------------------
// Implementation of SmartLifecycle interface
//---------------------------------------------------------------------
@Override
public void start() {
synchronized (this.lifecycleMonitor) {
try {
registerBeans();
registerNotificationListeners();
}
catch (RuntimeException ex) {
// Unregister beans already registered by this exporter.
doStop();
throw ex;
}
this.running = true;
}
}
@Override
public void stop() {
synchronized (this.lifecycleMonitor) {
doStop();
}
}
@Override
public void stop(Runnable callback) {
synchronized (this.lifecycleMonitor) {
doStop();
callback.run();
}
}
@Override
public boolean isRunning() {
synchronized (this.lifecycleMonitor) {
return this.running;
/**
* Kick off bean registration automatically when deployed in an {@code ApplicationContext}.
* @see #registerBeans()
*/
public void register() {
try {
logger.info("Registering beans for JMX exposure on startup");
registerBeans();
registerNotificationListeners();
}
catch (RuntimeException ex) {
// Unregister beans already registered by this exporter.
unregisterNotificationListeners();
unregisterBeans();
throw ex;
}
}
/**
* Unregisters all beans that this exported has exposed via JMX
* when the enclosing {@code ApplicationContext} is destroyed.
*/
@Override
public void destroy() {
synchronized (this.lifecycleMonitor) {
doStop();
}
}
private void doStop() {
logger.info("Unregistering JMX-exposed beans on shutdown");
unregisterNotificationListeners();
unregisterBeans();
this.running = false;
}
@ -568,8 +506,6 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
* implementation of the {@code ObjectNamingStrategy} interface being used.
*/
protected void registerBeans() {
logger.info("Registering beans for JMX exposure");
// The beans property may be null, for example if we are relying solely on autodetection.
if (this.beans == null) {
this.beans = new HashMap<String, Object>();
@ -587,7 +523,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo
}
if (mode == AUTODETECT_MBEAN || mode == AUTODETECT_ALL) {
// Autodetect any beans that are already MBeans.
logger.info("Autodetecting user-defined JMX MBeans");
logger.debug("Autodetecting user-defined JMX MBeans");
autodetectMBeans();
}
// Allow the assembler a chance to vote for bean inclusion.

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2014 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.
@ -22,6 +22,7 @@ import javax.management.ObjectName;
import org.junit.After;
import org.junit.Before;
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.support.GenericApplicationContext;
@ -52,12 +53,14 @@ public abstract class AbstractMBeanServerTests {
protected MBeanServer server;
@Before
public final void setUp() throws Exception {
this.server = MBeanServerFactory.createMBeanServer();
try {
onSetUp();
} catch (Exception ex) {
}
catch (Exception ex) {
releaseServer();
throw ex;
}
@ -94,13 +97,10 @@ public abstract class AbstractMBeanServerTests {
/**
* Start the specified {@link MBeanExporter}.
*
* @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
* @see org.springframework.context.Lifecycle#start()
*/
protected void start(MBeanExporter exporter) {
exporter.afterPropertiesSet();
exporter.start();
// exporter.register();
}
protected void assertIsRegistered(String message, ObjectName objectName) {

View File

@ -21,7 +21,6 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.management.Attribute;
import javax.management.InstanceNotFoundException;
import javax.management.JMException;
@ -40,11 +39,8 @@ import org.junit.rules.ExpectedException;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.support.ClassPathXmlApplicationContext;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.io.ClassPathResource;
import org.springframework.jmx.AbstractMBeanServerTests;
import org.springframework.jmx.IJmxTestBean;
import org.springframework.jmx.JmxTestBean;
@ -78,22 +74,10 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
private static final String OBJECT_NAME = "spring:test=jmxMBeanAdaptor";
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testRegisterNonNotificationListenerType() throws Exception {
Map listeners = new HashMap();
// put a non-NotificationListener instance in as a value...
listeners.put("*", this);
MBeanExporter exporter = new MBeanExporter();
thrown.expect(ClassCastException.class);
exporter.setNotificationListenerMappings(listeners);
}
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testRegisterNullNotificationListenerType() throws Exception {
Map listeners = new HashMap();
Map<String, NotificationListener> listeners = new HashMap<String, NotificationListener>();
// put null in as a value...
listeners.put("*", null);
MBeanExporter exporter = new MBeanExporter();
@ -102,10 +86,9 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
exporter.setNotificationListenerMappings(listeners);
}
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testRegisterNotificationListenerForNonExistentMBean() throws Exception {
Map listeners = new HashMap();
Map<String, NotificationListener> listeners = new HashMap<String, NotificationListener>();
NotificationListener dummyListener = new NotificationListener() {
@Override
public void handleNotification(Notification notification, Object handback) {
@ -139,7 +122,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
ObjectNameManager.getInstance(OBJECT_NAME));
}
finally {
exporter.stop();
exporter.destroy();
}
}
@ -162,7 +145,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
assertFalse("Assembler should not have been invoked", asm.invoked);
}
finally {
exporter.stop();
exporter.destroy();
}
}
@ -178,7 +161,8 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
assertNotNull(instance);
instance = server.getObjectInstance(ObjectNameManager.getInstance("spring:mbean3=true"));
assertNotNull(instance);
} finally {
}
finally {
ctx.close();
}
}
@ -194,7 +178,8 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
thrown.expect(InstanceNotFoundException.class);
server.getObjectInstance(ObjectNameManager.getInstance("spring:mbean=false"));
} finally {
}
finally {
ctx.close();
}
}
@ -215,7 +200,8 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
assertNotNull(server.getObjectInstance(oname));
name = (String) server.getAttribute(oname, "Name");
assertEquals("Invalid name returned", "Juergen Hoeller", name);
} finally {
}
finally {
ctx.close();
}
}
@ -225,31 +211,8 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
ConfigurableApplicationContext ctx = load("autodetectNoMBeans.xml");
try {
ctx.getBean("exporter");
} finally {
ctx.close();
}
}
@Test
public void testAutoStartupToFalse() throws Exception {
ConfigurableApplicationContext ctx = load("autodetectNoAutoStartup.xml");
try {
MBeanExporter exporter = ctx.getBean("exporter", MBeanExporter.class);
MBeanServer server = ctx.getBean("server", MBeanServer.class);
ObjectName on = ObjectNameManager.getInstance("spring:mbean=true");
try {
server.getObjectInstance(on);
fail("MBeans should not have been exported with autoStartup set to false");
}
catch (InstanceNotFoundException e) {
// expected
}
// Export manually
exporter.start();
assertNotNull(server.getObjectInstance(on)); // Should be exposed now.
} finally {
finally {
ctx.close();
}
}
@ -262,9 +225,9 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
MBeanExporter exporter = new MBeanExporter();
exporter.setBeans(getBeanMap());
exporter.setServer(server);
exporter.setListeners(new MBeanExporterListener[] { listener1, listener2 });
exporter.setListeners(listener1, listener2);
start(exporter);
exporter.stop();
exporter.destroy();
assertListener(listener1);
assertListener(listener2);
@ -279,7 +242,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
ProxyFactory factory = new ProxyFactory();
factory.setTarget(bean);
factory.addAdvice(new NopInterceptor());
factory.setInterfaces(new Class<?>[] { IJmxTestBean.class });
factory.setInterfaces(IJmxTestBean.class);
IJmxTestBean proxy = (IJmxTestBean) factory.getProxy();
String name = "bean:mmm=whatever";
@ -585,13 +548,13 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
exporter.setBeans(getBeanMap());
exporter.setServer(this.server);
MockMBeanExporterListener listener = new MockMBeanExporterListener();
exporter.setListeners(new MBeanExporterListener[] { listener });
exporter.setListeners(listener);
start(exporter);
assertIsRegistered("The bean was not registered with the MBeanServer",
ObjectNameManager.getInstance(OBJECT_NAME));
this.server.unregisterMBean(new ObjectName(OBJECT_NAME));
exporter.stop();
exporter.destroy();
assertEquals("Listener should not have been invoked (MBean previously unregistered by external agent)", 0,
listener.getUnregistered().size());
}
@ -698,6 +661,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
assertEquals("Incorrect ObjectName in unregister", desired, listener.getUnregistered().get(0));
}
private static class InvokeDetectAssembler implements MBeanInfoAssembler {
private boolean invoked = false;
@ -709,6 +673,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
private static class MockMBeanExporterListener implements MBeanExporterListener {
private List<ObjectName> registered = new ArrayList<ObjectName>();
@ -734,6 +699,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
private static class SelfNamingTestBean implements SelfNaming {
private ObjectName objectName;
@ -748,11 +714,13 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
public static interface PersonMBean {
String getName();
}
public static class Person implements PersonMBean {
private String name;
@ -767,6 +735,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
public static final class StubNotificationListener implements NotificationListener {
private List<Notification> notifications = new ArrayList<Notification>();
@ -781,6 +750,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
private static class RuntimeExceptionThrowingConstructorBean {
@SuppressWarnings("unused")
@ -789,6 +759,7 @@ public final class MBeanExporterTests extends AbstractMBeanServerTests {
}
}
private static final class NamedBeanAutodetectCapableMBeanInfoAssemblerStub extends
SimpleReflectiveMBeanInfoAssembler implements AutodetectCapableMBeanInfoAssembler {

View File

@ -1,17 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">
<bean id="server" class="org.springframework.jmx.support.MBeanServerFactoryBean"/>
<bean id="exporter" class="org.springframework.jmx.export.MBeanExporter">
<property name="server" ref="server"/>
<property name="autodetect" value="true"/>
<property name="autoStartup" value="false"/>
</bean>
<bean name="spring:mbean=true" class="org.springframework.jmx.export.TestDynamicMBean"/>
</beans>