From ecae664df06c77b443388ef7105f58d455b7752c Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Fri, 25 Oct 2024 21:35:57 +0200 Subject: [PATCH] fix issues/879 Signed-off-by: Ceki Gulcu --- .../logback/classic/issue/github879/Main.java | 41 ++++++++++++++ .../classic/issue/github879/logback-879.xml | 40 ++++++++++++++ ...imeBasedFileNamingAndTriggeringPolicy.java | 2 +- .../core/rolling/RollingFileAppender.java | 22 +++++++- .../rolling/SizeAndTimeBasedFNATP_Test.java | 55 +++++++++++++++++++ .../core/util/CachingDateFotmatterTest.java | 2 +- 6 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/Main.java create mode 100644 logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/logback-879.xml diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/Main.java b/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/Main.java new file mode 100644 index 0000000000..b4ed986f3f --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/Main.java @@ -0,0 +1,41 @@ +/* + * Logback: the reliable, generic, fast and flexible logging framework. + * Copyright (C) 1999-2024, QOS.ch. All rights reserved. + * + * This program and the accompanying materials are dual-licensed under + * either the terms of the Eclipse Public License v1.0 as published by + * the Eclipse Foundation + * + * or (per the licensee's choosing) + * + * under the terms of the GNU Lesser General Public License version 2.1 + * as published by the Free Software Foundation. + */ + +package ch.qos.logback.classic.issue.github879; + + +import ch.qos.logback.classic.ClassicConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class Main { + + + static { + + System.setProperty("outputPath", "logback-classic/target/test-output/issue879"); + String configFilePath = "logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/"; + System.setProperty("logback.statusListenerClass", "stdout"); + System.setProperty(ClassicConstants.CONFIG_FILE_PROPERTY, configFilePath+"logback-879.xml"); + } + + + public static void main(String[] args) { + final Logger LOGGER = LoggerFactory.getLogger(Main.class); + + for (int i = 0; i < 20_000; i++) { + LOGGER.info("X".repeat(45)); + } + } +} \ No newline at end of file diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/logback-879.xml b/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/logback-879.xml new file mode 100644 index 0000000000..45e0fc2368 --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/issue/github879/logback-879.xml @@ -0,0 +1,40 @@ + + + + + + + + + + + e + + + + ${outputPath}/mylog-%d{yyyy-MM-dd}.%i.txt + 1MB + 90 + 20GB + + + %msg%n + + + + + + + \ No newline at end of file diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java index 43b74ba68b..7b8612f042 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java +++ b/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java @@ -38,7 +38,7 @@ public void start() { return; if (tbrp.fileNamePattern.hasIntegerTokenCOnverter()) { addError("Filename pattern [" + tbrp.fileNamePattern - + "] contains an integer token converter, i.e. %i, INCOMPATIBLE with this configuration. Remove it."); + + "] contains an integer token converter, i.e. %i, INCOMPATIBLE with this configuration. Please remove it."); return; } diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java index 061ce4a34c..94770ff15e 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java +++ b/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java @@ -98,11 +98,14 @@ public void start() { } } - currentlyActiveFile = new File(getFile()); addInfo("Active log file name: " + getFile()); + currentlyActiveFile = new File(getFile()); + initializeLengthCounter(); super.start(); } + + private boolean checkForFileAndPatternCollisions() { if (triggeringPolicy instanceof RollingPolicyBase) { final RollingPolicyBase base = (RollingPolicyBase) triggeringPolicy; @@ -147,6 +150,14 @@ private boolean innerCheckForFileNamePatternCollisionInPreviousRFA(FileNamePatte return collisionsDetected; } + private void initializeLengthCounter() { + if(getLengthCounter() != null && currentlyActiveFile.exists()) { + long currentFileLength = currentlyActiveFile.length(); + addInfo("Setting currentFileLength to "+currentFileLength+ " for "+currentlyActiveFile); + incrementByteCount(currentFileLength); + } + } + @Override public void stop() { if (!isStarted()) { @@ -293,13 +304,18 @@ public void setTriggeringPolicy(TriggeringPolicy policy) { @Override protected void updateByteCount(byte[] byteArray) { + if(byteArray == null) + return; + incrementByteCount(byteArray.length); + } + void incrementByteCount(long increment) { LengthCounter lengthCounter = getLengthCounter(); if (lengthCounter == null) return; - if (byteArray != null && byteArray.length > 0) { - lengthCounter.add(byteArray.length); + if (increment > 0) { + lengthCounter.add(increment); } } diff --git a/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java b/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java index 20e0365b42..a9e486502b 100755 --- a/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java +++ b/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java @@ -15,12 +15,18 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.text.SimpleDateFormat; import java.util.Date; import java.util.List; import java.util.concurrent.ExecutionException; import java.util.function.UnaryOperator; +import ch.qos.logback.core.CoreConstants; +import ch.qos.logback.core.util.CachingDateFormatter; import ch.qos.logback.core.util.Duration; +import ch.qos.logback.core.util.StatusPrinter; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -31,6 +37,7 @@ import ch.qos.logback.core.status.testUtil.StatusChecker; import ch.qos.logback.core.util.FileSize; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; public class SizeAndTimeBasedFNATP_Test extends ScaffoldingForRollingTests { @@ -61,6 +68,7 @@ private void initRollingFileAppender(RollingFileAppender rfa, String fil private void initPolicies(RollingFileAppender rfa, TimeBasedRollingPolicy tbrp, String filenamePattern, int sizeThreshold, long givenTime, long lastCheck) { sizeAndTimeBasedFNATP = new SizeAndTimeBasedFileNamingAndTriggeringPolicy(); + sizeAndTimeBasedFNATP.setContext(context); sizeAndTimeBasedFNATP.setCheckIncrement(Duration.buildByMilliseconds(10)); tbrp.setContext(context); sizeAndTimeBasedFNATP.setMaxFileSize(new FileSize(sizeThreshold)); @@ -243,6 +251,53 @@ public void checkDateCollision() { checker.assertContainsMatch("The date format in FileNamePattern"); } + @Test + public void checkInitialFileSize_withFile() throws IOException { + String stem = "foo.log"; + String testId = "checkDateCollision"; + String fixedContent = "Hello world"; + byte[] fixedContentBytes = fixedContent.getBytes(); + + String fileProperty = randomOutputDir + stem; + Files.createDirectories(Paths.get(randomOutputDir)); + Files.write(Paths.get(fileProperty), fixedContentBytes); + + initRollingFileAppender(rfa1, fileProperty); + sizeThreshold = 300; + initPolicies(rfa1, tbrp1, randomOutputDir + testId + "-%d-%i.txt", sizeThreshold, + currentTime, 0); + + //StatusPrinter.print(context); + + assertEquals(fixedContentBytes.length, tbrp1.getLengthCounter().getLength()); + } + + + @Test + public void checkInitialFileSize_withoutFile() throws IOException { + String testId = "checkInitialFileSize_withoutFile"; + String fixedContent = "Hello world"; + byte[] fixedContentBytes = fixedContent.getBytes(); + + + CachingDateFormatter cdf = new CachingDateFormatter(CoreConstants.DAILY_DATE_PATTERN); + String nowString = cdf.format(currentTime); + String pathToFirstFile = randomOutputDir + testId + "-"+nowString+"-0.txt"; + + Files.createDirectories(Paths.get(randomOutputDir)); + Files.write(Paths.get(pathToFirstFile), fixedContentBytes); + + + initRollingFileAppender(rfa1, null); + sizeThreshold = 300; + initPolicies(rfa1, tbrp1, randomOutputDir + testId + "-%d-%i.txt", sizeThreshold, + currentTime, 0); + + StatusPrinter.print(context); + + assertEquals(fixedContentBytes.length, tbrp1.getLengthCounter().getLength()); + } + // @Test // public void testHistoryAsFileCount() throws IOException { // String testId = "testHistoryAsFileCount"; diff --git a/logback-core/src/test/java/ch/qos/logback/core/util/CachingDateFotmatterTest.java b/logback-core/src/test/java/ch/qos/logback/core/util/CachingDateFotmatterTest.java index fe6a8cf2eb..393333b7d4 100755 --- a/logback-core/src/test/java/ch/qos/logback/core/util/CachingDateFotmatterTest.java +++ b/logback-core/src/test/java/ch/qos/logback/core/util/CachingDateFotmatterTest.java @@ -13,7 +13,7 @@ public class CachingDateFotmatterTest { - final static String DATE_PATTERN = "yyyy-MM-dd'T'HH:mm"; + private final static String DATE_PATTERN = "yyyy-MM-dd'T'HH:mm"; SimpleDateFormat sdf = new SimpleDateFormat(DATE_PATTERN); TimeZone perthTZ = TimeZone.getTimeZone("Australia/Perth");