opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15'

Browse Source 
* stable-2.15:
  Fix infinite loop in IntraLineLoader

Change-Id: I1632c94b557de4987fe1edf2aba672568c1bc2e2
This commit is contained in:
David Pursehouse
2018-04-13 20:59:46 +09:00
parent fc628f4f75 54dfdec960
commit 2f90615543
5 changed files with 123 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
java/com/google/gerrit/extensions/common/testing/ContentEntrySubject.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.extensions.common.testing;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.Truth;
@@ -68,4 +69,16 @@ public class ContentEntrySubject extends Subject<ContentEntrySubject, ContentEnt
ContentEntry contentEntry = actual();
return ListSubject.assertThat(contentEntry.b, Truth::assertThat).named("lines of 'b'");
}
public IterableSubject intralineEditsOfA() {
isNotNull();
ContentEntry contentEntry = actual();
return Truth.assertThat(contentEntry.editA).named("intraline edits of 'a'");
}
public IterableSubject intralineEditsOfB() {
isNotNull();
ContentEntry contentEntry = actual();
return Truth.assertThat(contentEntry.editB).named("intraline edits of 'b'");
}
}

2
java/com/google/gerrit/server/patch/IntraLineDiffKey.java
View File

@@ -21,7 +21,7 @@ import org.eclipse.jgit.lib.ObjectId;
@AutoValue
public abstract class IntraLineDiffKey implements Serializable {
public static final long serialVersionUID = 11L;
public static final long serialVersionUID = 12L;
public static IntraLineDiffKey create(ObjectId aId, ObjectId bId, Whitespace whitespace) {
return new AutoValue_IntraLineDiffKey(aId, bId, whitespace);

1
java/com/google/gerrit/server/patch/IntraLineLoader.java
View File

@@ -271,6 +271,7 @@ class IntraLineLoader implements Callable<IntraLineDiff> {
if (editsDueToRebase.contains(c) || editsDueToRebase.contains(n)) {
// Don't combine any edits which were identified as being introduced by a rebase as we would
// lose that information because of the combination.
j++;
continue;
}

2
java/com/google/gerrit/server/patch/PatchListKey.java
View File

@@ -32,7 +32,7 @@ import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
public class PatchListKey implements Serializable {
public static final long serialVersionUID = 30L;
public static final long serialVersionUID = 31L;
public static final ImmutableBiMap<Whitespace, Character> WHITESPACE_TYPES =
ImmutableBiMap.of(

107
javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
View File

@@ -23,6 +23,7 @@ import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toMap;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -82,6 +83,11 @@ public class RevisionDiffIT extends AbstractDaemonTest {
@Before
public void setUp() throws Exception {
// Reduce flakiness of tests. (If tests aren't fast enough, we would use a fall-back
// computation, which might yield different results.)
baseConfig.setString("cache", "diff", "timeout", "1 minute");
baseConfig.setString("cache", "diff_intraline", "timeout", "1 minute");
intraline = baseConfig.getBoolean(TEST_PARAMETER_MARKER, "intraline", false);
ObjectId headCommit = testRepo.getRepository().resolve("HEAD");
@@ -1253,6 +1259,66 @@ public class RevisionDiffIT extends AbstractDaemonTest {
assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(1);
}
@Test
public void intralineEditsInNonRebaseHunksAreIdentified() throws Exception {
assume().that(intraline).isTrue();
Function<String, String> contentModification =
fileContent -> fileContent.replace("Line 1\n", "Line one\n");
addModifiedPatchSet(changeId, FILE_NAME, contentModification);
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo)
.content()
.element(0)
.intralineEditsOfA()
.containsExactly(ImmutableList.of(5, 1));
assertThat(diffInfo)
.content()
.element(0)
.intralineEditsOfB()
.containsExactly(ImmutableList.of(5, 3));
assertThat(diffInfo).content().element(0).isNotDueToRebase();
assertThat(diffInfo).content().element(1).commonLines().hasSize(99);
}
@Test
public void intralineEditsInRebaseHunksAreIdentified() throws Exception {
assume().that(intraline).isTrue();
String newFileContent = FILE_CONTENT.replace("Line 1\n", "Line one\n");
ObjectId commit2 = addCommit(commit1, FILE_NAME, newFileContent);
rebaseChangeOn(changeId, commit2);
Function<String, String> contentModification =
fileContent -> fileContent.replace("Line 50\n", "Line fifty\n");
addModifiedPatchSet(changeId, FILE_NAME, contentModification);
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo)
.content()
.element(0)
.intralineEditsOfA()
.containsExactly(ImmutableList.of(5, 1));
assertThat(diffInfo)
.content()
.element(0)
.intralineEditsOfB()
.containsExactly(ImmutableList.of(5, 3));
assertThat(diffInfo).content().element(0).isDueToRebase();
assertThat(diffInfo).content().element(1).commonLines().hasSize(48);
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
assertThat(diffInfo).content().element(3).commonLines().hasSize(50);
}
@Test
public void closeNonRebaseHunksAreCombinedForIntralineOptimizations() throws Exception {
assume().that(intraline).isTrue();
@@ -1365,6 +1431,47 @@ public class RevisionDiffIT extends AbstractDaemonTest {
assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(1);
}
@Test
public void closeNonRebaseHunksNextToRebaseHunksAreCombinedForIntralineOptimizations()
throws Exception {
assume().that(intraline).isTrue();
String fileContent = FILE_CONTENT.replace("Line 5\n", "{\n").replace("Line 7\n", "{\n");
ObjectId commit2 = addCommit(commit1, FILE_NAME, fileContent);
rebaseChangeOn(changeId, commit2);
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
String newFileContent = fileContent.replace("Line 8\n", "Line eight!\n");
ObjectId commit3 = addCommit(commit1, FILE_NAME, newFileContent);
rebaseChangeOn(changeId, commit3);
addModifiedPatchSet(
changeId,
FILE_NAME,
content -> content.replace("Line 4\n", "Line four\n").replace("Line 6\n", "Line six\n"));
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
assertThat(diffInfo).content().element(0).commonLines().hasSize(3);
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4", "{", "Line 6");
assertThat(diffInfo)
.content()
.element(1)
.linesOfB()
.containsExactly("Line four", "{", "Line six");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
assertThat(diffInfo).content().element(2).commonLines().hasSize(1);
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 8");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line eight!");
assertThat(diffInfo).content().element(3).isDueToRebase();
assertThat(diffInfo).content().element(4).commonLines().hasSize(92);
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
assertThat(changedFiles.get(FILE_NAME)).linesInserted().isEqualTo(2);
assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(2);
}
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =