Merge "Return empty context if the comment range is outside file boundaries"

java/com/google/gerrit/server/comment/CommentContextLoader.java

@@ -26,7 +26,6 @@ import com.google.gerrit.common.Nullable;
 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 @@ public class CommentContextLoader {
 
   private static CommentContext createContext(Text src, Range commentRange, int contextPadding) {
     if (commentRange.start() < 1 || commentRange.end() - 1 > src.size()) {
-      throw new StorageException(
+      // TODO(ghareeb): We should throw an exception in this case. See
-          "Invalid comment range "
+      // https://bugs.chromium.org/p/gerrit/issues/detail?id=14102 which is an example where the
-              + commentRange
+      // diff contains an extra line not in the original file.
-              + ". Text only contains "
+      return CommentContext.empty();
-              + src.size()
-              + " lines.");
     }
     commentRange = adjustRange(commentRange, contextPadding, src.size());
     ImmutableMap.Builder<Integer, String> context =

javatests/com/google/gerrit/acceptance/server/change/CommentContextIT.java

@@ -19,11 +19,9 @@ import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 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 @@ public class CommentContextIT extends AbstractDaemonTest {
         CommentsUtil.newComment(COMMIT_MSG, Side.REVISION, 100, "comment", false);
     CommentsUtil.addComments(gApi, changeId, ps1, comment);
 
-    Throwable thrown =
+    List<CommentInfo> comments =
-        assertThrows(
+        gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList();
-            UncheckedExecutionException.class,
+    assertThat(comments).hasSize(1);
-            () -> gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList());
+    assertThat(comments.get(0).contextLines).isEmpty();
-
-    assertThat(thrown).hasCauseThat().hasMessageThat().contains("Invalid comment range");
   }
 
   @Test