From 5aac35b99e9a13832a2dc2c4d2a2b189fa46d24a Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 2 Jul 2023 14:24:52 +0200 Subject: [PATCH] Check deps only once in MicrometerObservationRegistryTestExecutionListener Prior to this commit, the required runtime dependencies were checked via reflection each time an attempt was made to instantiate MicrometerObservationRegistryTestExecutionListener. Since it's sufficient to check for the presence of required runtime dependencies only once, this commit caches the results of the dependency checks in a static field. This commit also introduces automated tests for the runtime dependency checks in MicrometerObservationRegistryTestExecutionListener. See gh-30747 --- ...ervationRegistryTestExecutionListener.java | 63 +++++----- ...yTestExecutionListenerDependencyTests.java | 112 ++++++++++++++++++ 2 files changed, 142 insertions(+), 33 deletions(-) create mode 100644 spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerDependencyTests.java diff --git a/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java index 6e8f698ed83..b4df831b494 100644 --- a/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java @@ -16,8 +16,6 @@ package org.springframework.test.context.observation; -import java.lang.reflect.Method; - import io.micrometer.observation.ObservationRegistry; import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor; import org.apache.commons.logging.Log; @@ -27,7 +25,6 @@ import org.springframework.context.ApplicationContext; import org.springframework.core.Conventions; import org.springframework.test.context.TestContext; import org.springframework.test.context.support.AbstractTestExecutionListener; -import org.springframework.util.ReflectionUtils; /** * {@code TestExecutionListener} which provides support for Micrometer's @@ -45,11 +42,6 @@ class MicrometerObservationRegistryTestExecutionListener extends AbstractTestExe private static final Log logger = LogFactory.getLog(MicrometerObservationRegistryTestExecutionListener.class); - private static final String THREAD_LOCAL_ACCESSOR_CLASS_NAME = "io.micrometer.context.ThreadLocalAccessor"; - - private static final String OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME = - "io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor"; - /** * Attribute name for a {@link TestContext} attribute which contains the * {@link ObservationRegistry} that was previously stored in the @@ -62,41 +54,46 @@ class MicrometerObservationRegistryTestExecutionListener extends AbstractTestExe private static final String PREVIOUS_OBSERVATION_REGISTRY = Conventions.getQualifiedAttributeName( MicrometerObservationRegistryTestExecutionListener.class, "previousObservationRegistry"); + static final String DEPENDENCIES_ERROR_MESSAGE = """ + MicrometerObservationRegistryTestExecutionListener requires \ + io.micrometer:micrometer-observation:1.10.8 or higher and \ + io.micrometer:context-propagation:1.0.3 or higher."""; - public MicrometerObservationRegistryTestExecutionListener() { + static final String THREAD_LOCAL_ACCESSOR_CLASS_NAME = "io.micrometer.context.ThreadLocalAccessor"; + + static final String OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME = + "io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor"; + + private static final String ERROR_MESSAGE; + + static { // Trigger eager resolution of Micrometer Observation types to ensure that // this listener can be properly skipped when SpringFactoriesLoader attempts // to load it -- for example, if context-propagation and micrometer-observation // are not in the classpath or if the version of ObservationThreadLocalAccessor // present does not include the getObservationRegistry() method. - ClassLoader classLoader = getClass().getClassLoader(); - String errorMessage = """ - MicrometerObservationRegistryTestExecutionListener requires \ - io.micrometer:micrometer-observation:1.10.8 or higher and \ - io.micrometer:context-propagation:1.0.3 or higher."""; + String errorMessage = null; + ClassLoader classLoader = MicrometerObservationRegistryTestExecutionListener.class.getClassLoader(); + String classToCheck = THREAD_LOCAL_ACCESSOR_CLASS_NAME; try { - Class.forName(THREAD_LOCAL_ACCESSOR_CLASS_NAME, false, classLoader); - Class clazz = Class.forName(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME, false, classLoader); - Method method = ReflectionUtils.findMethod(clazz, "getObservationRegistry"); - if (method == null) { - // We simulate "class not found" even though it's "method not found", since - // the ClassNotFoundException will be processed in the subsequent catch-block. - throw new ClassNotFoundException(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME); - } + Class.forName(classToCheck, false, classLoader); + classToCheck = OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME; + Class clazz = Class.forName(classToCheck, false, classLoader); + clazz.getMethod("getObservationRegistry"); } catch (Throwable ex) { - // Ensure that we throw a NoClassDefFoundError so that the exception will be properly - // handled in TestContextFailureHandler. - // If the original exception was a ClassNotFoundException or NoClassDefFoundError, - // we throw a NoClassDefFoundError with an augmented error message and omit the cause. - if (ex instanceof ClassNotFoundException || ex instanceof NoClassDefFoundError) { - throw new NoClassDefFoundError(ex.getMessage() + ". " + errorMessage); - } - // Otherwise, we throw a NoClassDefFoundError with the cause initialized. - Error error = new NoClassDefFoundError(errorMessage); - error.initCause(ex); - throw error; + errorMessage = classToCheck + ". " + DEPENDENCIES_ERROR_MESSAGE; + } + ERROR_MESSAGE = errorMessage; + } + + + public MicrometerObservationRegistryTestExecutionListener() { + // If required dependencies are missing, throw a NoClassDefFoundError so + // that this listener will be properly skipped in TestContextFailureHandler. + if (ERROR_MESSAGE != null) { + throw new NoClassDefFoundError(ERROR_MESSAGE); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerDependencyTests.java b/spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerDependencyTests.java new file mode 100644 index 00000000000..42133eaf130 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerDependencyTests.java @@ -0,0 +1,112 @@ +/* + * Copyright 2002-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.test.context.observation; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.util.function.Predicate; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.OverridingClassLoader; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.springframework.test.context.observation.MicrometerObservationRegistryTestExecutionListener.DEPENDENCIES_ERROR_MESSAGE; +import static org.springframework.test.context.observation.MicrometerObservationRegistryTestExecutionListener.OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME; +import static org.springframework.test.context.observation.MicrometerObservationRegistryTestExecutionListener.THREAD_LOCAL_ACCESSOR_CLASS_NAME; + +/** + * Unit tests for {@link MicrometerObservationRegistryTestExecutionListener} + * behavior regarding required dependencies. + * + * @author Sam Brannen + * @since 6.0.11 + */ +class MicrometerObservationRegistryTestExecutionListenerDependencyTests { + + @Test + void allDependenciesArePresent() throws Exception { + FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader(), name -> false); + Class listenerClass = classLoader.loadClass(MicrometerObservationRegistryTestExecutionListener.class.getName()); + // Invoke multiple times to ensure consistency. + IntStream.rangeClosed(1, 5).forEach(n -> assertThatNoException().isThrownBy(() -> instantiateListener(listenerClass))); + } + + @Test + void threadLocalAccessorIsNotPresent() throws Exception { + assertNoClassDefFoundErrorIsThrown(THREAD_LOCAL_ACCESSOR_CLASS_NAME); + } + + @Test + void observationThreadLocalAccessorIsNotPresent() throws Exception { + assertNoClassDefFoundErrorIsThrown(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME); + } + + private void assertNoClassDefFoundErrorIsThrown(String missingClassName) throws Exception { + FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader(), missingClassName::equals); + Class listenerClass = classLoader.loadClass(MicrometerObservationRegistryTestExecutionListener.class.getName()); + // Invoke multiple times to ensure the same error message is generated every time. + IntStream.rangeClosed(1, 5).forEach(n -> assertExceptionThrown(missingClassName, listenerClass)); + } + + private void assertExceptionThrown(String missingClassName, Class listenerClass) { + assertThatExceptionOfType(InvocationTargetException.class) + .isThrownBy(() -> instantiateListener(listenerClass)) + .havingCause() + .isInstanceOf(NoClassDefFoundError.class) + .withMessage(missingClassName + ". " + DEPENDENCIES_ERROR_MESSAGE); + } + + private void instantiateListener(Class listenerClass) throws Exception { + assertThat(listenerClass).isNotNull(); + Constructor constructor = listenerClass.getDeclaredConstructor(); + constructor.setAccessible(true); + constructor.newInstance(); + } + + + static class FilteringClassLoader extends OverridingClassLoader { + + private static final Predicate isListenerClass = + MicrometerObservationRegistryTestExecutionListener.class.getName()::equals; + + private final Predicate classNameFilter; + + + FilteringClassLoader(ClassLoader parent, Predicate classNameFilter) { + super(parent); + this.classNameFilter = classNameFilter; + } + + @Override + protected boolean isEligibleForOverriding(String className) { + return this.classNameFilter.or(isListenerClass).test(className); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (this.classNameFilter.test(name)) { + throw new ClassNotFoundException(name); + } + return super.loadClass(name, resolve); + } + } + +}