From 218bacdc1f96942701503aa7891707bb3eccc111 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Sun, 13 Oct 2019 19:21:06 +0200 Subject: [PATCH] reftable: fix lookup by ID in merged reftables On changing a ref, the old SHA1 is not updated in the object => ref mapping. This means search by object ID may still turn up a ref from deeper within the stack. To fix this, check all refs produced by the merged iterator against the merged reftables. Signed-off-by: Han-Wen Nienhuys Change-Id: I41e9cd395b0608eedeeaead0a9fd997238d747c9 --- .../storage/reftable/MergedReftableTest.java | 13 +++++++ .../storage/reftable/MergedReftable.java | 38 ++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java index c20db7b96..5f5ab7267 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java @@ -187,6 +187,19 @@ public void twoTableById() throws IOException { } } + @Test + public void tableByIDDeletion() throws IOException { + List delta1 = Arrays.asList( + ref("refs/heads/apple", 1), + ref("refs/heads/master", 2)); + List delta2 = Arrays.asList(ref("refs/heads/master", 3)); + + MergedReftable mr = merge(write(delta1), write(delta2)); + try (RefCursor rc = mr.byObjectId(id(2))) { + assertFalse(rc.next()); + } + } + @SuppressWarnings("boxing") @Test public void fourTableScan() throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java index a4394066e..4de5e392f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java @@ -131,7 +131,7 @@ public RefCursor seekRefsWithPrefix(String prefix) throws IOException { /** {@inheritDoc} */ @Override public RefCursor byObjectId(AnyObjectId name) throws IOException { - MergedRefCursor m = new MergedRefCursor(); + MergedRefCursor m = new FilteringMergedRefCursor(name); for (int i = 0; i < tables.length; i++) { m.add(new RefQueueEntry(tables[i].byObjectId(name), i)); } @@ -250,6 +250,42 @@ public void close() { } } + private class FilteringMergedRefCursor extends MergedRefCursor { + final AnyObjectId filterId; + Ref filteredRef; + + FilteringMergedRefCursor(AnyObjectId id) { + filterId = id; + filteredRef = null; + } + + @Override + public Ref getRef() { + return filteredRef; + } + + @Override + public boolean next() throws IOException { + for (;;) { + boolean ok = super.next(); + if (!ok) { + return false; + } + + String name = super.getRef().getName(); + + try (RefCursor c = seekRef(name)) { + if (c.next()) { + if (filterId.equals(c.getRef().getObjectId())) { + filteredRef = c.getRef(); + return true; + } + } + } + } + } + } + private static class RefQueueEntry { static int compare(RefQueueEntry a, RefQueueEntry b) { int cmp = a.name().compareTo(b.name());