10000 [Mailer] Randomize the first transport used by the RoundRobin transport by fabpot · Pull Request #35525 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

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

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

Merged
merged 1 commit into from
Jan 31, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public function testSendFirstWork()
$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(''));
Expand Down Expand Up @@ -74,6 +77,9 @@ public function testSendOneDead()
$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(''));
Expand All @@ -93,6 +99,9 @@ public function testSendOneDeadAndRecoveryWithinRetryPeriod()
$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);
Expand Down Expand Up @@ -139,6 +148,9 @@ public function testSendOneDeadButRecover()
$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(''));
Expand Down
26 changes: 20 additions & 6 deletions src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,16 @@ public function testToString()
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()
Expand All @@ -73,6 +73,9 @@ public function testSendOneDead()
$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(''));
Expand All @@ -88,6 +91,9 @@ public function testSendOneDeadAndRecoveryNotWithinRetryPeriod()
$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(''));
Expand All @@ -106,6 +112,9 @@ public function testSendOneDeadAndRecoveryWithinRetryPeriod()
$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(''));
Expand All @@ -121,10 +130,15 @@ private function assertTransports(RoundRobinTransport $transport, int $cursor, a
{
$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;
}
}
8 changes: 7 additions & 1 deletion src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class RoundRobinTransport implements TransportInterface
private $deadTransports;
private $transports = [];
private $retryPeriod;
private $cursor = 0;
private $cursor = -1;

/**
* @param TransportInterface[] $transports
Expand Down Expand Up @@ -66,6 +66,12 @@ public function __toString(): string
*/
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];
Expand Down
0