feature #33424 [Mailer] Change the DSN semantics (fabpot)

This PR was merged into the 4.4 branch. Discussion ---------- [Mailer] Change the DSN semantics | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | n/a | License | MIT | Doc PR | symfony/symfony-docs#12258 I'm not very satisfied with the current DSNs for the Mailer. First, the scheme/protocol should use the provider name, then, there is no way to change the host, which would be nice to debug more easily (using a requestb.in service for instance). Before: ``` smtp://USERNAME:PASSWORD@mailgun http://KEY:DOMAIN@mailgun ``` After: ``` mailgun+smtp://USERNAME:PASSWORD@default mailgun+http://KEY:DOMAIN@default # New mailgun+http://KEY:DOMAIN@somewhere.com:99 ``` SMTP DSNs did not change, but the special sendmail one did: Before: ``` smtp://sendmail ``` After: ``` sendmail+smtp://default ``` And for the `null` transport: Before: ``` smtp://null ``` After: ``` null://null ``` Commits ------- 469c989 [Mailer] Change the DSN semantics
symfony · Sep 2, 2019 · 378abfa · 378abfa
2 parents ef2b65a + 469c989
commit 378abfa
Show file tree

Hide file tree

Showing 41 changed files with 704 additions and 187 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Mailer/config.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Mailer/config.yml
@@ -4,7 +4,7 @@ imports:
 
 framework:
     mailer:
-        dsn: 'smtp://null'
+        dsn: 'null://null'
         envelope:
             sender: sender@example.org
             recipients:

diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesApiTransportTest.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesApiTransportTest.php
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Mailer\Bridge\Amazon\Tests\Transport;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesApiTransport;
+
+class SesApiTransportTest extends TestCase
+{
+    /**
+     * @dataProvider getTransportData
+     */
+    public function testToString(SesApiTransport $transport, string $expected)
+    {
+        $this->assertSame($expected, (string) $transport);
+    }
+
+    public function getTransportData()
+    {
+        return [
+            [
+                new SesApiTransport('ACCESS_KEY', 'SECRET_KEY'),
+                'ses+api://ACCESS_KEY@email.eu-west-1.amazonaws.com',
+            ],
+            [
+                new SesApiTransport('ACCESS_KEY', 'SECRET_KEY', 'us-east-1'),
+                'ses+api://ACCESS_KEY@email.us-east-1.amazonaws.com',
+            ],
+            [
+                (new SesApiTransport('ACCESS_KEY', 'SECRET_KEY'))->setHost('example.com'),
+                'ses+api://ACCESS_KEY@example.com',
+            ],
+            [
+                (new SesApiTransport('ACCESS_KEY', 'SECRET_KEY'))->setHost('example.com')->setPort(99),
+                'ses+api://ACCESS_KEY@example.com:99',
+            ],
+        ];
+    }
+}
diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesHttpTransportTest.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesHttpTransportTest.php
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Mailer\Bridge\Amazon\Tests\Transport;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesHttpTransport;
+
+class SesHttpTransportTest extends TestCase
+{
+    /**
+     * @dataProvider getTransportData
+     */
+    public function testToString(SesHttpTransport $transport, string $expected)
+    {
+        $this->assertSame($expected, (string) $transport);
+    }
+
+    public function getTransportData()
+    {
+        return [
+            [
+                new SesHttpTransport('ACCESS_KEY', 'SECRET_KEY'),
+                'ses+https://ACCESS_KEY@email.eu-west-1.amazonaws.com',
+            ],
+            [
+                new SesHttpTransport('ACCESS_KEY', 'SECRET_KEY', 'us-east-1'),
+                'ses+https://ACCESS_KEY@email.us-east-1.amazonaws.com',
+            ],
+            [
+                (new SesHttpTransport('ACCESS_KEY', 'SECRET_KEY'))->setHost('example.com'),
+                'ses+https://ACCESS_KEY@example.com',
+            ],
+            [
+                (new SesHttpTransport('ACCESS_KEY', 'SECRET_KEY'))->setHost('example.com')->setPort(99),
+                'ses+https://ACCESS_KEY@example.com:99',
+            ],
+        ];
+    }
+}
diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesTransportFactoryTest.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Tests/Transport/SesTransportFactoryTest.php
@@ -29,28 +29,33 @@ public function getFactory(): TransportFactoryInterface
     public function supportsProvider(): iterable
     {
         yield [
-            new Dsn('api', 'ses'),
+            new Dsn('ses+api', 'default'),
             true,
         ];
 
         yield [
-            new Dsn('http', 'ses'),
+            new Dsn('ses+https', 'default'),
             true,
         ];
 
         yield [
-            new Dsn('smtp', 'ses'),
+            new Dsn('ses', 'default'),
             true,
         ];
 
         yield [
-            new Dsn('smtps', 'ses'),
+            new Dsn('ses+smtp', 'default'),
             true,
         ];
 
         yield [
-            new Dsn('smtp', 'example.com'),
-            false,
+            new Dsn('ses+smtps', 'default'),
+            true,
+        ];
+
+        yield [
+            new Dsn('ses+smtp', 'example.com'),
+            true,
         ];
     }
 
