From 0f05512e9af590a1aa73ff304df4d8b961fd4596 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 3 Mar 2015 21:36:14 +0200 Subject: [PATCH] INT-3664: Rework BPP in the `IntMBExporter` JIRA: https://jira.spring.io/browse/INT-3664 Since all metrics are already direct for the integration components, we don't do any proxying from `IntegrationMBeanExporter`, and even any other adjustments during BPP phases. Hence this stuff is already redundant for `IntegrationMBeanExporter`. In addition this change fix the `early access to the BeanFactory from BPP` issue. INT-3664: Address PR comments Doc Polish INT-3664: Polishing according the SF changes to the `PostProcessorRegistrationDelegate$BeanPostProcessorChecker` --- ...ltConfiguringBeanFactoryPostProcessor.java | 12 +- .../config/IntegrationRegistrar.java | 1 + .../monitor/IntegrationMBeanExporter.java | 213 +++++------------- .../MBeanExporterIntegrationTests.java | 9 + .../monitor/MessageMetricsAdviceTests.java | 41 +++- src/reference/docbook/jmx.xml | 10 + 6 files changed, 116 insertions(+), 170 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/DefaultConfiguringBeanFactoryPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/DefaultConfiguringBeanFactoryPostProcessor.java index a2a21181b5..cb062e6972 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/DefaultConfiguringBeanFactoryPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/DefaultConfiguringBeanFactoryPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -133,12 +133,16 @@ class DefaultConfiguringBeanFactoryPostProcessor implements BeanFactoryPostProce registry.registerBeanDefinition(IntegrationContextUtils.ERROR_CHANNEL_BEAN_NAME, new RootBeanDefinition(PublishSubscribeChannel.class)); - BeanDefinitionBuilder loggingHandlerBuilder = - BeanDefinitionBuilder.genericBeanDefinition(LoggingHandler.class).addConstructorArgValue("ERROR"); + BeanDefinition loggingHandler = + BeanDefinitionBuilder.genericBeanDefinition(LoggingHandler.class).addConstructorArgValue("ERROR") + .getBeanDefinition(); + + String errorLoggerBeanName = ERROR_LOGGER_BEAN_NAME + IntegrationConfigUtils.HANDLER_ALIAS_SUFFIX; + registry.registerBeanDefinition(errorLoggerBeanName, loggingHandler); BeanDefinitionBuilder loggingEndpointBuilder = BeanDefinitionBuilder.genericBeanDefinition(ConsumerEndpointFactoryBean.class) - .addPropertyValue("handler", loggingHandlerBuilder.getBeanDefinition()) + .addPropertyReference("handler", errorLoggerBeanName) .addPropertyValue("inputChannelName", IntegrationContextUtils.ERROR_CHANNEL_BEAN_NAME); BeanComponentDefinition componentDefinition = diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/IntegrationRegistrar.java b/spring-integration-core/src/main/java/org/springframework/integration/config/IntegrationRegistrar.java index f9fe88faad..11fb7235b5 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/IntegrationRegistrar.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/IntegrationRegistrar.java @@ -126,6 +126,7 @@ public class IntegrationRegistrar implements ImportBeanDefinitionRegistrar, Bean BeanDefinitionBuilder channelRegistryBuilder = BeanDefinitionBuilder .genericBeanDefinition(ChannelInitializer.AutoCreateCandidatesCollector.class); channelRegistryBuilder.addConstructorArgValue(new ManagedSet()); + channelRegistryBuilder.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); //SPR-12761 BeanDefinitionHolder channelRegistryHolder = new BeanDefinitionHolder(channelRegistryBuilder.getBeanDefinition(), IntegrationContextUtils.AUTO_CREATE_CHANNEL_CANDIDATES_BEAN_NAME); diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java index 33455a75bf..4d301e808e 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java +++ b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java @@ -24,7 +24,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.ReentrantLock; import javax.management.DynamicMBean; import javax.management.JMException; @@ -38,15 +37,10 @@ import org.springframework.aop.TargetSource; import org.springframework.aop.framework.Advised; import org.springframework.beans.BeansException; import org.springframework.beans.annotation.AnnotationBeanUtils; -import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.beans.factory.ListableBeanFactory; -import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.context.Lifecycle; -import org.springframework.context.SmartLifecycle; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.integration.channel.QueueChannel; import org.springframework.integration.channel.management.AbstractMessageChannelMetrics; @@ -117,8 +111,8 @@ import org.springframework.util.StringValueResolver; */ @ManagedResource @IntegrationManagedResource -public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostProcessor, BeanFactoryAware, - ApplicationContextAware, EmbeddedValueResolverAware, SmartLifecycle { +public class IntegrationMBeanExporter extends MBeanExporter implements ApplicationContextAware, + EmbeddedValueResolverAware { private static final Log logger = LogFactory.getLog(IntegrationMBeanExporter.class); @@ -126,8 +120,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP private final AnnotationJmxAttributeSource attributeSource = new IntegrationJmxAttributeSource(); - private ListableBeanFactory beanFactory; - private ApplicationContext applicationContext; private final Map anonymousHandlerCounters = new HashMap(); @@ -142,8 +134,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP private final Set channels = new HashSet(); - private final Map exposedBeans = new HashMap(); - private final Map channelsByName = new HashMap(); private final Map handlersByName = new HashMap(); @@ -158,14 +148,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP private final Map beansByEndpointName = new HashMap(); - private volatile boolean autoStartup = true; - - private volatile int phase = 0; - - private volatile boolean running; - - private final ReentrantLock lifecycleLock = new ReentrantLock(); - private String domain = DEFAULT_DOMAIN; private final Properties objectNameStaticProperties = new Properties(); @@ -184,8 +166,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP private final AtomicBoolean shuttingDown = new AtomicBoolean(); - private MessageHistoryConfigurer messageHistoryConfigurer; - private StringValueResolver embeddedValueResolver; private MetricsFactory metricsFactory = new DefaultMetricsFactory(); @@ -277,13 +257,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP this.enabledStatsPatterns = Arrays.copyOf(enabledStatsPatterns, enabledStatsPatterns.length); } - @Override - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { - super.setBeanFactory(beanFactory); - Assert.isInstanceOf(ListableBeanFactory.class, beanFactory, "A ListableBeanFactory is required."); - this.beanFactory = (ListableBeanFactory) beanFactory; - } - @Override public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { @@ -306,50 +279,73 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } @Override - public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - - if (IntegrationContextUtils.INTEGRATION_MESSAGE_HISTORY_CONFIGURER_BEAN_NAME.equals(beanName) - && bean instanceof MessageHistoryConfigurer) { - this.messageHistoryConfigurer = (MessageHistoryConfigurer) bean; - return bean; - } - - if (bean instanceof MessageHandlerMetrics) { + public void afterSingletonsInstantiated() { + Map messageHandlers = + this.applicationContext.getBeansOfType(MessageHandlerMetrics.class); + for (Entry entry : messageHandlers.entrySet()) { + String beanName = entry.getKey(); + MessageHandlerMetrics bean = entry.getValue(); if (this.handlerInAnonymousWrapper(bean) != null) { if (logger.isDebugEnabled()) { logger.debug("Skipping " + beanName + " because it wraps another handler"); } - return bean; + continue; } // If the handler is proxied, we have to extract the target to expose as an MBean. // The MetadataMBeanInfoAssembler does not support JDK dynamic proxies. MessageHandlerMetrics monitor = (MessageHandlerMetrics) extractTarget(bean); - handlers.add(monitor); + this.handlers.add(monitor); } - if (bean instanceof MessageSourceMetrics) { + Map messageSources = + this.applicationContext.getBeansOfType(MessageSourceMetrics.class); + for (Entry entry : messageSources.entrySet()) { // If the source is proxied, we have to extract the target to expose as an MBean. // The MetadataMBeanInfoAssembler does not support JDK dynamic proxies. - MessageSourceMetrics monitor = (MessageSourceMetrics) extractTarget(bean); - sources.add(monitor); + MessageSourceMetrics monitor = (MessageSourceMetrics) extractTarget(entry.getValue()); + this.sources.add(monitor); } - if (bean instanceof MessageChannel && bean instanceof MessageChannelMetrics - && bean instanceof NamedComponent) { + Map messageChannels = + this.applicationContext.getBeansOfType(MessageChannelMetrics.class); + for (Entry entry : messageChannels.entrySet()) { // If the channel is proxied, we have to extract the target to expose as an MBean. // The MetadataMBeanInfoAssembler does not support JDK dynamic proxies. - MessageChannelMetrics monitor = (MessageChannelMetrics) extractTarget(bean); - channels.add(monitor); + MessageChannelMetrics monitor = (MessageChannelMetrics) extractTarget(entry.getValue()); + this.channels.add(monitor); } - - if (bean instanceof MessageProducer && bean instanceof Lifecycle) { - Lifecycle target = (Lifecycle) extractTarget(bean); - if (!(target instanceof AbstractMessageProducingHandler)) { - this.inboundLifecycleMessageProducers.add(target); + Map messageProducers = + this.applicationContext.getBeansOfType(MessageProducer.class); + for (Entry entry : messageProducers.entrySet()) { + MessageProducer messageProducer = entry.getValue(); + if (messageProducer instanceof Lifecycle) { + Lifecycle target = (Lifecycle) extractTarget(messageProducer); + if (!(target instanceof AbstractMessageProducingHandler)) { + this.inboundLifecycleMessageProducers.add(target); + } } } + super.afterSingletonsInstantiated(); + try { + registerChannels(); + registerHandlers(); + registerSources(); + registerEndpoints(); - return bean; + if (this.applicationContext + .containsBean(IntegrationContextUtils.INTEGRATION_MESSAGE_HISTORY_CONFIGURER_BEAN_NAME)) { + Object messageHistoryConfigurer = this.applicationContext + .getBean(IntegrationContextUtils.INTEGRATION_MESSAGE_HISTORY_CONFIGURER_BEAN_NAME); + if (messageHistoryConfigurer instanceof MessageHistoryConfigurer) { + registerBeanInstance(messageHistoryConfigurer, + IntegrationContextUtils.INTEGRATION_MESSAGE_HISTORY_CONFIGURER_BEAN_NAME); + } + } + } + catch (RuntimeException e) { + unregisterBeans(); + throw e; + } } @@ -426,98 +422,11 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { - return bean; - } - - @Override - public final boolean isAutoStartup() { - return this.autoStartup; - } - - @Override - public final int getPhase() { - return this.phase; - } - - @Override - public final boolean isRunning() { - this.lifecycleLock.lock(); - try { - return this.running; - } - finally { - this.lifecycleLock.unlock(); - } - } - - @Override - public final void start() { - this.lifecycleLock.lock(); - try { - if (!this.running) { - this.doStart(); - this.running = true; - if (logger.isInfoEnabled()) { - logger.info("started " + this); - } - } - } - finally { - this.lifecycleLock.unlock(); - } - } - - @Override - public final void stop() { - this.lifecycleLock.lock(); - try { - if (this.running) { - this.doStop(); - this.running = false; - if (logger.isInfoEnabled()) { - logger.info("stopped " + this); - } - } - } - finally { - this.lifecycleLock.unlock(); - } - } - - @Override - public final void stop(Runnable callback) { - this.lifecycleLock.lock(); - try { - this.stop(); - callback.run(); - } - finally { - this.lifecycleLock.unlock(); - } - } - - protected void doStop() { - unregisterBeans(); + public void destroy() { + super.destroy(); channelsByName.clear(); handlersByName.clear(); sourcesByName.clear(); - } - - protected void doStart() { - registerChannels(); - registerHandlers(); - registerSources(); - registerEndpoints(); - if (this.messageHistoryConfigurer != null) { - this.registerBeanInstance(this.messageHistoryConfigurer, - IntegrationContextUtils.INTEGRATION_MESSAGE_HISTORY_CONFIGURER_BEAN_NAME); - } - } - - @Override - public void destroy() { - super.destroy(); for (MessageChannelMetrics monitor : channels) { logger.info("Summary on shutdown: " + monitor); } @@ -752,14 +661,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP return null; } - @Override - protected void registerBeans() { - if (!exposedBeans.isEmpty()) { - super.setBeans(exposedBeans); - super.registerBeans(); - } - } - @SuppressWarnings("unchecked") private void registerChannels() { for (MessageChannelMetrics monitor : channels) { @@ -852,11 +753,11 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } private void registerEndpoints() { - String[] names = beanFactory.getBeanNamesForType(AbstractEndpoint.class); + String[] names = this.applicationContext.getBeanNamesForType(AbstractEndpoint.class); Set endpointNames = new HashSet(); for (String name : names) { if (!beansByEndpointName.values().contains(name)) { - AbstractEndpoint endpoint = beanFactory.getBean(name, AbstractEndpoint.class); + AbstractEndpoint endpoint = this.applicationContext.getBean(name, AbstractEndpoint.class); String beanKey; name = endpoint.getComponentName(); String source; @@ -989,7 +890,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } // Assignment algorithm and bean id, with bean id pulled reflectively out of enclosing endpoint if possible - String[] names = beanFactory.getBeanNamesForType(AbstractEndpoint.class); + String[] names = this.applicationContext.getBeanNamesForType(AbstractEndpoint.class); String name = null; String endpointName = null; @@ -997,7 +898,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP Object endpoint = null; for (String beanName : names) { - endpoint = beanFactory.getBean(beanName); + endpoint = this.applicationContext.getBean(beanName); try { Object field = extractTarget(getField(endpoint, "handler")); if (field == monitor || @@ -1101,7 +1002,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } // Assignment algorithm and bean id, with bean id pulled reflectively out of enclosing endpoint if possible - String[] names = beanFactory.getBeanNamesForType(AbstractEndpoint.class); + String[] names = this.applicationContext.getBeanNamesForType(AbstractEndpoint.class); String name = null; String endpointName = null; @@ -1109,7 +1010,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP Object endpoint = null; for (String beanName : names) { - endpoint = beanFactory.getBean(beanName); + endpoint = this.applicationContext.getBean(beanName); Object field = null; try { field = extractTarget(getField(endpoint, "source")); diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java index 4bb93898e3..426037ce17 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java @@ -190,6 +190,15 @@ public class MBeanExporterIntegrationTests { } // Lifecycle method name assertEquals("start", startName); + + context.close(); + + context = new GenericXmlApplicationContext(getClass(), "lifecycle-no-source.xml"); + server = context.getBean(MBeanServer.class); + names = server.queryNames(ObjectName.getInstance("org.springframework.integration:type=ManagedEndpoint,*"), null); + assertEquals(1, names.size()); + names = server.queryNames(ObjectName.getInstance("org.springframework.integration:name=gateway,*"), null); + assertEquals(1, names.size()); } @Test diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessageMetricsAdviceTests.java b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessageMetricsAdviceTests.java index 3701a5e8c3..b616b9b986 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessageMetricsAdviceTests.java +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MessageMetricsAdviceTests.java @@ -10,6 +10,7 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ + package org.springframework.integration.monitor; import org.aopalliance.intercept.MethodInterceptor; @@ -19,9 +20,11 @@ import org.junit.Test; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.support.NameMatchMethodPointcutAdvisor; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.context.support.GenericApplicationContext; import org.springframework.integration.channel.NullChannel; import org.springframework.integration.support.MessageBuilder; +import org.springframework.integration.test.util.TestUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHandler; @@ -32,10 +35,13 @@ import org.springframework.util.ClassUtils; * @author Tareq Abedrabbo * @author Dave Syer * @author Gary Russell + * @author Artem Bilan * @since 2.0.4 */ public class MessageMetricsAdviceTests { + private ConfigurableListableBeanFactory beanFactory; + private IntegrationMBeanExporter mBeanExporter; private MessageHandler handler; @@ -44,12 +50,16 @@ public class MessageMetricsAdviceTests { @Before public void setUp() throws Exception { - channel = new NullChannel(); - mBeanExporter = new IntegrationMBeanExporter(); - mBeanExporter.setBeanFactory(new DefaultListableBeanFactory()); - mBeanExporter.setBeanClassLoader(ClassUtils.getDefaultClassLoader()); - mBeanExporter.afterPropertiesSet(); - handler = new DummyHandler(); + GenericApplicationContext applicationContext = TestUtils.createTestApplicationContext(); + this.beanFactory = applicationContext.getBeanFactory(); + this.channel = new NullChannel(); + this.mBeanExporter = new IntegrationMBeanExporter(); + this.mBeanExporter.setApplicationContext(applicationContext); + this.mBeanExporter.setBeanFactory(this.beanFactory); + this.mBeanExporter.setBeanClassLoader(ClassUtils.getDefaultClassLoader()); + this.mBeanExporter.afterPropertiesSet(); + this.handler = new DummyHandler(); + applicationContext.refresh(); } @Test @@ -59,11 +69,15 @@ public class MessageMetricsAdviceTests { NameMatchMethodPointcutAdvisor advisor = new NameMatchMethodPointcutAdvisor(interceptor); advisor.addMethodName("handleMessage"); - ProxyFactory factory = new ProxyFactory(handler); + ProxyFactory factory = new ProxyFactory(this.handler); factory.addAdvisor(advisor); MessageHandler advised = (MessageHandler) factory.getProxy(); - MessageHandler exported = (MessageHandler) mBeanExporter.postProcessAfterInitialization(advised, "test"); + this.beanFactory.registerSingleton("test", advised); + this.beanFactory.initializeBean(advised, "test"); + + mBeanExporter.afterSingletonsInstantiated(); + MessageHandler exported = this.beanFactory.getBean("test", MessageHandler.class); exported.handleMessage(MessageBuilder.withPayload("test").build()); } @@ -78,7 +92,11 @@ public class MessageMetricsAdviceTests { factory.addAdvisor(advisor); MessageChannel advised = (MessageChannel) factory.getProxy(); - MessageChannel exported = (MessageChannel) mBeanExporter.postProcessAfterInitialization(advised, "test"); + this.beanFactory.registerSingleton("test", advised); + this.beanFactory.initializeBean(advised, "test"); + + mBeanExporter.afterSingletonsInstantiated(); + MessageChannel exported = this.beanFactory.getBean("test", MessageChannel.class); exported.send(MessageBuilder.withPayload("test").build()); } @@ -91,6 +109,7 @@ public class MessageMetricsAdviceTests { public void handleMessage(Message message) throws MessagingException { invoked = true; } + } private static class DummyInterceptor implements MethodInterceptor { @@ -107,5 +126,7 @@ public class MessageMetricsAdviceTests { public String toString() { return super.toString() + "{" + "invoked=" + invoked + '}'; } + } + } diff --git a/src/reference/docbook/jmx.xml b/src/reference/docbook/jmx.xml index e8cc02d4ae..ea64dbe00b 100644 --- a/src/reference/docbook/jmx.xml +++ b/src/reference/docbook/jmx.xml @@ -860,6 +860,16 @@ public class ContextConfiguration { allowing the invocation of Lifecycle methods. + + IntegrationMBeanExporter changes + + The IntegrationMBeanExporter no longer implements + SmartLifecycle; this means that start() + and stop() operations are no longer available to register/unregister + MBeans. The MBeans are now registered during context initialization and unregistered + when the context is destroyed. + +