Merge pull request mautic#8703 from anton-vlasenko/fix-tokenization-i…

…ssue Fixes issue where support for tokenization was not detected in the MailHelper
dhilst · May 15, 2020 · 0286ac0 · 0286ac0
2 parents 82479f3 + f268d57
commit 0286ac0
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 7 deletions.
diff --git a/app/bundles/EmailBundle/Helper/MailHelper.php b/app/bundles/EmailBundle/Helper/MailHelper.php
@@ -21,6 +21,7 @@
 use Mautic\EmailBundle\Event\EmailSendEvent;
 use Mautic\EmailBundle\Swiftmailer\Exception\BatchQueueMaxException;
 use Mautic\EmailBundle\Swiftmailer\Message\MauticMessage;
+use Mautic\EmailBundle\Swiftmailer\Transport\SpoolTransport;
 use Mautic\EmailBundle\Swiftmailer\Transport\TokenTransportInterface;
 use Mautic\LeadBundle\Entity\Lead;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -266,7 +267,10 @@ public function __construct(MauticFactory $factory, \Swift_Mailer $mailer, $from
         $this->returnPath = $factory->getParameter('mailer_return_path');
 
         // Check if batching is supported by the transport
-        if ('memory' == $this->factory->getParameter('mailer_spool_type') && $this->transport instanceof TokenTransportInterface) {
+        if (
+            ('memory' == $this->factory->getParameter('mailer_spool_type') && $this->transport instanceof TokenTransportInterface)
+            || ($this->transport instanceof SpoolTransport && $this->transport->supportsTokenization())
+        ) {
             $this->tokenizationEnabled = true;
         }
 
@@ -737,7 +741,7 @@ public static function searchReplaceTokens($search, $replace, \Swift_Message &$m
         /** @var \Swift_Mime_SimpleMimeEntity $child */
         foreach ($children as $child) {
             $childType  = $child->getContentType();
-            list($type) = sscanf($childType, '%[^/]/%s');
+            [$type]     = sscanf($childType, '%[^/]/%s');
 
             if ('text' == $type) {
                 $childBody = $child->getBody();
@@ -1928,7 +1932,7 @@ public function generateBounceEmail($idHash = null)
 
         if ($settings = $this->isMontoringEnabled('EmailBundle', 'bounces')) {
             // Append the bounce notation
-            list($email, $domain) = explode('@', $settings['address']);
+            [$email, $domain] = explode('@', $settings['address']);
             $email .= '+bounce';
             if ($idHash || $this->idHash) {
                 $email .= '_'.($idHash ?: $this->idHash);
@@ -1952,7 +1956,7 @@ public function generateUnsubscribeEmail($idHash = null)
 
         if ($settings = $this->isMontoringEnabled('EmailBundle', 'unsubscribes')) {
             // Append the bounce notation
-            list($email, $domain) = explode('@', $settings['address']);
+            [$email, $domain] = explode('@', $settings['address']);
             $email .= '+unsubscribe';
             if ($idHash || $this->idHash) {
                 $email .= '_'.($idHash ?: $this->idHash);

diff --git a/app/bundles/EmailBundle/Swiftmailer/Spool/DelegatingSpool.php b/app/bundles/EmailBundle/Swiftmailer/Spool/DelegatingSpool.php
@@ -12,6 +12,7 @@
 namespace Mautic\EmailBundle\Swiftmailer\Spool;
 
 use Mautic\CoreBundle\Helper\CoreParametersHelper;
+use Mautic\EmailBundle\Swiftmailer\Transport\TokenTransportInterface;
 use Swift_Mime_SimpleMessage;
 
 /**
@@ -79,6 +80,11 @@ public function wasMessageSpooled(): bool
         return $this->messageSpooled;
     }
 
+    public function isTokenizationEnabled(): bool
+    {
+        return !$this->fileSpoolEnabled && $this->realTransport instanceof TokenTransportInterface;
+    }
+
     private function getSpoolDir(): string
     {
         $filePath = $this->coreParametersHelper->get('mailer_spool_path');
@@ -90,4 +96,9 @@ private function getSpoolDir(): string
 
         return str_replace('%kernel.root_dir%', $rootPath, $filePath);
     }
+
+    public function getRealTransport(): \Swift_Transport
+    {
+        return $this->realTransport;
+    }
 }
diff --git a/app/bundles/EmailBundle/Swiftmailer/Transport/SpoolTransport.php b/app/bundles/EmailBundle/Swiftmailer/Transport/SpoolTransport.php
@@ -64,4 +64,19 @@ public function send(\Swift_Mime_SimpleMessage $message, &$failedRecipients = nu
 
         return $count;
     }
+
+    public function supportsTokenization(): bool
+    {
+        return $this->spool->isTokenizationEnabled();
+    }
+
+    public function getMaxBatchLimit()
+    {
+        return $this->spool->getRealTransport()->getMaxBatchLimit();
+    }
+
+    public function getBatchRecipientCount(\Swift_Message $message, $toBeAdded = 1, $type = 'to')
+    {
+        return $this->spool->getRealTransport()->getBatchRecipientCount($message, $toBeAdded, $type);
+    }
 }
diff --git a/app/bundles/EmailBundle/Tests/Helper/MailHelperTest.php b/app/bundles/EmailBundle/Tests/Helper/MailHelperTest.php
@@ -16,6 +16,8 @@
 use Mautic\EmailBundle\Helper\MailHelper;
 use Mautic\EmailBundle\MonitoredEmail\Mailbox;
 use Mautic\EmailBundle\Swiftmailer\Exception\BatchQueueMaxException;
+use Mautic\EmailBundle\Swiftmailer\Spool\DelegatingSpool;
+use Mautic\EmailBundle\Swiftmailer\Transport\SpoolTransport;
 use Mautic\EmailBundle\Tests\Helper\Transport\BatchTransport;
 use Mautic\EmailBundle\Tests\Helper\Transport\BcInterfaceTokenTransport;
 use Mautic\EmailBundle\Tests\Helper\Transport\SmtpTransport;
@@ -25,6 +27,29 @@
 
 class MailHelperTest extends \PHPUnit\Framework\TestCase
 {
+    /**
+     * @var MauticFactory|\PHPUnit\Framework\MockObject\MockObject
+     */
+    private $mockFactory;
+
+    /**
+     * @var SpoolTransport
+     */
+    private $spoolTransport;
+
+    /**
+     * @var \Swift_Events_EventDispatcher
+     */
+    private $swiftEventsDispatcher;
+
+    /**
+     * @var DelegatingSpool
+     */
+    private $delegatingSpool;
+
+    /**
+     * @var array
+     */
     protected $contacts = [
         [
             'id'        => 1,
@@ -59,6 +84,12 @@ class MailHelperTest extends \PHPUnit\Framework\TestCase
     protected function setUp()
     {
         defined('MAUTIC_ENV') or define('MAUTIC_ENV', 'test');
+
+        $this->mockFactory           = $this->createMock(MauticFactory::class);
+        $this->swiftEventsDispatcher = $this->createMock(\Swift_Events_EventDispatcher::class);
+        $this->delegatingSpool       = $this->createMock(DelegatingSpool::class);
+
+        $this->spoolTransport = new SpoolTransport($this->swiftEventsDispatcher, $this->delegatingSpool);
     }
 
     /**
@@ -702,4 +733,26 @@ public function testArrayOfAddressesAreRemappedIntoEmailToNameKeyValuePair()
             $mailer->message->getTo()
         );
     }
+
+    public function testThatTokenizationIsEnabledIfTransportSupportsTokenization()
+    {
+        $swiftMailer = new \Swift_Mailer($this->spoolTransport);
+        $this->delegatingSpool->expects($this->once())
+            ->method('isTokenizationEnabled')
+            ->willReturn(true);
+
+        $mailer = new MailHelper($this->mockFactory, $swiftMailer, ['[email protected]' => 'No Body']);
+        $this->assertTrue($mailer->inTokenizationMode());
+    }
+
+    public function testThatTokenizationIsDisabledIfTransportDoesnotSupportTokenization()
+    {
+        $swiftMailer = new \Swift_Mailer($this->spoolTransport);
+        $this->delegatingSpool->expects($this->once())
+            ->method('isTokenizationEnabled')
+            ->willReturn(false);
+
+        $mailer = new MailHelper($this->mockFactory, $swiftMailer, ['[email protected]' => 'No Body']);
+        $this->assertFalse($mailer->inTokenizationMode());
+    }
 }
diff --git a/app/bundles/EmailBundle/Tests/Swiftmailer/Spool/DelegatingSpoolTest.php b/app/bundles/EmailBundle/Tests/Swiftmailer/Spool/DelegatingSpoolTest.php
@@ -13,6 +13,7 @@
 
 use Mautic\CoreBundle\Helper\CoreParametersHelper;
 use Mautic\EmailBundle\Swiftmailer\Spool\DelegatingSpool;
+use Mautic\EmailBundle\Swiftmailer\Transport\MomentumTransport;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Finder\Finder;
@@ -98,6 +99,41 @@ public function testEmailIsSentImmediatelyIfSpoolingIsDisabled()
         rmdir($spoolPath);
     }
 
+    public function testThatTokenizationIsDisabledIfFileSpoolIsEnabled()
+    {
+        $this->realTransport = $this->createMock(MomentumTransport::class);
+        $this->coreParametersHelper->expects($this->exactly(2))
+            ->method('get')
+            ->withConsecutive(['mailer_spool_type'], ['mailer_spool_path'])
+            ->willReturnOnConsecutiveCalls('file', null);
+
+        $spool = new DelegatingSpool($this->coreParametersHelper, $this->realTransport);
+        $this->assertFalse($spool->isTokenizationEnabled());
+    }
+
+    public function testThatTokenizationIsEnabledIfFileSpoolIsDisabled()
+    {
+        $this->realTransport = $this->createMock(MomentumTransport::class);
+        $this->coreParametersHelper->expects($this->exactly(2))
+            ->method('get')
+            ->withConsecutive(['mailer_spool_type'], ['mailer_spool_path'])
+            ->willReturnOnConsecutiveCalls('notFile', null);
+
+        $spool = new DelegatingSpool($this->coreParametersHelper, $this->realTransport);
+        $this->assertTrue($spool->isTokenizationEnabled());
+    }
+
+    public function testThatTokenizationIsDisabledIfRealTransposrtDoesNotImplementTokenTransportInterface()
+    {
+        $this->coreParametersHelper->expects($this->exactly(2))
+            ->method('get')
+            ->withConsecutive(['mailer_spool_type'], ['mailer_spool_path'])
+            ->willReturnOnConsecutiveCalls('notFile', null);
+
+        $spool = new DelegatingSpool($this->coreParametersHelper, $this->realTransport);
+        $this->assertFalse($spool->isTokenizationEnabled());
+    }
+
     public function testDelegateMessageWillReturnIntEvenIfTransportWillNot()
     {
         $spoolPath = __DIR__.'/tmp';

diff --git a/app/bundles/EmailBundle/Tests/Transport/SpoolTransportTest.php b/app/bundles/EmailBundle/Tests/Transport/SpoolTransportTest.php
@@ -44,14 +44,14 @@ protected function setUp()
         $this->spool           = $this->createMock(DelegatingSpool::class);
         $this->message         = $this->createMock(\Swift_Mime_SimpleMessage::class);
         $this->sendEvent       = $this->createMock(\Swift_Events_SendEvent::class);
+    }
 
+    public function testSpooledEventIsDispatched()
+    {
         $this->eventDispatcher->expects($this->once())
             ->method('createSendEvent')
             ->willReturn($this->sendEvent);
-    }
 
-    public function testSpooledEventIsDispatched()
-    {
         $this->spool->expects($this->once())
             ->method('delegateMessage')
             ->willReturn(1);
@@ -74,6 +74,10 @@ public function testSpooledEventIsDispatched()
 
     public function testSuccessEventIsDispatched()
     {
+        $this->eventDispatcher->expects($this->once())
+            ->method('createSendEvent')
+            ->willReturn($this->sendEvent);
+
         $this->spool->expects($this->once())
             ->method('delegateMessage')
             ->willReturn(1);
@@ -96,6 +100,10 @@ public function testSuccessEventIsDispatched()
 
     public function testFailedEventIsDispatched()
     {
+        $this->eventDispatcher->expects($this->once())
+            ->method('createSendEvent')
+            ->willReturn($this->sendEvent);
+
         $this->spool->expects($this->once())
             ->method('delegateMessage')
             ->willReturn(0);
@@ -115,4 +123,24 @@ public function testFailedEventIsDispatched()
 
         $this->assertEquals(0, $sent);
     }
+
+    public function testThatSupportsTokenizationMethodReturnsTrueIfTokenizationIsEnabled()
+    {
+        $this->spool->expects($this->once())
+            ->method('isTokenizationEnabled')
+            ->willReturn(true);
+
+        $transport = new SpoolTransport($this->eventDispatcher, $this->spool);
+        $this->assertTrue($transport->supportsTokenization());
+    }
+
+    public function testThatSupportsTokenizationMethodReturnsFalseIfTokenizationIsDisabled()
+    {
+        $this->spool->expects($this->once())
+            ->method('isTokenizationEnabled')
+            ->willReturn(false);
+
+        $transport = new SpoolTransport($this->eventDispatcher, $this->spool);
+        $this->assertFalse($transport->supportsTokenization());
+    }
 }