@@ -61,53 +66,68 @@ public function createProvider(): iterable
         $logger = $this->getLogger();
 
         yield [
-            new Dsn('api', 'ses', self::USER, self::PASSWORD),
+            new Dsn('ses+api', 'default', self::USER, self::PASSWORD),
             new SesApiTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('api', 'ses', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
+            new Dsn('ses+api', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
             new SesApiTransport(self::USER, self::PASSWORD, 'eu-west-1', $client, $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('http', 'ses', self::USER, self::PASSWORD),
+            new Dsn('ses+api', 'example.com', self::USER, self::PASSWORD, 8080),
+            (new SesApiTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
+        ];
+
+        yield [
+            new Dsn('ses+https', 'default', self::USER, self::PASSWORD),
             new SesHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('http', 'ses', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
+            new Dsn('ses', 'default', self::USER, self::PASSWORD),
+            new SesHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger),
+        ];
+
+        yield [
+            new Dsn('ses+https', 'example.com', self::USER, self::PASSWORD, 8080),
+            (new SesHttpTransport(self::USER, self::PASSWORD, null, $client, $dispatcher, $logger))->setHost('example.com')->setPort(8080),
+        ];
+
+        yield [
+            new Dsn('ses+https', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
             new SesHttpTransport(self::USER, self::PASSWORD, 'eu-west-1', $client, $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('smtp', 'ses', self::USER, self::PASSWORD),
+            new Dsn('ses+smtp', 'default', self::USER, self::PASSWORD),
             new SesSmtpTransport(self::USER, self::PASSWORD, null, $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('smtp', 'ses', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
+            new Dsn('ses+smtp', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
             new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', $dispatcher, $logger),
         ];
 
         yield [
-            new Dsn('smtps', 'ses', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
+            new Dsn('ses+smtps', 'default', self::USER, self::PASSWORD, null, ['region' => 'eu-west-1']),
             new SesSmtpTransport(self::USER, self::PASSWORD, 'eu-west-1', $dispatcher, $logger),
         ];
     }
 
     public function unsupportedSchemeProvider(): iterable
     {
         yield [
-            new Dsn('foo', 'ses', self::USER, self::PASSWORD),
-            'The "foo" scheme is not supported for mailer "ses". Supported schemes are: "api", "http", "smtp", "smtps".',
+            new Dsn('ses+foo', 'default', self::USER, self::PASSWORD),
+            'The "ses+foo" scheme is not supported. Supported schemes for mailer "ses" are: "ses", "ses+api", "ses+https", "ses+smtp", "ses+smtps".',
         ];
     }
 
     public function incompleteDsnProvider(): iterable
     {
-        yield [new Dsn('smtp', 'ses', self::USER)];
+        yield [new Dsn('ses+smtp', 'default', self::USER)];
 
-        yield [new Dsn('smtp', 'ses', null, self::PASSWORD)];
+        yield [new Dsn('ses+smtp', 'default', null, self::PASSWORD)];
     }
 }
diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesApiTransport.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesApiTransport.php
@@ -25,7 +25,7 @@
  */
 class SesApiTransport extends AbstractApiTransport
 {
-    private const ENDPOINT = 'https://email.%region%.amazonaws.com';
+    private const HOST = 'email.%region%.amazonaws.com';
 
     private $accessKey;
     private $secretKey;
@@ -45,16 +45,15 @@ public function __construct(string $accessKey, string $secretKey, string $region
 
     public function __toString(): string
     {
-        return sprintf('api://%s@ses?region=%s', $this->accessKey, $this->region);
+        return sprintf('ses+api://%s@%s', $this->accessKey, $this->getEndpoint());
     }
 
     protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
     {
         $date = gmdate('D, d M Y H:i:s e');
         $auth = sprintf('AWS3-HTTPS AWSAccessKeyId=%s,Algorithm=HmacSHA256,Signature=%s', $this->accessKey, $this->getSignature($date));
 
-        $endpoint = str_replace('%region%', $this->region, self::ENDPOINT);
-        $response = $this->client->request('POST', $endpoint, [
+        $response = $this->client->request('POST', 'https://'.$this->getEndpoint(), [
             'headers' => [
                 'X-Amzn-Authorization' => $auth,
                 'Date' => $date,
@@ -72,6 +71,11 @@ protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInte
         return $response;
     }
 
+    private function getEndpoint(): ?string
+    {
+        return ($this->host ?: str_replace('%region%', $this->region, self::HOST)).($this->port ? ':'.$this->port : '');
+    }
+
     private function getSignature(string $string): string
     {
         return base64_encode(hash_hmac('sha256', $string, $this->secretKey, true));

diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesHttpTransport.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesHttpTransport.php
@@ -24,7 +24,7 @@
  */
 class SesHttpTransport extends AbstractHttpTransport
 {
-    private const ENDPOINT = 'https://email.%region%.amazonaws.com';
+    private const HOST = 'email.%region%.amazonaws.com';
 
     private $accessKey;
     private $secretKey;
@@ -44,16 +44,15 @@ public function __construct(string $accessKey, string $secretKey, string $region
 
     public function __toString(): string
     {
-        return sprintf('http://%s@ses?region=%s', $this->accessKey, $this->region);
+        return sprintf('ses+https://%s@%s', $this->accessKey, $this->getEndpoint());
     }
 
     protected function doSendHttp(SentMessage $message): ResponseInterface
     {
         $date = gmdate('D, d M Y H:i:s e');
         $auth = sprintf('AWS3-HTTPS AWSAccessKeyId=%s,Algorithm=HmacSHA256,Signature=%s', $this->accessKey, $this->getSignature($date));
 
-        $endpoint = str_replace('%region%', $this->region, self::ENDPOINT);
-        $response = $this->client->request('POST', $endpoint, [
+        $response = $this->client->request('POST', 'https://'.$this->getEndpoint(), [
             'headers' => [
                 'X-Amzn-Authorization' => $auth,
                 'Date' => $date,
@@ -73,6 +72,11 @@ protected function doSendHttp(SentMessage $message): ResponseInterface
         return $response;
     }
 
+    private function getEndpoint(): ?string
+    {
+        return ($this->host ?: str_replace('%region%', $this->region, self::HOST)).($this->port ? ':'.$this->port : '');
+    }
+
     private function getSignature(string $string): string
     {
         return base64_encode(hash_hmac('sha256', $string, $this->secretKey, true));

diff --git a/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesTransportFactory.php b/src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesTransportFactory.php
@@ -27,24 +27,26 @@ public function create(Dsn $dsn): TransportInterface
         $user = $this->getUser($dsn);
         $password = $this->getPassword($dsn);
         $region = $dsn->getOption('region');
+        $host = 'default' === $dsn->getHost() ? null : $dsn->getHost();
+        $port = $dsn->getPort();
 
-        if ('api' === $scheme) {
-            return new SesApiTransport($user, $password, $region, $this->client, $this->dispatcher, $this->logger);
+        if ('ses+api' === $scheme) {
+            return (new SesApiTransport($user, $password, $region, $this->client, $this->dispatcher, $this->logger))->setHost($host)->setPort($port);
         }
 
-        if ('http' === $scheme) {
-            return new SesHttpTransport($user, $password, $region, $this->client, $this->dispatcher, $this->logger);
+        if ('ses+https' === $scheme || 'ses' === $scheme) {
+            return (new SesHttpTransport($user, $password, $region, $this->client, $this->dispatcher, $this->logger))->setHost($host)->setPort($port);
         }
 
-        if ('smtp' === $scheme || 'smtps' === $scheme) {
+        if ('ses+smtp' === $scheme || 'ses+smtps' === $scheme) {
             return new SesSmtpTransport($user, $password, $region, $this->dispatcher, $this->logger);
         }
 
-        throw new UnsupportedSchemeException($dsn, ['api', 'http', 'smtp', 'smtps']);
+        throw new UnsupportedSchemeException($dsn, 'ses', $this->getSupportedSchemes());
     }
 
-    public function supports(Dsn $dsn): bool
+    protected function getSupportedSchemes(): array
     {
-        return 'ses' === $dsn->getHost();
+        return ['ses', 'ses+api', 'ses+https', 'ses+smtp', 'ses+smtps'];
     }
 }