Skip to content

Commit 73050ed

Browse files
committed
GH-746 - Fix observability interception for components declaring event listeners.
1 parent 7ebee3c commit 73050ed

File tree

4 files changed

+66
-15
lines changed

4 files changed

+66
-15
lines changed

spring-modulith-observability/pom.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@
6060
<scope>test</scope>
6161
</dependency>
6262

63+
<dependency>
64+
<groupId>org.springframework.modulith</groupId>
65+
<artifactId>spring-modulith-events-api</artifactId>
66+
<version>${project.version}</version>
67+
<scope>test</scope>
68+
</dependency>
69+
6370
<dependency>
6471
<groupId>io.micrometer</groupId>
6572
<artifactId>micrometer-observation</artifactId>

spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ObservedModuleType.java

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.modulith.observability;
1717

1818
import java.lang.reflect.Method;
19+
import java.lang.reflect.Modifier;
1920
import java.util.Collection;
2021
import java.util.List;
2122
import java.util.function.Predicate;
@@ -38,10 +39,13 @@
3839
class ObservedModuleType {
3940

4041
private static Collection<Class<?>> IGNORED_TYPES = List.of(Advised.class, TargetClassAware.class);
42+
private static Predicate<Method> IS_USER_METHOD = it -> !Modifier.isPrivate(it.getModifiers())
43+
&& !(ReflectionUtils.isObjectMethod(it) || IGNORED_TYPES.contains(it.getDeclaringClass()));
4144

4245
private final ApplicationModules modules;
4346
private final ObservedModule module;
4447
private final ArchitecturallyEvidentType type;
48+
private final Predicate<Method> methodsToInterceptFilter;
4549

4650
/**
4751
* Creates a new {@link ObservedModuleType} for the given {@link ApplicationModules}, {@link ObservedModule} and
@@ -60,6 +64,12 @@ class ObservedModuleType {
6064
this.modules = modules;
6165
this.module = module;
6266
this.type = type;
67+
68+
Predicate<Method> isReferenceMethod = candidate -> type.isEventListener() && type.getReferenceMethods() //
69+
.map(ReferenceMethod::getMethod) //
70+
.anyMatch(it -> it.reflect().equals(candidate));
71+
72+
this.methodsToInterceptFilter = IS_USER_METHOD.or(isReferenceMethod);
6373
}
6474

6575
/**
@@ -80,20 +90,14 @@ public boolean shouldBeObserved() {
8090
}
8191

8292
/**
83-
* Returns a predicate to filter the methods to intercept. For event listeners it's the listener methods only. For
84-
* everything else, all (public) methods will be intercepted.
93+
* Returns a predicate to filter the methods to intercept. All user declared methods are intercepted, except from
94+
* well-known interfaces ({@code Advised}, {@code TargetClassAware}). For event listeners, package-protected methods
95+
* are supported as well.
8596
*
86-
* @return
97+
* @return will never be {@literal null}.
8798
*/
8899
public Predicate<Method> getMethodsToIntercept() {
89-
90-
if (!type.isEventListener()) {
91-
return it -> !(ReflectionUtils.isObjectMethod(it) || IGNORED_TYPES.contains(it.getDeclaringClass()));
92-
}
93-
94-
return candidate -> type.getReferenceMethods() //
95-
.map(ReferenceMethod::getMethod) //
96-
.anyMatch(it -> it.reflect().equals(candidate));
100+
return methodsToInterceptFilter;
97101
}
98102

99103
private boolean listensToOtherModulesEvents() {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright 2023-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package example.sample;
17+
18+
import org.springframework.aop.TargetClassAware;
19+
import org.springframework.aop.framework.Advised;
20+
import org.springframework.modulith.events.ApplicationModuleListener;
21+
import org.springframework.scheduling.annotation.Async;
22+
import org.springframework.stereotype.Component;
23+
24+
/**
25+
* @author Oliver Drotbohm
26+
*/
27+
@Component
28+
public abstract class ObservedComponent implements Advised, TargetClassAware {
29+
30+
@Async
31+
public void someMethod() {}
32+
33+
@SuppressWarnings("unused")
34+
private void someInternalMethod() {}
35+
36+
@ApplicationModuleListener
37+
void on(Object event) {}
38+
}

spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import static org.assertj.core.api.Assertions.*;
1919

20-
import example.sample.SampleComponent;
20+
import example.sample.ObservedComponent;
2121
import example.sample.SampleConfiguration;
2222

2323
import org.junit.jupiter.api.Test;
@@ -38,17 +38,19 @@ class ObservedModuleTypeUnitTests {
3838
static final ApplicationModules modules = TestApplicationModules.of("example");
3939

4040
ApplicationModule module = modules.getModuleByName("sample").orElseThrow();
41-
ArchitecturallyEvidentType type = module.getArchitecturallyEvidentType(SampleComponent.class);
41+
ArchitecturallyEvidentType type = module.getArchitecturallyEvidentType(ObservedComponent.class);
4242

4343
ObservedModuleType observedType = new ObservedModuleType(modules, new DefaultObservedModule(module), type);
4444

45-
@Test // GH-106
45+
@Test // GH-106, GH-744
4646
void onlyExposesUserMethodsAsToBeIntercepted() {
4747

4848
assertThat(observedType.getMethodsToIntercept()).satisfies(it -> {
4949

50-
assertThat(it.test(ReflectionUtils.findMethod(SampleComponent.class, "someMethod"))).isTrue();
50+
assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "someMethod"))).isTrue();
51+
assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "on", Object.class))).isTrue();
5152

53+
assertThat(it.test(ReflectionUtils.findMethod(ObservedComponent.class, "someInternalMethod"))).isFalse();
5254
assertThat(it.test(ReflectionUtils.findMethod(Object.class, "toString"))).isFalse();
5355
assertThat(it.test(ReflectionUtils.findMethod(Advised.class, "getTargetClass"))).isFalse();
5456
});

0 commit comments

Comments
 (0)