Guard CharSequence-based logging methods in LogAccessor

Prior to this commit, LogAccessor's CharSequence-based logging methods
delegated directly to the corresponding method on the underlying
commons-logging Log instance without first checking whether the target
level was enabled. This differed from the Supplier-based overloads,
which have checked isXxxEnabled() before delegating since Spring
Framework 5.2.9.

That asymmetry was harmless as long as spring-jcl supplied the
underlying Log implementation, since its SLF4J adapter itself checked
the level before rendering the message. However, since Spring Framework
7 replaced spring-jcl with Apache commons-logging, whose SLF4J adapters
call String.valueOf(message) unconditionally, any CharSequence argument
-- most notably a LogMessage supplied via LogMessage.format(...) or
LogMessage.of(...) -- is now rendered eagerly, even when the
corresponding level is disabled. Since LogMessage exists specifically
to defer that work, and the idiom is used extensively throughout the
framework and its portfolio projects, this leads to unnecessary
computation and allocation whenever logging is disabled.

To address this, this commit adds the same isXxxEnabled() guard to all
twelve CharSequence-based methods in LogAccessor, matching the
existing Supplier-based overloads and making LogAccessor's laziness
guarantee independent of the underlying Log implementation.

This commit also introduces LogAccessorTests, which verifies that a
lazily rendering LogMessage passed to one of the CharSequence-based
methods is only rendered when the corresponding level is enabled.

See gh-25741
Closes gh-37266

spring-core/src/main/java/org/springframework/core/log/LogAccessor.java

@@ -120,7 +120,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void fatal(CharSequence message) {
-		this.log.fatal(message);
+		if (this.log.isFatalEnabled()) {
+			this.log.fatal(message);
+		}
 	}
 
 	/**
@@ -129,7 +131,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void fatal(Throwable cause, CharSequence message) {
-		this.log.fatal(message, cause);
+		if (this.log.isFatalEnabled()) {
+			this.log.fatal(message, cause);
+		}
 	}
 
 	/**
@@ -137,7 +141,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void error(CharSequence message) {
-		this.log.error(message);
+		if (this.log.isErrorEnabled()) {
+			this.log.error(message);
+		}
 	}
 
 	/**
@@ -146,7 +152,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void error(Throwable cause, CharSequence message) {
-		this.log.error(message, cause);
+		if (this.log.isErrorEnabled()) {
+			this.log.error(message, cause);
+		}
 	}
 
 	/**
@@ -154,7 +162,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void warn(CharSequence message) {
-		this.log.warn(message);
+		if (this.log.isWarnEnabled()) {
+			this.log.warn(message);
+		}
 	}
 
 	/**
@@ -163,7 +173,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void warn(Throwable cause, CharSequence message) {
-		this.log.warn(message, cause);
+		if (this.log.isWarnEnabled()) {
+			this.log.warn(message, cause);
+		}
 	}
 
 	/**
@@ -171,7 +183,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void info(CharSequence message) {
-		this.log.info(message);
+		if (this.log.isInfoEnabled()) {
+			this.log.info(message);
+		}
 	}
 
 	/**
@@ -180,7 +194,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void info(Throwable cause, CharSequence message) {
-		this.log.info(message, cause);
+		if (this.log.isInfoEnabled()) {
+			this.log.info(message, cause);
+		}
 	}
 
 	/**
@@ -188,7 +204,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void debug(CharSequence message) {
-		this.log.debug(message);
+		if (this.log.isDebugEnabled()) {
+			this.log.debug(message);
+		}
 	}
 
 	/**
@@ -197,7 +215,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void debug(Throwable cause, CharSequence message) {
-		this.log.debug(message, cause);
+		if (this.log.isDebugEnabled()) {
+			this.log.debug(message, cause);
+		}
 	}
 
 	/**
@@ -205,7 +225,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void trace(CharSequence message) {
-		this.log.trace(message);
+		if (this.log.isTraceEnabled()) {
+			this.log.trace(message);
+		}
 	}
 
 	/**
@@ -214,7 +236,9 @@ public class LogAccessor {
 	 * @param message the message to log
 	 */
 	public void trace(Throwable cause, CharSequence message) {
-		this.log.trace(message, cause);
+		if (this.log.isTraceEnabled()) {
+			this.log.trace(message, cause);
+		}
 	}
 
 

spring-core/src/test/java/org/springframework/core/log/LogAccessorTests.java

