diff --git a/CHANGELOG.md b/CHANGELOG.md index 0312df2..1b6cd5c 100644 --- 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 index 6f20916..7bd08d1 100644 --- 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)`
`file(String)` | file accessed (e.g., `/tmp/app.log`) | -| `filePattern(RotatingFilePattern)`
`filePattern(String)`| rotating file pattern (e.g., `/tmp/app-%d{yyyyMMdd-HHmmss-SSS}.log`) | +| `filePattern(RotatingFilePattern)`
`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)`
`policies(Set 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)`
`callbacks(Set)` | 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 index 214a080..2d602a8 100644 --- 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 index 5c0867f..8ad32aa 100644 --- 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"); } } diff --git a/src/main/java/com/vlkan/rfos/RotationConfig.java b/src/main/java/com/vlkan/rfos/RotationConfig.java index 6db9fee..dd03b73 100644 --- a/src/main/java/com/vlkan/rfos/RotationConfig.java +++ b/src/main/java/com/vlkan/rfos/RotationConfig.java @@ -20,7 +20,6 @@ import java.io.File; import java.util.Collections; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Objects; import java.util.Set; @@ -30,6 +29,17 @@ public class RotationConfig { + private static final boolean DEFAULT_APPEND = true; + + private static final boolean DEFAULT_COMPRESS = false; + + private static final Clock DEFAULT_CLOCK = SystemClock.getInstance(); + + private static final Set DEFAULT_CALLBACKS = + Collections.singleton(LoggingRotationCallback.getInstance()); + + private static final int DEFAULT_MAX_BACKUP_COUNT = -1; + private enum DefaultExecutorServiceHolder {; private static final ScheduledExecutorService INSTANCE = createDefaultExecutorService(); @@ -74,6 +84,8 @@ private static int readDefaultThreadCount() { private final boolean compress; + private final int maxBackupCount; + private final Clock clock; private final Set callbacks; @@ -85,6 +97,7 @@ private RotationConfig(Builder builder) { this.policies = Collections.unmodifiableSet(builder.policies); this.append = builder.append; this.compress = builder.compress; + this.maxBackupCount = builder.maxBackupCount; this.clock = builder.clock; this.callbacks = Collections.unmodifiableSet(builder.callbacks); } @@ -109,18 +122,42 @@ public Set getPolicies() { return policies; } + public static boolean getDefaultAppend() { + return DEFAULT_APPEND; + } + public boolean isAppend() { return append; } + public static boolean getDefaultCompress() { + return DEFAULT_COMPRESS; + } + public boolean isCompress() { return compress; } - public Clock getClock() { + public static int getDefaultMaxBackupCount() { + return DEFAULT_MAX_BACKUP_COUNT; + } + + public int getMaxBackupCount() { + return maxBackupCount; + } + + public static Clock getDefaultClock() { + return DEFAULT_CLOCK; + } + + public Clock getClock() { return clock; } + public static Set getDefaultCallbacks() { + return DEFAULT_CALLBACKS; + } + /** * Returns the first callback in the registered set of callbacks. * @@ -142,6 +179,7 @@ public boolean equals(Object instance) { RotationConfig that = (RotationConfig) instance; return append == that.append && compress == that.compress && + maxBackupCount == that.maxBackupCount && Objects.equals(file, that.file) && Objects.equals(filePattern, that.filePattern) && Objects.equals(executorService, that.executorService) && @@ -152,7 +190,16 @@ public boolean equals(Object instance) { @Override public int hashCode() { - return Objects.hash(file, filePattern, executorService, policies, append, compress, clock, callbacks); + return Objects.hash( + file, + filePattern, + executorService, + policies, + append, + compress, + maxBackupCount, + clock, + callbacks); } @Override @@ -160,6 +207,11 @@ public String toString() { return String.format("RotationConfig{file=%s}", file); } + public static Builder builder(RotationConfig config) { + Objects.requireNonNull(config, "config"); + return new Builder(config); + } + public static Builder builder() { return new Builder(); } @@ -174,51 +226,67 @@ public static class Builder { private Set policies; - private boolean append = true; + private boolean append = DEFAULT_APPEND; + + private boolean compress = DEFAULT_COMPRESS; - private boolean compress = false; + private int maxBackupCount = DEFAULT_MAX_BACKUP_COUNT; - private Clock clock = SystemClock.getInstance(); + private Clock clock = DEFAULT_CLOCK; - private Set callbacks = - new HashSet<>(Collections.singleton( - LoggingRotationCallback.getInstance())); + private Set callbacks = new LinkedHashSet<>(DEFAULT_CALLBACKS); - private Builder() { - // Do nothing. + private Builder(RotationConfig config) { + this.file = config.file; + this.filePattern = config.filePattern; + this.executorService = config.executorService; + this.policies = config.policies; + this.append = config.append; + this.compress = config.append; + this.maxBackupCount = config.maxBackupCount; + this.clock = config.clock; + this.callbacks = config.callbacks; } + private Builder() {} + public Builder file(File file) { - this.file = file; + this.file = Objects.requireNonNull(file, "file"); return this; } public Builder file(String fileName) { + Objects.requireNonNull(fileName, "fileName"); this.file = new File(fileName); return this; } public Builder filePattern(RotatingFilePattern filePattern) { - this.filePattern = filePattern; + this.filePattern = Objects.requireNonNull(filePattern, "filePattern"); return this; } public Builder filePattern(String filePattern) { - this.filePattern = RotatingFilePattern.builder().pattern(filePattern).build(); + Objects.requireNonNull(filePattern, "filePattern"); + this.filePattern = RotatingFilePattern + .builder() + .pattern(filePattern) + .build(); return this; } public Builder executorService(ScheduledExecutorService executorService) { - this.executorService = executorService; + this.executorService = Objects.requireNonNull(executorService, "executorService"); return this; } public Builder policies(Set policies) { - this.policies = policies; + this.policies = Objects.requireNonNull(policies, "policies"); return this; } public Builder policy(RotationPolicy policy) { + Objects.requireNonNull(policy, "policy"); if (policies == null) { policies = new LinkedHashSet<>(); } @@ -236,8 +304,13 @@ public Builder compress(boolean compress) { return this; } + public Builder maxBackupCount(int maxBackupCount) { + this.maxBackupCount = maxBackupCount; + return this; + } + public Builder clock(Clock clock) { - this.clock = clock; + this.clock = Objects.requireNonNull(clock, "clock"); return this; } @@ -248,7 +321,7 @@ public Builder callback(RotationCallback callback) { } public Builder callbacks(Set callbacks) { - this.callbacks = callbacks; + this.callbacks = Objects.requireNonNull(callbacks, "callbacks"); return this; } @@ -266,12 +339,24 @@ private void prepare() { private void validate() { Objects.requireNonNull(file, "file"); - Objects.requireNonNull(filePattern, "filePattern"); + if (maxBackupCount > 0) { + String conflictingField = null; + if (filePattern != null) { + conflictingField = "filePattern"; + } else if (compress) { + conflictingField = "compress"; + } + if (conflictingField != null) { + throw new IllegalArgumentException( + "maxBackupCount and " + conflictingField + " cannot be combined"); + } + } else if (filePattern == null) { + throw new IllegalArgumentException( + "one of either maxBackupCount or filePattern must be provided"); + } if (policies == null || policies.isEmpty()) { - throw new IllegalArgumentException("empty policies"); + throw new IllegalArgumentException("no rotation policy is provided"); } - Objects.requireNonNull(clock, "clock"); - Objects.requireNonNull(callbacks, "callbacks"); } } diff --git a/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java b/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java index 7aef3d0..18d480c 100644 --- a/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java +++ b/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java @@ -523,6 +523,151 @@ private static byte[] copyArrays(byte[]... sources) { return target; } + @Test + void test_maxBackupCount_arg_conflicts() { + + // Prepare common builder fields. + File file = new File(tmpDir, "maxBackupCount_arg_conflicts.log"); + RotationPolicy policy = Mockito.mock(RotationPolicy.class); + Mockito.when(policy.toString()).thenReturn("MockedPolicy"); + + // Verify the absence of both maxBackupCount and filePattern. + Assertions + .assertThatThrownBy(() -> RotationConfig + .builder() + .file(file) + .executorService(executorService) + .policy(policy) + .build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("one of either maxBackupCount or filePattern must be provided"); + + // Verify the conflict of maxBackupCount and filePattern. + Assertions + .assertThatThrownBy(() -> RotationConfig + .builder() + .file(file) + .executorService(executorService) + .policy(policy) + .filePattern("filePattern_and_maxBackupCount_combination-%d{HHmmss}.log") + .maxBackupCount(10) + .build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxBackupCount and filePattern cannot be combined"); + + // Verify the conflict of maxBackupCount and compress. + Assertions + .assertThatThrownBy(() -> RotationConfig + .builder() + .file(file) + .executorService(executorService) + .policy(policy) + .compress(true) + .maxBackupCount(10) + .build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxBackupCount and compress cannot be combined"); + + } + + @Test + void test_maxBackupCount() throws Exception { + + // Create the policy and the callback. + RotationPolicy policy = new SizeBasedRotationPolicy(1); + RotationCallback callback = Mockito.mock(RotationCallback.class); + + // Create the stream. + File file = new File(tmpDir, "maxBackupCount.log"); + RotationConfig config = RotationConfig + .builder() + .executorService(executorService) + .file(file) + .maxBackupCount(2) + .policy(policy) + .callbacks(Collections.singleton(callback)) + .build(); + RotatingFileOutputStream stream = new RotatingFileOutputStream(config); + + // Determine the backup files. + File backupFile0 = new File(tmpDir, "maxBackupCount.log.0"); + File backupFile1 = new File(tmpDir, "maxBackupCount.log.1"); + File backupFile2 = new File(tmpDir, "maxBackupCount.log.2"); + + // Write some without triggering rotation. + byte[] content1 = {'1'}; + stream.write(content1); + stream.flush(); + + // Verify files. + Assertions.assertThat(file).hasBinaryContent(content1); + Assertions.assertThat(backupFile0).doesNotExist(); + Assertions.assertThat(backupFile1).doesNotExist(); + Assertions.assertThat(backupFile2).doesNotExist(); + + // Write some and trigger the 1st rotation. + byte[] content2 = {'2'}; + stream.write(content2); + stream.flush(); + + // Verify the 1st rotation. + InOrder inOrder = Mockito.inOrder(callback); + inOrder + .verify(callback) + .onSuccess( + Mockito.same(policy), + Mockito.any(), + Mockito.eq(backupFile0)); + + // Verify files after the 1st rotation. + Assertions.assertThat(file).hasBinaryContent(content2); + Assertions.assertThat(backupFile0).hasBinaryContent(content1); + Assertions.assertThat(backupFile1).doesNotExist(); + Assertions.assertThat(backupFile2).doesNotExist(); + + // Write some and trigger the 2nd rotation. + byte[] content3 = {'3'}; + stream.write(content3); + stream.flush(); + + // Verify the 2nd rotation. + inOrder + .verify(callback) + .onSuccess( + Mockito.same(policy), + Mockito.any(), + Mockito.eq(backupFile0)); + + // Verify files after the 2nd rotation. + Assertions.assertThat(file).hasBinaryContent(content3); + Assertions.assertThat(backupFile0).hasBinaryContent(content2); + Assertions.assertThat(backupFile1).hasBinaryContent(content1); + Assertions.assertThat(backupFile2).doesNotExist(); + + // Write some and trigger the 3rd rotation. + byte[] content4 = {'4'}; + stream.write(content4); + stream.flush(); + + // Verify the 3rd rotation. + inOrder + .verify(callback) + .onSuccess( + Mockito.same(policy), + Mockito.any(), + Mockito.eq(backupFile0)); + + // Verify files after the 3rd rotation. + Assertions.assertThat(file).hasBinaryContent(content4); + Assertions.assertThat(backupFile0).hasBinaryContent(content3); + Assertions.assertThat(backupFile1).hasBinaryContent(content2); + Assertions.assertThat(backupFile2).doesNotExist(); + + // Close the stream to avoid Windows failing to clean the temporary directory. + stream.close(); + + } + @Test void test_rotation_and_write_failure_after_close() throws Exception {