Polish "Add support for Log4j2 rolling policy configuration"

See spring-projectsgh-47260

Author: snicoll

core/spring-boot/src/main/java/org/springframework/boot/logging/AbstractLoggingSystem.java

@@ -26,7 +26,6 @@
 
 import org.jspecify.annotations.Nullable;
 
-import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.Environment;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.util.Assert;
@@ -181,10 +180,7 @@ protected final String getPackagedConfigFile(String fileName) {
 	}
 
 	protected final void applySystemProperties(Environment environment, @Nullable LogFile logFile) {
-		LoggingSystemProperties systemProperties = (environment instanceof ConfigurableEnvironment configurableEnvironment)
-				? getSystemProperties(configurableEnvironment)
-				: new LoggingSystemProperties(environment, getDefaultValueResolver(environment), null);
-		systemProperties.apply(logFile);
+		new LoggingSystemProperties(environment, getDefaultValueResolver(environment), null).apply(logFile);
 	}
 
 	/**

core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4j2LoggingSystemProperties.java

@@ -33,25 +33,15 @@
  * {@link LoggingSystemProperties} for Log4j2.
  *
  * @author HoJoo Moon
- * @since 4.0.0
- * @see Log4j2RollingPolicySystemProperty
+ * @since 4.1.0
+ * @see RollingPolicySystemProperty
  */
 public class Log4j2LoggingSystemProperties extends LoggingSystemProperties {
 
 	public Log4j2LoggingSystemProperties(Environment environment) {
 		super(environment);
 	}
 
-	/**
-	 * Create a new {@link Log4j2LoggingSystemProperties} instance.
-	 * @param environment the source environment
-	 * @param setter setter used to apply the property
-	 */
-	public Log4j2LoggingSystemProperties(Environment environment,
-			@Nullable BiConsumer<String, @Nullable String> setter) {
-		super(environment, setter);
-	}
-
 	/**
 	 * Create a new {@link Log4j2LoggingSystemProperties} instance.
 	 * @param environment the source environment
@@ -72,25 +62,22 @@ protected void apply(@Nullable LogFile logFile, PropertyResolver resolver) {
 	}
 
 	private void applyRollingPolicyProperties(PropertyResolver resolver) {
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.STRATEGY, resolver);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.TIME_INTERVAL, resolver, Integer.class);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.TIME_MODULATE, resolver, Boolean.class);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.CRON_SCHEDULE, resolver);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.FILE_NAME_PATTERN, resolver);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.MAX_FILE_SIZE, resolver, DataSize.class);
-		applyRollingPolicy(Log4j2RollingPolicySystemProperty.MAX_HISTORY, resolver);
+		applyRollingPolicy(RollingPolicySystemProperty.FILE_NAME_PATTERN, resolver);
+		applyRollingPolicy(RollingPolicySystemProperty.MAX_FILE_SIZE, resolver, DataSize.class);
+		applyRollingPolicy(RollingPolicySystemProperty.MAX_HISTORY, resolver);
+		applyRollingPolicy(RollingPolicySystemProperty.STRATEGY, resolver);
+		applyRollingPolicy(RollingPolicySystemProperty.TIME_INTERVAL, resolver, Integer.class);
+		applyRollingPolicy(RollingPolicySystemProperty.TIME_MODULATE, resolver, Boolean.class);
+		applyRollingPolicy(RollingPolicySystemProperty.CRON_SCHEDULE, resolver);
 	}
 
-	private void applyRollingPolicy(Log4j2RollingPolicySystemProperty property, PropertyResolver resolver) {
+	private void applyRollingPolicy(RollingPolicySystemProperty property, PropertyResolver resolver) {
 		applyRollingPolicy(property, resolver, String.class);
 	}
 
-	private <T> void applyRollingPolicy(Log4j2RollingPolicySystemProperty property, PropertyResolver resolver,
+	private <T> void applyRollingPolicy(RollingPolicySystemProperty property, PropertyResolver resolver,
 			Class<T> type) {
 		T value = getProperty(resolver, property.getApplicationPropertyName(), type);
-		if (value == null && property.getDeprecatedApplicationPropertyName() != null) {
-			value = getProperty(resolver, property.getDeprecatedApplicationPropertyName(), type);
-		}
 		if (value != null) {
 			String stringValue = String.valueOf((value instanceof DataSize dataSize) ? dataSize.toBytes() : value);
 			setSystemProperty(property.getEnvironmentVariableName(), stringValue);
@@ -106,19 +93,8 @@ private <T> void applyRollingPolicy(Log4j2RollingPolicySystemProperty property,
 			if (type != DataSize.class) {
 				throw ex;
 			}
-			// Fallback for Log4j2 compatibility - try parsing as string if DataSize
-			// conversion fails
 			String value = resolver.getProperty(key);
-			if (value != null) {
-				try {
-					return (T) DataSize.parse(value);
-				}
-				catch (Exception parseEx) {
-					ex.addSuppressed(parseEx);
-					throw ex;
-				}
-			}
-			return null;
+			return (value != null) ? (T) DataSize.parse(value) : null;
 		}
 	}
 

core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/RollingPolicyStrategy.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2012-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.boot.logging.log4j2;
+
+/**
+ * Available rolling policy strategies.
+ *
+ * @author Stephane Nicoll
+ * @since 4.1.0
+ */
+public enum RollingPolicyStrategy {
+
+	/**
+	 * Roll a file over based on its size.
+	 */
+	SIZE,
+
+	/**
+	 * Roll a file over based on time.
+	 */
+	TIME,
+
+	/**
+	 * Roll a file over based on its size and time.
+	 */
+	SIZE_AND_TIME,
+
+	/**
+	 * Roll a file over based on its size.
+	 */
+	CRON
+
+}

…2/Log4j2RollingPolicySystemProperty.java …/log4j2/RollingPolicySystemProperty.javacore/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4j2RollingPolicySystemProperty.java renamed to core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/RollingPolicySystemProperty.java core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4j2RollingPolicySystemProperty.java renamed to core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/RollingPolicySystemProperty.java

@@ -16,64 +16,59 @@
 
 package org.springframework.boot.logging.log4j2;
 
-import org.jspecify.annotations.Nullable;
-
 /**
  * Log4j2 rolling policy system properties that can later be used by log configuration
  * files.
  *
  * @author HoJoo Moon
- * @since 4.0.0
+ * @since 4.1.0
  * @see Log4j2LoggingSystemProperties
  */
-public enum Log4j2RollingPolicySystemProperty {
+public enum RollingPolicySystemProperty {
 
 	/**
 	 * Logging system property for the rolled-over log file name pattern.
 	 */
-	FILE_NAME_PATTERN("file-name-pattern", "logging.pattern.rolling-file-name"),
+	FILE_NAME_PATTERN("file-name-pattern"),
 
 	/**
 	 * Logging system property for the file log max size.
 	 */
-	MAX_FILE_SIZE("max-file-size", "logging.file.max-size"),
+	MAX_FILE_SIZE("max-file-size"),
 
 	/**
 	 * Logging system property for the file log max history.
 	 */
-	MAX_HISTORY("max-history", "logging.file.max-history"),
+	MAX_HISTORY("max-history"),
 
 	/**
-	 * Logging system property for the rolling policy strategy.
+	 * Logging system property for the {@linkplain RollingPolicyStrategy rolling policy
+	 * strategy}.
 	 */
-	STRATEGY("strategy", null),
+	STRATEGY("strategy"),
 
 	/**
 	 * Logging system property for the rolling policy time interval.
 	 */
-	TIME_INTERVAL("time-based.interval", null),
+	TIME_INTERVAL("time-based.interval"),
 
 	/**
 	 * Logging system property for the rolling policy time modulate flag.
 	 */
-	TIME_MODULATE("time-based.modulate", null),
+	TIME_MODULATE("time-based.modulate"),
 
 	/**
 	 * Logging system property for the cron based schedule.
 	 */
-	CRON_SCHEDULE("cron.schedule", null);
+	CRON_SCHEDULE("cron.schedule");
 
 	private final String environmentVariableName;
 
 	private final String applicationPropertyName;
 
-	private final @Nullable String deprecatedApplicationPropertyName;
-
-	Log4j2RollingPolicySystemProperty(String applicationPropertyName,
-			@Nullable String deprecatedApplicationPropertyName) {
+	RollingPolicySystemProperty(String applicationPropertyName) {
 		this.environmentVariableName = "LOG4J2_ROLLINGPOLICY_" + name();
 		this.applicationPropertyName = "logging.log4j2.rollingpolicy." + applicationPropertyName;
-		this.deprecatedApplicationPropertyName = deprecatedApplicationPropertyName;
 	}
 
 	/**
@@ -88,8 +83,4 @@ String getApplicationPropertyName() {
 		return this.applicationPropertyName;
 	}
 
-	@Nullable String getDeprecatedApplicationPropertyName() {
-		return this.deprecatedApplicationPropertyName;
-	}
-
 }

core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/SpringBootTriggeringPolicy.java

@@ -16,6 +16,8 @@
 
 package org.springframework.boot.logging.log4j2;
 
+import java.util.Objects;
+
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.appender.rolling.CompositeTriggeringPolicy;
 import org.apache.logging.log4j.core.appender.rolling.CronTriggeringPolicy;
@@ -32,6 +34,8 @@
 import org.apache.logging.log4j.core.util.Builder;
 import org.jspecify.annotations.Nullable;
 
+import org.springframework.boot.convert.ApplicationConversionService;
+import org.springframework.core.convert.ConversionException;
 import org.springframework.util.Assert;
 
 /**
@@ -40,7 +44,8 @@
  * {@code size-and-time}, and {@code cron}.
  *
  * @author HoJoo Moon
- * @since 4.0.0
+ * @author Stephane Nicoll
+ * @since 4.1.0
  */
 @Plugin(name = "SpringBootTriggeringPolicy", category = Node.CATEGORY, elementType = "TriggeringPolicy",
 		deferChildren = true, printObject = true)
@@ -51,12 +56,12 @@ private SpringBootTriggeringPolicy() {
 
 	@Override
 	public void initialize(RollingFileManager manager) {
-		throw new UnsupportedOperationException("This class should not be instantiated");
+		throw new UnsupportedOperationException();
 	}
 
 	@Override
 	public boolean isTriggeringEvent(LogEvent logEvent) {
-		throw new UnsupportedOperationException("This class should not be instantiated");
+		throw new UnsupportedOperationException();
 	}
 
 	@PluginBuilderFactory
@@ -73,7 +78,7 @@ public static class SpringBootTriggeringPolicyBuilder implements Builder<Trigger
 
 		private static final String DEFAULT_MAX_FILE_SIZE = "10MB";
 
-		private static final int DEFAULT_TIME_INTERVAL = 1;
+		private static final String DEFAULT_TIME_INTERVAL = "1";
 
 		private static final String DEFAULT_CRON_EXPRESSION = "0 0 0 * * ?";
 
@@ -97,79 +102,51 @@ public static class SpringBootTriggeringPolicyBuilder implements Builder<Trigger
 
 		@Override
 		public TriggeringPolicy build() {
-			// Read strategy from system properties first, then from attributes
-			String resolvedStrategy = System.getProperty("LOG4J2_ROLLINGPOLICY_STRATEGY");
-			if (resolvedStrategy == null) {
-				resolvedStrategy = (this.strategy != null) ? this.strategy : DEFAULT_STRATEGY;
-			}
+			RollingPolicyStrategy resolvedStrategy = getRollingPolicyStrategy();
 			return switch (resolvedStrategy) {
-				case "time" -> createTimePolicy();
-				case "size-and-time" -> CompositeTriggeringPolicy.createPolicy(createSizePolicy(), createTimePolicy());
-				case "cron" -> createCronPolicy();
-				case "size" -> createSizePolicy();
-				default -> throw new IllegalArgumentException(
-						"Unsupported rolling policy strategy '%s'".formatted(resolvedStrategy));
+				case TIME -> createTimePolicy();
+				case SIZE_AND_TIME -> CompositeTriggeringPolicy.createPolicy(createSizePolicy(), createTimePolicy());
+				case CRON -> createCronPolicy();
+				case SIZE -> createSizePolicy();
 			};
 		}
 
-		private TriggeringPolicy createSizePolicy() {
-			// Read from system properties first, then from attributes
-			String size = System.getProperty("LOG4J2_ROLLINGPOLICY_MAX_FILE_SIZE");
-			if (size == null) {
-				size = (this.maxFileSize != null) ? this.maxFileSize : DEFAULT_MAX_FILE_SIZE;
+		private static RollingPolicyStrategy getRollingPolicyStrategy() {
+			String resolvedStrategy = getSystemProperty(RollingPolicySystemProperty.STRATEGY, DEFAULT_STRATEGY);
+			try {
+				return Objects.requireNonNull(ApplicationConversionService.getSharedInstance()
+					.convert(resolvedStrategy, RollingPolicyStrategy.class));
 			}
+			catch (ConversionException ex) {
+				throw new IllegalArgumentException(
+						"Unsupported rolling policy strategy '%s'".formatted(resolvedStrategy));
+			}
+		}
+
+		private TriggeringPolicy createSizePolicy() {
+			String size = getSystemProperty(RollingPolicySystemProperty.MAX_FILE_SIZE, DEFAULT_MAX_FILE_SIZE);
 			return SizeBasedTriggeringPolicy.createPolicy(size);
 		}
 
 		private TriggeringPolicy createTimePolicy() {
-			// Read from system properties first, then from attributes
-			String intervalStr = System.getProperty("LOG4J2_ROLLINGPOLICY_TIME_INTERVAL");
-			int interval = (intervalStr != null) ? Integer.parseInt(intervalStr)
-					: (this.timeInterval != null) ? this.timeInterval : DEFAULT_TIME_INTERVAL;
-
-			String modulateStr = System.getProperty("LOG4J2_ROLLINGPOLICY_TIME_MODULATE");
-			boolean modulate = (modulateStr != null) ? Boolean.parseBoolean(modulateStr)
-					: (this.timeModulate != null) ? this.timeModulate : false;
-
+			int interval = Integer
+				.parseInt(getSystemProperty(RollingPolicySystemProperty.TIME_INTERVAL, DEFAULT_TIME_INTERVAL));
+			boolean modulate = Boolean
+				.parseBoolean(getSystemProperty(RollingPolicySystemProperty.TIME_MODULATE, Boolean.FALSE.toString()));
 			return TimeBasedTriggeringPolicy.newBuilder().withInterval(interval).withModulate(modulate).build();
 		}
 
 		private TriggeringPolicy createCronPolicy() {
 			Assert.notNull(this.configuration, "configuration must not be null");
 			Configuration configuration = this.configuration;
 
-			// Read from system properties first, then from attributes
-			String schedule = System.getProperty("LOG4J2_ROLLINGPOLICY_CRON_SCHEDULE");
-			if (schedule == null) {
-				schedule = (this.cronExpression != null) ? this.cronExpression : DEFAULT_CRON_EXPRESSION;
-			}
-
+			String schedule = getSystemProperty(RollingPolicySystemProperty.CRON_SCHEDULE, DEFAULT_CRON_EXPRESSION);
 			return CronTriggeringPolicy.createPolicy(configuration, null, schedule);
 		}
 
-		SpringBootTriggeringPolicyBuilder setStrategy(@Nullable String strategy) {
-			this.strategy = strategy;
-			return this;
-		}
-
-		SpringBootTriggeringPolicyBuilder setMaxFileSize(@Nullable String maxFileSize) {
-			this.maxFileSize = maxFileSize;
-			return this;
-		}
-
-		SpringBootTriggeringPolicyBuilder setTimeInterval(@Nullable Integer timeInterval) {
-			this.timeInterval = timeInterval;
-			return this;
-		}
-
-		SpringBootTriggeringPolicyBuilder setTimeModulate(@Nullable Boolean timeModulate) {
-			this.timeModulate = timeModulate;
-			return this;
-		}
-
-		SpringBootTriggeringPolicyBuilder setCronExpression(@Nullable String cronExpression) {
-			this.cronExpression = cronExpression;
-			return this;
+		private static String getSystemProperty(RollingPolicySystemProperty property, String fallback) {
+			String value = System.getProperty(property.getEnvironmentVariableName());
+			return (value != null) ? value : fallback;
 		}
 
 		SpringBootTriggeringPolicyBuilder setConfiguration(Configuration configuration) {