From 9ae8180bcf9103e7b5fa6a3da0f355e8093163c4 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 10 Mar 2011 14:30:27 +0000 Subject: [PATCH] INT-1830: add tests and workaround for MBeanExporters triggering eager loading --- .../monitor/IntegrationMBeanExporter.java | 112 ++++++------ .../src/test/java/log4j.properties | 3 +- .../MBeanExporterIntegrationTests.java | 169 ++++++++++++++++++ .../integration/monitor/oref-channel.xml | 33 ++++ .../oref-factory-channel-autodetect.xml | 24 +++ .../monitor/oref-factory-channel.xml | 28 +++ .../monitor/oref-factory-nonchannel.xml | 35 ++++ .../integration/monitor/oref-nonchannel.xml | 35 ++++ 8 files changed, 387 insertions(+), 52 deletions(-) create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-channel.xml create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel-autodetect.xml create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel.xml create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-nonchannel.xml create mode 100644 spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-nonchannel.xml 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 592b5939de..0a9197effc 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 @@ -40,6 +40,7 @@ import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.NameMatchMethodPointcutAdvisor; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.ListableBeanFactory; @@ -140,10 +141,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP private final MetadataNamingStrategy namingStrategy = new MetadataNamingStrategy(attributeSource); - private final Set mbeanExporters = new HashSet(); - - private final Set excludedBeansForOtherExporters = new HashSet(); - public IntegrationMBeanExporter() { super(); // Shouldn't be necessary, but to be on the safe side... @@ -186,11 +183,6 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - if (!initialized) { - initialized = true; - collectMBeanExporters(); - } - if (bean instanceof Advised) { for (Advisor advisor : ((Advised) bean).getAdvisors()) { Advice advice = advisor.getAdvice(); @@ -202,20 +194,22 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } } + boolean foundMetrics = false; + if (bean instanceof MessageHandler) { SimpleMessageHandlerMetrics monitor = new SimpleMessageHandlerMetrics((MessageHandler) bean); Object advised = applyHandlerInterceptor(bean, monitor, beanClassLoader); handlers.add(monitor); - excludeBeanInOtherExporters(beanName); - return advised; + foundMetrics = true; + bean = advised; } if (bean instanceof MessageSource) { SimpleMessageSourceMetrics monitor = new SimpleMessageSourceMetrics((MessageSource) bean); Object advised = applySourceInterceptor(bean, monitor, beanClassLoader); sources.add(monitor); - excludeBeanInOtherExporters(beanName); - return advised; + foundMetrics = true; + bean = advised; } if (bean instanceof MessageChannel) { @@ -224,33 +218,39 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP if (bean instanceof PollableChannel) { if (target instanceof QueueChannel) { monitor = new QueueChannelMetrics((QueueChannel) target, beanName); - } else { + } + else { monitor = new PollableChannelMetrics(target, beanName); } - } else { + } + else { monitor = new DirectChannelMetrics(target, beanName); } Object advised = applyChannelInterceptor(bean, monitor, beanClassLoader); channels.add(monitor); - excludeBeanInOtherExporters(beanName); - return advised; + foundMetrics = true; + bean = advised; } - return bean; - } - /** - * Make sure the named bean is not exposed in any other MBean exporters in the context. Called for beans that are - * being directly monitored by this exporter, but are wrapped in a proxy so cannot be directly exposed by a normal - * exporter. - * - * @param beanName the bean name to exclude - */ - private void excludeBeanInOtherExporters(String beanName) { - excludedBeansForOtherExporters.add(beanName); - String[] excluded = excludedBeansForOtherExporters.toArray(new String[0]); - for (MBeanExporter exporter : mbeanExporters) { - exporter.setExcludedBeans(excluded); + if (foundMetrics) { + // Only force the other exporters to initialize if we are sure we need to... + if (!initialized) { + try { + collectMBeanExporters(); + initialized = true; + } + catch (BeanCreationException e) { + // Ignore + if (logger.isDebugEnabled()) { + logger.debug("Ignoring BeanCreationException while instantiating MBeanExporter during creation of metrics for beanName=[" + + beanName + "]"); + } + } + } } + + return bean; + } /** @@ -267,7 +267,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP Object mbeanToExpose = null; if (isMBean(bean.getClass())) { mbeanToExpose = bean; - } else { + } + else { DynamicMBean adaptedBean = adaptMBeanIfPossible(bean); if (adaptedBean != null) { mbeanToExpose = adaptedBean; @@ -279,7 +280,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP + "]"); } doRegister(mbeanToExpose, objectName); - } else { + } + else { if (logger.isInfoEnabled()) { logger.info("Located managed bean '" + beanKey + "': registering with JMX server as MBean [" + objectName + "]"); @@ -289,7 +291,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP // injectNotificationPublisherIfNecessary(bean, mbean, objectName); } return objectName; - } catch (JMException e) { + } + catch (JMException e) { throw new UnableToRegisterMBeanException("Unable to register MBean [" + bean + "] with key '" + beanKey + "'", e); } @@ -311,7 +314,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP this.lifecycleLock.lock(); try { return this.running; - } finally { + } + finally { this.lifecycleLock.unlock(); } } @@ -326,7 +330,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP logger.info("started " + this); } } - } finally { + } + finally { this.lifecycleLock.unlock(); } } @@ -341,7 +346,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP logger.info("stopped " + this); } } - } finally { + } + finally { this.lifecycleLock.unlock(); } } @@ -351,7 +357,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP try { this.stop(); callback.run(); - } finally { + } + finally { this.lifecycleLock.unlock(); } } @@ -370,16 +377,13 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } /** - * Force early initialization of other MBean exporters and collect them for later use to ensure that they don't try - * to register the same beans that this exporter is covering. + * Force early initialization of other MBean exporters to ensure that they don't try to register the same beans that + * this exporter is covering. */ private void collectMBeanExporters() { String[] beanNames = beanFactory.getBeanNamesForType(MBeanExporter.class, false, false); for (String beanName : beanNames) { - MBeanExporter bean = beanFactory.getBean(beanName, MBeanExporter.class); - if (bean != this) { - mbeanExporters.add((MBeanExporter) bean); - } + beanFactory.getBean(beanName, MBeanExporter.class); } } @@ -568,7 +572,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP } try { return extractTarget(advised.getTargetSource().getTarget()); - } catch (Exception e) { + } + catch (Exception e) { logger.error("Could not extract target", e); return null; } @@ -580,7 +585,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP if (bean instanceof Advised) { ((Advised) bean).addAdvisor(advisor); return bean; - } else { + } + else { ProxyFactory proxyFactory = new ProxyFactory(bean); proxyFactory.addAdvisor(advisor); /** @@ -619,7 +625,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP return ""; } StringBuilder builder = new StringBuilder(); - + for (Object key : objectNameStaticProperties.keySet()) { builder.append("," + key + "=" + objectNameStaticProperties.get(key)); } @@ -646,7 +652,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP Object field = null; try { field = extractTarget(getField(endpoint, "handler")); - } catch (Exception e) { + } + catch (Exception e) { logger.trace("Could not get handler from bean = " + beanName); } if (field == monitor.getMessageHandler()) { @@ -665,7 +672,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP if (targetSource != null) { try { target = targetSource.getTarget(); - } catch (Exception e) { + } + catch (Exception e) { logger.debug("Could not get handler from bean = " + name); } } @@ -726,7 +734,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP Object field = null; try { field = extractTarget(getField(endpoint, "source")); - } catch (Exception e) { + } + catch (Exception e) { logger.trace("Could not get source from bean = " + beanName); } if (field == monitor.getMessageSource()) { @@ -745,7 +754,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP if (targetSource != null) { try { target = targetSource.getTarget(); - } catch (Exception e) { + } + catch (Exception e) { logger.debug("Could not get handler from bean = " + name); } } diff --git a/spring-integration-jmx/src/test/java/log4j.properties b/spring-integration-jmx/src/test/java/log4j.properties index f3be4a6292..9246efaccb 100644 --- a/spring-integration-jmx/src/test/java/log4j.properties +++ b/spring-integration-jmx/src/test/java/log4j.properties @@ -6,5 +6,6 @@ log4j.appender.stdout.layout.ConversionPattern=%d{ABSOLUTE} %5p %t %c{2}:%L - %m log4j.category.org.springframework=WARN -#log4j.category.org.springframework.integration=DEBUG +log4j.category.org.springframework.integration=DEBUG +log4j.category.org.springframework.beans.factory=DEBUG #log4j.category.org.springframework.integration.monitor=TRACE 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 new file mode 100644 index 0000000000..92f6e81c79 --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/MBeanExporterIntegrationTests.java @@ -0,0 +1,169 @@ +/* + * Copyright 2009-2010 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. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * 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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.Date; + +import javax.management.MBeanServer; +import javax.management.ObjectName; + +import org.junit.After; +import org.junit.Test; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.support.GenericXmlApplicationContext; +import org.springframework.integration.MessageChannel; +import org.springframework.jmx.export.annotation.ManagedResource; +import org.springframework.util.Assert; + +public class MBeanExporterIntegrationTests { + + private IntegrationMBeanExporter messageChannelsMonitor; + + private GenericXmlApplicationContext context; + + @After + public void close() { + if (context!=null) { + context.close(); + } + } + + @Test + public void testCircularReferenceNoChannel() throws Exception { + context = new GenericXmlApplicationContext(getClass(), "oref-nonchannel.xml"); + messageChannelsMonitor = context.getBean(IntegrationMBeanExporter.class); + assertNotNull(messageChannelsMonitor); + } + + @Test + public void testCircularReferenceNoChannelInFactoryBean() throws Exception { + context = new GenericXmlApplicationContext(getClass(), "oref-factory-nonchannel.xml"); + messageChannelsMonitor = context.getBean(IntegrationMBeanExporter.class); + assertNotNull(messageChannelsMonitor); + } + + @Test + public void testCircularReferenceWithChannel() throws Exception { + context = new GenericXmlApplicationContext(getClass(), "oref-channel.xml"); + messageChannelsMonitor = context.getBean(IntegrationMBeanExporter.class); + assertNotNull(messageChannelsMonitor); + } + + @Test + public void testCircularReferenceWithChannelInFactoryBean() throws Exception { + context = new GenericXmlApplicationContext(getClass(), "oref-factory-channel.xml"); + messageChannelsMonitor = context.getBean(IntegrationMBeanExporter.class); + assertNotNull(messageChannelsMonitor); + assertTrue(Arrays.asList(messageChannelsMonitor.getChannelNames()).contains("anonymous")); + MBeanServer server = context.getBean(MBeanServer.class); + assertEquals(1, server.queryNames(ObjectName.getInstance("com.foo:*"), null).size()); + assertEquals(1, server.queryNames(ObjectName.getInstance("org.springframework.integration:name=anonymous,*"), null).size()); + } + + @Test + public void testCircularReferenceWithChannelInFactoryBeanAutodetected() throws Exception { + context = new GenericXmlApplicationContext(getClass(), "oref-factory-channel-autodetect.xml"); + messageChannelsMonitor = context.getBean(IntegrationMBeanExporter.class); + assertNotNull(messageChannelsMonitor); + } + + public static class DateFactoryBean implements FactoryBean { + + private Date date; + + public void setDate(Date date) { + this.date = date; + } + + // This has the potential to blow up if called before setDate(). + // Depends on bean instantiation order and IntegrationMBeanExporter + // can influence that by aggressively instantiating other MBeanExporters + public Date getObject() throws Exception { + Assert.state(date != null, "A date must be provided"); + return date; + } + + public Class getObjectType() { + return Date.class; + } + + public boolean isSingleton() { + return true; + } + + } + + public static class DateHolder implements InitializingBean { + + private Date date; + + public void setDate(Date date) { + this.date = date; + } + + public void afterPropertiesSet() throws Exception { + Assert.state(date != null, "A date must be provided"); + } + + } + + public static class MetricFactoryBean implements FactoryBean { + + private MessageChannel channel; + private Metric date = new Metric(); + + public void setChannel(MessageChannel channel) { + this.channel = channel; + } + + public Metric getObject() throws Exception { + Assert.state(channel != null, "A channel must be provided"); + return date; + } + + public Class getObjectType() { + return Metric.class; + } + + public boolean isSingleton() { + return true; + } + + } + + @ManagedResource + public static class Metric { + + } + + public static class MetricHolder implements InitializingBean { + + private MessageChannel channel; + + public void setChannel(MessageChannel channel) { + this.channel = channel; + } + + public void afterPropertiesSet() throws Exception { + Assert.state(channel != null, "A channel must be provided"); + } + + } + +} diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-channel.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-channel.xml new file mode 100644 index 0000000000..f2574bb309 --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-channel.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel-autodetect.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel-autodetect.xml new file mode 100644 index 0000000000..1cc4fb5fa0 --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel-autodetect.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel.xml new file mode 100644 index 0000000000..08a4b3494d --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-channel.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-nonchannel.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-nonchannel.xml new file mode 100644 index 0000000000..362a154078 --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-factory-nonchannel.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-nonchannel.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-nonchannel.xml new file mode 100644 index 0000000000..436c59d277 --- /dev/null +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/monitor/oref-nonchannel.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +