Squash email comments to account for multi-block comments
When sending HTML emails to Gerrit, comments can span multiple HTML
blocks. In this case, we want to squash those comments and separate them
by two newlines to make a new paragraph.
Bug: Issue 6541
Change-Id: Ic3801a830aecefc4c374280cfa34e1e751c2557d
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 f282c2d..a190a45f 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 @@
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);
+ }
}
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 8afbe81..f7804b33 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 @@
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 62bc580..6c60db7 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 @@
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.
*
