[logging-log4j2] branch master updated: [LOG4j2-1946] Fix problem with purgeAscending if an old file was deleted

[rg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rgoers pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/master by this push:
     new 572eccd  [LOG4j2-1946] Fix problem with purgeAscending if an old file was deleted
     new 05ab7bf  Merge pull request #299 from IgorPerelyotov/LOG4J2-1946
572eccd is described below

commit 572eccde1ce98a3ff25e34e088b6e15b76b4c196
Author: Igor Perelyotov <pe...@mail.ru>
AuthorDate: Sat Jul 27 08:33:04 2019 +0300

    [LOG4j2-1946] Fix problem with purgeAscending if an old file was deleted
---
 .../appender/rolling/DefaultRolloverStrategy.java  |  2 +-
 .../rolling/RolloverWithDeletedOldFileTest.java    | 82 ++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java
index bd8f986..4b27223 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java
@@ -357,7 +357,7 @@ public class DefaultRolloverStrategy extends AbstractRolloverStrategy {
         final SortedMap<Integer, Path> eligibleFiles = getEligibleFiles(manager);
         final int maxFiles = highIndex - lowIndex + 1;
 
-        boolean renameFiles = false;
+        boolean renameFiles = !eligibleFiles.isEmpty() && eligibleFiles.lastKey() >= maxIndex;
         while (eligibleFiles.size() >= maxFiles) {
             try {
                 LOGGER.debug("Eligible files: {}", eligibleFiles);
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RolloverWithDeletedOldFileTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RolloverWithDeletedOldFileTest.java
new file mode 100644
index 0000000..7956024
--- /dev/null
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RolloverWithDeletedOldFileTest.java
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *      http://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.apache.logging.log4j.core.appender.rolling;
+
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.junit.LoggerContextRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.junit.Assert.*;
+
+/**
+ * Tests that files were rolled correctly if an old log file was deleted from the directory.
+ */
+public class RolloverWithDeletedOldFileTest {
+  private static final String CONFIG = "log4j-rolling-with-padding.xml";
+  private static final String DIR = "target/rolling-with-padding";
+
+  private final LoggerContextRule loggerContextRule = LoggerContextRule.createShutdownTimeoutLoggerContextRule(CONFIG);
+
+  @Rule
+  public RuleChain chain = loggerContextRule.withCleanFoldersRule(DIR);
+
+  @Test
+  public void testAppender() throws Exception {
+    final Logger logger = loggerContextRule.getLogger();
+    for (int i = 0; i < 10; ++i) {
+      // 30 chars per message: each message triggers a rollover
+      logger.fatal("This is a test message number " + i); // 30 chars:
+    }
+    Thread.sleep(100); // Allow time for rollover to complete
+
+    final File dir = new File(DIR);
+    assertTrue("Dir " + DIR + " should exist", dir.exists());
+    assertTrue("Dir " + DIR + " should contain files", dir.listFiles().length == 6);
+
+    File[] files = dir.listFiles();
+    final List<String> expected = Arrays.asList("rollingtest.log", "test-001.log", "test-002.log", "test-003.log", "test-004.log", "test-005.log");
+    assertEquals("Unexpected number of files", expected.size(), files.length);
+    File fileToRemove = null;
+    for (final File file : files) {
+      if (!expected.contains(file.getName())) {
+        fail("unexpected file" + file);
+      }
+      if (file.getName().equals("test-001.log")) {
+        fileToRemove = file;
+      }
+    }
+    fileToRemove.delete();
+    for (int i = 0; i < 10; ++i) {
+      // 30 chars per message: each message triggers a rollover
+      logger.fatal("This is a test message number " + i); // 30 chars:
+    }
+    Thread.sleep(100); // Allow time for rollover to complete again
+    files = dir.listFiles();
+    assertEquals("Unexpected number of files", expected.size(), files.length);
+    for (final File file : files) {
+      if (!expected.contains(file.getName())) {
+        fail("unexpected file" + file);
+      }
+    }
+  }
+}