8000 [mailgun-mailer] support EU-endpoint by bullder · Pull Request #31897 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[mailgun-mailer] support EU-endpoint #31897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
+130 −39
Closed

[mailgun-mailer] support EU-endpoint #31897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
359e214
[mailgun-mailer] support EU-endpoint
Jun 6, 2019
a8fe496
[mailgun-mailer] support EU-endpoint. Final to MailgunRegionConfigura…
bullder Jun 6, 2019
565cfe4
[mailgun-mailer] support EU-endpoint. Small improvements for mailgun …
Jun 7, 2019
File filter

Filter by extension

Filter by extension
8000
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/Symfony/Component/Mailer/Bridge/Mailgun/Http/Api/MailgunTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Mailer\Bridge\Mailgun\MailgunRegionConfiguration;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
Expand All @@ -27,15 +28,13 @@
*/
class MailgunTransport extends AbstractApiTransport
{
private const ENDPOINT = 'https://api.mailgun.net/v3/%domain%/messages';

private $key;
private $domain;
private $endpoint;

public function __construct(string $key, string $domain, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
public function __construct(string $key, string $domain, string $region = MailgunRegionConfiguration::REGION_DEFAULT, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
{
$this->key = $key;
$this->domain = $domain;
$this->endpoint = MailgunRegionConfiguration::resolveApiEndpoint($domain, $region);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the MailgunRegionConfiguration class is really needed.

You can, just change the ENDPOINT constant in each Mailgun Transports (or add it for the smtp transport) to add the region endpoint, something like : private const ENDPOINT = 'https://%region_endpoint%.mailgun.net/v3/%domain%/messages'
And store the region the same way as the domain, $this->regionEndpoint = 'us' === $region ? 'api' : 'api.eu';

Then (at line 51) replace the domain and the region endpoint at the same time
$endpoint = str_replace(['%domain%', '%region_endpoint%'], [urlencode($this->domain), $this->regionEndpoint], self::ENDPOINT);

What do you think ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be become tricky as soon as new region for example Asia might have host name https://mailgun.asia/

Me too don't like that helper class but we have to resolve endpoint in 3 different transports so sooner or later we have met case at which that will require to be extracted

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they would use the same pattern instead of using something different. But anyway, that's speculation at this point. I prefer #31998, which implements it the same as what we have in SesTransport, so it's more consistent across the board.


parent::__construct($client, $dispatcher, $logger);
}
Expand All @@ -48,8 +47,7 @@ protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
$headers[] = $header->toString();
}

$endpoint = str_replace('%domain%', urlencode($this->domain), self::ENDPOINT);
$response = $this->client->request('POST', $endpoint, [
$response = $this->client->request('POST', $this->endpoint, [
'auth_basic' => 'api:'.$this->key,
'headers' => $headers,
'body' => $body->bodyToIterable(),
Expand Down
11 changes: 5 additions & 6 deletions src/Symfony/Component/Mailer/Bridge/Mailgun/Http/MailgunTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Mailer\Bridge\Mailgun\MailgunRegionConfiguration;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\Http\AbstractHttpTransport;
Expand All @@ -27,14 +28,13 @@
*/
class MailgunTransport extends AbstractHttpTransport
{
private const ENDPOINT = 'https://api.mailgun.net/v3/%domain%/messages.mime';
private $key;
private $domain;
private $endpoint;

public function __construct(string $key, string $domain, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
public function __construct(string $key, string $domain, string $region = MailgunRegionConfiguration::REGION_DEFAULT, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
{
$this->key = $key;
$this->domain = $domain;
$this->endpoint = MailgunRegionConfiguration::resolveHttpEndpoint($domain, $region);

parent::__construct($client, $dispatcher, $logger);
}
Expand All @@ -49,8 +49,7 @@ protected function doSend(SentMessage $message): void
foreach ($body->getPreparedHeaders()->getAll() as $header) {
$headers[] = $header->toString();
}
$endpoint = str_replace('%domain%', urlencode($this->domain), self::ENDPOINT);
$response = $this->client->request('POST', $endpoint, [
$response = $this->client->request('POST', $this->endpoint, [
'auth_basic' => 'api:'.$this->key,
'headers' => $headers,
'body' => $body->bodyToIterable(),
Expand Down
68 changes: 68 additions & 0 deletions src/Symfony/Component/Mailer/Bridge/Mailgun/MailgunRegionConfiguration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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\Mailgun;

use Symfony\Component\Mailer\Exception\RuntimeException;

/**
* @author Michael Garifullin <garifullin@gmail.com>
*
* @experimental in 4.3
*/
final class MailgunRegionConfiguration
{
public const REGION_DEFAULT = self::REGION_US;
public const REGION_EU = 'EU';
public const REGION_US = 'US';

private const SMTP_HOSTS = [
self::REGION_EU => 'smtp.eu.mailgun.org',
self::REGION_US => 'smtp.mailgun.org',
];

private const ENDPOINT_DOMAINS = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API_HOSTS for consistency with SMTP_HOSTS.

self::REGION_EU => 'api.eu.mailgun.net',
self::REGION_US => 'api.mailgun.net',
];

private const HTTP_API_ENDPOINT = 'https://%s/v3/%s/messages';
private const HTTP_ENDPOINT = 'https://%s/v3/%s/messages.mime';

public static function resolveSmtpDomainByRegion(string $region = self::REGION_US): string
{
return self::resolveRegionArray(self::SMTP_HOSTS, $region);
}

public static function resolveApiEndpoint(string $domain, string $region = self::REGION_US): string
{
return sprintf(self::HTTP_API_ENDPOINT, self::resolveHttpDomainByRegion($region), urlencode($domain));
}

public static function resolveHttpEndpoint(string $domain, string $region = self::REGION_US): string
{
return sprintf(self::HTTP_ENDPOINT, self::resolveHttpDomainByRegion($region), urlencode($domain));
}

private static function resolveHttpDomainByRegion(string $region = self::REGION_DEFAULT): string
{
return self::resolveRegionArray(self::ENDPOINT_DOMAINS, $region);
}

private static function resolveRegionArray(array $regionMapping, string $region = self::REGION_DEFAULT): string
{
if (empty($regionMapping[$region])) {
throw new RuntimeException(sprintf('Region "%s" for Mailgun is incorrect', $region));
}

return $regionMapping[$region];
}
}
5 changes: 3 additions & 2 deletions src/Symfony/Component/Mailer/Bridge/Mailgun/Smtp/MailgunTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Mailer\Bridge\Mailgun\MailgunRegionConfiguration;
use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;

/**
Expand All @@ -22,9 +23,9 @@
*/
class MailgunTransport extends EsmtpTransport
{
public function __construct(string $username, string $password, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
public function __construct(string $username, string $password, string $region = MailgunRegionConfiguration::REGION_DEFAULT, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
{
parent::__construct('smtp.mailgun.org', 465, 'ssl', null, $dispatcher, $logger);
parent::__construct(MailgunRegionConfiguration::resolveSmtpDomainByRegion($region), 465, 'ssl', null, $dispatcher, $logger);

$this->setUsername($username);
$this->setPassword($password);
Expand Down
66 changes: 45 additions & 21 deletions src/Symfony/Component/Mailer/Tests/TransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Symfony\Component\Mailer\Bridge\Sendgrid;
use Symfony\Component\Mailer\Exception\InvalidArgumentException;
use Symfony\Component\Mailer\Exception\LogicException;
use Symfony\Component\Mailer\Exception\RuntimeException;
use Symfony\Component\Mailer\Transport;
use Symfony\Contracts\HttpClient\HttpClientInterface;

Expand Down Expand Up @@ -96,37 +97,60 @@ public function testFromDsnGmail()
Transport::fromDsn('http://gmail');
}

public function testFromDsnMailgun()
/**
* @dataProvider provideFromDsnMailgun
*/
public function testFromDsnMailgun(string $expectedInstance, string $expected, string $protocol, $region = null)
{
$dispatcher = $this->createMock(EventDispatcherInterface::class);
$client = $this->createMock(HttpClientInterface::class);
$logger = $this->createMock(LoggerInterface::class);
$transport = Transport::fromDsn('smtp://'.urlencode('u$er').':'.urlencode('pa$s').'@mailgun', $dispatcher, null, $logger);
$this->assertInstanceOf(Mailgun\Smtp\MailgunTransport::class, $transport);
$this->assertEquals('u$er', $transport->getUsername());
$this->assertEquals('pa$s', $transport->getPassword());
$this->assertProperties($transport, $dispatcher, $logger);

$client = $this->createMock(HttpClientInterface::class);
$transport = Transport::fromDsn('http://'.urlencode('u$er').':'.urlencode('pa$s').'@mailgun', $dispatcher, $client, $logger);
$this->assertInstanceOf(Mailgun\Http\MailgunTransport::class, $transport);
$this->assertProperties($transport, $dispatcher, $logger, [
'key' => 'u$er',
'domain' => 'pa$s',
'client' => $client,
]);
$dsn = $protocol.'://'.urlencode('u$er').':'.urlencode('client').'@mailgun'.$region;
$transport = Transport::fromDsn($dsn, $dispatcher, $client, $logger);
$this->assertInstanceOf($expectedInstance, $transport);
if (Mailgun\Smtp\MailgunTransport::class === $expectedInstance) {
$this->assertEquals('u$er', $transport->getUsername());
$this->assertEquals($expected, $transport->getPassword());
$this->assertProperties($transport, $dispatcher, $logger);
} else {
$this->assertProperties($transport, $dispatcher, $logger, [
'key' => 'u$er',
'endpoint' => $expected,
'client' => $client,
]);
}
}

$transport = Transport::fromDsn('api://'.urlencode('u$er').':'.urlencode('pa$s').'@mailgun', $dispatcher, $client, $logger);
$this->assertInstanceOf(Mailgun\Http\Api\MailgunTransport::class, $transport);
$this->assertProperties($transport, $dispatcher, $logger, [
'key' => 'u$er',
'domain' => 'pa$s',
'client' => $client,
]);
public function provideFromDsnMailgun()
{
yield [Mailgun\Smtp\MailgunTransport::class, 'client', 'smtp', null];
yield [Mailgun\Http\MailgunTransport::class, 'https://api.mailgun.net/v3/client/messages.mime', 'http', null];
yield [Mailgun\Http\Api\MailgunTransport::class, 'https://api.mailgun.net/v3/client/messages', 'api', null];
yield [Mailgun\Http\Api\MailgunTransport::class, 'https://api.eu.mailgun.net/v3/client/messages', 'api', '?region=EU'];
yield [Mailgun\Http\Api\MailgunTransport::class, 'https://api.mailgun.net/v3/client/messages', 'api', '?region=US'];
}

public function testFromDstMailgunWithExceptionWrongFormat()
{
$this->expectException(LogicException::class);
Transport::fromDsn('foo://mailgun');
}

/**
* @dataProvider provideFromDstMailgunWithExceptionWrongRegion
*/
public function testFromDstMailgunWithExceptionWrongRegion(string $schema)
{
$this->expectException(RuntimeException::class);
Transport::fromDsn($schema.'://user:password@mailgun?region=RU');
}

public function provideFromDstMailgunWithExceptionWrongRegion()
{
return [['smtp'], ['http'], ['api']];
}

public function testFromDsnPostmark()
{
$dispatcher = $this->createMock(EventDispatcherInterface::class);
Expand Down
7 changes: 4 additions & 3 deletions src/Symfony/Component/Mailer/Transport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,15 @@ private static function createTransport(string $dsn, EventDispatcherInterface $d
throw new \LogicException('Unable to send emails via Mailgun as the bridge is not installed. Try running "composer require symfony/mailgun-mailer".');
}

$region = $query['region'] ?? Mailgun\MailgunRegionConfiguration::REGION_DEFAULT;
if ('smtp' === $parsedDsn['scheme']) {
return new Mailgun\Smtp\MailgunTransport($user, $pass, $dispatcher, $logger);
return new Mailgun\Smtp\MailgunTransport($user, $pass, $region, $dispatcher, $logger);
}
if ('http' === $parsedDsn['scheme']) {
return new Mailgun\Http\MailgunTransport($user, $pass, $client, $dispatcher, $logger);
return new Mailgun\Http\MailgunTransport($user, $pass, $region, $client, $dispatcher, $logger);
}
if ('api' === $parsedDsn['scheme']) {
return new Mailgun\Http\Api\MailgunTransport($user, $pass, $client, $dispatcher, $logger);
return new Mailgun\Http\Api\MailgunTransport($user, $pass, $region, $client, $dispatcher, $logger);
}

throw new LogicException(sprintf('The "%s" scheme is not supported for mailer "%s".', $parsedDsn['scheme'], $parsedDsn['host']));
Expand Down
0