From a6b90b7ec5c238692dc323e25ef927e4433edb1d Mon Sep 17 00:00:00 2001 From: Gal Paikin Date: Mon, 30 Nov 2020 15:57:06 +0100 Subject: [PATCH] Add getsRefsByPrefixWithSkips (excluding prefixes) to ReftableDatabase We sometimes want to get all the refs except specific prefixes, similarly to getRefsByPrefix that gets all the refs of a specific prefix. We now create a new method that gets all refs matching a prefix except a set of specific prefixes. One use-case is for Gerrit to be able to get all the refs except refs/changes; in Gerrit we often have lots of refs/changes, but very little other refs. Currently, to get all the refs except refs/changes we need to get all the refs and then filter the refs/changes, which is very inefficient. With this method, we can simply skip the unneeded prefix so that we don't have to go over all the elements. RefDirectory still uses the inefficient implementation, since there isn't a simple way to use Refcursor to achieve the efficient implementation (as done in ReftableDatabase). Signed-off-by: Gal Paikin Change-Id: I8c5db581acdeb6698e3d3a2abde8da32f70c854c --- .../RefsUnreadableInMemoryRepository.java | 11 +++ .../storage/file/FileReftableTest.java | 72 +++++++++++++++++++ .../storage/file/RefDirectoryTest.java | 20 ++++++ .../tst/org/eclipse/jgit/lib/RefTest.java | 59 +++++++++++++++ .../storage/dfs/DfsReftableDatabase.java | 7 ++ .../storage/file/FileReftableDatabase.java | 8 +++ .../storage/reftable/ReftableDatabase.java | 50 +++++++++++++ .../src/org/eclipse/jgit/lib/RefDatabase.java | 28 ++++++++ 8 files changed, 255 insertions(+) diff --git a/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java b/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java index 80cbe8738..4167b038e 100644 --- a/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java +++ b/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java @@ -83,6 +83,17 @@ public List getRefsByPrefix(String prefix) throws IOException { return super.getRefsByPrefix(prefix); } + /** {@inheritDoc} */ + @Override + public List getRefsByPrefixWithExclusions(String include, Set excludes) + throws IOException { + if (failing) { + throw new IOException("disk failed, no refs found"); + } + + return super.getRefsByPrefixWithExclusions(include, excludes); + } + /** {@inheritDoc} */ @Override public Set getTipsWithSha1(ObjectId id) throws IOException { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java index 33bacbe3e..15c9109ca 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java @@ -28,14 +28,18 @@ import java.io.IOException; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.RefRename; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate.Result; @@ -579,6 +583,64 @@ public void reftableRefsStorageClass() throws IOException { assertEquals(Ref.Storage.PACKED, b.getStorage()); } + @Test + public void testGetRefsExcludingPrefix() throws IOException { + Set prefixes = new HashSet<>(); + prefixes.add("refs/tags"); + // HEAD + 12 refs/heads are present here. + List refs = + db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, prefixes); + assertEquals(13, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + checkContainsRef(refs, db.exactRef("refs/heads/a")); + for (Ref notInResult : db.getRefDatabase().getRefsByPrefix("refs/tags")) { + assertFalse(refs.contains(notInResult)); + } + } + + @Test + public void testGetRefsExcludingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/tags/"); + exclude.add("refs/heads/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + + @Test + public void testGetRefsExcludingNonExistingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/tags/"); + exclude.add("refs/heads/"); + exclude.add("refs/nonexistent/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + + @Test + public void testGetRefsWithPrefixExcludingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/heads/pa"); + String include = "refs/heads/p"; + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(include, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("refs/heads/prefix/a")); + } + + @Test + public void testGetRefsWithPrefixExcludingOverlappingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/heads/pa"); + exclude.add("refs/heads/"); + exclude.add("refs/heads/p"); + exclude.add("refs/tags/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + private RefUpdate updateRef(String name) throws IOException { final RefUpdate ref = db.updateRef(name); ref.setNewObjectId(db.resolve(Constants.HEAD)); @@ -596,4 +658,14 @@ private void writeSymref(String src, String dst) throws IOException { fail("link " + src + " to " + dst); } } + + private static void checkContainsRef(Collection haystack, Ref needle) { + for (Ref ref : haystack) { + if (ref.getName().equals(needle.getName()) && + ref.getObjectId().equals(needle.getObjectId())) { + return; + } + } + fail("list " + haystack + " does not contain ref " + needle); + } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java index 97ef5993b..38c545ef5 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java @@ -30,8 +30,10 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -352,6 +354,24 @@ public void testGetRefs_IgnoresGarbageRef4() throws IOException { assertEquals(A, c.getObjectId()); } + @Test + public void testGetRefs_ExcludingPrefixes() throws IOException { + writeLooseRef("refs/heads/A", A); + writeLooseRef("refs/heads/B", B); + writeLooseRef("refs/tags/tag", A); + writeLooseRef("refs/something/something", B); + writeLooseRef("refs/aaa/aaa", A); + + Set toExclude = new HashSet<>(); + toExclude.add("refs/aaa/"); + toExclude.add("refs/heads/"); + List refs = refdir.getRefsByPrefixWithExclusions(RefDatabase.ALL, toExclude); + + assertEquals(2, refs.size()); + assertTrue(refs.contains(refdir.exactRef("refs/tags/tag"))); + assertTrue(refs.contains(refdir.exactRef("refs/something/something"))); + } + @Test public void testFirstExactRef_IgnoresGarbageRef() throws IOException { writeLooseRef("refs/heads/A", A); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java index 88d17ec15..7590048a7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java @@ -26,6 +26,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -317,6 +318,64 @@ public void testGetRefsByPrefixes() throws IOException { checkContainsRef(refs, db.exactRef("refs/tags/A")); } + @Test + public void testGetRefsExcludingPrefix() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/tags"); + // HEAD + 12 refs/heads are present here. + List refs = + db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(13, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + checkContainsRef(refs, db.exactRef("refs/heads/a")); + for (Ref notInResult : db.getRefDatabase().getRefsByPrefix("refs/tags")) { + assertFalse(refs.contains(notInResult)); + } + } + + @Test + public void testGetRefsExcludingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/tags/"); + exclude.add("refs/heads/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + + @Test + public void testGetRefsExcludingNonExistingPrefixes() throws IOException { + Set prefixes = new HashSet<>(); + prefixes.add("refs/tags/"); + prefixes.add("refs/heads/"); + prefixes.add("refs/nonexistent/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, prefixes); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + + @Test + public void testGetRefsWithPrefixExcludingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/heads/pa"); + String include = "refs/heads/p"; + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(include, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("refs/heads/prefix/a")); + } + + @Test + public void testGetRefsWithPrefixExcludingOverlappingPrefixes() throws IOException { + Set exclude = new HashSet<>(); + exclude.add("refs/heads/pa"); + exclude.add("refs/heads/"); + exclude.add("refs/heads/p"); + exclude.add("refs/tags/"); + List refs = db.getRefDatabase().getRefsByPrefixWithExclusions(RefDatabase.ALL, exclude); + assertEquals(1, refs.size()); + checkContainsRef(refs, db.exactRef("HEAD")); + } + @Test public void testResolveTipSha1() throws IOException { ObjectId masterId = db.resolve("refs/heads/master"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableDatabase.java index 5561dc6a2..6c3b056ef 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableDatabase.java @@ -176,6 +176,13 @@ public List getRefsByPrefix(String prefix) throws IOException { return reftableDatabase.getRefsByPrefix(prefix); } + /** {@inheritDoc} */ + @Override + public List getRefsByPrefixWithExclusions(String include, Set excludes) + throws IOException { + return reftableDatabase.getRefsByPrefixWithExclusions(include, excludes); + } + /** {@inheritDoc} */ @Override public Set getTipsWithSha1(ObjectId id) throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java index ad1e75312..a80fa837b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java @@ -21,6 +21,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeSet; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -179,6 +180,13 @@ public Map getRefs(String prefix) throws IOException { RefList.emptyList()); } + /** {@inheritDoc} */ + @Override + public List getRefsByPrefixWithExclusions(String include, Set excludes) + throws IOException { + return reftableDatabase.getRefsByPrefixWithExclusions(include, excludes); + } + /** {@inheritDoc} */ @Override public List getAdditionalRefs() throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableDatabase.java index 4747be354..0c1682861 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableDatabase.java @@ -14,10 +14,12 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.lib.ObjectId; @@ -265,6 +267,54 @@ public List getRefsByPrefix(String prefix) throws IOException { return Collections.unmodifiableList(all); } + /** + * Returns refs whose names start with a given prefix excluding all refs that + * start with one of the given prefixes. + * + * @param include string that names of refs should start with; may be empty. + * @param excludes strings that names of refs can't start with; may be empty. + * @return immutable list of refs whose names start with {@code include} and + * none of the strings in {@code exclude}. + * @throws java.io.IOException the reference space cannot be accessed. + */ + public List getRefsByPrefixWithExclusions(String include, Set excludes) throws IOException { + if (excludes.isEmpty()) { + return getRefsByPrefix(include); + } + List results = new ArrayList<>(); + lock.lock(); + try { + Reftable table = reader(); + Iterator excludeIterator = + excludes.stream().sorted().collect(Collectors.toList()).iterator(); + String currentExclusion = excludeIterator.hasNext() ? excludeIterator.next() : null; + try (RefCursor rc = RefDatabase.ALL.equals(include) ? table.allRefs() : table.seekRefsWithPrefix(include)) { + while (rc.next()) { + Ref ref = table.resolve(rc.getRef()); + if (ref == null || ref.getObjectId() == null) { + continue; + } + // Skip prefixes that will never see since we are already further than those + // prefixes lexicographically. + while (excludeIterator.hasNext() && !ref.getName().startsWith(currentExclusion) + && ref.getName().compareTo(currentExclusion) > 0) { + currentExclusion = excludeIterator.next(); + } + + if (currentExclusion != null && ref.getName().startsWith(currentExclusion)) { + rc.seekPastPrefix(currentExclusion); + continue; + } + results.add(ref); + } + } + } finally { + lock.unlock(); + } + + return Collections.unmodifiableList(results); + } + /** * @return whether there is a fast SHA1 to ref map. * @throws IOException in case of I/O problems. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java index 6832c9cd8..7b7bdebac 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java @@ -21,6 +21,9 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; @@ -413,6 +416,31 @@ public List getRefsByPrefix(String prefix) throws IOException { return Collections.unmodifiableList(result); } + /** + * Returns refs whose names start with a given prefix excluding all refs that + * start with one of the given prefixes. + * + *

+ * The default implementation is not efficient. Implementors of {@link RefDatabase} + * should override this method directly if a better implementation is possible. + * + * @param include string that names of refs should start with; may be empty. + * @param excludes strings that names of refs can't start with; may be empty. + * @return immutable list of refs whose names start with {@code prefix} and none + * of the strings in {@code exclude}. + * @throws java.io.IOException the reference space cannot be accessed. + * @since 5.11 + */ + @NonNull + public List getRefsByPrefixWithExclusions(String include, Set excludes) + throws IOException { + Stream refs = getRefs(include).values().stream(); + for(String exclude: excludes) { + refs = refs.filter(r -> !r.getName().startsWith(exclude)); + } + return Collections.unmodifiableList(refs.collect(Collectors.toList())); + } + /** * Returns refs whose names start with one of the given prefixes. *