opendev/gerrit
Code Issues Proposed changes

Fix exception due to duplicate keys in EditTransformer

Browse Source 
As copies are treated as additions in the current version of JGit,
having multiple PatchListEntries referring to the same file path
didn't seem possible. However, when a file is renamed alongside
those copies, JGit identifies the copies as such.

The logic which transforms the edits from the parents' trees to the
children's trees assumed that one edit was only mapped to a
transformed edit. With the existence of copies, we need to map one
edit to several ones instead.

All in all, we compute the cross product of all possible combinations.
When computing a real diff between two trees, not all of them would
be present at the same time. As it seems difficult to figure out
exactly which edits are 'necessary' to keep, we rather keep all of
them for the moment and accept that we possibly waste some resources.

Change-Id: I6d57fb6a49b8dea58aab9565ac41301c7365159f
This commit is contained in:
Alice Kober-Sotzek
2017-07-13 18:05:52 +02:00
parent a29041ce24
commit 264143d5b5
3 changed files with 76 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
View File

@@ -138,6 +138,20 @@ public class RevisionDiffIT extends AbstractDaemonTest {
assertThat(changedFiles.keySet()).containsExactly(COMMIT_MSG, newFilePath);
}
@Test
public void copiedFileTreatedAsAddedFileInDiff() throws Exception {
String copyFilePath = "copy_of_some_file.txt";
gApi.changes().id(changeId).edit().modifyFile(copyFilePath, RawInputUtil.create(FILE_CONTENT));
gApi.changes().id(changeId).edit().publish();
Map<String, FileInfo> changedFiles = gApi.changes().id(changeId).current().files();
assertThat(changedFiles.keySet()).containsExactly(COMMIT_MSG, copyFilePath);
// If this ever changes, please add tests which cover copied files.
assertThat(changedFiles.get(copyFilePath)).status().isEqualTo('A');
assertThat(changedFiles.get(copyFilePath)).linesInserted().isEqualTo(100);
assertThat(changedFiles.get(copyFilePath)).linesDeleted().isNull();
}
@Test
public void addedBinaryFileIsIncludedInDiff() throws Exception {
String imageFileName = "an_image.png";
@@ -1039,6 +1053,43 @@ public class RevisionDiffIT extends AbstractDaemonTest {
assertThat(diffInfo).content().element(2).commonLines().hasSize(95);
}
@Test
public void copiedAndRenamedFilesWithOnlyRebaseHunksAreIdentified() throws Exception {
String newFileContent = FILE_CONTENT.replace("Line 5\n", "Line five\n");
ObjectId commit2 = addCommit(commit1, FILE_NAME, newFileContent);
rebaseChangeOn(changeId, commit2);
// Copies are only identified by JGit when paired with renaming.
String copyFileName = "copy_of_some_file.txt";
String renamedFileName = "renamed_some_file.txt";
gApi.changes()
.id(changeId)
.edit()
.modifyFile(copyFileName, RawInputUtil.create(newFileContent));
gApi.changes().id(changeId).edit().renameFile(FILE_NAME, renamedFileName);
gApi.changes().id(changeId).edit().publish();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
assertThat(changedFiles.keySet()).containsExactly(COMMIT_MSG, copyFileName, renamedFileName);
DiffInfo renamedFileDiffInfo =
getDiffRequest(changeId, CURRENT, renamedFileName).withBase(initialPatchSetId).get();
assertThat(renamedFileDiffInfo).content().element(0).commonLines().hasSize(4);
assertThat(renamedFileDiffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(renamedFileDiffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(renamedFileDiffInfo).content().element(1).isDueToRebase();
assertThat(renamedFileDiffInfo).content().element(2).commonLines().hasSize(95);
DiffInfo copiedFileDiffInfo =
getDiffRequest(changeId, CURRENT, copyFileName).withBase(initialPatchSetId).get();
assertThat(copiedFileDiffInfo).content().element(0).commonLines().hasSize(4);
assertThat(copiedFileDiffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(copiedFileDiffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(copiedFileDiffInfo).content().element(1).isDueToRebase();
assertThat(copiedFileDiffInfo).content().element(2).commonLines().hasSize(95);
}
/*
* change PS B
* |