From 6ebe83c14e491b2bd1642af9b6552ec8776ce7ee Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 30 Jan 2020 16:02:47 +0100 Subject: [PATCH] [Mailer] Randomize the first transport used by the RoundRobin transport --- .../Tests/Transport/FailoverTransportTest.php | 12 +++++++++ .../Transport/RoundRobinTransportTest.php | 26 ++++++++++++++----- .../Mailer/Transport/RoundRobinTransport.php | 8 +++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php b/src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php index 7e66309cab..c22a3bfaf3 100644 --- a/src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php +++ b/src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php @@ -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('')); diff --git a/src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php b/src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php index add578b233..2a8f49d4ac 100644 --- a/src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php +++ b/src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php @@ -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; } } diff --git a/src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php b/src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php index 37128dda62..873ab58481 100644 --- a/src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php +++ b/src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php @@ -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];