From ca9107d166ec2a06e8c16b73d05064ba28c049da Mon Sep 17 00:00:00 2001
From: Han-Wen Nienhuys <hanwen@google.com>
Date: Mon, 29 Jul 2019 10:58:50 +0200
Subject: [PATCH] reftable: fix seeking to refs in reflog implementation

Small reftables omit the log index. Currently,
ReftableWriter#shouldHaveIndex does this if there is a single-block
log, but other writers could decide on different criteria.

In the case that the log index is missing, we have to linearly search
for the right block. It is never appropriate to use binary search on
blocks for log data, as the blocks are compressed and therefore
irregularly sized.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Id59874edf6bf45c7dec502d9465888e077ffe198
---
 .../storage/reftable/ReftableTest.java        | 45 +++++++++++++++++++
 .../storage/reftable/ReftableReader.java      | 26 +++++++++++
 2 files changed, 71 insertions(+)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java
index a14216698..db9a4d0ac 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java
@@ -471,6 +471,51 @@ public void withReflog() throws IOException {
 		}
 	}
 
+	@Test
+	public void reflogSeek() throws IOException {
+		PersonIdent who = new PersonIdent("Log", "Ger", 1500079709, -8 * 60);
+		String msg = "test";
+		String msgNext = "test next";
+
+		ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+		ReftableWriter writer = new ReftableWriter()
+				.setMinUpdateIndex(1)
+				.setMaxUpdateIndex(1)
+				.begin(buffer);
+
+		writer.writeLog(MASTER, 1, who, ObjectId.zeroId(), id(1), msg);
+		writer.writeLog(NEXT, 1, who, ObjectId.zeroId(), id(2), msgNext);
+
+		writer.finish();
+		byte[] table = buffer.toByteArray();
+
+		ReftableReader t = read(table);
+		try (LogCursor c = t.seekLog(MASTER, Long.MAX_VALUE)) {
+			assertTrue(c.next());
+			assertEquals(c.getReflogEntry().getComment(), msg);
+		}
+		try (LogCursor c = t.seekLog(MASTER, 0)) {
+			assertFalse(c.next());
+		}
+		try (LogCursor c = t.seekLog(MASTER, 1)) {
+			assertTrue(c.next());
+			assertEquals(c.getUpdateIndex(), 1);
+			assertEquals(c.getReflogEntry().getComment(), msg);
+		}
+		try (LogCursor c = t.seekLog(NEXT, Long.MAX_VALUE)) {
+			assertTrue(c.next());
+			assertEquals(c.getReflogEntry().getComment(), msgNext);
+		}
+		try (LogCursor c = t.seekLog(NEXT, 0)) {
+			assertFalse(c.next());
+		}
+		try (LogCursor c = t.seekLog(NEXT, 1)) {
+			assertTrue(c.next());
+			assertEquals(c.getUpdateIndex(), 1);
+			assertEquals(c.getReflogEntry().getComment(), msgNext);
+		}
+	}
+
 	@Test
 	public void onlyReflog() throws IOException {
 		PersonIdent who = new PersonIdent("Log", "Ger", 1500079709, -8 * 60);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java
index bf3a9aeca..226e675e4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java
@@ -255,6 +255,32 @@ private BlockReader seek(byte blockType, byte[] key, BlockReader idx,
 			block.seekKey(key);
 			return block;
 		}
+		if (blockType == LOG_BLOCK_TYPE) {
+			// No index. Log blocks are irregularly sized, so we can't do binary search
+			// between blocks. Scan over blocks instead.
+			BlockReader block = readBlock(startPos, endPos);
+
+			for (;;) {
+				if (block == null || block.type() != LOG_BLOCK_TYPE) {
+					return null;
+				}
+
+				int result = block.seekKey(key);
+				if (result <= 0) {
+					// == 0 : we found the key.
+					// < 0 : the key is before this block. Either the ref name is there
+					// but only at a newer updateIndex, or it is absent. We leave it to
+					// logcursor to distinguish between both cases.
+					return block;
+				}
+
+				long pos = block.endPosition();
+				if (pos >= endPos) {
+					return null;
+				}
+				block = readBlock(pos, endPos);
+			}
+		}
 		return binarySearch(blockType, key, startPos, endPos);
 	}