Merge "Return empty context if the comment range is outside file boundaries"
diff --git a/java/com/google/gerrit/server/comment/CommentContextLoader.java b/java/com/google/gerrit/server/comment/CommentContextLoader.java
index 63bb8d0..c93f4b1 100644
--- a/java/com/google/gerrit/server/comment/CommentContextLoader.java
+++ b/java/com/google/gerrit/server/comment/CommentContextLoader.java
@@ -26,7 +26,6 @@
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentContext;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.common.ContextLineInfo;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.ComparisonType;
@@ -158,12 +157,10 @@
private static CommentContext createContext(Text src, Range commentRange, int contextPadding) {
if (commentRange.start() < 1 || commentRange.end() - 1 > src.size()) {
- throw new StorageException(
- "Invalid comment range "
- + commentRange
- + ". Text only contains "
- + src.size()
- + " lines.");
+ // TODO(ghareeb): We should throw an exception in this case. See
+ // https://bugs.chromium.org/p/gerrit/issues/detail?id=14102 which is an example where the
+ // diff contains an extra line not in the original file.
+ return CommentContext.empty();
}
commentRange = adjustRange(commentRange, contextPadding, src.size());
ImmutableMap.Builder<Integer, String> context =
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentContextIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentContextIT.java
index adfe56d..548e3fe 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentContextIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentContextIT.java
@@ -19,11 +19,9 @@
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
-import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.MoreCollectors;
-import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -148,12 +146,10 @@
CommentsUtil.newComment(COMMIT_MSG, Side.REVISION, 100, "comment", false);
CommentsUtil.addComments(gApi, changeId, ps1, comment);
- Throwable thrown =
- assertThrows(
- UncheckedExecutionException.class,
- () -> gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList());
-
- assertThat(thrown).hasCauseThat().hasMessageThat().contains("Invalid comment range");
+ List<CommentInfo> comments =
+ gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList();
+ assertThat(comments).hasSize(1);
+ assertThat(comments.get(0).contextLines).isEmpty();
}
@Test
