Merge branch '5.3.x'

Author: sbrannen

spring-context/src/main/java/org/springframework/cache/interceptor/LoggingCacheErrorHandler.java

@@ -16,6 +16,8 @@
 
 package org.springframework.cache.interceptor;
 
+import java.util.function.Supplier;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -78,47 +80,71 @@ public LoggingCacheErrorHandler(Log logger, boolean logStackTraces) {
 
 	@Override
 	public void handleCacheGetError(RuntimeException exception, Cache cache, Object key) {
-		logCacheError(logger,
-				createMessage(cache, "failed to get entry with key '" + key + "'"),
+		logCacheError(
+				() -> String.format("Cache '%s' failed to get entry with key '%s'", cache.getName(), key),
 				exception);
 	}
 
 	@Override
 	public void handleCachePutError(RuntimeException exception, Cache cache, Object key, @Nullable Object value) {
-		logCacheError(logger,
-				createMessage(cache, "failed to put entry with key '" + key + "'"),
+		logCacheError(
+				() -> String.format("Cache '%s' failed to put entry with key '%s'", cache.getName(), key),
 				exception);
 	}
 
 	@Override
 	public void handleCacheEvictError(RuntimeException exception, Cache cache, Object key) {
-		logCacheError(logger,
-				createMessage(cache, "failed to evict entry with key '" + key + "'"),
+		logCacheError(
+				() -> String.format("Cache '%s' failed to evict entry with key '%s'", cache.getName(), key),
 				exception);
 	}
 
 	@Override
 	public void handleCacheClearError(RuntimeException exception, Cache cache) {
-		logCacheError(logger, createMessage(cache, "failed to clear entries"), exception);
+		logCacheError(
+				() -> String.format("Cache '%s' failed to clear entries", cache.getName()),
+				exception);
 	}
 
+
 	/**
-	 * Log the specified message.
-	 * @param logger the logger
-	 * @param message the message
-	 * @param ex the exception
+	 * Get the logger for this {@code LoggingCacheErrorHandler}.
+	 * @return the logger
+	 * @since 5.3.22
 	 */
-	protected void logCacheError(Log logger, String message, RuntimeException ex) {
-		if (this.logStackTraces) {
-			logger.warn(message, ex);
-		}
-		else {
-			logger.warn(message);
-		}
+	protected final Log getLogger() {
+		return logger;
 	}
 
-	private String createMessage(Cache cache, String reason) {
-		return String.format("Cache '%s' %s", cache.getName(), reason);
+	/**
+	 * Get the {@code logStackTraces} flag for this {@code LoggingCacheErrorHandler}.
+	 * @return {@code true} if this {@code LoggingCacheErrorHandler} logs stack traces
+	 * @since 5.3.22
+	 */
+	protected final boolean isLogStackTraces() {
+		return this.logStackTraces;
+	}
+
+	/**
+	 * Log the cache error message in the given supplier.
+	 * <p>If {@link #isLogStackTraces()} is {@code true}, the given
+	 * {@code exception} will be logged as well.
+	 * <p>The default implementation logs the message as a warning.
+	 * @param messageSupplier the message supplier
+	 * @param exception the exception thrown by the cache provider
+	 * @since 5.3.22
+	 * @see #isLogStackTraces()
+	 * @see #getLogger()
+	 */
+	protected void logCacheError(Supplier<String> messageSupplier, RuntimeException exception) {
+		if (getLogger().isWarnEnabled()) {
+			if (isLogStackTraces()) {
+				getLogger().warn(messageSupplier.get(), exception);
+			}
+			else {
+				getLogger().warn(messageSupplier.get());
+			}
+		}
 	}
 
 }

spring-context/src/test/java/org/springframework/cache/interceptor/LoggingCacheErrorHandlerTests.java

@@ -17,13 +17,14 @@
 package org.springframework.cache.interceptor;
 
 import org.apache.commons.logging.Log;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
 
+import org.springframework.cache.Cache;
 import org.springframework.cache.support.NoOpCache;
 
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 /**
@@ -32,48 +33,55 @@
  * @author Adam Ostrožlík
  * @author Stephane Nicoll
  * @author Vedran Pavic
+ * @author Sam Brannen
  */
-@ExtendWith(MockitoExtension.class)
 class LoggingCacheErrorHandlerTests {
 
-	@Mock
-	private Log logger;
+	private static final Cache CACHE = new NoOpCache("NOOP");
+
+	private static final String KEY = "enigma";
+
+	private final Log logger = mock(Log.class);
+
+	private LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, false);
+
+
+	@BeforeEach
+	void setUp() {
+		given(this.logger.isWarnEnabled()).willReturn(true);
+	}
 
 
 	@Test
 	void handleGetCacheErrorLogsAppropriateMessage() {
-		LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, false);
-		handler.handleCacheGetError(new RuntimeException(), new NoOpCache("NOOP"), "key");
-		verify(this.logger).warn("Cache 'NOOP' failed to get entry with key 'key'");
+		this.handler.handleCacheGetError(new RuntimeException(), CACHE, KEY);
+		verify(this.logger).warn("Cache 'NOOP' failed to get entry with key 'enigma'");
 	}
 
 	@Test
 	void handlePutCacheErrorLogsAppropriateMessage() {
-		LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, false);
-		handler.handleCachePutError(new RuntimeException(), new NoOpCache("NOOP"), "key", new Object());
-		verify(this.logger).warn("Cache 'NOOP' failed to put entry with key 'key'");
+		this.handler.handleCachePutError(new RuntimeException(), CACHE, KEY, null);
+		verify(this.logger).warn("Cache 'NOOP' failed to put entry with key 'enigma'");
 	}
 
 	@Test
 	void handleEvictCacheErrorLogsAppropriateMessage() {
-		LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, false);
-		handler.handleCacheEvictError(new RuntimeException(), new NoOpCache("NOOP"), "key");
-		verify(this.logger).warn("Cache 'NOOP' failed to evict entry with key 'key'");
+		this.handler.handleCacheEvictError(new RuntimeException(), CACHE, KEY);
+		verify(this.logger).warn("Cache 'NOOP' failed to evict entry with key 'enigma'");
 	}
 
 	@Test
 	void handleClearErrorLogsAppropriateMessage() {
-		LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, false);
-		handler.handleCacheClearError(new RuntimeException(), new NoOpCache("NOOP"));
+		this.handler.handleCacheClearError(new RuntimeException(), CACHE);
 		verify(this.logger).warn("Cache 'NOOP' failed to clear entries");
 	}
 
 	@Test
-	void handleCacheErrorWithStacktrace() {
-		LoggingCacheErrorHandler handler = new LoggingCacheErrorHandler(this.logger, true);
+	void handleGetCacheErrorWithStackTraceLoggingEnabled() {
+		this.handler = new LoggingCacheErrorHandler(this.logger, true);
 		RuntimeException exception = new RuntimeException();
-		handler.handleCacheGetError(exception, new NoOpCache("NOOP"), "key");
-		verify(this.logger).warn("Cache 'NOOP' failed to get entry with key 'key'", exception);
+		this.handler.handleCacheGetError(exception, CACHE, KEY);
+		verify(this.logger).warn("Cache 'NOOP' failed to get entry with key 'enigma'", exception);
 	}
 
 }