diff --git a/spring-aop/src/main/java/org/springframework/aop/interceptor/CustomizableTraceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/interceptor/CustomizableTraceInterceptor.java index bc675836229..bc7b48f4c9f 100644 --- a/spring-aop/src/main/java/org/springframework/aop/interceptor/CustomizableTraceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/interceptor/CustomizableTraceInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-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. @@ -62,6 +62,7 @@ import org.springframework.util.StringUtils; * * @author Rob Harrop * @author Juergen Hoeller + * @author Sam Brannen * @since 1.2 * @see #setEnterMessage * @see #setExitMessage @@ -295,43 +296,38 @@ public class CustomizableTraceInterceptor extends AbstractTraceInterceptor { protected String replacePlaceholders(String message, MethodInvocation methodInvocation, @Nullable Object returnValue, @Nullable Throwable throwable, long invocationTime) { - Matcher matcher = PATTERN.matcher(message); Object target = methodInvocation.getThis(); Assert.state(target != null, "Target must not be null"); StringBuilder output = new StringBuilder(); + Matcher matcher = PATTERN.matcher(message); while (matcher.find()) { String match = matcher.group(); - if (PLACEHOLDER_METHOD_NAME.equals(match)) { - matcher.appendReplacement(output, Matcher.quoteReplacement(methodInvocation.getMethod().getName())); - } - else if (PLACEHOLDER_TARGET_CLASS_NAME.equals(match)) { - String className = getClassForLogging(target).getName(); - matcher.appendReplacement(output, Matcher.quoteReplacement(className)); - } - else if (PLACEHOLDER_TARGET_CLASS_SHORT_NAME.equals(match)) { - String shortName = ClassUtils.getShortName(getClassForLogging(target)); - matcher.appendReplacement(output, Matcher.quoteReplacement(shortName)); - } - else if (PLACEHOLDER_ARGUMENTS.equals(match)) { - matcher.appendReplacement(output, + switch (match) { + case PLACEHOLDER_METHOD_NAME -> matcher.appendReplacement(output, + Matcher.quoteReplacement(methodInvocation.getMethod().getName())); + case PLACEHOLDER_TARGET_CLASS_NAME -> { + String className = getClassForLogging(target).getName(); + matcher.appendReplacement(output, Matcher.quoteReplacement(className)); + } + case PLACEHOLDER_TARGET_CLASS_SHORT_NAME -> { + String shortName = ClassUtils.getShortName(getClassForLogging(target)); + matcher.appendReplacement(output, Matcher.quoteReplacement(shortName)); + } + case PLACEHOLDER_ARGUMENTS -> matcher.appendReplacement(output, Matcher.quoteReplacement(StringUtils.arrayToCommaDelimitedString(methodInvocation.getArguments()))); - } - else if (PLACEHOLDER_ARGUMENT_TYPES.equals(match)) { - appendArgumentTypes(methodInvocation, matcher, output); - } - else if (PLACEHOLDER_RETURN_VALUE.equals(match)) { - appendReturnValue(methodInvocation, matcher, output, returnValue); - } - else if (throwable != null && PLACEHOLDER_EXCEPTION.equals(match)) { - matcher.appendReplacement(output, Matcher.quoteReplacement(throwable.toString())); - } - else if (PLACEHOLDER_INVOCATION_TIME.equals(match)) { - matcher.appendReplacement(output, Long.toString(invocationTime)); - } - else { - // Should not happen since placeholders are checked earlier. - throw new IllegalArgumentException("Unknown placeholder [" + match + "]"); + case PLACEHOLDER_ARGUMENT_TYPES -> appendArgumentTypes(methodInvocation, matcher, output); + case PLACEHOLDER_RETURN_VALUE -> appendReturnValue(methodInvocation, matcher, output, returnValue); + case PLACEHOLDER_EXCEPTION -> { + if (throwable != null) { + matcher.appendReplacement(output, Matcher.quoteReplacement(throwable.toString())); + } + } + case PLACEHOLDER_INVOCATION_TIME -> matcher.appendReplacement(output, Long.toString(invocationTime)); + default -> { + // Should not happen since placeholders are checked earlier. + throw new IllegalArgumentException("Unknown placeholder [" + match + "]"); + } } } matcher.appendTail(output); @@ -348,7 +344,7 @@ public class CustomizableTraceInterceptor extends AbstractTraceInterceptor { * @param output the {@code StringBuilder} to write output to * @param returnValue the value returned by the method invocation. */ - private void appendReturnValue( + private static void appendReturnValue( MethodInvocation methodInvocation, Matcher matcher, StringBuilder output, @Nullable Object returnValue) { if (methodInvocation.getMethod().getReturnType() == void.class) { @@ -372,7 +368,7 @@ public class CustomizableTraceInterceptor extends AbstractTraceInterceptor { * @param matcher the {@code Matcher} containing the state of the output * @param output the {@code StringBuilder} containing the output */ - private void appendArgumentTypes(MethodInvocation methodInvocation, Matcher matcher, StringBuilder output) { + private static void appendArgumentTypes(MethodInvocation methodInvocation, Matcher matcher, StringBuilder output) { Class[] argumentTypes = methodInvocation.getMethod().getParameterTypes(); String[] argumentTypeShortNames = new String[argumentTypes.length]; for (int i = 0; i < argumentTypeShortNames.length; i++) { @@ -387,7 +383,7 @@ public class CustomizableTraceInterceptor extends AbstractTraceInterceptor { * that are not specified as constants on this class and throws an * {@code IllegalArgumentException} if so. */ - private void checkForInvalidPlaceholders(String message) throws IllegalArgumentException { + private static void checkForInvalidPlaceholders(String message) throws IllegalArgumentException { Matcher matcher = PATTERN.matcher(message); while (matcher.find()) { String match = matcher.group(); diff --git a/spring-aop/src/test/java/org/springframework/aop/interceptor/CustomizableTraceInterceptorTests.java b/spring-aop/src/test/java/org/springframework/aop/interceptor/CustomizableTraceInterceptorTests.java index 8b3be03d03b..e989a0753d9 100644 --- a/spring-aop/src/test/java/org/springframework/aop/interceptor/CustomizableTraceInterceptorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/interceptor/CustomizableTraceInterceptorTests.java @@ -27,73 +27,79 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_NAME; +import static org.springframework.aop.interceptor.CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_SHORT_NAME; /** + * Tests for {@link CustomizableTraceInterceptor}. + * * @author Rob Harrop * @author Rick Evans * @author Juergen Hoeller * @author Chris Beams + * @author Sam Brannen */ -public class CustomizableTraceInterceptorTests { +class CustomizableTraceInterceptorTests { + + private final CustomizableTraceInterceptor interceptor = new CustomizableTraceInterceptor(); + @Test - public void testSetEmptyEnterMessage() { + void setEmptyEnterMessage() { // Must not be able to set empty enter message - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setEnterMessage("")); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage("")); } @Test - public void testSetEnterMessageWithReturnValuePlaceholder() { + void setEnterMessageWithReturnValuePlaceholder() { // Must not be able to set enter message with return value placeholder - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_RETURN_VALUE)); } @Test - public void testSetEnterMessageWithExceptionPlaceholder() { + void setEnterMessageWithExceptionPlaceholder() { // Must not be able to set enter message with exception placeholder - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_EXCEPTION)); } @Test - public void testSetEnterMessageWithInvocationTimePlaceholder() { + void setEnterMessageWithInvocationTimePlaceholder() { // Must not be able to set enter message with invocation time placeholder - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setEnterMessage(CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setEnterMessage(PLACEHOLDER_INVOCATION_TIME)); } @Test - public void testSetEmptyExitMessage() { + void setEmptyExitMessage() { // Must not be able to set empty exit message - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setExitMessage("")); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExitMessage("")); } @Test - public void testSetExitMessageWithExceptionPlaceholder() { + void setExitMessageWithExceptionPlaceholder() { // Must not be able to set exit message with exception placeholder - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setExitMessage(CustomizableTraceInterceptor.PLACEHOLDER_EXCEPTION)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExitMessage(PLACEHOLDER_EXCEPTION)); } @Test - public void testSetEmptyExceptionMessage() { + void setEmptyExceptionMessage() { // Must not be able to set empty exception message - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setExceptionMessage("")); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExceptionMessage("")); } @Test - public void testSetExceptionMethodWithReturnValuePlaceholder() { + void setExceptionMethodWithReturnValuePlaceholder() { // Must not be able to set exception message with return value placeholder - assertThatIllegalArgumentException().isThrownBy(() -> - new CustomizableTraceInterceptor().setExceptionMessage(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.setExceptionMessage(PLACEHOLDER_RETURN_VALUE)); } @Test - public void testSunnyDayPathLogsCorrectly() throws Throwable { + void sunnyDayPathLogsCorrectly() throws Throwable { MethodInvocation methodInvocation = mock(); given(methodInvocation.getMethod()).willReturn(String.class.getMethod("toString")); given(methodInvocation.getThis()).willReturn(this); @@ -108,7 +114,7 @@ public class CustomizableTraceInterceptorTests { } @Test - public void testExceptionPathLogsCorrectly() throws Throwable { + void exceptionPathLogsCorrectly() throws Throwable { MethodInvocation methodInvocation = mock(); IllegalArgumentException exception = new IllegalArgumentException(); @@ -120,15 +126,14 @@ public class CustomizableTraceInterceptorTests { given(log.isTraceEnabled()).willReturn(true); CustomizableTraceInterceptor interceptor = new StubCustomizableTraceInterceptor(log); - assertThatIllegalArgumentException().isThrownBy(() -> - interceptor.invoke(methodInvocation)); + assertThatIllegalArgumentException().isThrownBy(() -> interceptor.invoke(methodInvocation)); verify(log).trace(anyString()); verify(log).trace(anyString(), eq(exception)); } @Test - public void testSunnyDayPathLogsCorrectlyWithPrettyMuchAllPlaceholdersMatching() throws Throwable { + void sunnyDayPathLogsCorrectlyWithPrettyMuchAllPlaceholdersMatching() throws Throwable { MethodInvocation methodInvocation = mock(); given(methodInvocation.getMethod()).willReturn(String.class.getMethod("toString", new Class[0])); @@ -141,18 +146,18 @@ public class CustomizableTraceInterceptorTests { CustomizableTraceInterceptor interceptor = new StubCustomizableTraceInterceptor(log); interceptor.setEnterMessage(new StringBuilder() - .append("Entering the '").append(CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME) - .append("' method of the [").append(CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_NAME) - .append("] class with the following args (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS) - .append(") and arg types (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES) + .append("Entering the '").append(PLACEHOLDER_METHOD_NAME) + .append("' method of the [").append(PLACEHOLDER_TARGET_CLASS_NAME) + .append("] class with the following args (").append(PLACEHOLDER_ARGUMENTS) + .append(") and arg types (").append(PLACEHOLDER_ARGUMENT_TYPES) .append(").").toString()); interceptor.setExitMessage(new StringBuilder() - .append("Exiting the '").append(CustomizableTraceInterceptor.PLACEHOLDER_METHOD_NAME) - .append("' method of the [").append(CustomizableTraceInterceptor.PLACEHOLDER_TARGET_CLASS_SHORT_NAME) - .append("] class with the following args (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENTS) - .append(") and arg types (").append(CustomizableTraceInterceptor.PLACEHOLDER_ARGUMENT_TYPES) - .append("), returning '").append(CustomizableTraceInterceptor.PLACEHOLDER_RETURN_VALUE) - .append("' and taking '").append(CustomizableTraceInterceptor.PLACEHOLDER_INVOCATION_TIME) + .append("Exiting the '").append(PLACEHOLDER_METHOD_NAME) + .append("' method of the [").append(PLACEHOLDER_TARGET_CLASS_SHORT_NAME) + .append("] class with the following args (").append(PLACEHOLDER_ARGUMENTS) + .append(") and arg types (").append(PLACEHOLDER_ARGUMENT_TYPES) + .append("), returning '").append(PLACEHOLDER_RETURN_VALUE) + .append("' and taking '").append(PLACEHOLDER_INVOCATION_TIME) .append("' this long.").toString()); interceptor.invoke(methodInvocation); @@ -165,7 +170,7 @@ public class CustomizableTraceInterceptorTests { private final Log log; - public StubCustomizableTraceInterceptor(Log log) { + StubCustomizableTraceInterceptor(Log log) { super.setUseDynamicLogger(false); this.log = log; }