Implement HTML Parser and Tests

This change implements an HTML parser that works for Gmail and most generic HTML clients and adds tests for the newly created parser. In future iterations, we will add tests for more email clients and adapt the parser if necessary. Change-Id: I0ae5f3d496d164e791a2dd0181fcb6299e98e427
2016-11-17 08:24:06 -08:00
parent 330a77a2c4
commit 0498fa1836
11 changed files with 537 additions and 27 deletions
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java
@@ -17,6 +17,8 @@ package com.google.gerrit.server.mail.receive;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Comment;
 import com.google.gerrit.server.mail.Address;
+import java.util.ArrayList;
+import java.util.List;
 import org.joda.time.DateTime;

 import java.sql.Timestamp;
@@ -26,6 +28,9 @@ import static com.google.common.truth.Truth.assertThat;

@Ignore
 public class AbstractParserTest {
+  protected static final String changeURL =
+      "https://gerrit-review.googlesource.com/#/changes/123";
+
  protected static void assertChangeMessage(String message,
      MailComment comment) {
    assertThat(comment.fileName).isNull();
@@ -67,4 +72,14 @@ public class AbstractParserTest {
    b.subject("");
    return b;
  }
+
+  /** Returns a List of default comments for testing. */
+  protected static List<Comment> defaultComments() {
+    List<Comment> comments = new ArrayList<>();
+    comments.add(newComment("c1", "gerrit-server/test.txt", "comment", 0));
+    comments.add(newComment("c2", "gerrit-server/test.txt", "comment", 2));
+    comments.add(newComment("c3", "gerrit-server/test.txt", "comment", 3));
+    comments.add(newComment("c4", "gerrit-server/readme.txt", "comment", 3));
+    return comments;
+  }
 }
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/GenericHtmlParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/GenericHtmlParserTest.java
@@ -0,0 +1,95 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+/** Test parser for a generic Html email client response */
+public class GenericHtmlParserTest extends HtmlParserTest {
+  @Override
+  protected String newHtmlBody(String changeMessage, String c1,
+      String c2, String c3, String f1, String f2, String fc1) {
+    String email = "" +
+        "<div dir=\"ltr\">" + (changeMessage != null ? changeMessage : "") +
+        "<div class=\"extra\"><br><div class=\"quote\">" +
+        "On Fri, Nov 18, 2016 at 11:15 AM, foobar (Gerrit) noreply@gerrit.com" +
+        "<span dir=\"ltr\">&lt;<a href=\"mailto:noreply@gerrit.com\" " +
+        "target=\"_blank\">noreply@gerrit.com</a>&gt;</span> wrote:<br>" +
+        "<blockquote class=\"quote\" " +
+        "<p>foobar <strong>posted comments</strong> on this change.</p>" +
+        "<p><a href=\"" + changeURL + "/1\" " +
+        "target=\"_blank\">View Change</a></p><div>Patch Set 2: CR-1\n" +
+        "\n" +
+        "(3 comments)</div><ul><li>" +
+        "<p>" + // File #1: test.txt
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt\">" +
+        "File gerrit-server/<wbr>test.txt:</a></p>" +
+        commentBlock(f1) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt\">" +
+        "Patch Set #2:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some comment on file 1</p>" +
+        "</li>" +
+        commentBlock(fc1) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@2\">" +
+        "Patch Set #2, Line 31:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some text from original comment</p>" +
+        "</li>" +
+        commentBlock(c1) +
+        "" + // Inline comment #2
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@3\">" +
+        "Patch Set #2, Line 47:</a> </p>" +
+        "<blockquote><pre>Some comment posted on Gerrit</pre>" +
+        "</blockquote><p>Some more comments from Gerrit</p>" +
+        "</li>" +
+        commentBlock(c2) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@115\">" +
+        "Patch Set #2, Line 115:</a> <code>some code</code></p>" +
+        "<p>some comment</p></li></ul></li>" +
+        "" +
+        "<li><p>" + // File #2: test.txt
+        "<a href=\"" + changeURL + "/1/gerrit-server/readme.txt\">" +
+        "File gerrit-server/<wbr>readme.txt:</a></p>" +
+        commentBlock(f2) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/readme.txt@3\">" +
+        "Patch Set #2, Line 31:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some text from original comment</p>" +
+        "</li>" +
+        commentBlock(c3) +
+        "" + // Inline comment #2
+        "</ul></li></ul>" +
+        "" + // Footer
+        "<p>To view, visit <a href=\"" + changeURL + "/1\">this change</a>. " +
+        "To unsubscribe, visit <a href=\"https://someurl\">settings</a>." +
+        "</p><p>Gerrit-MessageType: comment<br>" +
+        "Footer omitted</p>" +
+        "<div><div></div></div>" +
+        "<p>Gerrit-HasComments: Yes</p></blockquote></div><br></div></div>";
+    return email;
+  }
+
+  private static String commentBlock(String comment) {
+    if (comment == null) {
+      return "";
+    }
+    return "</ul></li></ul></blockquote><div>" + comment +
+        "</div><blockquote class=\"quote\"><ul><li><ul>";
+  }
+}
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/GmailHtmlParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/GmailHtmlParserTest.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+public class GmailHtmlParserTest extends HtmlParserTest {
+  @Override
+  protected String newHtmlBody(String changeMessage, String c1,
+      String c2, String c3, String f1, String f2, String fc1) {
+    String email = "" +
+        "<div dir=\"ltr\">" + (changeMessage != null ? changeMessage : "") +
+        "<div class=\"gmail_extra\"><br><div class=\"gmail_quote\">" +
+        "On Fri, Nov 18, 2016 at 11:15 AM, foobar (Gerrit) noreply@gerrit.com" +
+        "<span dir=\"ltr\">&lt;<a href=\"mailto:noreply@gerrit.com\" " +
+        "target=\"_blank\">noreply@gerrit.com</a>&gt;</span> wrote:<br>" +
+        "<blockquote class=\"gmail_quote\" " +
+        "<p>foobar <strong>posted comments</strong> on this change.</p>" +
+        "<p><a href=\"" + changeURL + "/1\" " +
+        "target=\"_blank\">View Change</a></p><div>Patch Set 2: CR-1\n" +
+        "\n" +
+        "(3 comments)</div><ul><li>" +
+        "<p>" + // File #1: test.txt
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt\">" +
+        "File gerrit-server/<wbr>test.txt:</a></p>" +
+        commentBlock(f1) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt\">" +
+        "Patch Set #2:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some comment on file 1</p>" +
+        "</li>" +
+        commentBlock(fc1) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@2\">" +
+        "Patch Set #2, Line 31:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some text from original comment</p>" +
+        "</li>" +
+        commentBlock(c1) +
+        "" + // Inline comment #2
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@3\">" +
+        "Patch Set #2, Line 47:</a> </p>" +
+        "<blockquote><pre>Some comment posted on Gerrit</pre>" +
+        "</blockquote><p>Some more comments from Gerrit</p>" +
+        "</li>" +
+        commentBlock(c2) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/test.txt@115\">" +
+        "Patch Set #2, Line 115:</a> <code>some code</code></p>" +
+        "<p>some comment</p></li></ul></li>" +
+        "" +
+        "<li><p>" + // File #2: test.txt
+        "<a href=\"" + changeURL + "/1/gerrit-server/readme.txt\">" +
+        "File gerrit-server/<wbr>readme.txt:</a></p>" +
+        commentBlock(f2) +
+        "<li><p>" +
+        "<a href=\"" + changeURL + "/1/gerrit-server/readme.txt@3\">" +
+        "Patch Set #2, Line 31:</a> </p>" +
+        "<blockquote><pre>Some inline comment from Gerrit</pre>" +
+        "</blockquote><p>Some text from original comment</p>" +
+        "</li>" +
+        commentBlock(c3) +
+        "" + // Inline comment #2
+        "</ul></li></ul>" +
+        "" + // Footer
+        "<p>To view, visit <a href=\"" + changeURL + "/1\">this change</a>. " +
+        "To unsubscribe, visit <a href=\"https://someurl\">settings</a>." +
+        "</p><p>Gerrit-MessageType: comment<br>" +
+        "Footer omitted</p>" +
+        "<div><div></div></div>" +
+        "<p>Gerrit-HasComments: Yes</p></blockquote></div><br></div></div>";
+    return email;
+  }
+
+  private static String commentBlock(String comment) {
+    if (comment == null) {
+      return "";
+    }
+    return "</ul></li></ul></blockquote><div>" + comment +
+        "</div><blockquote class=\"gmail_quote\"><ul><li><ul>";
+  }
+}
--- 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
@@ -0,0 +1,121 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.reviewdb.client.Comment;
+import java.util.List;
+import org.junit.Ignore;
+import org.junit.Test;
+
+@Ignore
+public abstract class HtmlParserTest extends AbstractParserTest {
+  @Test
+  public void simpleChangeMessage() {
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody("Looks good to me", null, null,
+        null, null, null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments =
+        HtmlParser.parse(b.build(), comments, "");
+
+    assertThat(parsedComments).hasSize(1);
+    assertChangeMessage("Looks good to me", parsedComments.get(0));
+  }
+
+  @Test
+  public void simpleInlineComments() {
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody("Looks good to me",
+        "I have a comment on this.", null, "Also have a comment here.",
+        null, null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments =
+        HtmlParser.parse(b.build(), comments, changeURL);
+
+    assertThat(parsedComments).hasSize(3);
+    assertChangeMessage("Looks good to me", parsedComments.get(0));
+    assertInlineComment("I have a comment on this.", parsedComments.get(1),
+        comments.get(1));
+    assertInlineComment("Also have a comment here.", parsedComments.get(2),
+        comments.get(3));
+  }
+
+  @Test
+  public void simpleFileComment() {
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody("Looks good to me",
+        null, null, "Also have a comment here.",
+        "This is a nice file", null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments =
+        HtmlParser.parse(b.build(), comments, changeURL);
+
+    assertThat(parsedComments).hasSize(3);
+    assertChangeMessage("Looks good to me", parsedComments.get(0));
+    assertFileComment("This is a nice file", parsedComments.get(1),
+        comments.get(1).key.filename);
+    assertInlineComment("Also have a comment here.", parsedComments.get(2),
+        comments.get(3));
+  }
+
+  @Test
+  public void noComments() {
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody(null, null, null, null, null, null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments =
+        HtmlParser.parse(b.build(), comments, changeURL);
+
+    assertThat(parsedComments).isEmpty();
+  }
+
+  @Test
+  public void noChangeMessage() {
+    MailMessage.Builder b = newMailMessageBuilder();
+    b.htmlContent(newHtmlBody(null, null, null,
+        "Also have a comment here.", "This is a nice file", null, null));
+
+    List<Comment> comments = defaultComments();
+    List<MailComment> parsedComments =
+        HtmlParser.parse(b.build(), comments, changeURL);
+
+    assertThat(parsedComments).hasSize(2);
+    assertFileComment("This is a nice file", parsedComments.get(0),
+        comments.get(1).key.filename);
+    assertInlineComment("Also have a comment here.", parsedComments.get(1),
+        comments.get(3));
+  }
+
+  /**
+   * Create an html message body with the specified comments.
+   *
+   * @param changeMessage
+   * @param c1 Comment in reply to first comment.
+   * @param c2 Comment in reply to second comment.
+   * @param c3 Comment in reply to third comment.
+   * @param f1 Comment on file one.
+   * @param f2 Comment on file two.
+   * @param fc1 Comment in reply to a comment on file 1.
+   * @return A string with all inline comments and the original quoted email.
+   */
+  protected abstract String newHtmlBody(String changeMessage, String c1,
+      String c2, String c3, String f1, String f2, String fc1);
+}
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/TextParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/TextParserTest.java
@@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat;
 import com.google.gerrit.reviewdb.client.Comment;
 import org.junit.Test;

-import java.util.ArrayList;
 import java.util.List;

 public class TextParserTest extends AbstractParserTest {
@@ -34,8 +33,6 @@ public class TextParserTest extends AbstractParserTest {
      "> Gerrit-Branch: master\n" +
      "> Gerrit-Owner: Foo Bar <foo@bar.com>\n" +
      "> Gerrit-HasComments: Yes";
-  private static final String changeURL =
-      "https://gerrit-review.googlesource.com/#/changes/123";

  @Test
  public void simpleChangeMessage() {
@@ -218,13 +215,4 @@ public class TextParserTest extends AbstractParserTest {
        "> Should this be EEE like in other places?\n" +
        (c3 == null ? "" : c3 + "\n");
  }
-
-  private List<Comment> defaultComments() {
-    List<Comment> comments = new ArrayList<>();
-    comments.add(newComment("c1", "gerrit-server/test.txt", "comment", 0));
-    comments.add(newComment("c2", "gerrit-server/test.txt", "comment", 2));
-    comments.add(newComment("c3", "gerrit-server/test.txt", "comment", 3));
-    comments.add(newComment("c4", "gerrit-server/readme.txt", "comment", 3));
-    return comments;
-  }
 }