@@ -0,0 +1,174 @@
+/*
+ * Copyright 2002-present 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.log;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.commons.logging.Log;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.FieldSource;
+import org.mockito.stubbing.Answer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.argumentSet;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for {@link LogAccessor}.
+ *
+ * @author Sam Brannen
+ * @since 7.0.10
+ */
+class LogAccessorTests {
+
+	private final Log log = mock();
+
+	private final LogAccessor logAccessor = new LogAccessor(this.log);
+
+
+	@BeforeEach
+	void renderMessagesWhenDelegatingToLog() {
+		// Simulate a Log implementation (such as commons-logging's SLF4J bridge) that renders the
+		// message unconditionally, in order to verify that LogAccessor itself guards against that.
+		Answer<Void> render = invocation -> {
+			invocation.getArgument(0).toString();
+			return null;
+		};
+		doAnswer(render).when(this.log).fatal(any());
+		doAnswer(render).when(this.log).fatal(any(), any());
+		doAnswer(render).when(this.log).error(any());
+		doAnswer(render).when(this.log).error(any(), any());
+		doAnswer(render).when(this.log).warn(any());
+		doAnswer(render).when(this.log).warn(any(), any());
+		doAnswer(render).when(this.log).info(any());
+		doAnswer(render).when(this.log).info(any(), any());
+		doAnswer(render).when(this.log).debug(any());
+		doAnswer(render).when(this.log).debug(any(), any());
+		doAnswer(render).when(this.log).trace(any());
+		doAnswer(render).when(this.log).trace(any(), any());
+	}
+
+	@ParameterizedTest
+	@FieldSource("levels")
+	void messageIsNotRenderedWhenLevelIsDisabled(LevelEnabler enabler, LogInvoker invoker) {
+		enabler.enable(this.log, false);
+
+		AtomicBoolean rendered = new AtomicBoolean();
+		invoker.invoke(this.logAccessor, lazyMessage(rendered));
+
+		assertThat(rendered).isFalse();
+	}
+
+	@ParameterizedTest
+	@FieldSource("levels")
+	void messageIsRenderedWhenLevelIsEnabled(LevelEnabler enabler, LogInvoker invoker) {
+		enabler.enable(this.log, true);
+
+		AtomicBoolean rendered = new AtomicBoolean();
+		invoker.invoke(this.logAccessor, lazyMessage(rendered));
+
+		assertThat(rendered).isTrue();
+	}
+
+	@ParameterizedTest
+	@FieldSource("levelsWithCause")
+	void messageWithCauseIsNotRenderedWhenLevelIsDisabled(LevelEnabler enabler, CauseLogInvoker invoker) {
+		enabler.enable(this.log, false);
+
+		AtomicBoolean rendered = new AtomicBoolean();
+		invoker.invoke(this.logAccessor, new RuntimeException(), lazyMessage(rendered));
+
+		assertThat(rendered).isFalse();
+	}
+
+	@ParameterizedTest
+	@FieldSource("levelsWithCause")
+	void messageWithCauseIsRenderedWhenLevelIsEnabled(LevelEnabler enabler, CauseLogInvoker invoker) {
+		enabler.enable(this.log, true);
+
+		AtomicBoolean rendered = new AtomicBoolean();
+		invoker.invoke(this.logAccessor, new RuntimeException(), lazyMessage(rendered));
+
+		assertThat(rendered).isTrue();
+	}
+
+
+	private static LogMessage lazyMessage(AtomicBoolean rendered) {
+		return LogMessage.of(() -> {
+			rendered.set(true);
+			return "message";
+		});
+	}
+
+	@SuppressWarnings("unused")
+	private static List<Arguments> levels = List.of(
+			argumentSet("fatal", (LevelEnabler) (log, enabled) -> when(log.isFatalEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::fatal),
+			argumentSet("error", (LevelEnabler) (log, enabled) -> when(log.isErrorEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::error),
+			argumentSet("warn", (LevelEnabler) (log, enabled) -> when(log.isWarnEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::warn),
+			argumentSet("info", (LevelEnabler) (log, enabled) -> when(log.isInfoEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::info),
+			argumentSet("debug", (LevelEnabler) (log, enabled) -> when(log.isDebugEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::debug),
+			argumentSet("trace", (LevelEnabler) (log, enabled) -> when(log.isTraceEnabled()).thenReturn(enabled),
+					(LogInvoker) LogAccessor::trace)
+		);
+
+	@SuppressWarnings("unused")
+	private static List<Arguments> levelsWithCause = List.of(
+			argumentSet("fatal", (LevelEnabler) (log, enabled) -> when(log.isFatalEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::fatal),
+			argumentSet("error", (LevelEnabler) (log, enabled) -> when(log.isErrorEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::error),
+			argumentSet("warn", (LevelEnabler) (log, enabled) -> when(log.isWarnEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::warn),
+			argumentSet("info", (LevelEnabler) (log, enabled) -> when(log.isInfoEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::info),
+			argumentSet("debug", (LevelEnabler) (log, enabled) -> when(log.isDebugEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::debug),
+			argumentSet("trace", (LevelEnabler) (log, enabled) -> when(log.isTraceEnabled()).thenReturn(enabled),
+					(CauseLogInvoker) LogAccessor::trace)
+		);
+
+
+	@FunctionalInterface
+	private interface LevelEnabler {
+
+		void enable(Log log, boolean enabled);
+	}
+
+	@FunctionalInterface
+	private interface LogInvoker {
+
+		void invoke(LogAccessor logAccessor, CharSequence message);
+	}
+
+	@FunctionalInterface
+	private interface CauseLogInvoker {
+
+		void invoke(LogAccessor logAccessor, Throwable cause, CharSequence message);
+	}
+
+}