Non-Fast-Forward Ref-Updates: Omit isMergedInto() calls

When the caller specifies to JGit in advance that a ref-update is a
non-fast-forward update, and that those are permitted, we should never
need to call the potentially expensive isMergedInto() check. Re-checking
that the older commit is /not/ reachable from the newer is superfluous.

http://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02258.html

Change-Id: I4bbf593de4dcea6b6f082881c1a33cb3a6a7fb89
Signed-off-by: Roberto Tyley <roberto.tyley@gmail.com>

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 87b917636848b1bf19ba3deb14f476cafd9b368c..8dbe64478e9af754731e3a129001a1489922d4a0 100644 (file)
--- 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
@@ -1225,6 +1225,27 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
                assertEquals(A.getId(), refs.get("refs/heads/masters").getObjectId());
        }
 
+       @Test
+       public void testBatchRefUpdateNonFastForwardDoesNotDoExpensiveMergeCheck()
+                       throws IOException {
+               writeLooseRef("refs/heads/master", B);
+               List<ReceiveCommand> commands = Arrays.asList(
+                               newCommand(B, A, "refs/heads/master",
+                                               ReceiveCommand.Type.UPDATE_NONFASTFORWARD));
+               BatchRefUpdate batchUpdate = refdir.newBatchUpdate();
+               batchUpdate.setAllowNonFastForwards(true);
+               batchUpdate.addCommand(commands);
+               batchUpdate.execute(new RevWalk(diskRepo) {
+                       @Override
+                       public boolean isMergedInto(RevCommit base, RevCommit tip) {
+                               throw new AssertionError("isMergedInto() should not be called");
+                       }
+               }, new StrictWorkMonitor());
+               Map<String, Ref> refs = refdir.getRefs(RefDatabase.ALL);
+               assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult());
+               assertEquals(A.getId(), refs.get("refs/heads/master").getObjectId());
+       }
+
        @Test
        public void testBatchRefUpdateConflict() throws IOException {
                writeLooseRef("refs/heads/master", A);

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
index 1731a33622f3e8ecb0aa604c491c741f221ffc37..098b31fccff3032a719c51525198e63156d7ef24 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
@@ -455,7 +455,6 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 
                // now update that ref
                updateRef = db.updateRef(Constants.HEAD);
-               updateRef.setForceUpdate(true);
                updateRef.setNewObjectId(oldValue);
                update = updateRef.update();
                assertEquals(Result.FAST_FORWARD, update);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
index 8d59cb47b404d29eb282744b9650c2c6f80158f1..f47dff75a84850e8faf40d517f15a0d550d6d6be 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
@@ -620,13 +620,14 @@ public abstract class RefUpdate {
                        if (newObj == oldObj && !detachingSymbolicRef)
                                return store.execute(Result.NO_CHANGE);
 
+                       if (isForceUpdate())
+                               return store.execute(Result.FORCED);
+
                        if (newObj instanceof RevCommit && oldObj instanceof RevCommit) {
                                if (walk.isMergedInto((RevCommit) oldObj, (RevCommit) newObj))
                                        return store.execute(Result.FAST_FORWARD);
                        }
 
-                       if (isForceUpdate())
-                               return store.execute(Result.FORCED);
                        return Result.REJECTED;
                } finally {
                        unlock();