From 679b668bbb2892f0b2883bc6726453dca69b7233 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 10 Jul 2023 19:00:40 +0200 Subject: [PATCH] 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 99aa376d6b78..c4f1718cd4d9 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.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,12 +134,18 @@ 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 @@ -235,10 +240,10 @@ public void setAutodetect(boolean autodetect) { * @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 void setAutodetectModeName(String constantName) { * @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 e2393e1292de..58e131b50f21 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.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.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 @@ void bonaFideMBeanIsNotExportedWhenAutodetectIsTotallyTurnedOff() { 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 @@ void bonaFideMBeanExplicitlyExportedAndAutodetectionIsOn() throws Exception { 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 ModelMBeanInfo getMBeanInfo(Object managedResource, String beanKey) throw } } + 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 {