Merge "Squash email comments to account for multi-block comments"

gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/HtmlParser.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.mail.receive;
 
 import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.PeekingIterator;
 import com.google.gerrit.reviewdb.client.Comment;
@@ -99,21 +100,46 @@ public class HtmlParser {
             content = ParserUtil.trimQuotation(content);
             // TODO(hiesel) Add more sanitizer
             if (!Strings.isNullOrEmpty(content)) {
-              parsedComments.add(
-                  new MailComment(content, null, null, MailComment.CommentType.CHANGE_MESSAGE));
+              appendOrAddNewComment(
+                  new MailComment(content, null, null, MailComment.CommentType.CHANGE_MESSAGE),
+                  parsedComments);
             }
           } else if (lastEncounteredComment == null) {
-            parsedComments.add(
+            appendOrAddNewComment(
                 new MailComment(
-                    content, lastEncounteredFileName, null, MailComment.CommentType.FILE_COMMENT));
+                    content, lastEncounteredFileName, null, MailComment.CommentType.FILE_COMMENT),
+                parsedComments);
           } else {
-            parsedComments.add(
+            appendOrAddNewComment(
                 new MailComment(
-                    content, null, lastEncounteredComment, MailComment.CommentType.INLINE_COMMENT));
+                    content, null, lastEncounteredComment, MailComment.CommentType.INLINE_COMMENT),
+                parsedComments);
           }
         }
       }
     }
     return parsedComments;
   }
+
+  /**
+   * When parsing HTML content, we need to append comments prematurely since we are parsing
+   * block-by-block and never know what comes next. This can result in a comment being parsed as two
+   * comments when it spans multiple blocks. This method takes care of merging those blocks or
+   * adding a new comment to the list of appropriate.
+   */
+  private static void appendOrAddNewComment(MailComment comment, List<MailComment> comments) {
+    if (comments.isEmpty()) {
+      comments.add(comment);
+      return;
+    }
+    MailComment lastComment = Iterables.getLast(comments);
+
+    if (comment.isSameCommentPath(lastComment)) {
+      // Merge the two comments
+      lastComment.message += "\n\n" + comment.message;
+      return;
+    }
+
+    comments.add(comment);
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailComment.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.mail.receive;
 
 import com.google.gerrit.reviewdb.client.Comment;
+import java.util.Objects;
 
 /** A comment parsed from inbound email */
 public class MailComment {
@@ -37,4 +38,14 @@ public class MailComment {
     this.inReplyTo = inReplyTo;
     this.type = type;
   }
+
+  /**
+   * Checks if the provided comment concerns the same exact spot in the change. This is basically an
+   * equals method except that the message is not checked.
+   */
+  public boolean isSameCommentPath(MailComment c) {
+    return Objects.equals(fileName, c.fileName)
+        && Objects.equals(inReplyTo, c.inReplyTo)
+        && Objects.equals(type, c.type);
+  }
 }

gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/HtmlParserTest.java

@@ -105,6 +105,23 @@ public abstract class HtmlParserTest extends AbstractParserTest {
     assertInlineComment("Also have a comment here.", parsedComments.get(1), comments.get(3));
   }
 
+  @Test
+  public void commentsSpanningMultipleBlocks() {
+    String htmlMessage =
+        "This is a very long test comment. <div><br></div><div>Now this is a new paragraph yay.</div>";
+    String txtMessage = "This is a very long test comment.\n\nNow this is a new paragraph yay.";
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody(htmlMessage, null, null, htmlMessage, htmlMessage, null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
+
+    assertThat(parsedComments).hasSize(3);
+    assertChangeMessage(txtMessage, parsedComments.get(0));
+    assertFileComment(txtMessage, parsedComments.get(1), comments.get(1).key.filename);
+    assertInlineComment(txtMessage, parsedComments.get(2), comments.get(3));
+  }
+
   /**
    * Create an html message body with the specified comments.
    *