ReftableCompactor: Use instant to set the reflog expire time

The reflog expire time is compared with the time in the
PersonIdent. PersonIdent has moved to the java.time API and prefers
the getWhenAsInstant() method.

Use the Instant method in PersonIdent and propagate the use of Instant
to the parameter and setter.

Change-Id: I14cfdc93437971737dc7e472ca5c9885e2d37a13

2 files changed, 137 insertions, 18 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java
index 6fc7f25475..62dbda47fd 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java
@@ -19,6 +19,8 @@ import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -27,6 +29,7 @@ import org.eclipse.jgit.internal.storage.io.BlockSource;
 import org.eclipse.jgit.internal.storage.reftable.ReftableWriter.Stats;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
+import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.junit.Test;
 
@@ -279,6 +282,95 @@ public class ReftableCompactorTest {
 		}
 	}
 
+	@Test
+	public void reflog_all() throws IOException {
+		byte[] inTab;
+		try (ByteArrayOutputStream inBuf = new ByteArrayOutputStream()) {
+			ReftableWriter writer = new ReftableWriter(inBuf)
+					.setMinUpdateIndex(0).setMaxUpdateIndex(2).begin();
+			writer.writeLog(MASTER, 2, person(Instant.ofEpochSecond(500)),
+					id(3), id(4), null);
+			writer.writeLog(MASTER, 1, person(Instant.ofEpochSecond(300)),
+					id(2), id(3), null);
+			writer.writeLog(MASTER, 0, person(Instant.ofEpochSecond(100)),
+					id(1), id(2), null);
+			writer.finish();
+			inTab = inBuf.toByteArray();
+		}
+
+		ReftableCompactor compactor;
+		try (ByteArrayOutputStream outBuf = new ByteArrayOutputStream()) {
+			compactor = new ReftableCompactor(outBuf);
+			// No setReflogExpire time is set
+			List<ReftableReader> readers = new ArrayList<>();
+			readers.add(read(inTab));
+			compactor.addAll(readers);
+			compactor.compact();
+		}
+		Stats stats = compactor.getStats();
+		assertEquals(3, stats.logCount());
+	}
+
+	@Test
+	public void reflog_setExpireOlderThan() throws IOException {
+		byte[] inTab;
+		try (ByteArrayOutputStream inBuf = new ByteArrayOutputStream()) {
+			ReftableWriter writer = new ReftableWriter(inBuf)
+					.setMinUpdateIndex(0).setMaxUpdateIndex(2).begin();
+			writer.writeLog(MASTER, 2, person(Instant.ofEpochSecond(500)),
+					id(3), id(4), null);
+			writer.writeLog(MASTER, 1, person(Instant.ofEpochSecond(300)),
+					id(2), id(3), null);
+			writer.writeLog(MASTER, 0, person(Instant.ofEpochSecond(100)),
+					id(1), id(2), null);
+			writer.finish();
+			inTab = inBuf.toByteArray();
+		}
+
+		ReftableCompactor compactor;
+		try (ByteArrayOutputStream outBuf = new ByteArrayOutputStream()) {
+			compactor = new ReftableCompactor(outBuf);
+			compactor.setReflogExpireOlderThan(Instant.ofEpochSecond(300));
+			List<ReftableReader> readers = new ArrayList<>();
+			readers.add(read(inTab));
+			compactor.addAll(readers);
+			compactor.compact();
+		}
+
+		Stats stats = compactor.getStats();
+		assertEquals(2, stats.logCount());
+	}
+
+	@Test
+	public void reflog_disable() throws IOException {
+		byte[] inTab;
+		try (ByteArrayOutputStream inBuf = new ByteArrayOutputStream()) {
+			ReftableWriter writer = new ReftableWriter(inBuf)
+					.setMinUpdateIndex(0).setMaxUpdateIndex(2).begin();
+			writer.writeLog(MASTER, 2, person(Instant.ofEpochSecond(500)),
+					id(3), id(4), null);
+			writer.writeLog(MASTER, 1, person(Instant.ofEpochSecond(300)),
+					id(2), id(3), null);
+			writer.writeLog(MASTER, 0, person(Instant.ofEpochSecond(100)),
+					id(1), id(2), null);
+			writer.finish();
+			inTab = inBuf.toByteArray();
+		}
+
+		ReftableCompactor compactor;
+		try (ByteArrayOutputStream outBuf = new ByteArrayOutputStream()) {
+			compactor = new ReftableCompactor(outBuf);
+			compactor.setReflogExpireOlderThan(Instant.MAX);
+			List<ReftableReader> readers = new ArrayList<>();
+			readers.add(read(inTab));
+			compactor.addAll(readers);
+			compactor.compact();
+		}
+
+		Stats stats = compactor.getStats();
+		assertEquals(0, stats.logCount());
+	}
+
 	private static Ref ref(String name, int id) {
 		return new ObjectIdRef.PeeledNonTag(PACKED, name, id(id));
 	}
