Fix ParentAwareNamingStrategy and JMX auto-config

Fix ParentAwareNamingStrategy to set ObjectName properties for the
'identity' and 'context' attributes. Also update JmxAutoConfiguration
to ensure that the ParentAwareNamingStrategy is created in each context
and that the `mbeanExporter` bean is created. Prior to this commit the
nested @EnableMBeanExport class always meant that the mbeanExporter
condition never matched.

Fixes gh-2243

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-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.
@@ -18,30 +18,28 @@ package org.springframework.boot.autoconfigure.jmx;
 
 import javax.management.MBeanServer;
 
+import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
-import org.springframework.beans.factory.FactoryBean;
-import org.springframework.beans.factory.InitializingBean;
-import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.SearchStrategy;
+import org.springframework.boot.bind.RelaxedPropertyResolver;
+import org.springframework.context.EnvironmentAware;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.EnableMBeanExport;
-import org.springframework.context.annotation.MBeanExportConfiguration;
+import org.springframework.context.annotation.MBeanExportConfiguration.SpecificPlatform;
 import org.springframework.core.env.Environment;
-import org.springframework.core.type.StandardAnnotationMetadata;
 import org.springframework.jmx.export.MBeanExporter;
 import org.springframework.jmx.export.annotation.AnnotationJmxAttributeSource;
 import org.springframework.jmx.export.annotation.AnnotationMBeanExporter;
 import org.springframework.jmx.export.naming.ObjectNamingStrategy;
 import org.springframework.jmx.support.MBeanServerFactoryBean;
