Skip to content

Commit 8d5abd1

Browse files
nosannosan
committed
Switching from FileSize to DataSize for logback size-based properties.
gh-15930
1 parent b28c31d commit 8d5abd1

File tree

3 files changed

+164
-19
lines changed

3 files changed

+164
-19
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
package org.springframework.boot.logging.logback;
1818

1919
import java.lang.reflect.Method;
20+
import java.util.Objects;
21+
import java.util.regex.Matcher;
22+
import java.util.regex.Pattern;
2023

2124
import ch.qos.logback.classic.Level;
2225
import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
@@ -35,7 +38,10 @@
3538
import org.springframework.core.env.Environment;
3639
import org.springframework.core.env.PropertyResolver;
3740
import org.springframework.core.env.PropertySourcesPropertyResolver;
41+
import org.springframework.util.Assert;
3842
import org.springframework.util.ReflectionUtils;
43+
import org.springframework.util.StringUtils;
44+
import org.springframework.util.unit.DataSize;
3945

4046
/**
4147
* Default logback configuration used by Spring Boot. Uses {@link LogbackConfigurator} to
@@ -58,6 +64,9 @@ class DefaultLogbackConfiguration {
5864
private static final String FILE_LOG_PATTERN = "%d{${LOG_DATEFORMAT_PATTERN:-yyyy-MM-dd HH:mm:ss.SSS}} "
5965
+ "${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- [%t] %-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}";
6066

67+
private static final Pattern SIZE_PATTERN = Pattern
68+
.compile("^([+\\-]?\\d+)\\s*([A-Z]{0,2})S?$", Pattern.CASE_INSENSITIVE);
69+
6170
private static final String MAX_FILE_SIZE = "10MB";
6271

6372
private final PropertyResolver patterns;
@@ -144,30 +153,51 @@ private void setRollingPolicy(RollingFileAppender<ILoggingEvent> appender,
144153
rollingPolicy.setCleanHistoryOnStart(this.patterns.getProperty(
145154
"logging.file.clean-history-on-start", Boolean.class, false));
146155
rollingPolicy.setFileNamePattern(logFile + ".%d{yyyy-MM-dd}.%i.gz");
147-
setMaxFileSize(rollingPolicy,
148-
this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE));
156+
setMaxFileSize(rollingPolicy, getDataSize(
157+
this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE)));
158+
setTotalSizeCap(rollingPolicy,
159+
getDataSize(this.patterns.getProperty("logging.file.total-size-cap",
160+
String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP))));
149161
rollingPolicy.setMaxHistory(this.patterns.getProperty("logging.file.max-history",
150162
Integer.class, CoreConstants.UNBOUND_HISTORY));
151-
rollingPolicy.setTotalSizeCap(
152-
FileSize.valueOf(this.patterns.getProperty("logging.file.total-size-cap",
153-
String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP))));
154163
appender.setRollingPolicy(rollingPolicy);
155164
rollingPolicy.setParent(appender);
156165
config.start(rollingPolicy);
157166
}
158167

168+
private void setTotalSizeCap(
169+
SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy,
170+
DataSize totalSizeCap) {
171+
Assert.isTrue(!totalSizeCap.isNegative(), "TotalSizeCap must not be a negative");
172+
rollingPolicy.setTotalSizeCap(new FileSize(totalSizeCap.toBytes()));
173+
}
174+
159175
private void setMaxFileSize(
160176
SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy,
161-
String maxFileSize) {
177+
DataSize maxFileSize) {
178+
Assert.isTrue(!maxFileSize.isNegative(), "MaxFileSize must not be a negative");
162179
try {
163-
rollingPolicy.setMaxFileSize(FileSize.valueOf(maxFileSize));
180+
rollingPolicy.setMaxFileSize(new FileSize(maxFileSize.toBytes()));
164181
}
165182
catch (NoSuchMethodError ex) {
166183
// Logback < 1.1.8 used String configuration
167184
Method method = ReflectionUtils.findMethod(
168185
SizeAndTimeBasedRollingPolicy.class, "setMaxFileSize", String.class);
169-
ReflectionUtils.invokeMethod(method, rollingPolicy, maxFileSize);
186+
ReflectionUtils.invokeMethod(method, rollingPolicy,
187+
Objects.toString(maxFileSize.toBytes()));
188+
}
189+
}
190+
191+
private static DataSize getDataSize(String value) {
192+
Matcher matcher = SIZE_PATTERN.matcher(value);
193+
if (matcher.matches()) {
194+
String size = matcher.group(1);
195+
String unit = matcher.group(2);
196+
return StringUtils.hasText(unit) ? DataSize.parse(size + unit.toUpperCase())
197+
: DataSize.parse(size);
170198
}
199+
throw new IllegalArgumentException(
200+
"'" + value + "' does not match data size pattern");
171201
}
172202

173203
}

‎spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
},
9898
{
9999
"name": "logging.file.max-size",
100-
"type": "java.lang.String",
100+
"type": "org.springframework.util.unit.DataSize",
101101
"description": "Maximum log file size. Only supported with the default logback setup.",
102102
"sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
103103
"defaultValue": "10MB"
@@ -111,7 +111,7 @@
111111
},
112112
{
113113
"name": "logging.file.total-size-cap",
114-
"type": "java.lang.String",
114+
"type": "org.springframework.util.unit.DataSize",
115115
"description": "Total size of log backups to be kept. Only supported with the default logback setup.",
116116
"sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
117117
"defaultValue": "0"

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java

Lines changed: 124 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.springframework.util.StringUtils;
5858

5959
import static org.assertj.core.api.Assertions.assertThat;
60+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
6061
import static org.assertj.core.api.Assertions.contentOf;
6162
import static org.hamcrest.Matchers.containsString;
6263
import static org.hamcrest.Matchers.not;
@@ -381,10 +382,53 @@ public void testCleanHistoryOnStartPropertyWithXmlConfiguration() {
381382
assertThat(getRollingPolicy().isCleanHistoryOnStart()).isTrue();
382383
}
383384

385+
@Test
386+
public void testMaxFileSizePropertyNegativeValue() {
387+
assertThatThrownBy(() -> {
388+
MockEnvironment environment = new MockEnvironment();
389+
environment.setProperty("logging.file.max-size", "-100MB");
390+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
391+
environment);
392+
File file = new File(tmpDir(), "logback-test.log");
393+
LogFile logFile = getLogFile(file.getPath(), null);
394+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
395+
}).isInstanceOf(IllegalArgumentException.class)
396+
.hasMessageContaining("MaxFileSize must not be a negative");
397+
}
398+
399+
@Test
400+
public void testMaxFileSizePropertyInvalidFormat() {
401+
assertThatThrownBy(() -> {
402+
MockEnvironment environment = new MockEnvironment();
403+
environment.setProperty("logging.file.max-size", "100 :)");
404+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
405+
environment);
406+
File file = new File(tmpDir(), "logback-test.log");
407+
LogFile logFile = getLogFile(file.getPath(), null);
408+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
409+
}).isInstanceOf(IllegalArgumentException.class)
410+
.hasMessageContaining("does not match data size pattern");
411+
}
412+
413+
@Test
414+
public void testMaxFileSizePropertyLogbackFormat() {
415+
MockEnvironment environment = new MockEnvironment();
416+
environment.setProperty("logging.file.max-size", "100 kbs");
417+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
418+
environment);
419+
File file = new File(tmpDir(), "logback-test.log");
420+
LogFile logFile = getLogFile(file.getPath(), null);
421+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
422+
this.logger.info("Hello world");
423+
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
424+
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
425+
.toString()).isEqualTo("100 KB");
426+
}
427+
384428
@Test
385429
public void testMaxFileSizeProperty() {
386430
MockEnvironment environment = new MockEnvironment();
387-
environment.setProperty("logging.file.max-size", "100MB");
431+
environment.setProperty("logging.file.max-size", "+10TB");
388432
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
389433
environment);
390434
File file = new File(tmpDir(), "logback-test.log");
@@ -393,7 +437,22 @@ public void testMaxFileSizeProperty() {
393437
this.logger.info("Hello world");
394438
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
395439
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
396-
.toString()).isEqualTo("100 MB");
440+
.toString()).isEqualTo("10240 GB");
441+
}
442+
443+
@Test
444+
public void testMaxFileSizePropertyRawBytes() {
445+
MockEnvironment environment = new MockEnvironment();
446+
environment.setProperty("logging.file.max-size", "1024");
447+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
448+
environment);
449+
File file = new File(tmpDir(), "logback-test.log");
450+
LogFile logFile = getLogFile(file.getPath(), null);
451+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
452+
this.logger.info("Hello world");
453+
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
454+
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
455+
.toString()).isEqualTo("1 KB");
397456
}
398457

399458
@Test
@@ -427,7 +486,7 @@ public void testMaxHistoryProperty() {
427486
}
428487

429488
@Test
430-
public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception {
489+
public void testMaxHistoryPropertyWithXmlConfiguration() {
431490
MockEnvironment environment = new MockEnvironment();
432491
environment.setProperty("logging.file.max-history", "30");
433492
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
@@ -441,11 +500,53 @@ public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception {
441500
assertThat(getRollingPolicy().getMaxHistory()).isEqualTo(30);
442501
}
443502

503+
@Test
504+
public void testTotalSizeCapPropertyNegativeValue() {
505+
assertThatThrownBy(() -> {
506+
MockEnvironment environment = new MockEnvironment();
507+
environment.setProperty("logging.file.total-size-cap", "-101 Mb");
508+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
509+
environment);
510+
File file = new File(tmpDir(), "logback-test.log");
511+
LogFile logFile = getLogFile(file.getPath(), null);
512+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
513+
}).isInstanceOf(IllegalArgumentException.class)
514+
.hasMessageContaining("TotalSizeCap must not be a negative");
515+
}
516+
517+
@Test
518+
public void testTotalSizeCapPropertyInvalidFormat() {
519+
assertThatThrownBy(() -> {
520+
MockEnvironment environment = new MockEnvironment();
521+
environment.setProperty("logging.file.total-size-cap", "101 :)");
522+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
523+
environment);
524+
File file = new File(tmpDir(), "logback-test.log");
525+
LogFile logFile = getLogFile(file.getPath(), null);
526+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
527+
}).isInstanceOf(IllegalArgumentException.class)
528+
.hasMessageContaining("does not match data size pattern");
529+
}
530+
531+
@Test
532+
public void testTotalSizeCapPropertyLogbackFormat() {
533+
MockEnvironment environment = new MockEnvironment();
534+
environment.setProperty("logging.file.total-size-cap", "101 mbs");
535+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
536+
environment);
537+
File file = new File(tmpDir(), "logback-test.log");
538+
LogFile logFile = getLogFile(file.getPath(), null);
539+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
540+
this.logger.info("Hello world");
541+
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
542+
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
543+
.toString()).isEqualTo("101 MB");
544+
}
545+
444546
@Test
445547
public void testTotalSizeCapProperty() {
446-
String expectedSize = "101 MB";
447548
MockEnvironment environment = new MockEnvironment();
448-
environment.setProperty("logging.file.total-size-cap", expectedSize);
549+
environment.setProperty("logging.file.total-size-cap", "101MB");
449550
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
450551
environment);
451552
File file = new File(tmpDir(), "logback-test.log");
@@ -454,14 +555,28 @@ public void testTotalSizeCapProperty() {
454555
this.logger.info("Hello world");
455556
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
456557
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
457-
.toString()).isEqualTo(expectedSize);
558+
.toString()).isEqualTo("101 MB");
559+
}
560+
561+
@Test
562+
public void testTotalSizeCapPropertyRawBytes() {
563+
MockEnvironment environment = new MockEnvironment();
564+
environment.setProperty("logging.file.total-size-cap", "10485760");
565+
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
566+
environment);
567+
File file = new File(tmpDir(), "logback-test.log");
568+
LogFile logFile = getLogFile(file.getPath(), null);
569+
this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
570+
this.logger.info("Hello world");
571+
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
572+
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
573+
.toString()).isEqualTo("10 MB");
458574
}
459575

460576
@Test
461577
public void testTotalSizeCapPropertyWithXmlConfiguration() {
462-
String expectedSize = "101 MB";
463578
MockEnvironment environment = new MockEnvironment();
464-
environment.setProperty("logging.file.total-size-cap", expectedSize);
579+
environment.setProperty("logging.file.total-size-cap", "101MB");
465580
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
466581
environment);
467582
File file = new File(tmpDir(), "logback-test.log");
@@ -471,7 +586,7 @@ public void testTotalSizeCapPropertyWithXmlConfiguration() {
471586
this.logger.info("Hello world");
472587
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
473588
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
474-
.toString()).isEqualTo(expectedSize);
589+
.toString()).isEqualTo("101 MB");
475590
}
476591

477592
@Test

0 commit comments

Comments
 (0)