Fix reading of ref names containing characters that sort before /

A set of ref names like ('a/b' and 'a+b') would cause the RefDirectory
to think that the set of refs have changed because it traversed the
'a' directory in the subtree before looking at 'a+b', but it then
compared with the know refs which are sorted with 'a+b' first.

Fix this by traversing the refs tree in another order. Treat a directory
as if they ends with a '/' before deciding on the order to traverse
the refs tree.

Bug: 348834
Change-Id: I23377f8df00c7252bf27dbcfba5da193c5403917
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>

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 dc2ccb97fb842997248839abb26e86dd0933aa03..3ca4f589dbcba4c267e528207cbff6c668d15363 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
@@ -61,6 +61,9 @@ import java.io.File;
 import java.io.IOException;
 import java.util.Map;
 
+import org.eclipse.jgit.events.ListenerHandle;
+import org.eclipse.jgit.events.RefsChangedEvent;
+import org.eclipse.jgit.events.RefsChangedListener;
 import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -465,6 +468,33 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
                assertEquals(B, refs.get("refs/heads/project1-B").getObjectId());
        }
 
+       @Test
+       public void testGetRefs_LooseSorting_Bug_348834() throws IOException {
+               Map<String, Ref> refs;
+
+               writeLooseRef("refs/heads/my/a+b", A);
+               writeLooseRef("refs/heads/my/a/b/c", B);
+
+               final int[] count = new int[1];
+
+               ListenerHandle listener = Repository.getGlobalListenerList()
+                               .addRefsChangedListener(new RefsChangedListener() {
+
+                                       public void onRefsChanged(RefsChangedEvent event) {
+                                               count[0]++;
+                                       }
+                               });
+
+               refs = refdir.getRefs(RefDatabase.ALL);
+               refs = refdir.getRefs(RefDatabase.ALL);
+               listener.remove();
+               assertEquals(1, count[0]); // Bug 348834 multiple RefsChangedEvents
+               assertEquals(2, refs.size());
+               assertEquals(A, refs.get("refs/heads/my/a+b").getObjectId());
+               assertEquals(B, refs.get("refs/heads/my/a/b/c").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 0590f905ec71ac0d3096721348ab0e77d7ceaea9..3917ff4716d125e26dd4de4d69b939d6a84c109d 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
@@ -385,10 +385,17 @@ public class RefDirectory extends RefDatabase {
                        if (entries == null) // not a directory or an I/O error
                                return false;
                        if (0 < entries.length) {
+                               for (int i = 0; i < entries.length; ++i) {
+                                       String e = entries[i];
+                                       File f = new File(dir, e);
+                                       if (f.isDirectory())
+                                               entries[i] += '/';
+                               }
                                Arrays.sort(entries);
                                for (String name : entries) {
-                                       File e = new File(dir, name);
-                                       if (!scanTree(prefix + name + '/', e))
+                                       if (name.charAt(name.length() - 1) == '/')
+                                               scanTree(prefix + name, new File(dir, name));
+                                       else
                                                scanOne(prefix + name);
                                }
                        }