feature #35525 [Mailer] Randomize the first transport used by the RoundRobin transport (fabpot)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Mailer] Randomize the first transport used by the RoundRobin transport

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #33723 <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

When not using Messenger, and so sending only one message, the RoundRobin class does not work as the first transport is always used. This PR randomizes the first transport used by the class to mitigate that problem.

Commits
-------

6ebe83c14e [Mailer] Randomize the first transport used by the RoundRobin transport
This commit is contained in:
Fabien Potencier 2020-01-31 09:29:43 +01:00
commit 09bdaf5553
3 changed files with 39 additions and 7 deletions

View File

@ -46,6 +46,9 @@ class FailoverTransportTest extends TestCase
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->never())->method('send');
$t = new FailoverTransport([$t1, $t2]);
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage(''));
@ -74,6 +77,9 @@ class FailoverTransportTest extends TestCase
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->exactly(3))->method('send');
$t = new FailoverTransport([$t1, $t2]);
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage(''));
@ -93,6 +99,9 @@ class FailoverTransportTest extends TestCase
$t2->expects($this->at(2))->method('send');
$t2->expects($this->at(3))->method('send')->will($this->throwException(new TransportException()));
$t = new FailoverTransport([$t1, $t2], 6);
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage('')); // t1>fail - t2>sent
$this->assertTransports($t, 0, [$t1]);
sleep(4);
@ -139,6 +148,9 @@ class FailoverTransportTest extends TestCase
$t2->expects($this->at(1))->method('send');
$t2->expects($this->at(2))->method('send')->will($this->throwException(new TransportException()));
$t = new FailoverTransport([$t1, $t2], 1);
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
sleep(1);
$t->send(new RawMessage(''));

View File

@ -41,16 +41,16 @@ class RoundRobinTransportTest extends TestCase
public function testSendAlternate()
{
$t1 = $this->createMock(TransportInterface::class);
$t1->expects($this->exactly(2))->method('send');
$t1->expects($this->atLeast(1))->method('send');
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->once())->method('send');
$t2->expects($this->atLeast(1))->method('send');
$t = new RoundRobinTransport([$t1, $t2]);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$cursor = $this->assertTransports($t, -1, []);
$t->send(new RawMessage(''));
$this->assertTransports($t, 0, []);
$cursor = $this->assertTransports($t, 0 === $cursor ? 1 : 0, []);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$this->assertTransports($t, 0 === $cursor ? 1 : 0, []);
}
public function testSendAllDead()
@ -73,6 +73,9 @@ class RoundRobinTransportTest extends TestCase
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->exactly(3))->method('send');
$t = new RoundRobinTransport([$t1, $t2]);
$p = new \ReflectionProperty($t, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage(''));
@ -88,6 +91,9 @@ class RoundRobinTransportTest extends TestCase
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->once())->method('send')->will($this->throwException(new TransportException()));
$t = new RoundRobinTransport([$t1, $t2], 60);
$p = new \ReflectionProperty($t, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage(''));
@ -106,6 +112,9 @@ class RoundRobinTransportTest extends TestCase
$t2->expects($this->at(0))->method('send')->will($this->throwException(new TransportException()));
$t2->expects($this->at(1))->method('send');
$t = new RoundRobinTransport([$t1, $t2], 3);
$p = new \ReflectionProperty($t, 'cursor');
$p->setAccessible(true);
$p->setValue($t, 0);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage(''));
@ -121,10 +130,15 @@ class RoundRobinTransportTest extends TestCase
{
$p = new \ReflectionProperty($transport, 'cursor');
$p->setAccessible(true);
$this->assertSame($cursor, $p->getValue($transport));
if (-1 !== $cursor) {
$this->assertSame($cursor, $p->getValue($transport));
}
$cursor = $p->getValue($transport);
$p = new \ReflectionProperty($transport, 'deadTransports');
$p->setAccessible(true);
$this->assertSame($deadTransports, iterator_to_array($p->getValue($transport)));
return $cursor;
}
}

View File

@ -27,7 +27,7 @@ class RoundRobinTransport implements TransportInterface
private $deadTransports;
private $transports = [];
private $retryPeriod;
private $cursor = 0;
private $cursor = -1;
/**
* @param TransportInterface[] $transports
@ -66,6 +66,12 @@ class RoundRobinTransport implements TransportInterface
*/
protected function getNextTransport(): ?TransportInterface
{
if (-1 === $this->cursor) {
// the cursor initial value is randomized so that
// when are not in a daemon, we are still rotating the transports
$this->cursor = mt_rand(0, \count($this->transports) - 1);
}
$cursor = $this->cursor;
while (true) {
$transport = $this->transports[$cursor];