From b263f126fa459749fc6bb12a8994734c5a6d1e30 Mon Sep 17 00:00:00 2001 From: nguyensach Date: Sat, 8 May 2021 14:26:25 +0900 Subject: [PATCH 1/2] Polish SpringApplicationAdminJmxAutoConfigurationTests See gh-26416 --- ...gApplicationAdminJmxAutoConfiguration.java | 11 +++--- ...icationAdminJmxAutoConfigurationTests.java | 34 +++++++++++++------ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java index 1ed6ca48994..947c717fa34 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java @@ -36,13 +36,13 @@ import org.springframework.jmx.export.MBeanExporter; * * @author Stephane Nicoll * @author Andy Wilkinson + * @author Nguyen Bao Sach * @since 1.3.0 * @see SpringApplicationAdminMXBean */ @Configuration(proxyBeanMethods = false) @AutoConfigureAfter(JmxAutoConfiguration.class) -@ConditionalOnProperty(prefix = "spring.application.admin", value = "enabled", havingValue = "true", - matchIfMissing = false) +@ConditionalOnProperty(prefix = "spring.application.admin", value = "enabled", havingValue = "true") public class SpringApplicationAdminJmxAutoConfiguration { /** @@ -61,11 +61,8 @@ public class SpringApplicationAdminJmxAutoConfiguration { public SpringApplicationAdminMXBeanRegistrar springApplicationAdminRegistrar( ObjectProvider mbeanExporters, Environment environment) throws MalformedObjectNameException { String jmxName = environment.getProperty(JMX_NAME_PROPERTY, DEFAULT_JMX_NAME); - if (mbeanExporters != null) { // Make sure to not register that MBean twice - for (MBeanExporter mbeanExporter : mbeanExporters) { - mbeanExporter.addExcludedBean(jmxName); - } - } + // Make sure to not register that MBean twice + mbeanExporters.forEach((mbeanExporter) -> mbeanExporter.addExcludedBean(jmxName)); return new SpringApplicationAdminMXBeanRegistrar(jmxName); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java index 12b2ebe7ac3..9c80ecec8de 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java @@ -50,6 +50,7 @@ import static org.junit.jupiter.api.Assertions.fail; * * @author Stephane Nicoll * @author Andy Wilkinson + * @author Nguyen Bao Sach */ class SpringApplicationAdminJmxAutoConfigurationTests { @@ -60,17 +61,10 @@ class SpringApplicationAdminJmxAutoConfigurationTests { private final MBeanServer server = ManagementFactory.getPlatformMBeanServer(); private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(MultipleMBeanExportersConfiguration.class, - SpringApplicationAdminJmxAutoConfiguration.class)); + .withConfiguration(AutoConfigurations.of(SpringApplicationAdminJmxAutoConfiguration.class)); @Test - void notRegisteredByDefault() { - this.contextRunner.run((context) -> assertThatExceptionOfType(InstanceNotFoundException.class) - .isThrownBy(() -> this.server.getObjectInstance(createDefaultObjectName()))); - } - - @Test - void registeredWithProperty() { + void WhenThereAreNotAnyMBeanExporters() { this.contextRunner.withPropertyValues(ENABLE_ADMIN_PROP).run((context) -> { ObjectName objectName = createDefaultObjectName(); ObjectInstance objectInstance = this.server.getObjectInstance(objectName); @@ -79,9 +73,27 @@ class SpringApplicationAdminJmxAutoConfigurationTests { } @Test - void registerWithCustomJmxName() { + void notRegisteredByDefaultWhenThereAreMultipleMBeanExporters() { + this.contextRunner.withUserConfiguration(MultipleMBeanExportersConfiguration.class) + .run((context) -> assertThatExceptionOfType(InstanceNotFoundException.class) + .isThrownBy(() -> this.server.getObjectInstance(createDefaultObjectName()))); + } + + @Test + void registeredWithPropertyWhenThereAreMultipleMBeanExporters() { + this.contextRunner.withUserConfiguration(MultipleMBeanExportersConfiguration.class) + .withPropertyValues(ENABLE_ADMIN_PROP).run((context) -> { + ObjectName objectName = createDefaultObjectName(); + ObjectInstance objectInstance = this.server.getObjectInstance(objectName); + assertThat(objectInstance).as("Lifecycle bean should have been registered").isNotNull(); + }); + } + + @Test + void registerWithCustomJmxNameWhenThereAreMultipleMBeanExporters() { String customJmxName = "org.acme:name=FooBar"; - this.contextRunner.withSystemProperties("spring.application.admin.jmx-name=" + customJmxName) + this.contextRunner.withUserConfiguration(MultipleMBeanExportersConfiguration.class) + .withSystemProperties("spring.application.admin.jmx-name=" + customJmxName) .withPropertyValues(ENABLE_ADMIN_PROP).run((context) -> { try { this.server.getObjectInstance(createObjectName(customJmxName)); From 0e2bb5f1792e8106eadebf8f813174ad00b27c05 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 11 May 2021 16:42:45 +0200 Subject: [PATCH 2/2] Polish "Polish SpringApplicationAdminJmxAutoConfigurationTests" See gh-26416 --- .../SpringApplicationAdminJmxAutoConfiguration.java | 11 +++++++---- ...ringApplicationAdminJmxAutoConfigurationTests.java | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java index 947c717fa34..1ed6ca48994 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfiguration.java @@ -36,13 +36,13 @@ import org.springframework.jmx.export.MBeanExporter; * * @author Stephane Nicoll * @author Andy Wilkinson - * @author Nguyen Bao Sach * @since 1.3.0 * @see SpringApplicationAdminMXBean */ @Configuration(proxyBeanMethods = false) @AutoConfigureAfter(JmxAutoConfiguration.class) -@ConditionalOnProperty(prefix = "spring.application.admin", value = "enabled", havingValue = "true") +@ConditionalOnProperty(prefix = "spring.application.admin", value = "enabled", havingValue = "true", + matchIfMissing = false) public class SpringApplicationAdminJmxAutoConfiguration { /** @@ -61,8 +61,11 @@ public class SpringApplicationAdminJmxAutoConfiguration { public SpringApplicationAdminMXBeanRegistrar springApplicationAdminRegistrar( ObjectProvider mbeanExporters, Environment environment) throws MalformedObjectNameException { String jmxName = environment.getProperty(JMX_NAME_PROPERTY, DEFAULT_JMX_NAME); - // Make sure to not register that MBean twice - mbeanExporters.forEach((mbeanExporter) -> mbeanExporter.addExcludedBean(jmxName)); + if (mbeanExporters != null) { // Make sure to not register that MBean twice + for (MBeanExporter mbeanExporter : mbeanExporters) { + mbeanExporter.addExcludedBean(jmxName); + } + } return new SpringApplicationAdminMXBeanRegistrar(jmxName); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java index 9c80ecec8de..389ec27b0a1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/admin/SpringApplicationAdminJmxAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 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. @@ -64,7 +64,7 @@ class SpringApplicationAdminJmxAutoConfigurationTests { .withConfiguration(AutoConfigurations.of(SpringApplicationAdminJmxAutoConfiguration.class)); @Test - void WhenThereAreNotAnyMBeanExporters() { + void notRegisteredWhenThereAreNoMBeanExporter() { this.contextRunner.withPropertyValues(ENABLE_ADMIN_PROP).run((context) -> { ObjectName objectName = createDefaultObjectName(); ObjectInstance objectInstance = this.server.getObjectInstance(objectName);