From 1f53aa27e4596754cd347c3a74fab3876c66f4c8 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 6 Feb 2018 08:24:00 -0800 Subject: [PATCH] Add unit tests for mail failover behaviors when multiple mailers are configured Summary: Depends on D19006. Ref T13053. Ref T12677. When multiple mailers are configured but one or more fail, test that we recover (or don't) appropriately. Test Plan: Ran unit tests. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13053, T12677 Differential Revision: https://secure.phabricator.com/D19007 --- .../storage/PhabricatorMetaMTAMail.php | 16 ++-- .../PhabricatorMetaMTAMailTestCase.php | 78 +++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index 141d3b5e1c..5f859fd1d9 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -315,6 +315,10 @@ final class PhabricatorMetaMTAMail return $this->getParam('stampMetadata', array()); } + public function getMailerKey() { + return $this->getParam('mailer.key'); + } + public function setHTMLBody($html) { $this->setParam('html-body', $html); return $this; @@ -588,6 +592,12 @@ final class PhabricatorMetaMTAMail continue; } + // Keep track of which mailer actually ended up accepting the message. + $mailer_key = $mailer->getKey(); + if ($mailer_key !== null) { + $this->setParam('mailer.key', $mailer_key); + } + return $this ->setStatus(PhabricatorMailOutboundStatus::STATUS_SENT) ->save(); @@ -919,12 +929,6 @@ final class PhabricatorMetaMTAMail $mailer->addCCs($add_cc); } - // Keep track of which mailer actually ended up accepting the message. - $mailer_key = $mailer->getKey(); - if ($mailer_key !== null) { - $this->setParam('mailer.key', $mailer_key); - } - return $mailer; } diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php index 6e72b129b1..c0045301fd 100644 --- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php @@ -253,4 +253,82 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase { ->executeOne(); } + public function testMailerFailover() { + $user = $this->generateNewTestUser(); + $phid = $user->getPHID(); + + $status_sent = PhabricatorMailOutboundStatus::STATUS_SENT; + $status_queue = PhabricatorMailOutboundStatus::STATUS_QUEUE; + $status_fail = PhabricatorMailOutboundStatus::STATUS_FAIL; + + $mailer1 = id(new PhabricatorMailImplementationTestAdapter()) + ->setKey('mailer1'); + + $mailer2 = id(new PhabricatorMailImplementationTestAdapter()) + ->setKey('mailer2'); + + $mailers = array( + $mailer1, + $mailer2, + ); + + // Send mail with both mailers active. The first mailer should be used. + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($phid)) + ->sendWithMailers($mailers); + $this->assertEqual($status_sent, $mail->getStatus()); + $this->assertEqual('mailer1', $mail->getMailerKey()); + + + // If the first mailer fails, the mail should be sent with the second + // mailer. Since we transmitted the mail, this doesn't raise an exception. + $mailer1->setFailTemporarily(true); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($phid)) + ->sendWithMailers($mailers); + $this->assertEqual($status_sent, $mail->getStatus()); + $this->assertEqual('mailer2', $mail->getMailerKey()); + + + // If both mailers fail, the mail should remain in queue. + $mailer2->setFailTemporarily(true); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($phid)); + + $caught = null; + try { + $mail->sendWithMailers($mailers); + } catch (Exception $ex) { + $caught = $ex; + } + + $this->assertTrue($caught instanceof Exception); + $this->assertEqual($status_queue, $mail->getStatus()); + $this->assertEqual(null, $mail->getMailerKey()); + + $mailer1->setFailTemporarily(false); + $mailer2->setFailTemporarily(false); + + + // If the first mailer fails permanently, the mail should fail even though + // the second mailer isn't configured to fail. + $mailer1->setFailPermanently(true); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($phid)); + + $caught = null; + try { + $mail->sendWithMailers($mailers); + } catch (Exception $ex) { + $caught = $ex; + } + + $this->assertTrue($caught instanceof Exception); + $this->assertEqual($status_fail, $mail->getStatus()); + $this->assertEqual(null, $mail->getMailerKey()); + } + }