Break dissimilar file pairs during diff

File pairs that are very dissimilar during a diff were not being
broken apart into their constituent ADD/DELETE pairs. The leads to
sub-optimal rename detection. Take, for example, this situation:

A file exists at src/a.txt containing "foo". A user renames src/a.txt
to src/b.txt, then adds a new src/a.txt containing "bar".

Even though the old a.txt and the new b.txt are identical, the
rename detection algorithm would not detect it as a rename since
it was already paired in a MODIFY. I added code to split all
MODIFYs below a certain score into their constituent ADD/DELETE
pairs. This allows situations like the one I described above to be
more correctly handled.

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

1 files changed, 121 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java
index bfc51b68f9..eb965cf601 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java
@@ -383,6 +383,116 @@ public class RenameDetectorTest extends RepositoryTestCase {
 		assertSame(b, entries.get(1));
 	}
 
+	public void testBreakModify_BreakAll() throws Exception {
+		ObjectId aId = blob("foo");
+		ObjectId bId = blob("bar");
+
+		DiffEntry m = DiffEntry.modify(PATH_A);
+		m.oldId = AbbreviatedObjectId.fromObjectId(aId);
+		m.newId = AbbreviatedObjectId.fromObjectId(bId);
+
+		DiffEntry a = DiffEntry.add(PATH_B, aId);
+
+		rd.add(a);
+		rd.add(m);
+
+		rd.setBreakScore(101);
+
+		List<DiffEntry> entries = rd.compute();
+		assertEquals(2, entries.size());
+		assertAdd(PATH_A, bId, FileMode.REGULAR_FILE, entries.get(0));
+		assertRename(DiffEntry.breakModify(m).get(0), a, 100, entries.get(1));
+	}
+
+	public void testBreakModify_BreakNone() throws Exception {
+		ObjectId aId = blob("foo");
+		ObjectId bId = blob("bar");
+
+		DiffEntry m = DiffEntry.modify(PATH_A);
+		m.oldId = AbbreviatedObjectId.fromObjectId(aId);
+		m.newId = AbbreviatedObjectId.fromObjectId(bId);
+
+		DiffEntry a = DiffEntry.add(PATH_B, aId);
+
+		rd.add(a);
+		rd.add(m);
+
+		rd.setBreakScore(-1);
+
+		List<DiffEntry> entries = rd.compute();
+		assertEquals(2, entries.size());
+		assertSame(m, entries.get(0));
+		assertSame(a, entries.get(1));
+	}
+
+	public void testBreakModify_BreakBelowScore() throws Exception {
+		ObjectId aId = blob("foo");
+		ObjectId bId = blob("bar");
+
+		DiffEntry m = DiffEntry.modify(PATH_A);
+		m.oldId = AbbreviatedObjectId.fromObjectId(aId);
+		m.newId = AbbreviatedObjectId.fromObjectId(bId);
+
+		DiffEntry a = DiffEntry.add(PATH_B, aId);
+
+		rd.add(a);
+		rd.add(m);
+
+		rd.setBreakScore(20); // Should break the modify
+
+		List<DiffEntry> entries = rd.compute();
+		assertEquals(2, entries.size());
+		assertAdd(PATH_A, bId, FileMode.REGULAR_FILE, entries.get(0));
+		assertRename(DiffEntry.breakModify(m).get(0), a, 100, entries.get(1));
+	}
+
+	public void testBreakModify_DontBreakAboveScore() throws Exception {
+		ObjectId aId = blob("blah\nblah\nfoo");
+		ObjectId bId = blob("blah\nblah\nbar");
+
+		DiffEntry m = DiffEntry.modify(PATH_A);
+		m.oldId = AbbreviatedObjectId.fromObjectId(aId);
+		m.newId = AbbreviatedObjectId.fromObjectId(bId);
+
+		DiffEntry a = DiffEntry.add(PATH_B, aId);
+
+		rd.add(a);
+		rd.add(m);
+
+		rd.setBreakScore(20); // Should not break the modify
+
+		List<DiffEntry> entries = rd.compute();
+		assertEquals(2, entries.size());
+		assertSame(m, entries.get(0));
+		assertSame(a, entries.get(1));
+	}
+
+	public void testBreakModify_RejoinIfUnpaired() throws Exception {
+		ObjectId aId = blob("foo");
+		ObjectId bId = blob("bar");
+
+		DiffEntry m = DiffEntry.modify(PATH_A);
+		m.oldId = AbbreviatedObjectId.fromObjectId(aId);
+		m.newId = AbbreviatedObjectId.fromObjectId(bId);
+
+		rd.add(m);
+
+		rd.setBreakScore(101); // Ensure m is broken apart
+
+		List<DiffEntry> entries = rd.compute();
+		assertEquals(1, entries.size());
+
+		DiffEntry modify = entries.get(0);
+		assertEquals(m.oldName, modify.oldName);
+		assertEquals(m.oldId, modify.oldId);
+		assertEquals(m.oldMode, modify.oldMode);
+		assertEquals(m.newName, modify.newName);
+		assertEquals(m.newId, modify.newId);
+		assertEquals(m.newMode, modify.newMode);
+		assertEquals(m.changeType, modify.changeType);
+		assertEquals(0, modify.score);
+	}
+
 	public void testSetRenameScore_IllegalArgs() throws Exception {
 		try {
 			rd.setRenameScore(-1);
@@ -462,4 +572,15 @@ public class RenameDetectorTest extends RepositoryTestCase {
 
 		assertEquals(score, copy.getScore());
 	}
+
+	private static void assertAdd(String newName, ObjectId newId,
+			FileMode newMode, DiffEntry add) {
+		assertEquals(DiffEntry.DEV_NULL, add.oldName);
+		assertEquals(DiffEntry.A_ZERO, add.oldId);
+		assertEquals(FileMode.MISSING, add.oldMode);
+		assertEquals(ChangeType.ADD, add.changeType);
+		assertEquals(newName, add.newName);
+		assertEquals(AbbreviatedObjectId.fromObjectId(newId), add.newId);
+		assertEquals(newMode, add.newMode);
+	}
 }