diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/HtmlParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/HtmlParser.java index f282c2d70d..a190a45fbd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/HtmlParser.java +++ b/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 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); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailComment.java index 8afbe81c17..f7804b3357 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailComment.java +++ b/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); + } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/HtmlParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/HtmlParserTest.java index 62bc580974..6c60db74d7 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/HtmlParserTest.java +++ b/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.

Now this is a new paragraph yay.
"; + 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 comments = defaultComments(); + List 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. *