Skip to content

Commit a941aa3

Browse files
fabpotfabpot
committed
feature #33494 [Mailer] Change DSN syntax (fabpot)
This PR was merged into the 4.4 branch. Discussion ---------- [Mailer] Change DSN syntax | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | n/a | License | MIT | Doc PR | - The current syntax for failover and roundrobin is confusing. `&&` and `||` do not really convey the right meaning. I realized that while working on a new transport that will send on more than one transport in parallel. `&&` would be a natural fit, but that's already taken. So, this pull request changes the syntax to be more explicit. Commits ------- 39dd213960 [Mailer] Change the syntax for DSNs using failover or roundrobin
2 parents 332dfc2 + 518f4ac commit a941aa3

12 files changed

+137
-39
lines changed

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,18 @@ CHANGELOG
44
4.4.0
55
-----
66

7+
* [BC BREAK] changed the syntax for failover and roundrobin DSNs
8+
9+
Before:
10+
11+
dummy://a || dummy://b (for failover)
12+
dummy://a && dummy://b (for roundrobin)
13+
14+
After:
15+
16+
failover(dummy://a dummy://b)
17+
roundrobin(dummy://a dummy://b)
18+
719
* added support for multiple transports on a `Mailer` instance
820
* [BC BREAK] removed the `auth_mode` DSN option (it is now always determined automatically)
921
* STARTTLS cannot be enabled anymore (it is used automatically if TLS is disabled and the server supports STARTTLS)

‎FailoverTransport.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ protected function getNextTransport(): ?TransportInterface
3131

3232
protected function getNameSymbol(): string
3333
{
34-
return '||';
34+
return 'failover';
3535
}
3636
}

‎FailoverTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function testToString()
3636
$t2 = $this->createMock(TransportInterface::class);
3737
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
3838
$t = new FailoverTransport([$t1, $t2]);
39-
$this->assertEquals('t1://local || t2://local', (string) $t);
39+
$this->assertEquals('failover(t1://local t2://local)', (string) $t);
4040
}
4141

4242
public function testSendFirstWork()

‎RoundRobinTransport.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentM
5858

5959
public function __toString(): string
6060
{
61-
return implode(' '.$this->getNameSymbol().' ', array_map(function (TransportInterface $transport) {
61+
return $this->getNameSymbol().'('.implode(' ', array_map(function (TransportInterface $transport) {
6262
return (string) $transport;
63-
}, $this->transports));
63+
}, $this->transports)).')';
6464
}
6565

6666
/**
@@ -99,7 +99,7 @@ protected function isTransportDead(TransportInterface $transport): bool
9999

100100
protected function getNameSymbol(): string
101101
{
102-
return '&&';
102+
return 'roundrobin';
103103
}
104104

105105
private function moveCursor(int $cursor): int

‎RoundRobinTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function testToString()
3535
$t2 = $this->createMock(TransportInterface::class);
3636
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
3737
$t = new RoundRobinTransport([$t1, $t2]);
38-
$this->assertEquals('t1://local && t2://local', (string) $t);
38+
$this->assertEquals('roundrobin(t1://local t2://local)', (string) $t);
3939
}
4040

4141
public function testSendAlternate()

‎Tests/Transport/FailoverTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function testToString()
3636
$t2 = $this->createMock(TransportInterface::class);
3737
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
3838
$t = new FailoverTransport([$t1, $t2]);
39-
$this->assertEquals('t1://local || t2://local', (string) $t);
39+
$this->assertEquals('failover(t1://local t2://local)', (string) $t);
4040
}
4141

4242
public function testSendFirstWork()

‎Tests/Transport/RoundRobinTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function testToString()
3535
$t2 = $this->createMock(TransportInterface::class);
3636
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
3737
$t = new RoundRobinTransport([$t1, $t2]);
38-
$this->assertEquals('t1://local && t2://local', (string) $t);
38+
$this->assertEquals('roundrobin(t1://local t2://local)', (string) $t);
3939
}
4040

4141
public function testSendAlternate()

‎Tests/TransportTest.php‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Mailer\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Mailer\Exception\InvalidArgumentException;
1516
use Symfony\Component\Mailer\SentMessage;
1617
use Symfony\Component\Mailer\SmtpEnvelope;
1718
use Symfony\Component\Mailer\Transport;
@@ -44,14 +45,42 @@ public function fromStringProvider(): iterable
4445
];
4546

4647
yield 'failover transport' => [
47-
'dummy://a || dummy://b',
48+
'failover(dummy://a dummy://b)',
4849
new FailoverTransport([$transportA, $transportB]),
4950
];
5051

5152
yield 'round robin transport' => [
52-
'dummy://a && dummy://b',
53+
'roundrobin(dummy://a dummy://b)',
5354
new RoundRobinTransport([$transportA, $transportB]),
5455
];
56+
57+
yield 'mixed transport' => [
58+
'roundrobin(dummy://a failover(dummy://b dummy://a) dummy://b)',
59+
new RoundRobinTransport([$transportA, new FailoverTransport([$transportB, $transportA]), $transportB]),
60+
];
61+
}
62+
63+
/**
64+
* @dataProvider fromWrongStringProvider
65+
*/
66+
public function testFromWrongString(string $dsn, string $error): void
67+
{
68+
$transportFactory = new Transport([new DummyTransportFactory()]);
69+
70+
$this->expectExceptionMessage($error);
71+
$this->expectException(InvalidArgumentException::class);
72+
$transportFactory->fromString($dsn);
73+
}
74+
75+
public function fromWrongStringProvider(): iterable
76+
{
77+
yield 'garbage at the end' => ['dummy://a some garbage here', 'The DSN has some garbage at the end: some garbage here.'];
78+
79+
yield 'not a valid DSN' => ['something not a dsn', 'The "something" mailer DSN must contain a scheme.'];
80+
81+
yield 'failover not closed' => ['failover(dummy://a', 'The "(dummy://a" mailer DSN must contain a scheme.'];
82+
83+
yield 'not a valid keyword' => ['foobar(dummy://a)', 'The "foobar" keyword is not valid (valid ones are "failover", "roundrobin")'];
5584
}
5685
}
5786

