From f5055848090101b6863ce821a74de290a396c516 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 1 Jun 2011 08:33:14 -0700
Subject: [PATCH] Fix reply email parsing for linebreaks in "On <date>, <user>
 wrote:" quote identifier

Summary:
Move the parser to a separate class so it can be easily unit tested, add some
tests. Properly parse emails with linebreaks in the quote line.

Test Plan:
Ran unit tests, used mail receiver to reply to an object.

Reviewed By: cadamo
Reviewers: aran, jungejason, tuomaspelkonen, cadamo
CC: aran, cadamo, epriestley
Differential Revision: 392
---
 src/__phutil_library_map__.php                |  3 +
 .../PhabricatorMetaMTAEmailBodyParser.php     | 36 +++++++++++
 src/applications/metamta/parser/__init__.php  | 10 +++
 ...bricatorMetaMTAEmailBodyParserTestCase.php | 64 +++++++++++++++++++
 .../metamta/parser/__tests__/__init__.php     | 13 ++++
 .../PhabricatorMetaMTAReceivedMail.php        | 13 +---
 .../metamta/storage/receivedmail/__init__.php |  1 +
 7 files changed, 129 insertions(+), 11 deletions(-)
 create mode 100644 src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
 create mode 100644 src/applications/metamta/parser/__init__.php
 create mode 100644 src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
 create mode 100644 src/applications/metamta/parser/__tests__/__init__.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 81a9d42b03..0aca76d524 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -349,6 +349,8 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAController' => 'applications/metamta/controller/base',
     'PhabricatorMetaMTADAO' => 'applications/metamta/storage/base',
     'PhabricatorMetaMTADaemon' => 'applications/metamta/daemon/mta',
+    'PhabricatorMetaMTAEmailBodyParser' => 'applications/metamta/parser',
+    'PhabricatorMetaMTAEmailBodyParserTestCase' => 'applications/metamta/parser/__tests__',
     'PhabricatorMetaMTAListController' => 'applications/metamta/controller/list',
     'PhabricatorMetaMTAMail' => 'applications/metamta/storage/mail',
     'PhabricatorMetaMTAMailTestCase' => 'applications/metamta/storage/mail/__tests__',
@@ -790,6 +792,7 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAController' => 'PhabricatorController',
     'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO',
     'PhabricatorMetaMTADaemon' => 'PhabricatorDaemon',
+    'PhabricatorMetaMTAEmailBodyParserTestCase' => 'PhabricatorTestCase',
     'PhabricatorMetaMTAListController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAMail' => 'PhabricatorMetaMTADAO',
     'PhabricatorMetaMTAMailTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
new file mode 100644
index 0000000000..e101429070
--- /dev/null
+++ b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * 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.
+ */
+
+final class PhabricatorMetaMTAEmailBodyParser {
+
+  public function __construct($corpus) {
+    $this->corpus = $corpus;
+  }
+
+  public function stripQuotedText() {
+    $body = $this->corpus;
+
+    $body = preg_replace(
+      '/^\s*On\b.*\bwrote:.*?/msU',
+      '',
+      $body);
+
+    return rtrim($body);
+  }
+
+}
diff --git a/src/applications/metamta/parser/__init__.php b/src/applications/metamta/parser/__init__.php
new file mode 100644
index 0000000000..a088211fd8
--- /dev/null
+++ b/src/applications/metamta/parser/__init__.php
@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('PhabricatorMetaMTAEmailBodyParser.php');
diff --git a/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php b/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
new file mode 100644
index 0000000000..3b8bbce033
--- /dev/null
+++ b/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * 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.
+ */
+
+final class PhabricatorMetaMTAEmailBodyParserTestCase
+  extends PhabricatorTestCase {
+
+  public function testQuotedTextStripping() {
+    $bodies = $this->getEmailBodies();
+    foreach ($bodies as $body) {
+      $parser = new PhabricatorMetaMTAEmailBodyParser($body);
+      $stripped = $parser->stripQuotedText();
+      $this->assertEqual("OKAY", $stripped);
+    }
+  }
+
+  private function getEmailBodies() {
+    return array(
+<<<EOEMAIL
+OKAY
+
+On May 30, 2011, at 8:36 PM, Someone wrote:
+
+> ...
+
+EOEMAIL
+,
+<<<EOEMAIL
+OKAY
+
+On Fri, May 27, 2011 at 9:39 AM, Someone <
+somebody@somewhere.com> wrote:
+
+> ...
+
+EOEMAIL
+,
+<<<EOEMAIL
+OKAY
+
+On Fri, May 27, 2011 at 9:39 AM, Someone
+<somebody@somewhere.com> wrote:
+
+> ...
+
+EOEMAIL
+    );
+  }
+
+}
diff --git a/src/applications/metamta/parser/__tests__/__init__.php b/src/applications/metamta/parser/__tests__/__init__.php
new file mode 100644
index 0000000000..e2cbf69d4f
--- /dev/null
+++ b/src/applications/metamta/parser/__tests__/__init__.php
@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/metamta/parser');
+phutil_require_module('phabricator', 'infrastructure/testing/testcase');
+
+
+phutil_require_source('PhabricatorMetaMTAEmailBodyParserTestCase.php');
diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php
index 3896c6bb1a..cafaf174a6 100644
--- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php
+++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php
@@ -109,17 +109,8 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
   public function getCleanTextBody() {
     $body = idx($this->bodies, 'text');
 
-    // TODO: Refine this "algorithm".
-
-    $lines = explode("\n", trim($body));
-    for ($ii = 0; $ii < count($lines); $ii++) {
-      if (preg_match('/^\s*On\b.*\bwrote:\s*$/', $lines[$ii])) {
-        $lines = array_slice($lines, 0, $ii);
-        break;
-      }
-    }
-
-    return trim(implode("\n", $lines));
+    $parser = new PhabricatorMetaMTAEmailBodyParser($body);
+    return $parser->stripQuotedText();
   }
 
   public static function loadReceiverObject($receiver_name) {
diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php
index 72c98b9068..afd38aa144 100644
--- a/src/applications/metamta/storage/receivedmail/__init__.php
+++ b/src/applications/metamta/storage/receivedmail/__init__.php
@@ -8,6 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/differential/mail/base');
 phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
+phutil_require_module('phabricator', 'applications/metamta/parser');
 phutil_require_module('phabricator', 'applications/metamta/storage/base');
 phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/env');