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>

3 files changed, 24 insertions, 3 deletions

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 87b9176368..8dbe64478e 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
@@ -1226,6 +1226,27 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 	}
 
 	@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);
 		writeLooseRef("refs/heads/masters", B);
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 1731a33622..098b31fccf 100644
--- 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 8d59cb47b4..f47dff75a8 100644
--- 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();