From e5a09b3b319777fae2b4ae07edd28918d7fabc9a Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Fri, 18 Mar 2022 09:35:29 -0500 Subject: [PATCH] Apply unique-names consistently in JmxAutoConfiguration Ensure that the `spring.jmx.unique-names` property is applied to the auto-configured `MBeanExporter` as well as the `ObjectNamingStrategy`. Fixes gh-29968 --- .../jmx/JmxAutoConfiguration.java | 3 + .../jmx/JmxAutoConfigurationTests.java | 135 ++++++++---------- 2 files changed, 61 insertions(+), 77 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfiguration.java index 75e76d1f540..33f15e5af47 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfiguration.java @@ -48,6 +48,7 @@ import org.springframework.util.StringUtils; * @author Christian Dupuis * @author Madhura Bhave * @author Artsiom Yudovin + * @author Scott Frederick * @since 1.0.0 */ @AutoConfiguration @@ -72,6 +73,8 @@ public class JmxAutoConfiguration { if (StringUtils.hasLength(serverBean)) { exporter.setServer(beanFactory.getBean(serverBean, MBeanServer.class)); } + boolean uniqueNames = this.environment.getProperty("spring.jmx.unique-names", Boolean.class, false); + exporter.setEnsureUniqueRuntimeObjectNames(uniqueNames); return exporter; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfigurationTests.java index 5caddce4197..ce11238e443 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jmx/JmxAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2022 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,12 +16,12 @@ package org.springframework.boot.autoconfigure.jmx; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.integration.IntegrationAutoConfiguration; -import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.boot.context.annotation.UserConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -32,114 +32,95 @@ import org.springframework.jmx.export.annotation.ManagedAttribute; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.jmx.export.naming.MetadataNamingStrategy; -import org.springframework.mock.env.MockEnvironment; +import org.springframework.jmx.export.naming.ObjectNamingStrategy; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests for {@link JmxAutoConfiguration}. * * @author Christian Dupuis * @author Artsiom Yudovin + * @author Scott Frederick */ class JmxAutoConfigurationTests { - private AnnotationConfigApplicationContext context; - - @AfterEach - void tearDown() { - if (this.context != null) { - this.context.close(); - if (this.context.getParent() != null) { - ((ConfigurableApplicationContext) this.context.getParent()).close(); - } - } - } + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(JmxAutoConfiguration.class)); @Test void testDefaultMBeanExport() { - this.context = new AnnotationConfigApplicationContext(); - this.context.register(JmxAutoConfiguration.class); - this.context.refresh(); - assertThatExceptionOfType(NoSuchBeanDefinitionException.class) - .isThrownBy(() -> this.context.getBean(MBeanExporter.class)); - } - - @Test - void testEnabledMBeanExport() { - MockEnvironment env = new MockEnvironment(); - env.setProperty("spring.jmx.enabled", "true"); - this.context = new AnnotationConfigApplicationContext(); - this.context.setEnvironment(env); - this.context.register(JmxAutoConfiguration.class); - this.context.refresh(); - assertThat(this.context.getBean(MBeanExporter.class)).isNotNull(); + this.contextRunner.run((context) -> { + assertThat(context).doesNotHaveBean(MBeanExporter.class); + assertThat(context).doesNotHaveBean(ObjectNamingStrategy.class); + }); } @Test void testDisabledMBeanExport() { - MockEnvironment env = new MockEnvironment(); - env.setProperty("spring.jmx.enabled", "false"); - this.context = new AnnotationConfigApplicationContext(); - this.context.setEnvironment(env); - this.context.register(TestConfiguration.class, JmxAutoConfiguration.class); - this.context.refresh(); - assertThatExceptionOfType(NoSuchBeanDefinitionException.class) - .isThrownBy(() -> this.context.getBean(MBeanExporter.class)); + this.contextRunner.withPropertyValues("spring.jmx.enabled=false").run((context) -> { + assertThat(context).doesNotHaveBean(MBeanExporter.class); + assertThat(context).doesNotHaveBean(ObjectNamingStrategy.class); + }); + } + + @Test + void testEnabledMBeanExport() { + this.contextRunner.withPropertyValues("spring.jmx.enabled=true").run((context) -> { + assertThat(context).hasSingleBean(MBeanExporter.class); + assertThat(context).hasSingleBean(ParentAwareNamingStrategy.class); + MBeanExporter exporter = context.getBean(MBeanExporter.class); + assertThat(exporter).hasFieldOrPropertyWithValue("ensureUniqueRuntimeObjectNames", false); + MetadataNamingStrategy naming = (MetadataNamingStrategy) ReflectionTestUtils.getField(exporter, + "namingStrategy"); + assertThat(naming).hasFieldOrPropertyWithValue("ensureUniqueRuntimeObjectNames", false); + }); } @Test void testDefaultDomainConfiguredOnMBeanExport() { - MockEnvironment env = new MockEnvironment(); - env.setProperty("spring.jmx.enabled", "true"); - env.setProperty("spring.jmx.default-domain", "my-test-domain"); - env.setProperty("spring.jmx.unique-names", "true"); - this.context = new AnnotationConfigApplicationContext(); - this.context.setEnvironment(env); - this.context.register(TestConfiguration.class, JmxAutoConfiguration.class); - this.context.refresh(); - MBeanExporter mBeanExporter = this.context.getBean(MBeanExporter.class); - assertThat(mBeanExporter).isNotNull(); - MetadataNamingStrategy naming = (MetadataNamingStrategy) ReflectionTestUtils.getField(mBeanExporter, - "namingStrategy"); - assertThat(naming).hasFieldOrPropertyWithValue("defaultDomain", "my-test-domain"); - assertThat(naming).hasFieldOrPropertyWithValue("ensureUniqueRuntimeObjectNames", true); + this.contextRunner.withPropertyValues("spring.jmx.enabled=true", "spring.jmx.default-domain=my-test-domain", + "spring.jmx.unique-names=true").run((context) -> { + assertThat(context).hasSingleBean(MBeanExporter.class); + MBeanExporter exporter = context.getBean(MBeanExporter.class); + assertThat(exporter).hasFieldOrPropertyWithValue("ensureUniqueRuntimeObjectNames", true); + MetadataNamingStrategy naming = (MetadataNamingStrategy) ReflectionTestUtils.getField(exporter, + "namingStrategy"); + assertThat(naming).hasFieldOrPropertyWithValue("defaultDomain", "my-test-domain"); + assertThat(naming).hasFieldOrPropertyWithValue("ensureUniqueRuntimeObjectNames", true); + }); } @Test void testBasicParentContext() { - this.context = new AnnotationConfigApplicationContext(); - this.context.register(JmxAutoConfiguration.class); - this.context.refresh(); - AnnotationConfigApplicationContext parent = this.context; - this.context = new AnnotationConfigApplicationContext(); - this.context.setParent(parent); - this.context.register(JmxAutoConfiguration.class); - this.context.refresh(); + try (AnnotationConfigApplicationContext parent = new AnnotationConfigApplicationContext()) { + parent.register(JmxAutoConfiguration.class); + parent.refresh(); + this.contextRunner.withParent(parent).run((context) -> assertThat(context.isRunning())); + } } @Test void testParentContext() { - 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(); + try (AnnotationConfigApplicationContext parent = new AnnotationConfigApplicationContext()) { + parent.register(JmxAutoConfiguration.class, TestConfiguration.class); + parent.refresh(); + this.contextRunner.withParent(parent).withConfiguration(UserConfigurations.of(TestConfiguration.class)) + .run((context) -> assertThat(context.isRunning())); + } } @Test void customJmxDomain() { - this.context = new AnnotationConfigApplicationContext(); - this.context.register(CustomJmxDomainConfiguration.class, JmxAutoConfiguration.class, - IntegrationAutoConfiguration.class); - this.context.refresh(); - IntegrationMBeanExporter mbeanExporter = this.context.getBean(IntegrationMBeanExporter.class); - assertThat(mbeanExporter).hasFieldOrPropertyWithValue("domain", "foo.my"); + this.contextRunner.withConfiguration(UserConfigurations.of(CustomJmxDomainConfiguration.class)) + .withConfiguration( + AutoConfigurations.of(JmxAutoConfiguration.class, IntegrationAutoConfiguration.class)) + .run((context) -> { + assertThat(context).hasSingleBean(IntegrationMBeanExporter.class); + IntegrationMBeanExporter exporter = context.getBean(IntegrationMBeanExporter.class); + assertThat(exporter).hasFieldOrPropertyWithValue("domain", "foo.my"); + }); } @Configuration(proxyBeanMethods = false)