Add rolling scheme via maxBackupCount. (#14)

vy · Jul 29, 2021 · 8b07a0c · 8b07a0c
1 parent d22a005
commit 8b07a0c
Show file tree

Hide file tree

Showing 6 changed files with 365 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,12 @@
+### (2021-07-29) v0.9.3
+
+- Switched from `File#renameTo(File)` to the more robust
+  `Files.move(Path, Path, CopyOptions...)` alternative. (#14)
+
+- Add rolling support via `maxBackupCount`. (#14)
+
+- Stop policies after stream close. (#26)
+
 ### (2020-01-10) v0.9.2
 
 - Shutdown the default `ScheduledExecutorService` at JVM exit. (#12)

diff --git a/README.md b/README.md
@@ -32,6 +32,7 @@ RotationConfig config = RotationConfig
         .file("/tmp/app.log")
         .filePattern("/tmp/app-%d{yyyyMMdd-HHmmss.SSS}.log")
         .policy(new SizeBasedRotationPolicy(1024 * 1024 * 100 /* 100MiB */))
+        .compress(true)
         .policy(DailyRotationPolicy.getInstance())
         .build();
 
@@ -40,16 +41,35 @@ try (RotatingFileOutputStream stream = new RotatingFileOutputStream(config)) {
 }
 ```
 
+Using `maxBackupCount`, one can also introduce a rolling scheme where rotated
+files will be named as `file.0`, `file.1`, `file.2`, ..., `file.N` in the order
+from the newest to the oldest, `N` denoting the `maxBackupCount`:
+
+```java
+RotationConfig config = RotationConfig
+        .builder()
+        .file("/tmp/app.log")
+        .maxBackupCount(10)         // Set `filePattern` to `file.%i` and keep
+                                    // the most recent 10 files.
+        .policy(new SizeBasedRotationPolicy(1024 * 1024 * 100 /* 100MiB */))
+        .build();
+
+try (RotatingFileOutputStream stream = new RotatingFileOutputStream(config)) {
+    stream.write("Hello, world!".getBytes(StandardCharsets.UTF_8));
+}
+```
+
 `RotationConfig.Builder` supports the following methods:
 
 | Method(s) | Description |
 | --------- | ----------- |
 | `file(File)`<br/>`file(String)` | file accessed (e.g., `/tmp/app.log`) |
-| `filePattern(RotatingFilePattern)`<br/>`filePattern(String)`| rotating file pattern (e.g., `/tmp/app-%d{yyyyMMdd-HHmmss-SSS}.log`) |
+| `filePattern(RotatingFilePattern)`<br/>`filePattern(String)`| The pattern used to generate files for moving after rotation, e.g., `/tmp/app-%d{yyyyMMdd-HHmmss-SSS}.log`. This option cannot be combined with `maxBackupCount`. |
 | `policy(RotationPolicy)`<br/>`policies(Set<RotationPolicy> policies)` | rotation policies |
+| `maxBackupCount(int)` | If greater than zero, rotated files will be named as `file.0`, `file.1`, `file.2`, ..., `file.N` in the order from the newest to the oldest, where `N` denoting the `maxBackupCount`. `maxBackupCount` defaults to `-1`, that is, no rolling. This option cannot be combined with `filePattern` or `compress`. |
 | `executorService(ScheduledExecutorService)` | scheduler for time-based policies and compression tasks |
 | `append(boolean)` | append while opening the `file` (defaults to `true`) |
-| `compress(boolean)` | GZIP compression after rotation (defaults to `false`) |
+| `compress(boolean)` | Toggles GZIP compression after rotation and defaults to `false`. This option cannot be combined with `maxBackupCount`. |
 | `clock(Clock)` | clock for retrieving date and time (defaults to `SystemClock`) |
 | `callback(RotationCallback)`<br/>`callbacks(Set<RotationCallback>)` | rotation callbacks (defaults to `LoggingRotationCallback`) |
 
@@ -136,6 +156,7 @@ methods.
 - [Jonas (yawkat) Konrad](https://yawk.at/) (`RotatingFileOutputStream`
   thread-safety improvements)
 - [Lukas Bradley](https://github.com/lukasbradley/)
+- [Liran Mendelovich](https://github.com/liran2000/) (rolling via `maxBackupCount`)
 
 # License
 

diff --git a/src/main/java/com/vlkan/rfos/RotatingFileOutputStream.java b/src/main/java/com/vlkan/rfos/RotatingFileOutputStream.java
@@ -20,9 +20,20 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.*;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
 import java.time.Instant;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.zip.GZIPOutputStream;
 
@@ -106,15 +117,18 @@ private synchronized void unsafeRotate(RotationPolicy policy, Instant instant) t
         invokeCallbacks(callback -> callback.onClose(policy, instant, stream));
         stream.close();
 
-        // Rename the file.
-        File rotatedFile = config.getFilePattern().create(instant).getAbsoluteFile();
-        LOGGER.debug("renaming {file={}, rotatedFile={}}", config.getFile(), rotatedFile);
-        boolean renamed = config.getFile().renameTo(rotatedFile);
-        if (!renamed) {
-            String message = String.format("rename failure {file=%s, rotatedFile=%s}", config.getFile(), rotatedFile);
-            IOException error = new IOException(message);
-            invokeCallbacks(callback -> callback.onFailure(policy, instant, rotatedFile, error));
-            return;
+        // Backup file, if enabled.
+        File rotatedFile;
+        if (config.getMaxBackupCount() > 0) {
+            renameBackups();
+            rotatedFile = backupFile();
+        }
+
+        // Otherwise, rename using the provided file pattern.
+        else {
+        	rotatedFile = config.getFilePattern().create(instant).getAbsoluteFile();
+            LOGGER.debug("renaming {file={}, rotatedFile={}}", config.getFile(), rotatedFile);
+            renameFile(config.getFile(), rotatedFile);
         }
 
         // Re-open the file.
@@ -132,6 +146,45 @@ private synchronized void unsafeRotate(RotationPolicy policy, Instant instant) t
 
     }
 
+    private void renameBackups() throws IOException {
+        File dstFile = getBackupFile(config.getMaxBackupCount() - 1);
+        for (int backupIndex = config.getMaxBackupCount() - 2; backupIndex >= 0; backupIndex--) {
+            File srcFile = getBackupFile(backupIndex);
+            if (!srcFile.exists()) {
+                continue;
+            }
+            LOGGER.debug("renaming backup {srcFile={}, dstFile={}}", srcFile, dstFile);
+            renameFile(srcFile, dstFile);
+            dstFile = srcFile;
+        }
+    }
+
+    private File backupFile() throws IOException {
+        File dstFile = getBackupFile(0);
+        File srcFile = config.getFile();
+        LOGGER.debug("renaming for backup {srcFile={}, dstFile={}}", srcFile, dstFile);
+        renameFile(srcFile, dstFile);
+        return dstFile;
+    }
+
+    private static void renameFile(File srcFile, File dstFile) throws IOException {
+        Files.move(
+                srcFile.toPath(),
+                dstFile.toPath(),
+                StandardCopyOption.REPLACE_EXISTING/*,      // The rest of the arguments (atomic & copy-attr) are pretty
+                StandardCopyOption.ATOMIC_MOVE,             // much platform-dependent and JVM throws an "unsupported
+                StandardCopyOption.COPY_ATTRIBUTES*/);      // option" exception at runtime.
+    }
+
+    private File getBackupFile(int backupIndex) {
+        String parent = config.getFile().getParent();
+        if (parent == null) {
+            parent = ".";
+        }
+        String fileName = config.getFile().getName() + '.' + backupIndex;
+        return Paths.get(parent, fileName).toFile();
+    }
+
     private void asyncCompress(RotationPolicy policy, Instant instant, File rotatedFile) {
         config.getExecutorService().execute(new Runnable() {
 

diff --git a/src/main/java/com/vlkan/rfos/RotatingFilePattern.java b/src/main/java/com/vlkan/rfos/RotatingFilePattern.java
@@ -24,6 +24,10 @@
 
 public class RotatingFilePattern {
 
+    private static final Locale DEFAULT_LOCALE = Locale.getDefault();
+
+    private static final ZoneId DEFAULT_TIME_ZONE_ID = TimeZone.getDefault().toZoneId();
+
     private static final char ESCAPE_CHAR = '%';
 
     private static final char DATE_TIME_DIRECTIVE_CHAR = 'd';
@@ -190,10 +194,18 @@ public String getPattern() {
         return pattern;
     }
 
+    public static Locale getDefaultLocale() {
+        return DEFAULT_LOCALE;
+    }
+
     public Locale getLocale() {
         return locale;
     }
 
+    public static ZoneId getDefaultTimeZoneId() {
+        return DEFAULT_TIME_ZONE_ID;
+    }
+
     public ZoneId getTimeZoneId() {
         return timeZoneId;
     }
@@ -226,24 +238,24 @@ public static final class Builder {
 
         private String pattern;
 
-        private Locale locale = Locale.getDefault();
+        private Locale locale = DEFAULT_LOCALE;
 
-        private ZoneId timeZoneId = TimeZone.getDefault().toZoneId();
+        private ZoneId timeZoneId = DEFAULT_TIME_ZONE_ID;
 
         private Builder() {}
 
         public Builder pattern(String pattern) {
-            this.pattern = pattern;
+            this.pattern = Objects.requireNonNull(pattern, "pattern");
             return this;
         }
 
         public Builder locale(Locale locale) {
-            this.locale = locale;
+            this.locale = Objects.requireNonNull(locale, "locale");
             return this;
         }
 
         public Builder timeZoneId(ZoneId timeZoneId) {
-            this.timeZoneId = timeZoneId;
+            this.timeZoneId = Objects.requireNonNull(timeZoneId, "timeZoneId");
             return this;
         }
 
@@ -254,8 +266,6 @@ public RotatingFilePattern build() {
 
         private void validate() {
             Objects.requireNonNull(pattern, "file");
-            Objects.requireNonNull(locale, "locale");
-            Objects.requireNonNull(timeZoneId, "timeZoneId");
         }
 
     }