From cdc5c4501ba1c6d6facbe4ef5d69de5600e7a344 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 15 Feb 2017 14:57:01 +1300 Subject: [PATCH] CRM-20016 preliminary work - add basis for unit testing and failing unit test. In writing the test I identified the transation was not rolling back due to the api not following our crud format. I fixed it to 'fail' because we should not be passing despite mysql errors & ... we should fix the errors... The BAO code in fact holds transaction handling to ensure that a fail causes a fail - but we were bypassing it without passing in the 'is_transactional' param. --- CRM/Mailing/Event/BAO/Bounce.php | 4 +- CRM/Utils/Mail/EmailProcessor.php | 16 ++- .../CRM/Utils/Mail/EmailProcessorTest.php | 107 ++++++++++++++++++ .../Mail/data/bounces/bounce_no_verp.txt | 72 ++++++++++++ tests/phpunit/CiviTest/CiviUnitTestCase.php | 11 +- 5 files changed, 200 insertions(+), 10 deletions(-) create mode 100644 tests/phpunit/CRM/Utils/Mail/EmailProcessorTest.php create mode 100644 tests/phpunit/CRM/Utils/Mail/data/bounces/bounce_no_verp.txt diff --git a/CRM/Mailing/Event/BAO/Bounce.php b/CRM/Mailing/Event/BAO/Bounce.php index 612b1783af..8154cd8326 100644 --- a/CRM/Mailing/Event/BAO/Bounce.php +++ b/CRM/Mailing/Event/BAO/Bounce.php @@ -46,8 +46,8 @@ class CRM_Mailing_Event_BAO_Bounce extends CRM_Mailing_Event_DAO_Bounce { * * @return bool|null */ - public static function &create(&$params) { - $q = &CRM_Mailing_Event_BAO_Queue::verify($params['job_id'], + public static function create(&$params) { + $q = CRM_Mailing_Event_BAO_Queue::verify($params['job_id'], $params['event_queue_id'], $params['hash'] ); diff --git a/CRM/Utils/Mail/EmailProcessor.php b/CRM/Utils/Mail/EmailProcessor.php index a995b7af8f..336c820352 100644 --- a/CRM/Utils/Mail/EmailProcessor.php +++ b/CRM/Utils/Mail/EmailProcessor.php @@ -137,10 +137,10 @@ class CRM_Utils_Mail_EmailProcessor { $emailActivityTypeId = (defined('EMAIL_ACTIVITY_TYPE_ID') && EMAIL_ACTIVITY_TYPE_ID) ? EMAIL_ACTIVITY_TYPE_ID : CRM_Core_OptionGroup::getValue( - 'activity_type', - 'Inbound Email', - 'name' - ); + 'activity_type', + 'Inbound Email', + 'name' + ); if (!$emailActivityTypeId) { CRM_Core_Error::fatal(ts('Could not find a valid Activity Type ID for Inbound Email')); @@ -376,6 +376,14 @@ class CRM_Utils_Mail_EmailProcessor { 'hash' => $hash, 'body' => $text, 'version' => 3, + // Setting is_transactional means it will rollback if + // it crashes part way through creating the bounce. + // If the api were standard & had a create this would be the + // default. Adding the standard api & deprecating this one + // would probably be the + // most consistent way to address this - but this is + // a quick hack. + 'is_transactional' => 1, ); $result = civicrm_api('Mailing', 'event_bounce', $params); break; diff --git a/tests/phpunit/CRM/Utils/Mail/EmailProcessorTest.php b/tests/phpunit/CRM/Utils/Mail/EmailProcessorTest.php new file mode 100644 index 0000000000..45208024b1 --- /dev/null +++ b/tests/phpunit/CRM/Utils/Mail/EmailProcessorTest.php @@ -0,0 +1,107 @@ +callAPISuccess('MailSettings', 'get', array( + 'api.MailSettings.create' => array( + 'name' => 'local', + 'protocol' => 'Localdir', + 'source' => __DIR__ . '/data/mail', + 'domain' => 'example.com', + ), + )); + } + + /** + * Test the job processing function works and processes a bounce. + */ + public function testBounceProcessing() { + $this->setUpMailing(); + + copy(__DIR__ . '/data/bounces/bounce_no_verp.txt', __DIR__ . '/data/mail/bounce_no_verp.txt'); + $this->assertTrue(file_exists(__DIR__ . '/data/mail/bounce_no_verp.txt')); + $this->callAPISuccess('job', 'fetch_bounces', array()); + $this->assertFalse(file_exists(__DIR__ . '/data/mail/bounce_no_verp.txt')); + $this->checkMailingBounces(1); + } + + /** + * Test that a deleted email does not cause a hard fail. + * + * The civicrm_mailing_event_queue table tracks email ids to represent an + * email address. The id may not represent the same email by the time the bounce may + * come in - a weakness of storing the id not the email. Relevant here + * is that it might have been deleted altogether, in which case the bounce should be + * silently ignored. This ignoring is also at the expense of the contact + * having the same email address with a different id. + * + * Longer term it would make sense to track the email address & track bounces back to that + * rather than an id that may not reflect the email used. Issue logged CRM-20021. + * + * For not however, we are testing absence of mysql error in conjunction with CRM-20016. + */ + public function testBounceProcessingDeletedEmail() { + $this->setUpMailing(); + $this->callAPISuccess('Email', 'get', array( + 'contact_id' => $this->contactID, + 'api.email.delete' => 1, + )); + + copy(__DIR__ . '/data/bounces/bounce_no_verp.txt', __DIR__ . '/data/mail/bounce_no_verp.txt'); + $this->assertTrue(file_exists(__DIR__ . '/data/mail/bounce_no_verp.txt')); + $this->callAPISuccess('job', 'fetch_bounces', array()); + $this->assertFalse(file_exists(__DIR__ . '/data/mail/bounce_no_verp.txt')); + $this->checkMailingBounces(1); + } + + /** + * Wrapper to check for mailing bounces. + * + * Normally we would call $this->callAPISuccessGetCount but there is not one & there is resistance to + * adding apis for 'convenience' so just adding a hacky function to get past the impasse. + * + * @param int $expectedCount + */ + public function checkMailingBounces($expectedCount) { + $this->assertEquals($expectedCount, CRM_Core_DAO::singleValueQuery( + "SELECT count(*) FROM civicrm_mailing_event_bounce WHERE event_queue_id = " . $this->eventQueue['id'] + )); + } + + /** + * Set up a mailing. + */ + public function setUpMailing() { + $this->contactID = $this->individualCreate(array('email' => 'undeliverable@example.com')); + $groupID = $this->callAPISuccess('Group', 'create', array( + 'title' => 'Mailing group', + 'api.GroupContact.create' => array( + 'contact_id' => $this->contactID, + ))); + $this->createMailing(array('scheduled_date' => 'now', 'groups' => array('include' => array($groupID)))); + $this->callAPISuccess('job', 'process_mailing', array()); + $this->eventQueue = $this->callAPISuccess('MailingEventQueue', 'get', array('api.MailingEventQueue.create' => array('hash' => 'aaaaaaaaaaaaaaaa'))); + } + +} diff --git a/tests/phpunit/CRM/Utils/Mail/data/bounces/bounce_no_verp.txt b/tests/phpunit/CRM/Utils/Mail/data/bounces/bounce_no_verp.txt new file mode 100644 index 0000000000..26e07f2284 --- /dev/null +++ b/tests/phpunit/CRM/Utils/Mail/data/bounces/bounce_no_verp.txt @@ -0,0 +1,72 @@ +Delivered-To: my@example.com +Received: by 10.112.67.71 with SMTP id l7csp45921lbt; + Thu, 23 Jan 2014 12:45:42 -0800 (PST) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=googlemail.com; s=20120113; + h=mime-version:from:to:subject:message-id:date:content-type; + bh=UGTp5DX2SZvxoM2GPLjZwRcpcz4/3zeLJyMyYuTvOI0=; + b=lbtNQ3EnmzubKV0sbek27d5E0+yCG4FXwcKrk3+mfgJ2nDUHeBvQNz/fyL+ppmJs6h + XHZzakNN0JAeQh9AxELPYlRM5iTYcRe2zFiBGvU0YMcoIoDZOV3r8RvpfcRm5IYQCNS5 + LJmRJsvPN+mwgKhdYmBqWezjasQSVVlaHMVt59CdH106pp1FixWFrSxl0r465/IhlbWR + p1geZmq89piISVsNemXT8n5src0OuqMQCFW4m4LZbk6JKdCuS3ErnJ6t6ODSmzRW+0Lv + WkiXuxKefj/d5OHdgUb9YHDQMb8EU3Gh4q0udoqkTV1l87Lfq2c/2NpH9tk/2dbN9MaA + 65hA== +X-Received: by 10.67.22.67 with SMTP id hq3mr9981350pad.132.1390509942011; + Thu, 23 Jan 2014 12:45:42 -0800 (PST) +MIME-Version: 1.0 +Return-Path: <> +Received: by 10.67.22.67 with SMTP id hq3mr12327267pad.132; Thu, 23 Jan 2014 + 12:45:42 -0800 (PST) +From: Mail Delivery Subsystem +To: b.2.1.aaaaaaaaaaaaaaaa@example.com +X-Failed-Recipients: undeliverable@example.com +Subject: Delivery Status Notification (Failure) +Message-ID: <047d7b5dbbd42675d704f0a953a8@google.com> +Date: Thu, 23 Jan 2014 20:45:42 +0000 +Content-Type: text/plain; charset=ISO-8859-1 + +Delivery to the following recipient failed permanently: + + undeliverable@example.com + +Technical details of permanent failure: +Google tried to deliver your message, but it was rejected by the server for the recipient domain example.com by aspmx.l.google.com. [74.125.129.26]. + +The error that the other server returned was: +550-5.1.1 The email account that you tried to reach does not exist. Please try +550-5.1.1 double-checking the recipient's email address for typos or +550-5.1.1 unnecessary spaces. Learn more at +550 5.1.1 http://support.google.com/mail/bin/answer.py?answer=6596 yy4si15401666pbc.69 - gsmtp + +----- Original message ----- + +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=1e100.net; s=20130820; + h=x-gm-message-state:from:content-type:subject:message-id:date:to + :mime-version; + bh=pmXAuF/5WEKUL4czT4KPvdj7HdW4O6/bnJxxnzhDxS4=; + b=OLUdt6zYl0SwEk+7rmWCcpxDxpELnUMFnU8LGHDZxefjkFcugCUCGb4SEFo0uW+FEv + JYxv9xteYtVZ4pfy40ggFUtN5mXnn/B8WSI0Y+/BF5Ow2FpKXKk932+Jhi+DPRDc7fB0 + YdpwO9CqEDx0FHi6r1G7uKmse8Y6ekfO8zCq48t4SQ9A39P1pNESj3KSIhaaBP/PdMVu + VezOOwae71dMviH6WSiDksIJgw+cRXcxpWNU/mjn+Yf7lk2PHYn2vrhCni2Q3Trr2PpK + oNQBiGHbxSGDeHX8r+DsoWeSIvaXxH8y0AICSnTMAXqFuzEEhFHFFkqrXwn0ZWgLyK3F + Vjaw== +X-Gm-Message-State: ALoCoQkkZ5zeZN9IR1iF+yKdq/RaFqQbP/XzNV3rJ2PRHd9eagL83ZI9HrH5oEWtcQWqI11+gALC +X-Received: by 10.67.22.67 with SMTP id hq3mr9981324pad.132.1390509941690; + Thu, 23 Jan 2014 12:45:41 -0800 (PST) +Return-Path: +Received: from [10.1.1.2] ([121.99.62.224]) + by mx.google.com with ESMTPSA id vx10sm65825229pac.17.2014.01.23.12.45.38 + for + (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); + Thu, 23 Jan 2014 12:45:40 -0800 (PST) +From: My Mail +Content-Type: multipart/signed; boundary="Apple-Mail=_2E4B9F97-76BD-4F9D-94C0-5DEC92528786"; protocol="application/pgp-signature"; micalg=pgp-sha1 +Subject: test +Message-Id: <20E88533-8A36-4405-8125-FDC61CD6AB56@example.co> +Date: Fri, 24 Jan 2014 09:45:37 +1300 +To: undeliverable@example.com +Mime-Version: 1.0 (Mac OS X Mail 7.1 \(1827\)) +X-Mailer: Apple Mail (2.1827) + +test diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 5e41fc68dc..cfb4531416 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3199,15 +3199,18 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) /** * Helper function to create new mailing. - * @return mixed + * + * @param array $params + * + * @return int */ - public function createMailing() { - $params = array( + public function createMailing($params) { + $params = array_merge(array( 'subject' => 'maild' . rand(), 'body_text' => 'bdkfhdskfhduew{domain.address}{action.optOutUrl}', 'name' => 'mailing name' . rand(), 'created_id' => 1, - ); + ), $params); $result = $this->callAPISuccess('Mailing', 'create', $params); return $result['id']; -- 2.25.1