-import org.springframework.jmx.support.WebSphereMBeanServerFactoryBean;
-import org.springframework.jndi.JndiObjectFactoryBean;
-import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
+import org.springframework.jmx.support.RegistrationPolicy;
+import org.springframework.util.StringUtils;
 
 /**
  * {@link EnableAutoConfiguration Auto-configuration} to enable/disable Spring's
@@ -54,35 +52,45 @@ import org.springframework.util.ClassUtils;
 @Configuration
 @ConditionalOnClass({ MBeanExporter.class })
 @ConditionalOnExpression("${spring.jmx.enabled:true}")
-public class JmxAutoConfiguration {
+public class JmxAutoConfiguration implements EnvironmentAware, BeanFactoryAware {
 
-	@Autowired
-	private Environment environment;
+	private RelaxedPropertyResolver propertyResolver;
 
-	@Autowired
 	private BeanFactory beanFactory;
 
-	@Autowired
-	private ObjectNamingStrategy namingStrategy;
+	@Override
+	public void setEnvironment(Environment environment) {
+		this.propertyResolver = new RelaxedPropertyResolver(environment, "spring.jmx.");
+	}
+
+	@Override
+	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
+		this.beanFactory = beanFactory;
+	}
 
 	@Bean
 	@ConditionalOnMissingBean(value = MBeanExporter.class, search = SearchStrategy.CURRENT)
-	public AnnotationMBeanExporter mbeanExporter() {
-		// Re-use the @EnableMBeanExport configuration
-		MBeanExportConfiguration config = new MBeanExportConfiguration();
-		config.setEnvironment(this.environment);
-		config.setBeanFactory(this.beanFactory);
-		config.setImportMetadata(new StandardAnnotationMetadata(Empty.class));
-		// But add a custom naming strategy
-		AnnotationMBeanExporter exporter = config.mbeanExporter();
-		exporter.setNamingStrategy(this.namingStrategy);
+	public AnnotationMBeanExporter mbeanExporter(ObjectNamingStrategy namingStrategy) {
+		AnnotationMBeanExporter exporter = new AnnotationMBeanExporter();
+		exporter.setRegistrationPolicy(RegistrationPolicy.FAIL_ON_EXISTING);
+		exporter.setNamingStrategy(namingStrategy);
+		String server = this.propertyResolver.getProperty("server", "mbeanServer");
+		if (StringUtils.hasLength(server)) {
+			exporter.setServer(this.beanFactory.getBean(server, MBeanServer.class));
+		}
 		return exporter;
 	}
 
 	@Bean
-	@ConditionalOnMissingBean(ObjectNamingStrategy.class)
+	@ConditionalOnMissingBean(value = ObjectNamingStrategy.class, search = SearchStrategy.CURRENT)
 	public ParentAwareNamingStrategy objectNamingStrategy() {
-		return new ParentAwareNamingStrategy(new AnnotationJmxAttributeSource());
+		ParentAwareNamingStrategy namingStrategy = new ParentAwareNamingStrategy(
+				new AnnotationJmxAttributeSource());
+		String defaultDomain = this.propertyResolver.getProperty("default-domain");
+		if (StringUtils.hasLength(defaultDomain)) {
+			namingStrategy.setDefaultDomain(defaultDomain);
+		}
+		return namingStrategy;
 	}
 
 	@Bean
@@ -99,63 +107,4 @@ public class JmxAutoConfiguration {
 
 	}
 
-	@EnableMBeanExport(defaultDomain = "${spring.jmx.default_domain:}", server = "${spring.jmx.server:mbeanServer}")
-	private static class Empty {
-
-	}
-
-	// Copied and adapted from MBeanExportConfiguration
-	private static enum SpecificPlatform {
-
-		WEBLOGIC("weblogic.management.Helper") {
-			@Override
-			public FactoryBean<?> getMBeanServerFactory() {
-				JndiObjectFactoryBean factory = new JndiObjectFactoryBean();
-				factory.setJndiName("java:comp/env/jmx/runtime");
-				return factory;
-			}
-		},
-
-		WEBSPHERE("com.ibm.websphere.management.AdminServiceFactory") {
-			@Override
-			public FactoryBean<MBeanServer> getMBeanServerFactory() {
-				return new WebSphereMBeanServerFactoryBean();
-			}
-		};
-
-		private final String identifyingClass;
-
-		private SpecificPlatform(String identifyingClass) {
-			this.identifyingClass = identifyingClass;
-		}
-
-		public MBeanServer getMBeanServer() {
-			try {
-				FactoryBean<?> factory = getMBeanServerFactory();
-				if (factory instanceof InitializingBean) {
-					((InitializingBean) factory).afterPropertiesSet();
-				}
-				Object server = factory.getObject();
-				Assert.isInstanceOf(MBeanServer.class, server);
-				return (MBeanServer) server;
-			}
-			catch (Exception ex) {
-				throw new IllegalStateException(ex);
-			}
-		}
-
-		protected abstract FactoryBean<?> getMBeanServerFactory();
-
-		public static SpecificPlatform get() {
-			ClassLoader classLoader = MBeanExportConfiguration.class.getClassLoader();
-			for (SpecificPlatform environment : values()) {
-				if (ClassUtils.isPresent(environment.identifyingClass, classLoader)) {
-					return environment;
-				}
-			}
-			return null;
-		}
-
-	}
-
 }

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/ParentAwareNamingStrategy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2013 the original author or authors.
+ * Copyright 2012-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.
@@ -16,6 +16,8 @@
 
 package org.springframework.boot.autoconfigure.jmx;
 
+import java.util.Hashtable;
+
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 
@@ -24,9 +26,13 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.jmx.export.metadata.JmxAttributeSource;
 import org.springframework.jmx.export.naming.MetadataNamingStrategy;
+import org.springframework.jmx.support.ObjectNameManager;
 import org.springframework.util.ObjectUtils;
 
 /**
+ * Extension of {@link MetadataNamingStrategy} that supports a parent
+ * {@link ApplicationContext}.
+ *
  * @author Dave Syer
  * @since 1.1.1
  */
