From 9ad92b16b0c5b034ff0b9e0aa4f4f92a4e4ac81f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Jul 2023 18:20:39 +0200 Subject: [PATCH 1/4] Polish MBeanExporterTests --- .../jmx/export/MBeanExporterTests.java | 202 ++++++++---------- 1 file changed, 91 insertions(+), 111 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java index 79087637426..d851d456f74 100644 --- a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java +++ b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java @@ -62,7 +62,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** - * Integration tests for the {@link MBeanExporter} class. + * Integration tests for {@link MBeanExporter}. * * @author Rob Harrop * @author Juergen Hoeller @@ -76,9 +76,11 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { private static final String OBJECT_NAME = "spring:test=jmxMBeanAdaptor"; + private final MBeanExporter exporter = new MBeanExporter(); + @Test - void testRegisterNullNotificationListenerType() throws Exception { + void registerNullNotificationListenerType() throws Exception { Map listeners = new HashMap<>(); // put null in as a value... listeners.put("*", null); @@ -89,14 +91,12 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testRegisterNotificationListenerForNonExistentMBean() throws Exception { - Map listeners = new HashMap<>(); + void registerNotificationListenerForNonExistentMBean() throws Exception { NotificationListener dummyListener = (notification, handback) -> { throw new UnsupportedOperationException(); }; // the MBean with the supplied object name does not exist... - listeners.put("spring:type=Test", dummyListener); - MBeanExporter exporter = new MBeanExporter(); + Map listeners = Map.of("spring:type=Test", dummyListener); exporter.setBeans(getBeanMap()); exporter.setServer(server); exporter.setNotificationListenerMappings(listeners); @@ -107,8 +107,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testWithSuppliedMBeanServer() throws Exception { - MBeanExporter exporter = new MBeanExporter(); + void withSuppliedMBeanServer() throws Exception { exporter.setBeans(getBeanMap()); exporter.setServer(server); try { @@ -122,13 +121,11 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testUserCreatedMBeanRegWithDynamicMBean() throws Exception { - Map map = new HashMap<>(); - map.put("spring:name=dynBean", new TestDynamicMBean()); + void userCreatedMBeanRegWithDynamicMBean() throws Exception { + Map map = Map.of("spring:name=dynBean", new TestDynamicMBean()); InvokeDetectAssembler asm = new InvokeDetectAssembler(); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(server); exporter.setBeans(map); exporter.setAssembler(asm); @@ -145,7 +142,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testAutodetectMBeans() throws Exception { + void autodetectMBeans() throws Exception { try (ConfigurableApplicationContext ctx = load("autodetectMBeans.xml")) { ctx.getBean("exporter"); MBeanServer server = ctx.getBean("server", MBeanServer.class); @@ -159,7 +156,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testAutodetectWithExclude() throws Exception { + void autodetectWithExclude() throws Exception { try (ConfigurableApplicationContext ctx = load("autodetectMBeans.xml")) { ctx.getBean("exporter"); MBeanServer server = ctx.getBean("server", MBeanServer.class); @@ -172,7 +169,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testAutodetectLazyMBeans() throws Exception { + void autodetectLazyMBeans() throws Exception { try (ConfigurableApplicationContext ctx = load("autodetectLazyMBeans.xml")) { ctx.getBean("exporter"); MBeanServer server = ctx.getBean("server", MBeanServer.class); @@ -190,18 +187,17 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testAutodetectNoMBeans() throws Exception { + void autodetectNoMBeans() throws Exception { try (ConfigurableApplicationContext ctx = load("autodetectNoMBeans.xml")) { ctx.getBean("exporter"); } } @Test - void testWithMBeanExporterListeners() throws Exception { + void withMBeanExporterListeners() throws Exception { MockMBeanExporterListener listener1 = new MockMBeanExporterListener(); MockMBeanExporterListener listener2 = new MockMBeanExporterListener(); - MBeanExporter exporter = new MBeanExporter(); exporter.setBeans(getBeanMap()); exporter.setServer(server); exporter.setListeners(listener1, listener2); @@ -213,7 +209,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testExportJdkProxy() throws Exception { + void exportJdkProxy() throws Exception { JmxTestBean bean = new JmxTestBean(); bean.setName("Rob Harrop"); @@ -225,10 +221,8 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { IJmxTestBean proxy = (IJmxTestBean) factory.getProxy(); String name = "bean:mmm=whatever"; - Map beans = new HashMap<>(); - beans.put(name, proxy); + Map beans = Map.of(name, proxy); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(server); exporter.setBeans(beans); exporter.registerBeans(); @@ -239,15 +233,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testSelfNaming() throws Exception { + void selfNaming() throws Exception { ObjectName objectName = ObjectNameManager.getInstance(OBJECT_NAME); SelfNamingTestBean testBean = new SelfNamingTestBean(); testBean.setObjectName(objectName); - Map beans = new HashMap<>(); - beans.put("foo", testBean); + Map beans = Map.of("foo", testBean); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(server); exporter.setBeans(beans); @@ -258,7 +250,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testRegisterIgnoreExisting() throws Exception { + void registerIgnoreExisting() throws Exception { ObjectName objectName = ObjectNameManager.getInstance(OBJECT_NAME); Person preRegistered = new Person(); @@ -271,11 +263,11 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { String objectName2 = "spring:test=equalBean"; - Map beans = new HashMap<>(); - beans.put(objectName.toString(), springRegistered); - beans.put(objectName2, springRegistered); + Map beans = Map.of( + objectName.toString(), springRegistered, + objectName2, springRegistered + ); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(server); exporter.setBeans(beans); exporter.setRegistrationPolicy(RegistrationPolicy.IGNORE_EXISTING); @@ -292,7 +284,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testRegisterReplaceExisting() throws Exception { + void registerReplaceExisting() throws Exception { ObjectName objectName = ObjectNameManager.getInstance(OBJECT_NAME); Person preRegistered = new Person(); @@ -303,10 +295,8 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { Person springRegistered = new Person(); springRegistered.setName("Sally Greenwood"); - Map beans = new HashMap<>(); - beans.put(objectName.toString(), springRegistered); + Map beans = Map.of(objectName.toString(), springRegistered); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(server); exporter.setBeans(beans); exporter.setRegistrationPolicy(RegistrationPolicy.REPLACE_EXISTING); @@ -321,7 +311,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testWithExposeClassLoader() throws Exception { + void withExposeClassLoader() throws Exception { String name = "Rob Harrop"; String otherName = "Juergen Hoeller"; @@ -329,10 +319,8 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { bean.setName(name); ObjectName objectName = ObjectNameManager.getInstance("spring:type=Test"); - Map beans = new HashMap<>(); - beans.put(objectName.toString(), bean); + Map beans = Map.of(objectName.toString(), bean); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setBeans(beans); exporter.setExposeManagedResourceClassLoader(true); @@ -351,16 +339,14 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testBonaFideMBeanIsNotExportedWhenAutodetectIsTotallyTurnedOff() { + void bonaFideMBeanIsNotExportedWhenAutodetectIsTotallyTurnedOff() { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(Person.class); DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition("^&_invalidObjectName_(*", builder.getBeanDefinition()); String exportedBeanName = "export.me.please"; factory.registerSingleton(exportedBeanName, new TestBean()); - MBeanExporter exporter = new MBeanExporter(); - Map beansToExport = new HashMap<>(); - beansToExport.put(OBJECT_NAME, exportedBeanName); + Map beansToExport = Map.of(OBJECT_NAME, exportedBeanName); exporter.setBeans(beansToExport); exporter.setServer(getServer()); exporter.setBeanFactory(factory); @@ -371,14 +357,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testOnlyBonaFideMBeanIsExportedWhenAutodetectIsMBeanOnly() throws Exception { + void onlyBonaFideMBeanIsExportedWhenAutodetectIsMBeanOnly() throws Exception { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(Person.class); DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition(OBJECT_NAME, builder.getBeanDefinition()); String exportedBeanName = "spring:type=TestBean"; factory.registerSingleton(exportedBeanName, new TestBean()); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(exportedBeanName)); exporter.setBeanFactory(factory); @@ -392,7 +377,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testBonaFideMBeanAndRegularBeanExporterWithAutodetectAll() throws Exception { + void bonaFideMBeanAndRegularBeanExporterWithAutodetectAll() throws Exception { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(Person.class); DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition(OBJECT_NAME, builder.getBeanDefinition()); @@ -401,7 +386,6 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { String notToBeExportedBeanName = "spring:type=NotToBeExported"; factory.registerSingleton(notToBeExportedBeanName, new TestBean()); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(exportedBeanName)); exporter.setBeanFactory(factory); @@ -416,14 +400,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testBonaFideMBeanIsNotExportedWithAutodetectAssembler() throws Exception { + void bonaFideMBeanIsNotExportedWithAutodetectAssembler() throws Exception { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(Person.class); DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition(OBJECT_NAME, builder.getBeanDefinition()); String exportedBeanName = "spring:type=TestBean"; factory.registerSingleton(exportedBeanName, new TestBean()); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(exportedBeanName)); exporter.setBeanFactory(factory); @@ -439,15 +422,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { * Want to ensure that said MBean is not exported twice. */ @Test - void testBonaFideMBeanExplicitlyExportedAndAutodetectionIsOn() throws Exception { + void bonaFideMBeanExplicitlyExportedAndAutodetectionIsOn() throws Exception { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(Person.class); DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition(OBJECT_NAME, builder.getBeanDefinition()); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); - Map beansToExport = new HashMap<>(); - beansToExport.put(OBJECT_NAME, OBJECT_NAME); + Map beansToExport = Map.of(OBJECT_NAME, OBJECT_NAME); exporter.setBeans(beansToExport); exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(OBJECT_NAME)); exporter.setBeanFactory(factory); @@ -458,61 +439,68 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testSetAutodetectModeToOutOfRangeNegativeValue() { - MBeanExporter exporter = new MBeanExporter(); - assertThatIllegalArgumentException().isThrownBy(() -> - exporter.setAutodetectMode(-1)); + void setAutodetectModeToSupportedValue() { + exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ASSEMBLER); } @Test - void testSetAutodetectModeToOutOfRangePositiveValue() { - MBeanExporter exporter = new MBeanExporter(); - assertThatIllegalArgumentException().isThrownBy(() -> - exporter.setAutodetectMode(5)); + void setAutodetectModeToOutOfRangeNegativeValue() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectMode(-1)); } @Test - void testSetAutodetectModeNameToAnEmptyString() { - MBeanExporter exporter = new MBeanExporter(); - assertThatIllegalArgumentException().isThrownBy(() -> - exporter.setAutodetectModeName("")); + void setAutodetectModeToOutOfRangePositiveValue() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectMode(5)); } @Test - void testSetAutodetectModeNameToAWhitespacedString() { - MBeanExporter exporter = new MBeanExporter(); - assertThatIllegalArgumentException().isThrownBy(() -> - exporter.setAutodetectModeName(" \t")); + void setAutodetectModeNameToSupportedValue() { + exporter.setAutodetectModeName("AUTODETECT_ASSEMBLER"); } @Test - void testSetAutodetectModeNameToARubbishValue() { - MBeanExporter exporter = new MBeanExporter(); - assertThatIllegalArgumentException().isThrownBy(() -> - exporter.setAutodetectModeName("That Hansel is... *sssooo* hot right now!")); + void setAutodetectModeNameToNull() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectModeName(null)); } @Test - void testNotRunningInBeanFactoryAndPassedBeanNameToExport() throws Exception { - MBeanExporter exporter = new MBeanExporter(); - Map beans = new HashMap<>(); - beans.put(OBJECT_NAME, "beanName"); + void setAutodetectModeNameToAnEmptyString() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectModeName("")); + } + + @Test + void setAutodetectModeNameToAWhitespacedString() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectModeName(" \t")); + } + + @Test + void setAutodetectModeNameToARubbishValue() { + assertThatIllegalArgumentException() + .isThrownBy(() -> exporter.setAutodetectModeName("That Hansel is... *sssooo* hot right now!")); + } + + @Test + void notRunningInBeanFactoryAndPassedBeanNameToExport() throws Exception { + Map beans = Map.of(OBJECT_NAME, "beanName"); exporter.setBeans(beans); - assertThatExceptionOfType(MBeanExportException.class).isThrownBy(() -> - start(exporter)); + assertThatExceptionOfType(MBeanExportException.class) + .isThrownBy(() -> start(exporter)); } @Test - void testNotRunningInBeanFactoryAndAutodetectionIsOn() throws Exception { - MBeanExporter exporter = new MBeanExporter(); + void notRunningInBeanFactoryAndAutodetectionIsOn() throws Exception { exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ALL); - assertThatExceptionOfType(MBeanExportException.class).isThrownBy(() -> - start(exporter)); + assertThatExceptionOfType(MBeanExportException.class) + .isThrownBy(() -> start(exporter)); } @Test // SPR-2158 - void testMBeanIsNotUnregisteredSpuriouslyIfSomeExternalProcessHasUnregisteredMBean() throws Exception { - MBeanExporter exporter = new MBeanExporter(); + void mbeanIsNotUnregisteredSpuriouslyIfSomeExternalProcessHasUnregisteredMBean() throws Exception { exporter.setBeans(getBeanMap()); exporter.setServer(this.server); MockMBeanExporterListener listener = new MockMBeanExporterListener(); @@ -523,12 +511,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { this.server.unregisterMBean(new ObjectName(OBJECT_NAME)); exporter.destroy(); - assertThat(listener.getUnregistered()).as("Listener should not have been invoked (MBean previously unregistered by external agent)") + assertThat(listener.getUnregistered()) + .as("Listener should not have been invoked (MBean previously unregistered by external agent)") .isEmpty(); } @Test // SPR-3302 - void testBeanNameCanBeUsedInNotificationListenersMap() throws Exception { + void beanNameCanBeUsedInNotificationListenersMap() throws Exception { String beanName = "charlesDexterWard"; BeanDefinitionBuilder testBean = BeanDefinitionBuilder.rootBeanDefinition(JmxTestBean.class); @@ -537,10 +526,8 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { factory.preInstantiateSingletons(); Object testBeanInstance = factory.getBean(beanName); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); - Map beansToExport = new HashMap<>(); - beansToExport.put("test:what=ever", testBeanInstance); + Map beansToExport = Map.of("test:what=ever", testBeanInstance); exporter.setBeans(beansToExport); exporter.setBeanFactory(factory); StubNotificationListener listener = new StubNotificationListener(); @@ -550,7 +537,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testWildcardCanBeUsedInNotificationListenersMap() throws Exception { + void wildcardCanBeUsedInNotificationListenersMap() throws Exception { String beanName = "charlesDexterWard"; BeanDefinitionBuilder testBean = BeanDefinitionBuilder.rootBeanDefinition(JmxTestBean.class); @@ -559,10 +546,8 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { factory.preInstantiateSingletons(); Object testBeanInstance = factory.getBean(beanName); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); - Map beansToExport = new HashMap<>(); - beansToExport.put("test:what=ever", testBeanInstance); + Map beansToExport = Map.of("test:what=ever", testBeanInstance); exporter.setBeans(beansToExport); exporter.setBeanFactory(factory); StubNotificationListener listener = new StubNotificationListener(); @@ -572,7 +557,7 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test // SPR-3625 - void testMBeanIsUnregisteredForRuntimeExceptionDuringInitialization() throws Exception { + void mbeanIsUnregisteredForRuntimeExceptionDuringInitialization() throws Exception { BeanDefinitionBuilder builder1 = BeanDefinitionBuilder.rootBeanDefinition(Person.class); BeanDefinitionBuilder builder2 = BeanDefinitionBuilder .rootBeanDefinition(RuntimeExceptionThrowingConstructorBean.class); @@ -584,16 +569,16 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { factory.registerBeanDefinition(objectName1, builder1.getBeanDefinition()); factory.registerBeanDefinition(objectName2, builder2.getBeanDefinition()); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); - Map beansToExport = new HashMap<>(); - beansToExport.put(objectName1, objectName1); - beansToExport.put(objectName2, objectName2); + Map beansToExport = Map.of( + objectName1, objectName1, + objectName2, objectName2 + ); exporter.setBeans(beansToExport); exporter.setBeanFactory(factory); assertThatRuntimeException().as("failed during creation of RuntimeExceptionThrowingConstructorBean") - .isThrownBy(() -> start(exporter)); + .isThrownBy(() -> start(exporter)); assertIsNotRegistered("Must have unregistered all previously registered MBeans due to RuntimeException", ObjectNameManager.getInstance(objectName1)); @@ -602,14 +587,13 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testIgnoreBeanName() throws MalformedObjectNameException { + void ignoreBeanName() throws MalformedObjectNameException { DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); String firstBeanName = "spring:type=TestBean"; factory.registerSingleton(firstBeanName, new TestBean("test")); String secondBeanName = "spring:type=TestBean2"; factory.registerSingleton(secondBeanName, new TestBean("test2")); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setAssembler(new NamedBeanAutodetectCapableMBeanInfoAssemblerStub(firstBeanName, secondBeanName)); exporter.setBeanFactory(factory); @@ -624,11 +608,10 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testRegisterFactoryBean() throws MalformedObjectNameException { + void registerFactoryBean() throws MalformedObjectNameException { DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition("spring:type=FactoryBean", new RootBeanDefinition(ProperSomethingFactoryBean.class)); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setBeanFactory(factory); exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ALL); @@ -639,11 +622,10 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } @Test - void testIgnoreNullObjectFromFactoryBean() throws MalformedObjectNameException { + void ignoreNullObjectFromFactoryBean() throws MalformedObjectNameException { DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); factory.registerBeanDefinition("spring:type=FactoryBean", new RootBeanDefinition(NullSomethingFactoryBean.class)); - MBeanExporter exporter = new MBeanExporter(); exporter.setServer(getServer()); exporter.setBeanFactory(factory); exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ALL); @@ -658,13 +640,11 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { return new ClassPathXmlApplicationContext(context, getClass()); } - private Map getBeanMap() { - Map map = new HashMap<>(); - map.put(OBJECT_NAME, new JmxTestBean()); - return map; + private static Map getBeanMap() { + return Map.of(OBJECT_NAME, new JmxTestBean()); } - private void assertListener(MockMBeanExporterListener listener) throws MalformedObjectNameException { + private static void assertListener(MockMBeanExporterListener listener) throws MalformedObjectNameException { ObjectName desired = ObjectNameManager.getInstance(OBJECT_NAME); assertThat(listener.getRegistered()).as("Incorrect number of registrations").hasSize(1); assertThat(listener.getUnregistered()).as("Incorrect number of unregistrations").hasSize(1); From 7c7fa6955848cb73179dd2f77d436bac6043de8f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Jul 2023 18:26:33 +0200 Subject: [PATCH 2/4] Introduce getAutodetectMode() in MBeanExporter Closes gh-30855 --- .../jmx/export/MBeanExporter.java | 16 ++++++++++++++++ .../jmx/export/MBeanExporterTests.java | 8 ++++++++ 2 files changed, 24 insertions(+) diff --git a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java index f4ce6bc207d..0657b8e6a20 100644 --- a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java +++ b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java @@ -92,6 +92,7 @@ import org.springframework.util.ObjectUtils; * @author Rick Evans * @author Mark Fisher * @author Stephane Nicoll + * @author Sam Brannen * @since 1.2 * @see #setBeans * @see #setAutodetect @@ -227,6 +228,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo * @throws IllegalArgumentException if the supplied value is not * one of the {@code AUTODETECT_} constants * @see #setAutodetectModeName(String) + * @see #getAutodetectMode() * @see #AUTODETECT_ALL * @see #AUTODETECT_ASSEMBLER * @see #AUTODETECT_MBEAN @@ -244,6 +246,7 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo * @throws IllegalArgumentException if the supplied value is not resolvable * to one of the {@code AUTODETECT_} constants or is {@code null} * @see #setAutodetectMode(int) + * @see #getAutodetectMode() * @see #AUTODETECT_ALL * @see #AUTODETECT_ASSEMBLER * @see #AUTODETECT_MBEAN @@ -256,6 +259,19 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo this.autodetectMode = (Integer) constants.asNumber(constantName); } + /** + * Get the autodetect mode to use for this {@code MBeanExporter}. + * @return the configured autodetect mode, or {@code null} if not explicitly + * configured + * @since 6.0.11 + * @see #setAutodetectModeName(String) + * @see #setAutodetectMode(int) + */ + @Nullable + public Integer getAutodetectMode() { + return this.autodetectMode; + } + /** * Specify whether to allow eager initialization of candidate beans * when autodetecting MBeans in the Spring application context. diff --git a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java index d851d456f74..e2393e1292d 100644 --- a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java +++ b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java @@ -441,47 +441,55 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { @Test void setAutodetectModeToSupportedValue() { exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ASSEMBLER); + assertThat(exporter.getAutodetectMode()).isEqualTo(MBeanExporter.AUTODETECT_ASSEMBLER); } @Test void setAutodetectModeToOutOfRangeNegativeValue() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectMode(-1)); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeToOutOfRangePositiveValue() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectMode(5)); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeNameToSupportedValue() { exporter.setAutodetectModeName("AUTODETECT_ASSEMBLER"); + assertThat(exporter.getAutodetectMode()).isEqualTo(MBeanExporter.AUTODETECT_ASSEMBLER); } @Test void setAutodetectModeNameToNull() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectModeName(null)); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeNameToAnEmptyString() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectModeName("")); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeNameToAWhitespacedString() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectModeName(" \t")); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeNameToARubbishValue() { assertThatIllegalArgumentException() .isThrownBy(() -> exporter.setAutodetectModeName("That Hansel is... *sssooo* hot right now!")); + assertThat(exporter.getAutodetectMode()).isNull(); } @Test From 676daa990b9540896ec8d8216f2156bb8a4cf43d Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Jul 2023 18:28:45 +0200 Subject: [PATCH 3/4] Reorganize methods in MBeanExporter --- .../jmx/export/MBeanExporter.java | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java index 0657b8e6a20..99aa376d6b7 100644 --- a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java +++ b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java @@ -223,24 +223,6 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo this.autodetectMode = (autodetect ? AUTODETECT_ALL : AUTODETECT_NONE); } - /** - * Set the autodetection mode to use. - * @throws IllegalArgumentException if the supplied value is not - * one of the {@code AUTODETECT_} constants - * @see #setAutodetectModeName(String) - * @see #getAutodetectMode() - * @see #AUTODETECT_ALL - * @see #AUTODETECT_ASSEMBLER - * @see #AUTODETECT_MBEAN - * @see #AUTODETECT_NONE - */ - public void setAutodetectMode(int autodetectMode) { - if (!constants.getValues(CONSTANT_PREFIX_AUTODETECT).contains(autodetectMode)) { - throw new IllegalArgumentException("Only values of autodetect constants allowed"); - } - this.autodetectMode = autodetectMode; - } - /** * Set the autodetection mode to use by name. * @throws IllegalArgumentException if the supplied value is not resolvable @@ -259,6 +241,24 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo this.autodetectMode = (Integer) constants.asNumber(constantName); } + /** + * Set the autodetection mode to use. + * @throws IllegalArgumentException if the supplied value is not + * one of the {@code AUTODETECT_} constants + * @see #setAutodetectModeName(String) + * @see #getAutodetectMode() + * @see #AUTODETECT_ALL + * @see #AUTODETECT_ASSEMBLER + * @see #AUTODETECT_MBEAN + * @see #AUTODETECT_NONE + */ + public void setAutodetectMode(int autodetectMode) { + if (!constants.getValues(CONSTANT_PREFIX_AUTODETECT).contains(autodetectMode)) { + throw new IllegalArgumentException("Only values of autodetect constants allowed"); + } + this.autodetectMode = autodetectMode; + } + /** * Get the autodetect mode to use for this {@code MBeanExporter}. * @return the configured autodetect mode, or {@code null} if not explicitly From 679b668bbb2892f0b2883bc6726453dca69b7233 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Jul 2023 19:00:40 +0200 Subject: [PATCH 4/4] Avoid need for reflection hints for MBeanExporter in native image Prior to this commit, MBeanExporter used org.springframework.core.Constants which used reflection to find constant fields in the MBeanExporter class. Consequently, one had to register reflection hints in order to use MBeanExporter in a GraalVM native image. This commit addresses this by replacing the use of the `Constants` class with a simple java.util.Map which maps constant names to constant values for the autodetect constants defined in MBeanExporter. See gh-30851 Closes gh-30846 --- .../jmx/export/MBeanExporter.java | 30 +++---- .../jmx/export/MBeanExporterTests.java | 78 +++++++++++++++---- 2 files changed, 78 insertions(+), 30 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java index 99aa376d6b7..c4f1718cd4d 100644 --- a/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java +++ b/spring-context/src/main/java/org/springframework/jmx/export/MBeanExporter.java @@ -53,7 +53,6 @@ import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.core.Constants; import org.springframework.jmx.export.assembler.AutodetectCapableMBeanInfoAssembler; import org.springframework.jmx.export.assembler.MBeanInfoAssembler; import org.springframework.jmx.export.assembler.SimpleReflectiveMBeanInfoAssembler; @@ -135,13 +134,19 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo /** Constant for the JMX {@code mr_type} "ObjectReference". */ private static final String MR_TYPE_OBJECT_REFERENCE = "ObjectReference"; - /** Prefix for the autodetect constants defined in this class. */ - private static final String CONSTANT_PREFIX_AUTODETECT = "AUTODETECT_"; + /** + * Map of constant names to constant values for the autodetect constants defined + * in this class. + * @since 6.0.11 + */ + private static final Map constants = Map.of( + "AUTODETECT_NONE", AUTODETECT_NONE, + "AUTODETECT_MBEAN", AUTODETECT_MBEAN, + "AUTODETECT_ASSEMBLER", AUTODETECT_ASSEMBLER, + "AUTODETECT_ALL", AUTODETECT_ALL + ); - /** Constants instance for this class. */ - private static final Constants constants = new Constants(MBeanExporter.class); - /** The beans to be exposed as JMX managed resources, with JMX names as keys. */ @Nullable private Map beans; @@ -235,10 +240,10 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo * @see #AUTODETECT_NONE */ public void setAutodetectModeName(String constantName) { - if (!constantName.startsWith(CONSTANT_PREFIX_AUTODETECT)) { - throw new IllegalArgumentException("Only autodetect constants allowed"); - } - this.autodetectMode = (Integer) constants.asNumber(constantName); + Assert.hasText(constantName, "'constantName' must not be null or blank"); + Integer mode = constants.get(constantName); + Assert.notNull(mode, "Only autodetect constants allowed"); + this.autodetectMode = mode; } /** @@ -253,9 +258,8 @@ public class MBeanExporter extends MBeanRegistrationSupport implements MBeanExpo * @see #AUTODETECT_NONE */ public void setAutodetectMode(int autodetectMode) { - if (!constants.getValues(CONSTANT_PREFIX_AUTODETECT).contains(autodetectMode)) { - throw new IllegalArgumentException("Only values of autodetect constants allowed"); - } + Assert.isTrue(constants.containsValue(autodetectMode), + "Only values of autodetect constants allowed"); this.autodetectMode = autodetectMode; } diff --git a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java index e2393e1292d..58e131b50f2 100644 --- a/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java +++ b/spring-context/src/test/java/org/springframework/jmx/export/MBeanExporterTests.java @@ -16,6 +16,7 @@ package org.springframework.jmx.export; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -23,6 +24,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import javax.management.Attribute; import javax.management.InstanceNotFoundException; @@ -55,10 +57,12 @@ import org.springframework.jmx.export.assembler.SimpleReflectiveMBeanInfoAssembl import org.springframework.jmx.export.naming.SelfNaming; import org.springframework.jmx.support.ObjectNameManager; import org.springframework.jmx.support.RegistrationPolicy; +import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** @@ -353,7 +357,6 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { exporter.setAutodetectMode(MBeanExporter.AUTODETECT_NONE); // MBean has a bad ObjectName, so if said MBean is autodetected, an exception will be thrown... start(exporter); - } @Test @@ -438,60 +441,88 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { ObjectNameManager.getInstance(OBJECT_NAME)); } - @Test - void setAutodetectModeToSupportedValue() { - exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ASSEMBLER); - assertThat(exporter.getAutodetectMode()).isEqualTo(MBeanExporter.AUTODETECT_ASSEMBLER); - } - @Test void setAutodetectModeToOutOfRangeNegativeValue() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectMode(-1)); + .isThrownBy(() -> exporter.setAutodetectMode(-1)) + .withMessage("Only values of autodetect constants allowed"); assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeToOutOfRangePositiveValue() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectMode(5)); + .isThrownBy(() -> exporter.setAutodetectMode(5)) + .withMessage("Only values of autodetect constants allowed"); assertThat(exporter.getAutodetectMode()).isNull(); } + /** + * This test effectively verifies that the internal 'constants' map is properly + * configured for all autodetect constants defined in {@link MBeanExporter}. + */ @Test - void setAutodetectModeNameToSupportedValue() { - exporter.setAutodetectModeName("AUTODETECT_ASSEMBLER"); + void setAutodetectModeToAllSupportedValues() { + streamConstants(MBeanExporter.class) + .map(MBeanExporterTests::getFieldValue) + .forEach(mode -> assertThatNoException().isThrownBy(() -> exporter.setAutodetectMode(mode))); + } + + @Test + void setAutodetectModeToSupportedValue() { + exporter.setAutodetectMode(MBeanExporter.AUTODETECT_ASSEMBLER); assertThat(exporter.getAutodetectMode()).isEqualTo(MBeanExporter.AUTODETECT_ASSEMBLER); } @Test void setAutodetectModeNameToNull() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectModeName(null)); + .isThrownBy(() -> exporter.setAutodetectModeName(null)) + .withMessage("'constantName' must not be null or blank"); assertThat(exporter.getAutodetectMode()).isNull(); } @Test void setAutodetectModeNameToAnEmptyString() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectModeName("")); + .isThrownBy(() -> exporter.setAutodetectModeName("")) + .withMessage("'constantName' must not be null or blank"); assertThat(exporter.getAutodetectMode()).isNull(); } @Test - void setAutodetectModeNameToAWhitespacedString() { + void setAutodetectModeNameToWhitespace() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectModeName(" \t")); + .isThrownBy(() -> exporter.setAutodetectModeName(" \t")) + .withMessage("'constantName' must not be null or blank"); assertThat(exporter.getAutodetectMode()).isNull(); } @Test - void setAutodetectModeNameToARubbishValue() { + void setAutodetectModeNameToBogusValue() { assertThatIllegalArgumentException() - .isThrownBy(() -> exporter.setAutodetectModeName("That Hansel is... *sssooo* hot right now!")); + .isThrownBy(() -> exporter.setAutodetectModeName("Bogus")) + .withMessage("Only autodetect constants allowed"); assertThat(exporter.getAutodetectMode()).isNull(); } + /** + * This test effectively verifies that the internal 'constants' map is properly + * configured for all autodetect constants defined in {@link MBeanExporter}. + */ + @Test + void setAutodetectModeNameToAllSupportedValues() { + streamConstants(MBeanExporter.class) + .map(Field::getName) + .forEach(name -> assertThatNoException().isThrownBy(() -> exporter.setAutodetectModeName(name))); + } + + @Test + void setAutodetectModeNameToSupportedValue() { + exporter.setAutodetectModeName("AUTODETECT_ASSEMBLER"); + assertThat(exporter.getAutodetectMode()).isEqualTo(MBeanExporter.AUTODETECT_ASSEMBLER); + } + @Test void notRunningInBeanFactoryAndPassedBeanNameToExport() throws Exception { Map beans = Map.of(OBJECT_NAME, "beanName"); @@ -672,6 +703,19 @@ public class MBeanExporterTests extends AbstractMBeanServerTests { } } + private static Stream streamConstants(Class clazz) { + return Arrays.stream(clazz.getFields()).filter(ReflectionUtils::isPublicStaticFinal); + } + + private static Integer getFieldValue(Field field) { + try { + return (Integer) field.get(null); + } + catch (Exception ex) { + throw new RuntimeException(ex); + } + } + private static class MockMBeanExporterListener implements MBeanExporterListener {