Merge "Skip empty lines in emails when using TextParser"
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 4803d83..5bd8d46 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -271,12 +271,14 @@
** [[delete]]`Delete Change` / `Delete Revision`:
+
-Deletes the draft change / the currently viewed draft patch set.
+Deletes the change / the currently viewed draft patch set.
+
-The `Delete Change` / `Delete Revision` buttons are only available if a
-draft patch set is viewed and the user is the change owner or has the
-link:access-control.html#category_delete_drafts[Delete Drafts] access
-right assigned.
+For normal changes, the `Delete Change` button will only be available if the
+user is an administrator and the change hasn't been merged. For draft changes,
+the `Delete Change` / `Delete Revision` buttons will be available if the user
+is the change owner or has the
+link:access-control.html#category_delete_drafts[Delete Drafts] access right
+assigned.
** [[plugin-actions]]Further actions may be available if plugins are installed.
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 9e04f89..163d414 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
@@ -96,7 +96,7 @@
if (lastEncounteredComment == null && lastEncounteredFileName == null) {
// Remove quotation line, email signature and
// "Sent from my xyz device"
- content = ParserUtil.trimQuotationLine(content);
+ content = ParserUtil.trimQuotation(content);
// TODO(hiesel) Add more sanitizer
if (!Strings.isNullOrEmpty(content)) {
parsedComments.add(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ParserUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ParserUtil.java
index 72eb18a..bfead94 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ParserUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ParserUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.mail.receive;
import com.google.gerrit.reviewdb.client.Comment;
+import java.util.StringJoiner;
import java.util.regex.Pattern;
public class ParserUtil {
@@ -24,39 +25,44 @@
+ "(\\.[A-Za-z0-9]+)*(\\.[A-Za-z]{2,})");
/**
- * Trims the quotation line that email clients add Example: On Sun, Nov 20, 2016 at 10:33 PM,
- * <gerrit@hiesel.it> wrote:
+ * Trims the quotation that email clients add Example: On Sun, Nov 20, 2016 at 10:33 PM,
+ * <gerrit@gerritcodereview.com> wrote:
*
* @param comment Comment parsed from an email.
* @return Trimmed comment.
*/
- public static String trimQuotationLine(String comment) {
- // Identifying the quotation line is hard, as it can be in any language.
- // We identify this line by it's characteristics: It usually contains a
- // valid email address, some digits for the date in groups of 1-4 in a row
- // as well as some characters.
- StringBuilder b = new StringBuilder();
- for (String line : comment.split("\n")) {
- // Count occurrences of digit groups
- int numConsecutiveDigits = 0;
- int maxConsecutiveDigits = 0;
- int numDigitGroups = 0;
- for (char c : line.toCharArray()) {
- if (c >= '0' && c <= '9') {
- numConsecutiveDigits++;
- } else if (numConsecutiveDigits > 0) {
- maxConsecutiveDigits = Integer.max(maxConsecutiveDigits, numConsecutiveDigits);
- numConsecutiveDigits = 0;
- numDigitGroups++;
- }
+ public static String trimQuotation(String comment) {
+ StringJoiner j = new StringJoiner("\n");
+ String[] lines = comment.split("\n");
+ for (int i = 0; i < lines.length - 2; i++) {
+ j.add(lines[i]);
+ }
+
+ // Check if the last line contains the full quotation pattern (date + email)
+ String lastLine = lines[lines.length - 1];
+ if (containsQuotationPattern(lastLine)) {
+ if (lines.length > 1) {
+ j.add(lines[lines.length - 2]);
}
- if (numDigitGroups < 4
- || maxConsecutiveDigits > 4
- || !SIMPLE_EMAIL_PATTERN.matcher(line).find()) {
- b.append(line);
+ return j.toString().trim();
+ }
+
+ // Check if the second last line + the last line contain the full quotation pattern. This is
+ // necessary, as the quotation line can be split across the last two lines if it gets too long.
+ if (lines.length > 1) {
+ String lastLines = lines[lines.length - 2] + lastLine;
+ if (containsQuotationPattern(lastLines)) {
+ return j.toString().trim();
}
}
- return b.toString().trim();
+
+ // Add the last two lines
+ if (lines.length > 1) {
+ j.add(lines[lines.length - 2]);
+ }
+ j.add(lines[lines.length - 1]);
+
+ return j.toString().trim();
}
/** Check if string is an inline comment url on a patch set or the base */
@@ -69,4 +75,31 @@
public static String filePath(String changeUrl, Comment comment) {
return changeUrl + "/" + comment.key.patchSetId + "/" + comment.key.filename;
}
+
+ private static boolean containsQuotationPattern(String s) {
+ // Identifying the quotation line is hard, as it can be in any language.
+ // We identify this line by it's characteristics: It usually contains a
+ // valid email address, some digits for the date in groups of 1-4 in a row
+ // as well as some characters.
+
+ // Count occurrences of digit groups
+ int numConsecutiveDigits = 0;
+ int maxConsecutiveDigits = 0;
+ int numDigitGroups = 0;
+ for (char c : s.toCharArray()) {
+ if (c >= '0' && c <= '9') {
+ numConsecutiveDigits++;
+ } else if (numConsecutiveDigits > 0) {
+ maxConsecutiveDigits = Integer.max(maxConsecutiveDigits, numConsecutiveDigits);
+ numConsecutiveDigits = 0;
+ numDigitGroups++;
+ }
+ }
+ if (numDigitGroups < 4 || maxConsecutiveDigits > 4) {
+ return false;
+ }
+
+ // Check if the string contains an email address
+ return SIMPLE_EMAIL_PATTERN.matcher(s).find();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/TextParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/TextParser.java
index 3164645..fa33cc6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/TextParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/TextParser.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail.receive;
+import com.google.common.base.Strings;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
import com.google.gerrit.reviewdb.client.Comment;
@@ -73,7 +74,12 @@
// This is not a comment, try to advance the file/comment pointers and
// add previous comment to list if applicable
if (currentComment != null) {
- parsedComments.add(currentComment);
+ if (currentComment.type == MailComment.CommentType.CHANGE_MESSAGE) {
+ currentComment.message = ParserUtil.trimQuotation(currentComment.message);
+ }
+ if (!Strings.isNullOrEmpty(currentComment.message)) {
+ parsedComments.add(currentComment);
+ }
currentComment = null;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/ParserUtilTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/ParserUtilTest.java
new file mode 100644
index 0000000..dfa492c
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/ParserUtilTest.java
@@ -0,0 +1,58 @@
+// 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 org.junit.Test;
+
+public class ParserUtilTest {
+ @Test
+ public void trimQuotationLineOnMessageWithoutQuoatationLine() throws Exception {
+ assertThat(ParserUtil.trimQuotation("One line")).isEqualTo("One line");
+ assertThat(ParserUtil.trimQuotation("Two\nlines")).isEqualTo("Two\nlines");
+ assertThat(ParserUtil.trimQuotation("Thr\nee\nlines")).isEqualTo("Thr\nee\nlines");
+ }
+
+ @Test
+ public void trimQuotationLineOnMixedMessages() throws Exception {
+ assertThat(
+ ParserUtil.trimQuotation(
+ "One line\n"
+ + "On Thu, Feb 9, 2017 at 8:21 AM, ekempin (Gerrit)\n"
+ + "<noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com> wrote:"))
+ .isEqualTo("One line");
+ assertThat(
+ ParserUtil.trimQuotation(
+ "One line\n"
+ + "On Thu, Feb 9, 2017 at 8:21 AM, ekempin (Gerrit) "
+ + "<noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com> wrote:"))
+ .isEqualTo("One line");
+ }
+
+ @Test
+ public void trimQuotationLineOnMessagesContainingQuoationLine() throws Exception {
+ assertThat(
+ ParserUtil.trimQuotation(
+ "On Thu, Feb 9, 2017 at 8:21 AM, ekempin (Gerrit)\n"
+ + "<noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com> wrote:"))
+ .isEqualTo("");
+ assertThat(
+ ParserUtil.trimQuotation(
+ "On Thu, Feb 9, 2017 at 8:21 AM, ekempin (Gerrit) "
+ + "<noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com> wrote:"))
+ .isEqualTo("");
+ }
+}
diff --git 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
index 4033897..a98835b 100644
--- 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
@@ -180,6 +180,8 @@
private static String newPlaintextBody(
String changeMessage, String c1, String c2, String c3, String f1, String f2, String fc1) {
return (changeMessage == null ? "" : changeMessage + "\n")
+ + "On Thu, Feb 9, 2017 at 8:21 AM, ekempin (Gerrit)\n"
+ + "<noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com> wrote: \n"
+ "> Foo Bar has posted comments on this change. ( \n"
+ "> "
+ changeURL
