Skip to content

Commit 9c1346d

Browse files
committed
Merge pull request #34316 from JoshuaChen
* pr/34316: Polish "Handle arbitrary JoinPoint argument index" Handle arbitrary JoinPoint argument index Closes gh-34316
2 parents a4d99d6 + fb6e865 commit 9c1346d

File tree

2 files changed

+148
-9
lines changed

2 files changed

+148
-9
lines changed

Diff for: spring-aop/src/main/java/org/springframework/aop/aspectj/AbstractAspectJAdvice.java

+13-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -276,14 +276,18 @@ public void setArgumentNamesFromStringArray(String... argumentNames) {
276276
}
277277
if (this.aspectJAdviceMethod.getParameterCount() == this.argumentNames.length + 1) {
278278
// May need to add implicit join point arg name...
279-
Class<?> firstArgType = this.aspectJAdviceMethod.getParameterTypes()[0];
280-
if (firstArgType == JoinPoint.class ||
281-
firstArgType == ProceedingJoinPoint.class ||
282-
firstArgType == JoinPoint.StaticPart.class) {
283-
String[] oldNames = this.argumentNames;
284-
this.argumentNames = new String[oldNames.length + 1];
285-
this.argumentNames[0] = "THIS_JOIN_POINT";
286-
System.arraycopy(oldNames, 0, this.argumentNames, 1, oldNames.length);
279+
for (int i = 0; i < this.aspectJAdviceMethod.getParameterCount(); i++) {
280+
Class<?> argType = this.aspectJAdviceMethod.getParameterTypes()[i];
281+
if (argType == JoinPoint.class ||
282+
argType == ProceedingJoinPoint.class ||
283+
argType == JoinPoint.StaticPart.class) {
284+
String[] oldNames = this.argumentNames;
285+
this.argumentNames = new String[oldNames.length + 1];
286+
System.arraycopy(oldNames, 0, this.argumentNames, 0, i);
287+
this.argumentNames[i] = "THIS_JOIN_POINT";
288+
System.arraycopy(oldNames, i, this.argumentNames, i + 1, oldNames.length - i);
289+
break;
290+
}
287291
}
288292
}
289293
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
* Copyright 2002-2025 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+
17+
package org.springframework.aop.aspectj;
18+
19+
import java.lang.reflect.Method;
20+
import java.util.Arrays;
21+
import java.util.function.Consumer;
22+
23+
import org.aspectj.lang.JoinPoint;
24+
import org.aspectj.lang.ProceedingJoinPoint;
25+
import org.assertj.core.api.InstanceOfAssertFactories;
26+
import org.junit.jupiter.api.Test;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
import static org.mockito.Mockito.mock;
30+
31+
/**
32+
* Tests for {@link AbstractAspectJAdvice}.
33+
*
34+
* @author Joshua Chen
35+
* @author Stephane Nicoll
36+
*/
37+
class AbstractAspectJAdviceTests {
38+
39+
@Test
40+
void setArgumentNamesFromStringArray_withoutJoinPointParameter() {
41+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithNoJoinPoint");
42+
assertThat(advice).satisfies(hasArgumentNames("arg1", "arg2"));
43+
}
44+
45+
@Test
46+
void setArgumentNamesFromStringArray_withJoinPointAsFirstParameter() {
47+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithJoinPointAsFirstParameter");
48+
assertThat(advice).satisfies(hasArgumentNames("THIS_JOIN_POINT", "arg1", "arg2"));
49+
}
50+
51+
@Test
52+
void setArgumentNamesFromStringArray_withJoinPointAsLastParameter() {
53+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithJoinPointAsLastParameter");
54+
assertThat(advice).satisfies(hasArgumentNames("arg1", "arg2", "THIS_JOIN_POINT"));
55+
}
56+
57+
@Test
58+
void setArgumentNamesFromStringArray_withJoinPointAsMiddleParameter() {
59+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithJoinPointAsMiddleParameter");
60+
assertThat(advice).satisfies(hasArgumentNames("arg1", "THIS_JOIN_POINT", "arg2"));
61+
}
62+
63+
@Test
64+
void setArgumentNamesFromStringArray_withProceedingJoinPoint() {
65+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithProceedingJoinPoint");
66+
assertThat(advice).satisfies(hasArgumentNames("THIS_JOIN_POINT", "arg1", "arg2"));
67+
}
68+
69+
@Test
70+
void setArgumentNamesFromStringArray_withStaticPart() {
71+
AbstractAspectJAdvice advice = getAspectJAdvice("methodWithStaticPart");
72+
assertThat(advice).satisfies(hasArgumentNames("THIS_JOIN_POINT", "arg1", "arg2"));
73+
}
74+
75+
private Consumer<AbstractAspectJAdvice> hasArgumentNames(String... argumentNames) {
76+
return advice -> assertThat(advice).extracting("argumentNames")
77+
.asInstanceOf(InstanceOfAssertFactories.array(String[].class))
78+
.containsExactly(argumentNames);
79+
}
80+
81+
private AbstractAspectJAdvice getAspectJAdvice(final String methodName) {
82+
AbstractAspectJAdvice advice = new TestAspectJAdvice(getMethod(methodName),
83+
mock(AspectJExpressionPointcut.class), mock(AspectInstanceFactory.class));
84+
advice.setArgumentNamesFromStringArray("arg1", "arg2");
85+
return advice;
86+
}
87+
88+
private Method getMethod(final String methodName) {
89+
return Arrays.stream(Sample.class.getDeclaredMethods())
90+
.filter(method -> method.getName().equals(methodName)).findFirst()
91+
.orElseThrow();
92+
}
93+
94+
@SuppressWarnings("serial")
95+
public static class TestAspectJAdvice extends AbstractAspectJAdvice {
96+
97+
public TestAspectJAdvice(Method aspectJAdviceMethod, AspectJExpressionPointcut pointcut,
98+
AspectInstanceFactory aspectInstanceFactory) {
99+
super(aspectJAdviceMethod, pointcut, aspectInstanceFactory);
100+
}
101+
102+
@Override
103+
public boolean isBeforeAdvice() {
104+
return false;
105+
}
106+
107+
@Override
108+
public boolean isAfterAdvice() {
109+
return false;
110+
}
111+
}
112+
113+
@SuppressWarnings("unused")
114+
static class Sample {
115+
116+
void methodWithNoJoinPoint(String arg1, String arg2) {
117+
}
118+
119+
void methodWithJoinPointAsFirstParameter(JoinPoint joinPoint, String arg1, String arg2) {
120+
}
121+
122+
void methodWithJoinPointAsLastParameter(String arg1, String arg2, JoinPoint joinPoint) {
123+
}
124+
125+
void methodWithJoinPointAsMiddleParameter(String arg1, JoinPoint joinPoint, String arg2) {
126+
}
127+
128+
void methodWithProceedingJoinPoint(ProceedingJoinPoint joinPoint, String arg1, String arg2) {
129+
}
130+
131+
void methodWithStaticPart(JoinPoint.StaticPart staticPart, String arg1, String arg2) {
132+
}
133+
}
134+
135+
}

0 commit comments

Comments
 (0)