From 1a37a0d5ebac23c95530bda2aa63220350dd6ba4 Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Fri, 20 Jan 2017 16:57:06 +0100 Subject: [PATCH] unit test file name collisions --- .../logback/classic/LoggerContextTest.java | 2 +- .../java/ch/qos/logback/core/ContextBase.java | 7 +- .../ch/qos/logback/core/CoreConstants.java | 2 +- .../ch/qos/logback/core/FileAppender.java | 2 +- .../core/rolling/RollingFileAppender.java | 10 +- .../core/rolling/helper/FileNamePattern.java | 28 +++++ .../core/rolling/CollisionDetectionTest.java | 109 ++++++++++++++++++ .../qos/logback/core/rolling/PackageTest.java | 3 +- 8 files changed, 151 insertions(+), 12 deletions(-) create mode 100755 logback-core/src/test/java/ch/qos/logback/core/rolling/CollisionDetectionTest.java diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/LoggerContextTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/LoggerContextTest.java index 7ce22f4e6..7eb5b3e5f 100644 --- a/logback-classic/src/test/java/ch/qos/logback/classic/LoggerContextTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/LoggerContextTest.java @@ -245,7 +245,7 @@ public class LoggerContextTest { public void collisionMapsPostReset() { lc.reset(); - Map fileCollisions = (Map) lc.getObject(CoreConstants.RFA_FILENAME_COLLISION_MAP); + Map fileCollisions = (Map) lc.getObject(CoreConstants.FA_FILENAME_COLLISION_MAP); assertNotNull(fileCollisions); assertTrue(fileCollisions.isEmpty()); diff --git a/logback-core/src/main/java/ch/qos/logback/core/ContextBase.java b/logback-core/src/main/java/ch/qos/logback/core/ContextBase.java index 8c739b770..edd3a3219 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/ContextBase.java +++ b/logback-core/src/main/java/ch/qos/logback/core/ContextBase.java @@ -14,7 +14,7 @@ package ch.qos.logback.core; import static ch.qos.logback.core.CoreConstants.CONTEXT_NAME_KEY; -import static ch.qos.logback.core.CoreConstants.RFA_FILENAME_COLLISION_MAP; +import static ch.qos.logback.core.CoreConstants.FA_FILENAME_COLLISION_MAP; import static ch.qos.logback.core.CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP; import java.util.ArrayList; @@ -25,6 +25,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; +import ch.qos.logback.core.rolling.helper.FileNamePattern; import ch.qos.logback.core.spi.LifeCycle; import ch.qos.logback.core.spi.LogbackLock; import ch.qos.logback.core.status.StatusManager; @@ -84,8 +85,8 @@ public class ContextBase implements Context, LifeCycle { } protected void initCollisionMaps() { - putObject(RFA_FILENAME_COLLISION_MAP, new HashMap()); - putObject(RFA_FILENAME_PATTERN_COLLISION_MAP, new HashMap()); + putObject(FA_FILENAME_COLLISION_MAP, new HashMap()); + putObject(RFA_FILENAME_PATTERN_COLLISION_MAP, new HashMap()); } /** diff --git a/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java b/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java index 46671188f..03d786c47 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java +++ b/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java @@ -78,7 +78,7 @@ public class CoreConstants { * * The collision map consists of enties of the type (appender name, File option) */ - public static final String RFA_FILENAME_COLLISION_MAP = "RFA_FILENAME_COLLISION_MAP"; + public static final String FA_FILENAME_COLLISION_MAP = "FA_FILENAME_COLLISION_MAP"; /** * Key used to locate a collision map for RollingFileAppender instances in context's object map. diff --git a/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java b/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java index 1079192a2..ad9f1135d 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java +++ b/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java @@ -139,7 +139,7 @@ public class FileAppender extends OutputStreamAppender { return false; } @SuppressWarnings("unchecked") - Map map = (Map) context.getObject(CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP); + Map map = (Map) context.getObject(CoreConstants.FA_FILENAME_COLLISION_MAP); if (map == null) { return collisionsDetected; } 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 eda207c52..af840d73c 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 @@ -117,23 +117,23 @@ public class RollingFileAppender extends FileAppender { if (triggeringPolicy instanceof RollingPolicyBase) { final RollingPolicyBase base = (RollingPolicyBase) triggeringPolicy; final FileNamePattern fileNamePattern = base.fileNamePattern; - boolean collisionsDetected = innerCheckForFileNamePatternCollisionInPreviousRFA(fileNamePattern.toString()); + boolean collisionsDetected = innerCheckForFileNamePatternCollisionInPreviousRFA(fileNamePattern); if (collisionsDetected) collisionResult = true; } return collisionResult; } - private boolean innerCheckForFileNamePatternCollisionInPreviousRFA(String fileNamePattern) { + private boolean innerCheckForFileNamePatternCollisionInPreviousRFA(FileNamePattern fileNamePattern) { boolean collisionsDetected = false; @SuppressWarnings("unchecked") - Map map = (Map) context.getObject(CoreConstants.RFA_FILENAME_COLLISION_MAP); + Map map = (Map) context.getObject(CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP); if (map == null) { return collisionsDetected; } - for (Entry entry : map.entrySet()) { + for (Entry entry : map.entrySet()) { if (fileNamePattern.equals(entry.getValue())) { - addErrorForCollision("FileNamePattern", entry.getValue(), entry.getKey()); + addErrorForCollision("FileNamePattern", entry.getValue().toString(), entry.getKey()); collisionsDetected = true; } } diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java index 7a11a4405..0b0f6dd9e 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java +++ b/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java @@ -54,6 +54,7 @@ public class FileNamePattern extends ContextAwareBase { ConverterUtil.startConverters(this.headTokenConverter); } + void parse() { try { // http://jira.qos.ch/browse/LOGBACK-197 @@ -79,6 +80,33 @@ public class FileNamePattern extends ContextAwareBase { return pattern; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((pattern == null) ? 0 : pattern.hashCode()); + return result; + } + + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + FileNamePattern other = (FileNamePattern) obj; + if (pattern == null) { + if (other.pattern != null) + return false; + } else if (!pattern.equals(other.pattern)) + return false; + return true; + } + + public DateTokenConverter getPrimaryDateTokenConverter() { Converter p = headTokenConverter; diff --git a/logback-core/src/test/java/ch/qos/logback/core/rolling/CollisionDetectionTest.java b/logback-core/src/test/java/ch/qos/logback/core/rolling/CollisionDetectionTest.java new file mode 100755 index 000000000..9e8847103 --- /dev/null +++ b/logback-core/src/test/java/ch/qos/logback/core/rolling/CollisionDetectionTest.java @@ -0,0 +1,109 @@ +package ch.qos.logback.core.rolling; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import ch.qos.logback.core.Context; +import ch.qos.logback.core.ContextBase; +import ch.qos.logback.core.FileAppender; +import ch.qos.logback.core.encoder.NopEncoder; +import ch.qos.logback.core.status.Status; +import ch.qos.logback.core.status.StatusChecker; +import ch.qos.logback.core.testUtil.RandomUtil; +import ch.qos.logback.core.util.CoreTestConstants; +import ch.qos.logback.core.util.StatusPrinter; + +public class CollisionDetectionTest { + + Context context = new ContextBase(); + StatusChecker statusChecker = new StatusChecker(context); + int diff = RandomUtil.getPositiveInt(); + protected String randomOutputDir = CoreTestConstants.OUTPUT_DIR_PREFIX + diff + "/"; + + @Before + public void setUp() throws Exception { + } + + @After + public void tearDown() throws Exception { + } + + + FileAppender buildFileAppender(String name, String filenameSuffix) { + FileAppender fileAppender = new FileAppender(); + fileAppender.setName(name); + fileAppender.setContext(context); + fileAppender.setFile(randomOutputDir+filenameSuffix); + fileAppender.setEncoder(new NopEncoder()); + return fileAppender; + } + + RollingFileAppender buildRollingFileAppender(String name, String filenameSuffix, String patternSuffix) { + RollingFileAppender rollingFileAppender = new RollingFileAppender(); + rollingFileAppender.setName(name); + rollingFileAppender.setContext(context); + rollingFileAppender.setFile(randomOutputDir+filenameSuffix); + rollingFileAppender.setEncoder(new NopEncoder()); + + TimeBasedRollingPolicy tbrp = new TimeBasedRollingPolicy(); + tbrp.setContext(context); + tbrp.setFileNamePattern(randomOutputDir+patternSuffix); + tbrp.setParent(rollingFileAppender); + //tbrp.timeBasedFileNamingAndTriggeringPolicy = new DefaultTimeBasedFileNamingAndTriggeringPolicy(); + //tbrp.timeBasedFileNamingAndTriggeringPolicy.setCurrentTime(givenTime); + rollingFileAppender.setRollingPolicy(tbrp); + tbrp.start(); + + + return rollingFileAppender; + } + + + @Test + public void collisionImpossibleForSingleAppender() { + FileAppender fileAppender = buildFileAppender("FA", "collisionImpossibleForSingleAppender"); + fileAppender.start(); + statusChecker.assertIsErrorFree(); + } + + @Test + public void collisionWithTwoFileAppenders() { + String suffix = "collisionWithToFileAppenders"; + + FileAppender fileAppender1 = buildFileAppender("FA1", suffix); + fileAppender1.start(); + + FileAppender fileAppender2 = buildFileAppender("FA2", suffix); + fileAppender2.start(); + statusChecker.assertContainsMatch(Status.ERROR, "'File' option has the same value"); + //StatusPrinter.print(context); + } + + @Test + public void collisionWith_FA_RFA() { + String suffix = "collisionWith_FA_RFA"; + + FileAppender fileAppender1 = buildFileAppender("FA", suffix); + fileAppender1.start(); + + RollingFileAppender rollingfileAppender = buildRollingFileAppender("RFA", suffix, "bla-%d.log"); + rollingfileAppender.start(); + StatusPrinter.print(context); + statusChecker.assertContainsMatch(Status.ERROR, "'File' option has the same value"); + } + + @Test + public void collisionWith_2RFA() { + String suffix = "collisionWith_2RFA"; + + RollingFileAppender rollingfileAppender1 = buildRollingFileAppender("RFA1", suffix, "bla-%d.log"); + rollingfileAppender1.start(); + RollingFileAppender rollingfileAppender2 = buildRollingFileAppender("RFA1", suffix, "bla-%d.log"); + rollingfileAppender2.start(); + + StatusPrinter.print(context); + statusChecker.assertContainsMatch(Status.ERROR, "'FileNamePattern' option has the same value"); + } + +} diff --git a/logback-core/src/test/java/ch/qos/logback/core/rolling/PackageTest.java b/logback-core/src/test/java/ch/qos/logback/core/rolling/PackageTest.java index a1d506aa3..df9b6a0b2 100755 --- a/logback-core/src/test/java/ch/qos/logback/core/rolling/PackageTest.java +++ b/logback-core/src/test/java/ch/qos/logback/core/rolling/PackageTest.java @@ -18,6 +18,7 @@ import org.junit.runners.Suite; @RunWith(Suite.class) @Suite.SuiteClasses({ RenameUtilTest.class, SizeBasedRollingTest.class, TimeBasedRollingTest.class, TimeBasedRollingWithArchiveRemoval_Test.class, - MultiThreadedRollingTest.class, SizeAndTimeBasedFNATP_Test.class, RollingFileAppenderTest.class, ch.qos.logback.core.rolling.helper.PackageTest.class }) + MultiThreadedRollingTest.class, SizeAndTimeBasedFNATP_Test.class, RollingFileAppenderTest.class, + CollisionDetectionTest.class, ch.qos.logback.core.rolling.helper.PackageTest.class }) public class PackageTest { } -- GitLab