From 36edab34e6f7fb0cbafed2dad0de074cf3d2df65 Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Thu, 31 Mar 2022 15:12:05 +0200 Subject: [PATCH] Move logic for return path fallback address from method classes to factory [MAILPOET-4115] --- mailpoet/lib/Mailer/MailerFactory.php | 6 +++--- mailpoet/lib/Mailer/Methods/AmazonSES.php | 4 +--- mailpoet/lib/Mailer/Methods/PHPMail.php | 4 +--- mailpoet/lib/Mailer/Methods/SMTP.php | 4 +--- .../integration/Mailer/MailerFactoryTest.php | 8 ++++++++ .../integration/Mailer/Methods/AmazonSESTest.php | 13 ------------- .../integration/Mailer/Methods/PHPMailTest.php | 10 ---------- .../integration/Mailer/Methods/SMTPTest.php | 16 ---------------- 8 files changed, 14 insertions(+), 51 deletions(-) diff --git a/mailpoet/lib/Mailer/MailerFactory.php b/mailpoet/lib/Mailer/MailerFactory.php index 91cadad44c..3187d4e03f 100644 --- a/mailpoet/lib/Mailer/MailerFactory.php +++ b/mailpoet/lib/Mailer/MailerFactory.php @@ -47,7 +47,7 @@ class MailerFactory { $sender = $this->getSenderNameAndAddress($sender); $replyTo = $this->getReplyToNameAndAddress($sender, $replyTo); $mailerConfig = $mailerConfig ?? $this->getMailerConfig(); - $returnPath = $returnPath ?? $this->getReturnPathAddress(); + $returnPath = $returnPath ?? $this->getReturnPathAddress($sender); switch ($mailerConfig['method']) { case Mailer::METHOD_AMAZONSES: $mailerInstance = new AmazonSES( @@ -147,9 +147,9 @@ class MailerFactory { ]; } - private function getReturnPathAddress(): ?string { + private function getReturnPathAddress(array $sender): ?string { $bounceAddress = $this->settings->get('bounce.address'); - return $this->wp->isEmail($bounceAddress) ? $bounceAddress : null; + return $this->wp->isEmail($bounceAddress) ? $bounceAddress : $sender['from_email']; } private function encodeAddressNamePart($name): string { diff --git a/mailpoet/lib/Mailer/Methods/AmazonSES.php b/mailpoet/lib/Mailer/Methods/AmazonSES.php index 6dcd91bce9..d5fb640ce0 100644 --- a/mailpoet/lib/Mailer/Methods/AmazonSES.php +++ b/mailpoet/lib/Mailer/Methods/AmazonSES.php @@ -83,9 +83,7 @@ class AmazonSES implements MailerMethod { $this->url = 'https://' . $this->awsEndpoint; $this->sender = $sender; $this->replyTo = $replyTo; - $this->returnPath = ($returnPath) ? - $returnPath : - $this->sender['from_email']; + $this->returnPath = $returnPath; $this->date = gmdate('Ymd\THis\Z'); $this->dateWithoutTime = gmdate('Ymd'); $this->errorMapper = $errorMapper; diff --git a/mailpoet/lib/Mailer/Methods/PHPMail.php b/mailpoet/lib/Mailer/Methods/PHPMail.php index a44af2918f..234a4a50cf 100644 --- a/mailpoet/lib/Mailer/Methods/PHPMail.php +++ b/mailpoet/lib/Mailer/Methods/PHPMail.php @@ -29,9 +29,7 @@ class PHPMail implements MailerMethod { ) { $this->sender = $sender; $this->replyTo = $replyTo; - $this->returnPath = ($returnPath) ? - $returnPath : - $this->sender['from_email']; + $this->returnPath = $returnPath; $this->mailer = $this->buildMailer(); $this->errorMapper = $errorMapper; $this->blacklist = new BlacklistCheck(); diff --git a/mailpoet/lib/Mailer/Methods/SMTP.php b/mailpoet/lib/Mailer/Methods/SMTP.php index bba5ed188f..090ecd6ea6 100644 --- a/mailpoet/lib/Mailer/Methods/SMTP.php +++ b/mailpoet/lib/Mailer/Methods/SMTP.php @@ -55,9 +55,7 @@ class SMTP implements MailerMethod { $this->encryption = $encryption; $this->sender = $sender; $this->replyTo = $replyTo; - $this->returnPath = ($returnPath) ? - $returnPath : - $this->sender['from_email']; + $this->returnPath = $returnPath; $this->mailer = $this->buildMailer(); $this->mailerLogger = new Swift_Plugins_Loggers_ArrayLogger(); $this->mailer->registerPlugin(new Swift_Plugins_LoggerPlugin($this->mailerLogger)); diff --git a/mailpoet/tests/integration/Mailer/MailerFactoryTest.php b/mailpoet/tests/integration/Mailer/MailerFactoryTest.php index 77b843b5a8..5399ef3bbf 100644 --- a/mailpoet/tests/integration/Mailer/MailerFactoryTest.php +++ b/mailpoet/tests/integration/Mailer/MailerFactoryTest.php @@ -153,6 +153,14 @@ class MailerFactoryTest extends \MailPoetTest { ]); } + public function testItIgnoresInvalidBounceAddressAndUsesSenderAddressInstead() { + $this->settings->set('bounce.address', 'invalid'); + $mailer = $this->factory->getDefaultMailer(); + $mailerMethod = $mailer->mailerInstance; + $this->assertInstanceOf(PHPMail::class, $mailerMethod); + expect($mailerMethod->returnPath)->equals('sender@email.com'); + } + public function testItUsesSenderAddressInReplyToInCaseReplyToHasOnlyName() { $this->settings->set('reply_to', ['name' => 'Reply To']); $mailer = $this->factory->getDefaultMailer(); diff --git a/mailpoet/tests/integration/Mailer/Methods/AmazonSESTest.php b/mailpoet/tests/integration/Mailer/Methods/AmazonSESTest.php index 496ff6875f..b87ec32bf3 100644 --- a/mailpoet/tests/integration/Mailer/Methods/AmazonSESTest.php +++ b/mailpoet/tests/integration/Mailer/Methods/AmazonSESTest.php @@ -79,19 +79,6 @@ class AmazonSESTest extends \MailPoetTest { expect(preg_match('!^\d{8}$!', $this->mailer->dateWithoutTime))->equals(1); } - public function testWhenReturnPathIsNullItIsSetToSenderEmail() { - $mailer = new AmazonSES( - $this->settings['region'], - $this->settings['access_key'], - $this->settings['secret_key'], - $this->sender, - $this->replyTo, - $returnPath = false, - new AmazonSESMapper() - ); - expect($mailer->returnPath)->equals($this->sender['from_email']); - } - public function testItChecksForValidRegion() { try { $mailer = new AmazonSES( diff --git a/mailpoet/tests/integration/Mailer/Methods/PHPMailTest.php b/mailpoet/tests/integration/Mailer/Methods/PHPMailTest.php index ecff80cf14..522ebd673b 100644 --- a/mailpoet/tests/integration/Mailer/Methods/PHPMailTest.php +++ b/mailpoet/tests/integration/Mailer/Methods/PHPMailTest.php @@ -58,16 +58,6 @@ class PHPMailTest extends \MailPoetTest { expect($mailer->Mailer)->equals('mail'); // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps } - public function testWhenReturnPathIsNullItIsSetToSenderEmail() { - $mailer = new PHPMail( - $this->sender, - $this->replyTo, - $returnPath = false, - new PHPMailMapper() - ); - expect($mailer->returnPath)->equals($this->sender['from_email']); - } - public function testItCanConfigureMailerWithMessage() { $mailer = $this->mailer ->configureMailerWithMessage($this->newsletter, $this->subscriber, $this->extraParams); diff --git a/mailpoet/tests/integration/Mailer/Methods/SMTPTest.php b/mailpoet/tests/integration/Mailer/Methods/SMTPTest.php index 855f8355ff..124042f907 100644 --- a/mailpoet/tests/integration/Mailer/Methods/SMTPTest.php +++ b/mailpoet/tests/integration/Mailer/Methods/SMTPTest.php @@ -87,22 +87,6 @@ class SMTPTest extends \MailPoetTest { ->equals($this->settings['encryption']); } - public function testWhenReturnPathIsNullItIsSetToSenderEmail() { - $mailer = new SMTP( - $this->settings['host'], - $this->settings['port'], - $this->settings['authentication'], - $this->settings['encryption'], - $this->sender, - $this->replyTo, - $returnPath = false, - new SMTPMapper(), - $this->settings['login'], - $this->settings['password'] - ); - expect($mailer->returnPath)->equals($this->sender['from_email']); - } - public function testItCanCreateMessage() { $message = $this->mailer ->createMessage($this->newsletter, $this->subscriber, $this->extraParams);