@@ -296,6 +388,10 @@ public class ReftableCompactorTest {
 		return ObjectId.fromRaw(buf);
 	}
 
+	private static PersonIdent person(Instant when) {
+		return new PersonIdent("a. u. thor", "author@jgit.com", when, ZoneId.systemDefault());
+	}
+
 	private static ReftableReader read(byte[] table) {
 		return new ReftableReader(BlockSource.from(table));
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java
index 3c4bc75792..7e5f4ebbd4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java
@@ -12,6 +12,7 @@ package org.eclipse.jgit.internal.storage.reftable;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.time.Instant;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.List;
@@ -28,20 +29,26 @@ import org.eclipse.jgit.lib.ReflogEntry;
  * to shadow any lower reftable that may have the reference present.
  * <p>
  * By default all log entries within the range defined by
- * {@link #setReflogExpireMinUpdateIndex(long)} and {@link #setReflogExpireMaxUpdateIndex(long)} are
- * copied, even if no references in the output file match the log records.
- * Callers may truncate the log to a more recent time horizon with
- * {@link #setReflogExpireOldestReflogTimeMillis(long)}, or disable the log altogether with
- * {@code setOldestReflogTimeMillis(Long.MAX_VALUE)}.
+ * {@link #setReflogExpireMinUpdateIndex(long)} and
+ * {@link #setReflogExpireMaxUpdateIndex(long)} are copied, even if no
+ * references in the output file match the log records. Callers may truncate the
+ * log to a more recent time horizon with
+ * {@link #setReflogExpireOlderThan(Instant)}, or disable the log
+ * altogether with {@code setReflogExpireOldestReflogTime(Instant.MAX)}.
  */
 public class ReftableCompactor {
 	private final ReftableWriter writer;
+
 	private final ArrayDeque<ReftableReader> tables = new ArrayDeque<>();
 
 	private boolean includeDeletes;
+
 	private long reflogExpireMinUpdateIndex = 0;
+
 	private long reflogExpireMaxUpdateIndex = Long.MAX_VALUE;
-	private long reflogExpireOldestReflogTimeMillis;
+
+	private Instant reflogExpireOldestReflogTime = Instant.EPOCH;
+
 	private Stats stats;
 
 	/**
@@ -122,9 +129,29 @@ public class ReftableCompactor {
 	 *            entries that predate {@code timeMillis} will be discarded.
 	 *            Specified in Java standard milliseconds since the epoch.
 	 * @return {@code this}
+	 *
+	 * @deprecated Use {@link #setReflogExpireOlderThan(Instant)} instead
+	 */
+	@Deprecated(since="7.3")
+	public ReftableCompactor setReflogExpireOldestReflogTimeMillis(
+			long timeMillis) {
+		return setReflogExpireOlderThan(timeMillis == Long.MAX_VALUE
+				? Instant.MAX
+				: Instant.ofEpochMilli(timeMillis));
+	}
+
+	/**
+	 * Set oldest reflog time to preserve.
+	 *
+	 * @param cutTime
+	 *            oldest log time to preserve. Entries whose timestamps are
+	 *            {@code >= cutTime} will be copied into the output file. Log
+	 *            entries that predate {@code cutTime} will be discarded.
+	 * @return {@code this}
 	 */
-	public ReftableCompactor setReflogExpireOldestReflogTimeMillis(long timeMillis) {
-		reflogExpireOldestReflogTimeMillis = timeMillis;
+	public ReftableCompactor setReflogExpireOlderThan(
+			Instant cutTime) {
+		reflogExpireOldestReflogTime = cutTime;
 		return this;
 	}
 
@@ -182,14 +209,15 @@ public class ReftableCompactor {
 	}
 
 	private void mergeLogs(MergedReftable mr) throws IOException {
-		if (reflogExpireOldestReflogTimeMillis == Long.MAX_VALUE) {
+		if (reflogExpireOldestReflogTime == Instant.MAX) {
 			return;
 		}
 
 		try (LogCursor lc = mr.allLogs()) {
 			while (lc.next()) {
 				long updateIndex = lc.getUpdateIndex();
-				if (updateIndex > reflogExpireMaxUpdateIndex || updateIndex < reflogExpireMinUpdateIndex) {
+				if (updateIndex > reflogExpireMaxUpdateIndex
+						|| updateIndex < reflogExpireMinUpdateIndex) {
 					continue;
 				}
 
@@ -203,14 +231,9 @@ public class ReftableCompactor {
 				}
 
 				PersonIdent who = log.getWho();
-				if (who.getWhen().getTime() >= reflogExpireOldestReflogTimeMillis) {
-					writer.writeLog(
-							refName,
-							updateIndex,
-							who,
-							log.getOldId(),
-							log.getNewId(),
-							log.getComment());
+				if (who.getWhenAsInstant().compareTo(reflogExpireOldestReflogTime) >= 0) {
+					writer.writeLog(refName, updateIndex, who, log.getOldId(),
+							log.getNewId(), log.getComment());
 				}
 			}
 		}