vy · liran2000 · Jun 9, 2021
diff --git a/README.md b/README.md
@@ -50,6 +50,7 @@ try (RotatingFileOutputStream stream = new RotatingFileOutputStream(config)) {
 | `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`) |
+| `rollingFile(boolean)` | rolling file by indexes, based on SizeBasedRotationPolicy, similar to log4j RollingFileAppender. (defaults to `false`) |
 | `clock(Clock)` | clock for retrieving date and time (defaults to `SystemClock`) |
 | `callback(RotationCallback)` | rotation callback (defaults to `LoggingRotationCallback`) |
 
@@ -135,6 +136,7 @@ methods.
 - [Jonas (yawkat) Konrad](https://yawk.at/) (`RotatingFileOutputStream`
   thread-safety improvements)
 - [Lukas Bradley](https://github.com/lukasbradley/)
+- [Liran Mendelovich](https://github.com/liran2000/) (`rollingFile` enhancement)
 
 # License
 

diff --git a/src/main/java/com/vlkan/rfos/RotatingFileOutputStream.java b/src/main/java/com/vlkan/rfos/RotatingFileOutputStream.java
@@ -16,14 +16,24 @@
 
 package com.vlkan.rfos;
 
-import com.vlkan.rfos.policy.RotationPolicy;
+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.Paths;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.zip.GZIPOutputStream;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.*;
-import java.time.Instant;
-import java.util.*;
-import java.util.zip.GZIPOutputStream;
+import com.vlkan.rfos.policy.RotationPolicy;
 
 public class RotatingFileOutputStream extends OutputStream implements Rotatable {
 
@@ -101,16 +111,57 @@ private synchronized void unsafeRotate(RotationPolicy policy, Instant instant) t
         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);
+        File rotatedFile = null;
+        boolean renamed = false;
+
+        if (config.isRollingFile()) {
+        	int maxBackupIndex = config.getMaxBackupIndex();
+        	LOGGER.debug("Rotating using rolling file, maxBackupIndex: {}", maxBackupIndex);
+        	renamed = true;
+    		String parent = config.getFile().getParent();
+    		if (parent == null) {
+    			parent = ".";
+    		}
+			File file = Paths.get(parent,
+				config.getFile().getName() + '.' + maxBackupIndex).toFile();
+    		if (file.exists()) {
+    			LOGGER.debug("Deleting oldest file: {}", file);
+    			renamed = file.delete();
+    		}
+
+    		// Map {(maxBackupIndex - 1), ..., 2, 1} to {maxBackupIndex, ..., 3, 2}
+    		for (int i = maxBackupIndex - 1; i >= 1 && renamed; i--) {
+    			file = Paths.get(parent,
+    				config.getFile().getName() + '.' + i).toFile();
+    			if (file.exists()) {
+    				rotatedFile = Paths.get(parent,
+        				config.getFile().getName() + '.' + (i+1)).toFile();
+    				LOGGER.debug("Renaming file {} to {}", file, rotatedFile);
+    				renamed = file.renameTo(rotatedFile);
+    			}
+    		}
+
+    		if(renamed) {
+    			rotatedFile =  Paths.get(parent,
+    				config.getFile().getName() + '.' + 1).toFile();
+    			file = Paths.get(parent,
+    				config.getFile().getName()).toFile();
+    			LOGGER.debug("Renaming file {} to {}", file, rotatedFile);
+    			renamed = file.renameTo(rotatedFile);
+    		}
+        } else {
+        	rotatedFile = config.getFilePattern().create(instant).getAbsoluteFile();
+            LOGGER.debug("renaming {file={}, rotatedFile={}}", config.getFile(), rotatedFile);
+            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);
             callback.onFailure(policy, instant, rotatedFile, error);
             return;
         }
-
+        
         // Re-open the file.
         LOGGER.debug("re-opening file {file={}}", config.getFile());
         stream = open(policy, instant);

diff --git a/src/main/java/com/vlkan/rfos/RotationConfig.java b/src/main/java/com/vlkan/rfos/RotationConfig.java
@@ -16,8 +16,6 @@
 
 package com.vlkan.rfos;
 
-import com.vlkan.rfos.policy.RotationPolicy;
-
 import java.io.File;
 import java.util.LinkedHashSet;
 import java.util.Objects;
@@ -26,6 +24,14 @@
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadFactory;
 
+import com.vlkan.rfos.policy.RotationPolicy;
+import com.vlkan.rfos.policy.SizeBasedRotationPolicy;
+
+/**
+ * RotationConfig
+ * 
+ * rollingFile - rolling file by indexes, based on SizeBasedRotationPolicy, similar to log4j RollingFileAppender.
+ */
 public class RotationConfig {
 
     private enum DefaultExecutorServiceHolder {;
@@ -71,6 +77,10 @@ private static int readDefaultThreadCount() {
     private final boolean append;
 
     private final boolean compress;
+
+    private final boolean rollingFile;
+
+    private final Integer maxBackupIndex;
 
     private final Clock clock;
 
@@ -83,6 +93,8 @@ private RotationConfig(Builder builder) {
         this.policies = builder.policies;
         this.append = builder.append;
         this.compress = builder.compress;
+        this.rollingFile = builder.rollingFile;
+        this.maxBackupIndex = builder.maxBackupIndex;
         this.clock = builder.clock;
         this.callback = builder.callback;
     }
@@ -115,7 +127,15 @@ public boolean isCompress() {
         return compress;
     }
 
-    public Clock getClock() {
+    public boolean isRollingFile() {
+		return rollingFile;
+	}
+
+	public Integer getMaxBackupIndex() {
+		return maxBackupIndex;
+	}
+
+	public Clock getClock() {
         return clock;
     }
 
@@ -130,6 +150,8 @@ public boolean equals(Object instance) {
         RotationConfig that = (RotationConfig) instance;
         return append == that.append &&
                 compress == that.compress &&
+        		rollingFile == that.rollingFile &&
+				Objects.equals(maxBackupIndex, that.maxBackupIndex) &&
                 Objects.equals(file, that.file) &&
                 Objects.equals(filePattern, that.filePattern) &&
                 Objects.equals(executorService, that.executorService) &&
@@ -140,7 +162,7 @@ public boolean equals(Object instance) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(file, filePattern, executorService, policies, append, compress, clock, callback);
+        return Objects.hash(file, filePattern, executorService, policies, append, compress, rollingFile, maxBackupIndex, clock, callback);
     }
 
     @Override
@@ -165,6 +187,9 @@ public static class Builder {
         private boolean append = true;
 
         private boolean compress = false;
+
+        private boolean rollingFile = false;
+        private Integer maxBackupIndex = null;
 
         private Clock clock = SystemClock.getInstance();
 
@@ -221,6 +246,16 @@ public Builder compress(boolean compress) {
             this.compress = compress;
             return this;
         }
+
+        public Builder rollingFile(boolean rollingFile) {
+            this.rollingFile = rollingFile;
+            return this;
+        }
+
+        public Builder maxBackupIndex(int maxBackupIndex) {
+            this.maxBackupIndex = maxBackupIndex;
+            return this;
+        }
 
         public Builder clock(Clock clock) {
             this.clock = clock;
@@ -246,9 +281,29 @@ private void prepare() {
 
         private void validate() {
             Objects.requireNonNull(file, "file");
-            Objects.requireNonNull(filePattern, "filePattern");
-            if (policies == null || policies.isEmpty()) {
-                throw new IllegalArgumentException("empty policies");
+            if (rollingFile) {
+            	boolean singleRotationPolicy = false;
+            	if (policies != null && policies.size() == 1) {
+            		RotationPolicy rotationPolicy = policies.iterator().next();
+            		if (rotationPolicy instanceof SizeBasedRotationPolicy) {
+            			singleRotationPolicy = true;
+            		}
+            	}
+            	if (!singleRotationPolicy) {
+            		throw new IllegalArgumentException("Expecting single SizeBasedRotationPolicy when using rollingFile.");
+            	}
+            	if (filePattern != null) {
+            		throw new IllegalArgumentException("filePattern cannot be set when using rollingFile.");
+            	}
+            	Objects.requireNonNull(maxBackupIndex, "maxBackupIndex");
+            	if (maxBackupIndex <= 0) {
+            		throw new IllegalArgumentException("maxBackupIndex is out of range, must be positive.");
+            	}
+            } else {
+	            Objects.requireNonNull(filePattern, "filePattern");
+	            if (policies == null || policies.isEmpty()) {
+	                throw new IllegalArgumentException("empty policies");
+	            }
             }
             Objects.requireNonNull(clock, "clock");
             Objects.requireNonNull(callback, "callback");

diff --git a/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java b/src/test/java/com/vlkan/rfos/RotatingFileOutputStreamTest.java
@@ -16,28 +16,35 @@
 
 package com.vlkan.rfos;
 
-import com.vlkan.rfos.policy.RotationPolicy;
-import com.vlkan.rfos.policy.SizeBasedRotationPolicy;
-import org.assertj.core.api.Assertions;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.mockito.InOrder;
-import org.mockito.Mockito;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
 import java.time.Instant;
 import java.util.Calendar;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.zip.GZIPOutputStream;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import org.assertj.core.api.Assertions;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.internal.ArrayComparisonFailure;
+import org.junit.rules.TemporaryFolder;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.vlkan.rfos.policy.RotationPolicy;
+import com.vlkan.rfos.policy.SizeBasedRotationPolicy;
 
 public class RotatingFileOutputStreamTest {
 
@@ -490,5 +497,56 @@ private static byte[] copyArrays(byte[]... sources) {
         }
         return target;
     }
+
+    @Test
+    public void rollingFileTest() throws Exception {
+    	LOGGER.debug("rollingFileTest begin");
+    	File dir = Paths.get(tmpDir.getRoot().toString(), "rollingFileTest").toFile();
+    	dir.mkdir();
+    	int maxByteCount = 10;
+		int maxBackupIndex = 10;
+		String fileName = "app.log";
+		RotationConfig config = RotationConfig
+	        .builder()
+	        .file(dir + File.separator + fileName)
+	        .rollingFile(true)
+	        .policy(new SizeBasedRotationPolicy(maxByteCount))
+	        .maxBackupIndex(maxBackupIndex )
+	        .build();
+		try (RotatingFileOutputStream stream = new RotatingFileOutputStream(config)) {
+			for (int i = 0; i < 50; i++) {
+				stream.write(String.format("a%04d",i).getBytes(StandardCharsets.UTF_8));
+			}
+		}
+
+		File[] outputFiles = dir.listFiles();
+		assertEquals("maxBackupIndex value not as expected", maxBackupIndex + 1, outputFiles.length);
+		Map<String, String> suffixToContent = new HashMap<>();
+		suffixToContent.put("", "a0048a0049");
+		suffixToContent.put(".1", "a0046a0047");
+		suffixToContent.put(".10", "a0028a0029");
+		int validatedCount = 0;
+		for (File file : outputFiles) {
+			LOGGER.debug("Validating file: {}", file);
+			String suffix = file.getName().substring(fileName.length());
+			LOGGER.debug("suffix: {}", suffix);
+			String expectedContent = suffixToContent.get(suffix);
+			if (expectedContent != null &&(fileName + suffix).equals(file.getName())) {
+				LOGGER.debug("Validating file content for: {}", fileName);
+				validateContent(maxByteCount, fileName, file, expectedContent);
+				validatedCount++;
+			}
+		}
+		assertEquals("Did not validate content of all files", validatedCount, suffixToContent.size());
+		LOGGER.debug("rollingFileTest end");
+    }
+
+	private void validateContent(int maxByteCount, String fileName, File file, String expectedContent)
+			throws IOException, ArrayComparisonFailure {
+		byte[] fileBytes = readFileBytes(file, maxByteCount);
+		byte[] expectedBytes = expectedContent.getBytes(StandardCharsets.UTF_8);
+		assertArrayEquals("File content not as expected for: " + fileName + ". Expected: " + 
+			new String(expectedBytes, StandardCharsets.UTF_8) + ", actual: " + new String(fileBytes, StandardCharsets.UTF_8), expectedBytes, fileBytes);
+	}
 
 }