@@ -34,7 +40,8 @@ public class ParentAwareNamingStrategy extends MetadataNamingStrategy implements
 		ApplicationContextAware {
 
 	private ApplicationContext applicationContext;
-	private boolean ensureUniqueRuntimeObjectNames = false;
+
+	private boolean ensureUniqueRuntimeObjectNames;
 
 	public ParentAwareNamingStrategy(JmxAttributeSource attributeSource) {
 		super(attributeSource);
@@ -50,16 +57,17 @@ public class ParentAwareNamingStrategy extends MetadataNamingStrategy implements
 	@Override
 	public ObjectName getObjectName(Object managedBean, String beanKey)
 			throws MalformedObjectNameException {
-		StringBuilder builder = new StringBuilder(beanKey);
-		if (parentContextContainsSameBean(this.applicationContext, beanKey)) {
-			builder.append(",context="
-					+ ObjectUtils.getIdentityHexString(this.applicationContext));
-		}
-		if (this.ensureUniqueRuntimeObjectNames) {
-			builder.append(",identity=" + ObjectUtils.getIdentityHexString(managedBean));
-		}
 		ObjectName name = super.getObjectName(managedBean, beanKey);
-		return name;
+		Hashtable<String, String> properties = new Hashtable<String, String>();
+		properties.putAll(name.getKeyPropertyList());
+		if (this.ensureUniqueRuntimeObjectNames) {
+			properties.put("identity", ObjectUtils.getIdentityHexString(managedBean));
+		}
+		else if (parentContextContainsSameBean(this.applicationContext, beanKey)) {
+			properties.put("context",
+					ObjectUtils.getIdentityHexString(this.applicationContext));
+		}
+		return ObjectNameManager.getInstance(name.getDomain(), properties);
 	}
 
 	@Override
@@ -68,19 +76,18 @@ public class ParentAwareNamingStrategy extends MetadataNamingStrategy implements
 		this.applicationContext = applicationContext;
 	}
 
-	private boolean parentContextContainsSameBean(ApplicationContext applicationContext,
+	private boolean parentContextContainsSameBean(ApplicationContext context,
 			String beanKey) {
-		if (applicationContext.getParent() != null) {
-			try {
-				this.applicationContext.getParent().getBean(beanKey);
-				return true;
-			}
-			catch (BeansException ex) {
-				return parentContextContainsSameBean(applicationContext.getParent(),
-						beanKey);
-			}
+		if (context.getParent() == null) {
+			return false;
+		}
+		try {
+			this.applicationContext.getParent().getBean(beanKey);
+			return true;
+		}
+		catch (BeansException ex) {
+			return parentContextContainsSameBean(context.getParent(), beanKey);
 		}
-		return false;
 	}
 
 }

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfigurationTests.java

@@ -74,7 +74,6 @@ public class JmxAutoConfigurationTests {
 		this.context.setEnvironment(env);
 		this.context.register(JmxAutoConfiguration.class);
 		this.context.refresh();
-
 		assertNotNull(this.context.getBean(MBeanExporter.class));
 	}
 
@@ -86,7 +85,6 @@ public class JmxAutoConfigurationTests {
 		this.context.setEnvironment(env);
 		this.context.register(TestConfiguration.class, JmxAutoConfiguration.class);
 		this.context.refresh();
-
 		this.context.getBean(MBeanExporter.class);
 	}
 
@@ -99,17 +97,16 @@ public class JmxAutoConfigurationTests {
 		this.context.setEnvironment(env);
 		this.context.register(TestConfiguration.class, JmxAutoConfiguration.class);
 		this.context.refresh();
-
 		MBeanExporter mBeanExporter = this.context.getBean(MBeanExporter.class);
 		assertNotNull(mBeanExporter);
 		MetadataNamingStrategy naming = (MetadataNamingStrategy) ReflectionTestUtils
-				.getField(mBeanExporter, "metadataNamingStrategy");
+				.getField(mBeanExporter, "namingStrategy");
 		assertEquals("my-test-domain",
 				ReflectionTestUtils.getField(naming, "defaultDomain"));
 	}
 
 	@Test
-	public void testParentContext() {
+	public void testBasicParentContext() {
 		this.context = new AnnotationConfigApplicationContext();
 		this.context.register(JmxAutoConfiguration.class);
 		this.context.refresh();
@@ -120,6 +117,18 @@ public class JmxAutoConfigurationTests {
 		this.context.refresh();
 	}
 
+	@Test
+	public void testParentContext() throws Exception {
+		this.context = new AnnotationConfigApplicationContext();
+		this.context.register(JmxAutoConfiguration.class, TestConfiguration.class);
+		this.context.refresh();
+		AnnotationConfigApplicationContext parent = this.context;
+		this.context = new AnnotationConfigApplicationContext();
+		this.context.setParent(parent);
+		this.context.register(JmxAutoConfiguration.class, TestConfiguration.class);
+		this.context.refresh();
+	}
+
 	@Configuration
 	public static class TestConfiguration {