‎Transport.php‎

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Mailer\Bridge\Mailgun\Transport\MailgunTransportFactory;
1919
use Symfony\Component\Mailer\Bridge\Postmark\Transport\PostmarkTransportFactory;
2020
use Symfony\Component\Mailer\Bridge\Sendgrid\Transport\SendgridTransportFactory;
21+
use Symfony\Component\Mailer\Exception\InvalidArgumentException;
2122
use Symfony\Component\Mailer\Exception\UnsupportedHostException;
2223
use Symfony\Component\Mailer\Transport\Dsn;
2324
use Symfony\Component\Mailer\Transport\FailoverTransport;
@@ -82,17 +83,59 @@ public function fromStrings(array $dsns): Transports
8283

8384
public function fromString(string $dsn): TransportInterface
8485
{
85-
$dsns = preg_split('/\s++\|\|\s++/', $dsn);
86-
if (\count($dsns) > 1) {
87-
return new FailoverTransport($this->createFromDsns($dsns));
86+
list($transport, $offset) = $this->parseDsn($dsn);
87+
if ($offset !== \strlen($dsn)) {
88+
throw new InvalidArgumentException(sprintf('The DSN has some garbage at the end: %s.', substr($dsn, $offset)));
8889
}
8990

90-
$dsns = preg_split('/\s++&&\s++/', $dsn);
91-
if (\count($dsns) > 1) {
92-
return new RoundRobinTransport($this->createFromDsns($dsns));
93-
}
91+
return $transport;
92+
}
93+
94+
private function parseDsn(string $dsn, int $offset = 0): array
95+
{
96+
static $keywords = [
97+
'failover' => FailoverTransport::class,
98+
'roundrobin' => RoundRobinTransport::class,
99+
];
100+
101+
while (true) {
102+
foreach ($keywords as $name => $class) {
103+
$name .= '(';
104+
if ($name === substr($dsn, $offset, \strlen($name))) {
105+
$offset += \strlen($name) - 1;
106+
preg_match('{\(([^()]|(?R))*\)}A', $dsn, $matches, 0, $offset);
107+
if (!isset($matches[0])) {
108+
continue;
109+
}
110+
111+
++$offset;
112+
$args = [];
113+
while (true) {
114+
list($arg, $offset) = $this->parseDsn($dsn, $offset);
115+
$args[] = $arg;
116+
if (\strlen($dsn) === $offset) {
117+
break;
118+
}
119+
++$offset;
120+
if (')' === $dsn[$offset - 1]) {
121+
break;
122+
}
123+
}
124+
125+
return [new $class($args), $offset];
126+
}
127+
}
128+
129+
if (preg_match('{(\w+)\(}A', $dsn, $matches, 0, $offset)) {
130+
throw new InvalidArgumentException(sprintf('The "%s" keyword is not valid (valid ones are "%s"), ', $matches[1], implode('", "', array_keys($keywords))));
131+
}
132+
133+
if ($pos = strcspn($dsn, ' )', $offset)) {
134+
return [$this->fromDsnObject(Dsn::fromString(substr($dsn, $offset, $pos))), $offset + $pos];
135+
}
94136

95-
return $this->fromDsnObject(Dsn::fromString($dsn));
137+
return [$this->fromDsnObject(Dsn::fromString(substr($dsn, $offset))), \strlen($dsn)];
138+
}
96139
}
97140

98141
public function fromDsnObject(Dsn $dsn): TransportInterface
@@ -106,21 +149,6 @@ public function fromDsnObject(Dsn $dsn): TransportInterface
106149
throw new UnsupportedHostException($dsn);
107150
}
108151

109-
/**
110-
* @param string[] $dsns
111-
*
112-
* @return TransportInterface[]
113-
*/
114-
private function createFromDsns(array $dsns): array
115-
{
116-
$transports = [];
117-
foreach ($dsns as $dsn) {
118-
$transports[] = $this->fromDsnObject(Dsn::fromString($dsn));
119-
}
120-
121-
return $transports;
122-
}
123-
124152
private static function getDefaultFactories(EventDispatcherInterface $dispatcher = null, HttpClientInterface $client = null, LoggerInterface $logger = null): iterable
125153
{
126154
foreach (self::FACTORY_CLASSES as $factoryClass) {

‎Transport/FailoverTransport.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ protected function getNextTransport(): ?TransportInterface
3131

3232
protected function getNameSymbol(): string
3333
{
34-
return '||';
34+
return 'failover';
3535
}
3636
}

0 commit comments

Comments
 (0)