Revise internals of LoggingCacheErrorHandler

Since LoggingCacheErrorHandler was only recently introduced in 5.3.16,
we have decided to completely revise its internals (protected API) in
5.3.x while retaining the current public API.

Specifically, this commit:

- introduces protected getLogger() and isLogStackTraces() methods to
  improve extensibility

- revises logCacheError() to accept a Supplier<String> for lazy
  resolution of error messages

Closes gh-28672
See gh-28670, gh-28648

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);
 	}
 
 }