From 73050ed23a5543cee9f5aff6106ebe9188f1edd4 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 29 Jul 2024 23:43:53 +0200 Subject: [PATCH] GH-746 - Fix observability interception for components declaring event listeners. --- spring-modulith-observability/pom.xml | 7 ++++ .../observability/ObservedModuleType.java | 26 +++++++------ .../example/sample/ObservedComponent.java | 38 +++++++++++++++++++ .../ObservedModuleTypeUnitTests.java | 10 +++-- 4 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 spring-modulith-observability/src/test/java/example/sample/ObservedComponent.java diff --git a/spring-modulith-observability/pom.xml b/spring-modulith-observability/pom.xml index 4fdc127e..2b45747a 100644 --- a/spring-modulith-observability/pom.xml +++ b/spring-modulith-observability/pom.xml @@ -60,6 +60,13 @@ test + + org.springframework.modulith + spring-modulith-events-api + ${project.version} + test + + io.micrometer micrometer-observation diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModuleType.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModuleType.java index aa98bb03..357a1cc2 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModuleType.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModuleType.java @@ -16,6 +16,7 @@ package org.springframework.modulith.observability; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Collection; import java.util.List; import java.util.function.Predicate; @@ -38,10 +39,13 @@ import org.springframework.util.ReflectionUtils; class ObservedModuleType { private static Collection> IGNORED_TYPES = List.of(Advised.class, TargetClassAware.class); + private static Predicate IS_USER_METHOD = it -> !Modifier.isPrivate(it.getModifiers()) + && !(ReflectionUtils.isObjectMethod(it) || IGNORED_TYPES.contains(it.getDeclaringClass())); private final ApplicationModules modules; private final ObservedModule module; private final ArchitecturallyEvidentType type; + private final Predicate methodsToInterceptFilter; /** * Creates a new {@link ObservedModuleType} for the given {@link ApplicationModules}, {@link ObservedModule} and @@ -60,6 +64,12 @@ class ObservedModuleType { this.modules = modules; this.module = module; this.type = type; + + Predicate isReferenceMethod = candidate -> type.isEventListener() && type.getReferenceMethods() // + .map(ReferenceMethod::getMethod) // + .anyMatch(it -> it.reflect().equals(candidate)); + + this.methodsToInterceptFilter = IS_USER_METHOD.or(isReferenceMethod); } /** @@ -80,20 +90,14 @@ class ObservedModuleType { } /** - * Returns a predicate to filter the methods to intercept. For event listeners it's the listener methods only. For - * everything else, all (public) methods will be intercepted. + * Returns a predicate to filter the methods to intercept. All user declared methods are intercepted, except from + * well-known interfaces ({@code Advised}, {@code TargetClassAware}). For event listeners, package-protected methods + * are supported as well. * - * @return + * @return will never be {@literal null}. */ public Predicate getMethodsToIntercept() { - - if (!type.isEventListener()) { - return it -> !(ReflectionUtils.isObjectMethod(it) || IGNORED_TYPES.contains(it.getDeclaringClass())); - } - - return candidate -> type.getReferenceMethods() // - .map(ReferenceMethod::getMethod) // - .anyMatch(it -> it.reflect().equals(candidate)); + return methodsToInterceptFilter; } private boolean listensToOtherModulesEvents() { diff --git a/spring-modulith-observability/src/test/java/example/sample/ObservedComponent.java b/spring-modulith-observability/src/test/java/example/sample/ObservedComponent.java new file mode 100644 index 00000000..d5ce1dcb --- /dev/null +++ b/spring-modulith-observability/src/test/java/example/sample/ObservedComponent.java @@ -0,0 +1,38 @@ +/* + * Copyright 2023-2024 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 example.sample; + +import org.springframework.aop.TargetClassAware; +import org.springframework.aop.framework.Advised; +import org.springframework.modulith.events.ApplicationModuleListener; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Component; + +/** + * @author Oliver Drotbohm + */ +@Component +public abstract class ObservedComponent implements Advised, TargetClassAware { + + @Async + public void someMethod() {} + + @SuppressWarnings("unused") + private void someInternalMethod() {} + + @ApplicationModuleListener + void on(Object event) {} +} diff --git a/spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java b/spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java index 31542e01..b20e5893 100644 --- a/spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java +++ b/spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java @@ -17,7 +17,7 @@ package org.springframework.modulith.observability; import static org.assertj.core.api.Assertions.*; -import example.sample.SampleComponent; +import example.sample.ObservedComponent; import example.sample.SampleConfiguration; import org.junit.jupiter.api.Test; @@ -38,17 +38,19 @@ class ObservedModuleTypeUnitTests { static final ApplicationModules modules = TestApplicationModules.of("example"); ApplicationModule module = modules.getModuleByName("sample").orElseThrow(); - ArchitecturallyEvidentType type = module.getArchitecturallyEvidentType(SampleComponent.class); + ArchitecturallyEvidentType type = module.getArchitecturallyEvidentType(ObservedComponent.class); ObservedModuleType observedType = new ObservedModuleType(modules, new DefaultObservedModule(module), type); - @Test // GH-106 + @Test // GH-106, GH-744 void onlyExposesUserMethodsAsToBeIntercepted() { assertThat(observedType.getMethodsToIntercept()).satisfies(it -> { - assertThat(it.test(ReflectionUtils.findMethod(SampleComponent.class, "someMethod"))).isTrue(); + assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "someMethod"))).isTrue(); + assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "on", Object.class))).isTrue(); + assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "someInternalMethod"))).isFalse(); assertThat(it.test(ReflectionUtils.findMethod(Object.class, "toString"))).isFalse(); assertThat(it.test(ReflectionUtils.findMethod(Advised.class, "getTargetClass"))).isFalse(); });