Merge branch 'stable-3.0' into stable-3.1
* stable-3.0:
Switch off intraline diffs for some cases (invalid cache key sharing)
Fix an internal server error when diffing patch sets (non-merge commit)
Set HTTP thread name to describe the request it is processing
Change-Id: I2ec9d44bc7a9dd372e074a4030a7dd6b6d2ab657
diff --git a/java/com/google/gerrit/httpd/SetThreadNameFilter.java b/java/com/google/gerrit/httpd/SetThreadNameFilter.java
new file mode 100644
index 0000000..c7d977d
--- /dev/null
+++ b/java/com/google/gerrit/httpd/SetThreadNameFilter.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import com.google.gerrit.server.CurrentUser;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.servlet.ServletModule;
+import java.io.IOException;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+
+@Singleton
+public class SetThreadNameFilter implements Filter {
+ private static final int MAX_PATH_LENGTH = 120;
+
+ public static Module module() {
+ return new ServletModule() {
+ @Override
+ protected void configureServlets() {
+ filter("/*").through(SetThreadNameFilter.class);
+ }
+ };
+ }
+
+ private final Provider<CurrentUser> user;
+
+ @Inject
+ public SetThreadNameFilter(Provider<CurrentUser> user) {
+ this.user = user;
+ }
+
+ @Override
+ public void init(FilterConfig filterConfig) {}
+
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+ Thread current = Thread.currentThread();
+ String old = current.getName();
+ try {
+ current.setName(computeName((HttpServletRequest) request));
+ chain.doFilter(request, response);
+ } finally {
+ current.setName(old);
+ }
+ }
+
+ private String computeName(HttpServletRequest req) {
+ StringBuilder s = new StringBuilder();
+ s.append("HTTP ");
+ s.append(req.getMethod());
+ s.append(" ");
+ s.append(req.getRequestURI());
+ String query = req.getQueryString();
+ if (query != null) {
+ s.append("?").append(query);
+ }
+ if (s.length() > MAX_PATH_LENGTH) {
+ s.delete(MAX_PATH_LENGTH, s.length());
+ }
+ s.append(" (");
+ s.append(user.get().getUserName().orElse("N/A"));
+ s.append(" from ");
+ s.append(req.getRemoteAddr());
+ s.append(")");
+ return s.toString();
+ }
+
+ @Override
+ public void destroy() {}
+}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index e6504e2..0befbd3 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -31,6 +31,7 @@
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
import com.google.gerrit.httpd.RequireSslFilter;
+import com.google.gerrit.httpd.SetThreadNameFilter;
import com.google.gerrit.httpd.WebModule;
import com.google.gerrit.httpd.WebSshGlueModule;
import com.google.gerrit.httpd.auth.oauth.OAuthModule;
@@ -383,6 +384,7 @@
modules.add(sysInjector.getInstance(GerritAuthModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
modules.add(RequestCleanupFilter.module());
+ modules.add(SetThreadNameFilter.module());
modules.add(AllRequestFilter.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 88ccf18..2457b6a 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -35,6 +35,7 @@
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
import com.google.gerrit.httpd.RequireSslFilter;
+import com.google.gerrit.httpd.SetThreadNameFilter;
import com.google.gerrit.httpd.WebModule;
import com.google.gerrit.httpd.WebSshGlueModule;
import com.google.gerrit.httpd.auth.oauth.OAuthModule;
@@ -547,6 +548,7 @@
}
modules.add(RequestCleanupFilter.module());
modules.add(AllRequestFilter.module());
+ modules.add(SetThreadNameFilter.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
modules.add(new HttpPluginModule());
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index f3c8eab..1c1c639 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -133,7 +133,7 @@
edits = new ArrayList<>(content.getEdits());
ImmutableSet<Edit> editsDueToRebase = content.getEditsDueToRebase();
- if (isModify(content) && diffPrefs.intralineDifference) {
+ if (isModify(content) && diffPrefs.intralineDifference && isIntralineModeAllowed(b)) {
IntraLineDiff d =
patchListCache.getIntraLineDiff(
IntraLineDiffKey.create(a.id, b.id, diffPrefs.ignoreWhitespace),
@@ -267,6 +267,16 @@
}
}
+ private static boolean isIntralineModeAllowed(Side side) {
+ // The intraline diff cache keys are the same for these cases. It's better to not show
+ // intraline results than showing completely wrong diffs or to run into a server error.
+ return !Patch.isMagic(side.path) && !isSubmoduleCommit(side.mode);
+ }
+
+ private static boolean isSubmoduleCommit(FileMode mode) {
+ return mode.getObjectType() == Constants.OBJ_COMMIT;
+ }
+
private void correctForDifferencesInNewlineAtEnd() {
// a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
int aSize = a.src.size();
@@ -279,6 +289,12 @@
return;
}
+ if (edits.isEmpty() && (aSize != bSize)) {
+ // Only edits due to rebase were present. If we now added the edits for the newlines, the
+ // code which later assembles the file contents would fail.
+ return;
+ }
+
Optional<Edit> lastEdit = getLast(edits);
if (isNewlineAtEndDeleted()) {
Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 24ac209..d4a4c45 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
+import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.FileInfoSubject.assertThat;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
@@ -41,6 +42,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
@@ -401,6 +403,24 @@
}
@Test
+ public void diffBetweenPatchSetsOfMergeCommitCanBeRetrievedForCommitMessageAndMergeList()
+ throws Exception {
+ PushOneCommit.Result result = createMergeCommitChange("refs/for/master", "my_file.txt");
+ String changeId = result.getChangeId();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, "my_file.txt", content -> content.concat("Line I\nLine II\n"));
+
+ // Call both of them in succession to ensure that they don't share the same cache keys.
+ DiffInfo commitMessageDiffInfo =
+ getDiffRequest(changeId, CURRENT, COMMIT_MSG).withBase(previousPatchSetId).get();
+ DiffInfo mergeListDiffInfo =
+ getDiffRequest(changeId, CURRENT, MERGE_LIST).withBase(previousPatchSetId).get();
+
+ assertThat(commitMessageDiffInfo).content().hasSize(3);
+ assertThat(mergeListDiffInfo).content().hasSize(1);
+ }
+
+ @Test
public void diffOfUnmodifiedFileMarksAllLinesAsCommon() throws Exception {
String filePath = "a_new_file.txt";
String fileContent = "Line 1\nLine 2\nLine 3\n";
@@ -578,6 +598,109 @@
}
@Test
+ public void addedNewlineAtEndOfFileIsMarkedInDiffWhenOtherwiseOnlyEditsDueToRebaseExist()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70 modification
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(102);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void addedNewlineAtEndOfFileIsNotIdentifiedAsDueToRebaseEvenThoughItShould()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ // Add a comment so that file contents are not 'skipped'. To be able to add a comment, touch
+ // (= modify) the file in the change.
+ addModifiedPatchSet(
+ changeId, FILE_NAME, fileContent -> fileContent.replace("Line 2\n", "Line two\n"));
+ CommentInput comment = createCommentInput(3, 0, 4, 0, "Comment to not skip file content.");
+ addCommentTo(changeId, CURRENT, FILE_NAME, comment);
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfA().isNull();
+ assertThat(diffInfo).content().element(1).linesOfB().containsExactly("");
+ // This should actually be isDueToRebase().
+ assertThat(diffInfo).content().element(1).isNotDueToRebase();
+ }
+
+ @Test
+ public void
+ addedNewlineAtEndOfFileIsMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceConsidered()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70.5 insertion
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(103);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void
+ addedNewlineAtEndOfFileIsNotMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceIgnoredEvenThoughItShould()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
+ }
+
+ @Test
public void addedLastLineWithoutNewlineBeforeAndAfterwardsIsMarkedInDiff() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
@@ -2374,9 +2497,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2395,6 +2516,112 @@
}
@Test
+ public void
+ diffOfFileWithOnlyRebaseHunksAndWithCommentAndConsideringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void diffOfFileWithOnlyRebaseHunksAndWithCommentAndIgnoringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\nLine 102\nLine 103\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.concat("Line 101\nLine 103\nLine 104");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
public void diffOfNonExistentFileIsAnEmptyDiffResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
@@ -2432,9 +2659,7 @@
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(20, 0, 21, 0, "Should be 'Line 20'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2476,9 +2701,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
String newFilePath = "a_new_file.txt";
gApi.changes().id(changeId).edit().renameFile(FILE_NAME, newFilePath);
gApi.changes().id(changeId).edit().publish();
@@ -2507,6 +2730,14 @@
return comment;
}
+ private void addCommentTo(
+ String changeId, String previousPatchSetId, String fileName, CommentInput comment)
+ throws RestApiException {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.comments = ImmutableMap.of(fileName, ImmutableList.of(comment));
+ gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ }
+
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =
