Fix sorting of names in RefDirectory

RefDirectory did not correctly follow the contract of RefList.  The
contract says if you use add() method of RefList builder, you MUST
sort() it afterwards, and later every other method assumes that list
is properly sorted (especially the binary search in the find() and
get() methods).  Instead RefDirectory class tried to scan the refs
recursively while sorting every folder in the process before
processing and did not call sort().

For example, when scanning the contents of refs/tags project1 string
is smaller than project1-*, so it will recursively go into the folder
and add these tags first and only then will add project-* ones. This
will result in a broken list (any project1-* string is less than
project1/* one, but they all appear after them in the list), that's
why binary search will fail making loose RefList and the whole local
RefMap completely unusable.

Change-Id: Ibad90017e3b2435b1396b69a22520db4b1b022bb
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
index a61580dd1f1f57fd5d96c6cbd683a0680cce17e9..56e5549b841fef2394edd6e46abbeff82da6695d 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
@@ -455,6 +455,19 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
                assertEquals(v1_0, a.getObjectId());
        }
 
+       @Test
+       public void testGetRefs_LooseSortedCorrectly() throws IOException {
+               Map<String, Ref> refs;
+
+               writeLooseRef("refs/heads/project1/A", A);
+               writeLooseRef("refs/heads/project1-B", B);
+
+               refs = refdir.getRefs(RefDatabase.ALL);
+               assertEquals(2, refs.size());
+               assertEquals(A, refs.get("refs/heads/project1/A").getObjectId());
+               assertEquals(B, refs.get("refs/heads/project1-B").getObjectId());
+       }
+
        @Test
        public void testGetRefs_TagsOnly_AllPacked() throws IOException {
                Map<String, Ref> tags;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
index bba634a6b646af1a1a1c3f11b0eec9f60699f12e..cd199dcf92a01e907e20e76f922db2b0a55f478e 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
@@ -288,6 +288,7 @@ public class RefDirectory extends RefDatabase {
 
                RefList<LooseRef> loose;
                if (scan.newLoose != null) {
+                       scan.newLoose.sort();
                        loose = scan.newLoose.toRefList();
                        if (looseRefs.compareAndSet(oldLoose, loose))
                                modCnt.incrementAndGet();
@@ -312,6 +313,7 @@ public class RefDirectory extends RefDatabase {
                                        loose = loose.remove(toRemove);
                        }
                }
+               symbolic.sort();
 
                return new RefMap(prefix, packed, upcast(loose), symbolic.toRefList());
        }