From 262efb58282ef3a83e6a0ec156a1b63c6042d9d6 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 5 Jan 2023 17:18:40 +0100 Subject: [PATCH] GH-106 - Avoid intercepting Object and proxy-related methods for module interaction tracing. We now skip Object methods and ones declared on Spring's Advised or TargetClassAware types when intercepting invocations to trigger a module entry. --- .../modulith/model/ApplicationModule.java | 17 ++++++ spring-modulith-observability/pom.xml | 7 +++ .../observability/DefaultObservedModule.java | 12 +++- .../observability/ObservedModule.java | 8 ++- .../observability/ObservedModuleType.java | 11 +++- .../java/example/sample/SampleComponent.java | 27 +++++++++ .../ObservedModuleTypeUnitTests.java | 60 +++++++++++++++++++ 7 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 spring-modulith-observability/src/test/java/example/sample/SampleComponent.java create mode 100644 spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java index 18eeafa8..32a92aa4 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java @@ -245,6 +245,23 @@ public class ApplicationModule { .toList(); } + /** + * Returns the {@link ArchitecturallyEvidentType} for the given type. + * + * @param type must not be {@literal null}. + * @return will never be {@literal null}. + * @throws IllegalArgumentException if the given type is not a module type. + */ + public ArchitecturallyEvidentType getArchitecturallyEvidentType(Class type) { + + Assert.notNull(type, "Type must not be null!"); + + return getType(type.getName()) + .map(it -> ArchitecturallyEvidentType.of(it, getSpringBeansInternal())) + .orElseThrow(() -> new IllegalArgumentException("Couldn't find type %s in module %s!".formatted( + FormatableType.of(type).getAbbreviatedFullName(this), getName()))); + } + public boolean contains(JavaClass type) { return basePackage.contains(type); } diff --git a/spring-modulith-observability/pom.xml b/spring-modulith-observability/pom.xml index 94383d8e..95eef10f 100644 --- a/spring-modulith-observability/pom.xml +++ b/spring-modulith-observability/pom.xml @@ -47,6 +47,13 @@ spring-boot-starter-test test + + + org.springframework.experimental + spring-modulith-test + ${project.version} + test + io.micrometer diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java index c8cf4d9d..28035bd0 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java @@ -23,10 +23,11 @@ import java.util.Arrays; import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.ProxyMethodInvocation; import org.springframework.aop.framework.Advised; -import org.springframework.modulith.model.FormatableType; import org.springframework.modulith.model.ApplicationModule; import org.springframework.modulith.model.ApplicationModules; +import org.springframework.modulith.model.FormatableType; import org.springframework.modulith.model.SpringBean; +import org.springframework.util.Assert; import com.tngtech.archunit.core.domain.JavaClass; @@ -98,6 +99,9 @@ class DefaultObservedModule implements ObservedModule { */ @Override public boolean exposes(JavaClass type) { + + Assert.notNull(type, "Type must not be null!"); + return module.isExposed(type); } @@ -107,6 +111,9 @@ class DefaultObservedModule implements ObservedModule { */ @Override public boolean isObservedModule(ApplicationModule module) { + + Assert.notNull(module, "AppliucationModule must not be null!"); + return this.module.equals(module); } @@ -116,6 +123,9 @@ class DefaultObservedModule implements ObservedModule { */ public ObservedModuleType getObservedModuleType(Class type, ApplicationModules modules) { + Assert.notNull(type, "Type must not be null!"); + Assert.notNull(modules, "ApplicationModules must not be null!"); + return module.getSpringBeans().stream() .filter(it -> it.getFullyQualifiedTypeName().equals(type.getName())) .map(SpringBean::toArchitecturallyEvidentType) diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModule.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModule.java index 62c22b12..e31daae2 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModule.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModule.java @@ -18,6 +18,7 @@ package org.springframework.modulith.observability; import java.lang.reflect.Method; import org.aopalliance.intercept.MethodInvocation; +import org.springframework.lang.Nullable; import org.springframework.modulith.model.ApplicationModule; import org.springframework.modulith.model.ApplicationModules; @@ -53,9 +54,10 @@ interface ObservedModule { /** * Returns the {@link ObservedModuleType} for the given type and {@link ApplicationModules}. * - * @param type - * @param modules - * @return + * @param type must not be {@literal null}. + * @param modules must not be {@literal null}. + * @return the {@link ObservedModuleType} for the given type or {@literal null} if the type is not to be observed. */ + @Nullable ObservedModuleType getObservedModuleType(Class type, ApplicationModules modules); } 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 2f1cb535..01a8c934 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 @@ -18,12 +18,17 @@ package org.springframework.modulith.observability; import lombok.RequiredArgsConstructor; import java.lang.reflect.Method; +import java.util.Collection; +import java.util.List; import java.util.function.Predicate; import java.util.stream.Stream; +import org.springframework.aop.TargetClassAware; +import org.springframework.aop.framework.Advised; +import org.springframework.modulith.model.ApplicationModules; import org.springframework.modulith.model.ArchitecturallyEvidentType; import org.springframework.modulith.model.ArchitecturallyEvidentType.ReferenceMethod; -import org.springframework.modulith.model.ApplicationModules; +import org.springframework.util.ReflectionUtils; /** * Represents a type in an {@link ObservedModule}. @@ -33,6 +38,8 @@ import org.springframework.modulith.model.ApplicationModules; @RequiredArgsConstructor public class ObservedModuleType { + private static Collection> IGNORED_TYPES = List.of(Advised.class, TargetClassAware.class); + private final ApplicationModules modules; private final ObservedModule module; private final ArchitecturallyEvidentType type; @@ -59,7 +66,7 @@ public class ObservedModuleType { public Predicate getMethodsToIntercept() { if (!type.isEventListener()) { - return it -> true; + return it -> !(ReflectionUtils.isObjectMethod(it) || IGNORED_TYPES.contains(it.getDeclaringClass())); } return candidate -> type.getReferenceMethods() // diff --git a/spring-modulith-observability/src/test/java/example/sample/SampleComponent.java b/spring-modulith-observability/src/test/java/example/sample/SampleComponent.java new file mode 100644 index 00000000..71013cea --- /dev/null +++ b/spring-modulith-observability/src/test/java/example/sample/SampleComponent.java @@ -0,0 +1,27 @@ +/* + * Copyright 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 example.sample; + +import org.springframework.stereotype.Component; + +/** + * @author Oliver Drotbohm + */ +@Component +public class SampleComponent { + + void someMethod() {} +} 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 new file mode 100644 index 00000000..534425d2 --- /dev/null +++ b/spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java @@ -0,0 +1,60 @@ +/* + * Copyright 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.modulith.observability; + +import static org.assertj.core.api.Assertions.*; + +import example.sample.SampleComponent; + +import org.junit.jupiter.api.Test; +import org.springframework.aop.framework.Advised; +import org.springframework.modulith.model.ApplicationModule; +import org.springframework.modulith.model.ApplicationModules; +import org.springframework.modulith.model.ArchitecturallyEvidentType; +import org.springframework.modulith.test.TestApplicationModules; +import org.springframework.util.ReflectionUtils; + +/** + * Unit tests for {@link ObservedModuleType}. + * + * @author Oliver Drotbohm + */ +public class ObservedModuleTypeUnitTests { + + static final ApplicationModules modules = TestApplicationModules.of("example"); + + ApplicationModule module = modules.getModuleByName("sample").orElseThrow(); + ArchitecturallyEvidentType type = module.getArchitecturallyEvidentType(SampleComponent.class); + + ObservedModuleType observedType = new ObservedModuleType(modules, new DefaultObservedModule(module), type); + + @Test // GH-106 + void onlyExposesUserMethodsAsToBeIntercepted() { + + assertThat(observedType.getMethodsToIntercept()).satisfies(it -> { + + assertThat(it.test(ReflectionUtils.findMethod(SampleComponent.class, "someMethod"))).isTrue(); + + assertThat(it.test(ReflectionUtils.findMethod(Object.class, "toString"))).isFalse(); + assertThat(it.test(ReflectionUtils.findMethod(Advised.class, "getTargetClass"))).isFalse(); + }); + } + + @Test // GH-106 + void considersExposedTypeAsToBeIntercepted() { + assertThat(observedType.shouldBeTraced()).isTrue(); + } +}