Skip to content

Commit 5e31856

Browse files
sbrannensbrannen
committed
Polish CustomizableTraceInterceptor[Tests]
1 parent 1058fed commit 5e31856

File tree

2 files changed

+76
-75
lines changed

2 files changed

+76
-75
lines changed

‎spring-aop/src/main/java/org/springframework/aop/interceptor/CustomizableTraceInterceptor.java

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2023 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.
@@ -62,6 +62,7 @@
6262
*
6363
* @author Rob Harrop
6464
* @author Juergen Hoeller
65+
* @author Sam Brannen
6566
* @since 1.2
6667
* @see #setEnterMessage
6768
* @see #setExitMessage
@@ -295,43 +296,38 @@ protected Object invokeUnderTrace(MethodInvocation invocation, Log logger) throw
295296
protected String replacePlaceholders(String message, MethodInvocation methodInvocation,
296297
@Nullable Object returnValue, @Nullable Throwable throwable, long invocationTime) {
297298

298-
Matcher matcher = PATTERN.matcher(message);
299299
Object target = methodInvocation.getThis();
300300
Assert.state(target != null, "Target must not be null");
301301

302302
StringBuilder output = new StringBuilder();
303+
Matcher matcher = PATTERN.matcher(message);
303304
while (matcher.find()) {
304305
String match = matcher.group();
305-
if (PLACEHOLDER_METHOD_NAME.equals(match)) {
306-
matcher.appendReplacement(output, Matcher.quoteReplacement(methodInvocation.getMethod().getName()));
307-
}
308-
else if (PLACEHOLDER_TARGET_CLASS_NAME.equals(match)) {
309-
String className = getClassForLogging(target).getName();
310-
matcher.appendReplacement(output, Matcher.quoteReplacement(className));
311-
}
312-
else if (PLACEHOLDER_TARGET_CLASS_SHORT_NAME.equals(match)) {
313-
String shortName = ClassUtils.getShortName(getClassForLogging(target));
314-
matcher.appendReplacement(output, Matcher.quoteReplacement(shortName));
315-
}
316-
else if (PLACEHOLDER_ARGUMENTS.equals(match)) {
317-
matcher.appendReplacement(output,
306+
switch (match) {
307+
case PLACEHOLDER_METHOD_NAME -> matcher.appendReplacement(output,
308+
Matcher.quoteReplacement(methodInvocation.getMethod().getName()));
309+
case PLACEHOLDER_TARGET_CLASS_NAME -> {
310+
String className = getClassForLogging(target).getName();
311+
matcher.appendReplacement(output, Matcher.quoteReplacement(className));
312+
}
313+
case PLACEHOLDER_TARGET_CLASS_SHORT_NAME -> {
314+
String shortName = ClassUtils.getShortName(getClassForLogging(target));
315+
matcher.appendReplacement(output, Matcher.quoteReplacement(shortName));
316+
}
317+
case PLACEHOLDER_ARGUMENTS -> matcher.appendReplacement(output,
318318
Matcher.quoteReplacement(StringUtils.arrayToCommaDelimitedString(methodInvocation.getArguments())));
319-
}
320-
else if (PLACEHOLDER_ARGUMENT_TYPES.equals(match)) {
321-
appendArgumentTypes(methodInvocation, matcher, output);
322-
}
323-
else if (PLACEHOLDER_RETURN_VALUE.equals(match)) {
324-
appendReturnValue(methodInvocation, matcher, output, returnValue);
325-
}
326-
else if (throwable != null && PLACEHOLDER_EXCEPTION.equals(match)) {
327-
matcher.appendReplacement(output, Matcher.quoteReplacement(throwable.toString()));
328-
}
329-
else if (PLACEHOLDER_INVOCATION_TIME.equals(match)) {
330-
matcher.appendReplacement(output, Long.toString(invocationTime));
331-
}
332-
else {
333-
// Should not happen since placeholders are checked earlier.
334-
throw new IllegalArgumentException("Unknown placeholder [" + match + "]");
319+
case PLACEHOLDER_ARGUMENT_TYPES -> appendArgumentTypes(methodInvocation, matcher, output);
320+
case PLACEHOLDER_RETURN_VALUE -> appendReturnValue(methodInvocation, matcher, output, returnValue);
321+
case PLACEHOLDER_EXCEPTION -> {
322+
if (throwable != null) {
323+
matcher.appendReplacement(output, Matcher.quoteReplacement(throwable.toString()));
324+
}
325+
}
326+
case PLACEHOLDER_INVOCATION_TIME -> matcher.appendReplacement(output, Long.toString(invocationTime));
327+
default -> {
328+
// Should not happen since placeholders are checked earlier.
329+
throw new IllegalArgumentException("Unknown placeholder [" + match + "]");
330+
}
335331
}
336332
}
337333
matcher.appendTail(output);
@@ -348,7 +344,7 @@ else if (PLACEHOLDER_INVOCATION_TIME.equals(match)) {
348344
* @param output the {@code StringBuilder} to write output to
349345
* @param returnValue the value returned by the method invocation.
350346
*/
351-
private void appendReturnValue(
347+
private static void appendReturnValue(
352348
MethodInvocation methodInvocation, Matcher matcher, StringBuilder output, @Nullable Object returnValue) {
353349

354350
if (methodInvocation.getMethod().getReturnType() == void.class) {
@@ -372,7 +368,7 @@ else if (returnValue == null) {
372368
* @param matcher the {@code Matcher} containing the state of the output
373369
* @param output the {@code StringBuilder} containing the output
374370
*/
375-
private void appendArgumentTypes(MethodInvocation methodInvocation, Matcher matcher, StringBuilder output) {
371+
private static void appendArgumentTypes(MethodInvocation methodInvocation, Matcher matcher, StringBuilder output) {
376372
Class<?>[] argumentTypes = methodInvocation.getMethod().getParameterTypes();
377373
String[] argumentTypeShortNames = new String[argumentTypes.length];
378374
for (int i = 0; i < argumentTypeShortNames.length; i++) {
@@ -387,7 +383,7 @@ private void appendArgumentTypes(MethodInvocation methodInvocation, Matcher matc
387383
* that are not specified as constants on this class and throws an
388384
* {@code IllegalArgumentException} if so.
389385
*/
390-
private void checkForInvalidPlaceholders(String message) throws IllegalArgumentException {
386+
private static void checkForInvalidPlaceholders(String message) throws IllegalArgumentException {
391387
Matcher matcher = PATTERN.matcher(message);
392388
while (matcher.find()) {
393389
String match = matcher.group();

‎spring-aop/src/test/java/org/springframework/aop/interceptor/CustomizableTraceInterceptorTests.java

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,73 +27,79 @@
2727
import static org.mockito.Mockito.mock;
2828
import static org.mockito.Mockito.times;
2929
import static org.mockito.Mockito.verify;
30+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS;
31+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES;
32+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION;
33+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME;
34+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME;
35+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE;
36+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_NAME;
37+
import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_SHORT_NAME;
3038

3139
/**
40+
* Tests for {@link CustomizableTraceInterceptor}.
41+
*
3242
* @author Rob Harrop
3343
* @author Rick Evans
3444
* @author Juergen Hoeller
3545
* @author Chris Beams
46+
* @author Sam Brannen
3647
*/
37-
public class CustomizableTraceInterceptorTests {
48+
class CustomizableTraceInterceptorTests {
49+
50+
private final CustomizableTraceInterceptor interceptor = new CustomizableTraceInterceptor();
51+
3852

3953
@Test
40-
public void testSetEmptyEnterMessage() {
54+
void setEmptyEnterMessage() {
4155
// Must not be able to set empty enter message
42-
assertThatIllegalArgumentException().isThrownBy(() ->
43-
new CustomizableTraceInterceptor().setEnterMessage(""));
56+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(""));
4457
}
4558

4659
@Test
47-
public void testSetEnterMessageWithReturnValuePlaceholder() {
60+
void setEnterMessageWithReturnValuePlaceholder() {
4861
// Must not be able to set enter message with return value placeholder
49-
assertThatIllegalArgumentException().isThrownBy(() ->
50-
new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE));
62+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_RETURN_VALUE));
5163
}
5264

5365
@Test
54-
public void testSetEnterMessageWithExceptionPlaceholder() {
66+
void setEnterMessageWithExceptionPlaceholder() {
5567
// Must not be able to set enter message with exception placeholder
56-
assertThatIllegalArgumentException().isThrownBy(() ->
57-
new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION));
68+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_EXCEPTION));
5869
}
5970

6071
@Test
61-
public void testSetEnterMessageWithInvocationTimePlaceholder() {
72+
void setEnterMessageWithInvocationTimePlaceholder() {
6273
// Must not be able to set enter message with invocation time placeholder
63-
assertThatIllegalArgumentException().isThrownBy(() ->
64-
new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME));
74+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_INVOCATION_TIME));
6575
}
6676

6777
@Test
68-
public void testSetEmptyExitMessage() {
78+
void setEmptyExitMessage() {
6979
// Must not be able to set empty exit message
70-
assertThatIllegalArgumentException().isThrownBy(() ->
71-
new CustomizableTraceInterceptor().setExitMessage(""));
80+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExitMessage(""));
7281
}
7382

7483
@Test
75-
public void testSetExitMessageWithExceptionPlaceholder() {
84+
void setExitMessageWithExceptionPlaceholder() {
7685
// Must not be able to set exit message with exception placeholder
77-
assertThatIllegalArgumentException().isThrownBy(() ->
78-
new CustomizableTraceInterceptor().setExitMessage(CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION));
86+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExitMessage(PLACEHOLDER_EXCEPTION));
7987
}
8088

8189
@Test
82-
public void testSetEmptyExceptionMessage() {
90+
void setEmptyExceptionMessage() {
8391
// Must not be able to set empty exception message
84-
assertThatIllegalArgumentException().isThrownBy(() ->
85-
new CustomizableTraceInterceptor().setExceptionMessage(""));
92+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExceptionMessage(""));
8693
}
8794

8895
@Test
89-
public void testSetExceptionMethodWithReturnValuePlaceholder() {
96+
void setExceptionMethodWithReturnValuePlaceholder() {
9097
// Must not be able to set exception message with return value placeholder
91-
assertThatIllegalArgumentException().isThrownBy(() ->
92-
new CustomizableTraceInterceptor().setExceptionMessage(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE));
98+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExceptionMessage(PLACEHOLDER_RETURN_VALUE));
9399
}
94100

95101
@Test
96-
public void testSunnyDayPathLogsCorrectly() throws Throwable {
102+
void sunnyDayPathLogsCorrectly() throws Throwable {
97103
MethodInvocation methodInvocation = mock();
98104
given(methodInvocation.getMethod()).willReturn(String.class.getMethod("toString"));
99105
given(methodInvocation.getThis()).willReturn(this);
@@ -108,7 +114,7 @@ public void testSunnyDayPathLogsCorrectly() throws Throwable {
108114
}
109115

110116
@Test
111-
public void testExceptionPathLogsCorrectly() throws Throwable {
117+
void exceptionPathLogsCorrectly() throws Throwable {
112118
MethodInvocation methodInvocation = mock();
113119

114120
IllegalArgumentException exception = new IllegalArgumentException();
@@ -120,15 +126,14 @@ public void testExceptionPathLogsCorrectly() throws Throwable {
120126
given(log.isTraceEnabled()).willReturn(true);
121127

122128
CustomizableTraceInterceptor interceptor = new StubCustomizableTraceInterceptor(log);
123-
assertThatIllegalArgumentException().isThrownBy(() ->
124-
interceptor.invoke(methodInvocation));
129+
assertThatIllegalArgumentException().isThrownBy(() -> interceptor.invoke(methodInvocation));
125130

126131
verify(log).trace(anyString());
127132
verify(log).trace(anyString(), eq(exception));
128133
}
129134

130135
@Test
131-
public void testSunnyDayPathLogsCorrectlyWithPrettyMuchAllPlaceholdersMatching() throws Throwable {
136+
void sunnyDayPathLogsCorrectlyWithPrettyMuchAllPlaceholdersMatching() throws Throwable {
132137
MethodInvocation methodInvocation = mock();
133138

134139
given(methodInvocation.getMethod()).willReturn(String.class.getMethod("toString", new Class[0]));
@@ -141,18 +146,18 @@ public void testSunnyDayPathLogsCorrectlyWithPrettyMuchAllPlaceholdersMatching()
141146

142147
CustomizableTraceInterceptor interceptor = new StubCustomizableTraceInterceptor(log);
143148
interceptor.setEnterMessage(new StringBuilder()
144-
.append("Entering the '").append(CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME)
145-
.append("' method of the [").append(CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_NAME)
146-
.append("] class with the following args (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS)
147-
.append(") and arg types (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES)
149+
.append("Entering the '").append(PLACEHOLDER_METHOD_NAME)
150+
.append("' method of the [").append(PLACEHOLDER_TARGET_CLASS_NAME)
151+
.append("] class with the following args (").append(PLACEHOLDER_ARGUMENTS)
152+
.append(") and arg types (").append(PLACEHOLDER_ARGUMENT_TYPES)
148153
.append(").").toString());
149154
interceptor.setExitMessage(new StringBuilder()
150-
.append("Exiting the '").append(CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME)
151-
.append("' method of the [").append(CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_SHORT_NAME)
152-
.append("] class with the following args (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS)
153-
.append(") and arg types (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES)
154-
.append("), returning '").append(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE)
155-
.append("' and taking '").append(CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME)
155+
.append("Exiting the '").append(PLACEHOLDER_METHOD_NAME)
156+
.append("' method of the [").append(PLACEHOLDER_TARGET_CLASS_SHORT_NAME)
157+
.append("] class with the following args (").append(PLACEHOLDER_ARGUMENTS)
158+
.append(") and arg types (").append(PLACEHOLDER_ARGUMENT_TYPES)
159+
.append("), returning '").append(PLACEHOLDER_RETURN_VALUE)
160+
.append("' and taking '").append(PLACEHOLDER_INVOCATION_TIME)
156161
.append("' this long.").toString());
157162
interceptor.invoke(methodInvocation);
158163

@@ -165,7 +170,7 @@ private static class StubCustomizableTraceInterceptor extends CustomizableTraceI
165170

166171
private final Log log;
167172

168-
public StubCustomizableTraceInterceptor(Log log) {
173+
StubCustomizableTraceInterceptor(Log log) {
169174
super.setUseDynamicLogger(false);
170175
this.log = log;
171176
}

0 commit comments

Comments
 (0)