8000 [Mailer] Randomize the first transport used by the RoundRobin transport · symfony/symfony@6ebe83c · GitHub
[go: up one dir, main page]
Skip to content

Commit 6ebe83c

Browse files
fabpotfabpot
committed
[Mailer] Randomize the first transport used by the RoundRobin transport
1 parent 75fc3fa commit 6ebe83c

File tree

3 files changed

+39
-7
lines changed

3 files changed

+39
-7
lines changed

‎src/Symfony/Component/Mailer/Tests/Transport/FailoverTransportTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ public function testSendFirstWork()
4646
$t2 = $this->createMock(TransportInterface::class);
4747
$t2->expects($this->never())->method('send');
4848
$t = new FailoverTransport([$t1, $t2]);
49+
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
50+
$p->setAccessible(true);
51+
$p->setValue($t, 0);
4952
$t->send(new RawMessage(''));
5053
$this->assertTransports($t, 1, []);
5154
$t->send(new RawMessage(''));
@@ -74,6 +77,9 @@ public function testSendOneDead()
7477
$t2 = $this->createMock(TransportInterface::class);
7578
$t2->expects($this->exactly(3))->method('send');
7679
$t = new FailoverTransport([$t1, $t2]);
80+
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
81+
$p->setAccessible(true);
82+
$p->setValue($t, 0);
7783
$t->send(new RawMessage(''));
7884
$this->assertTransports($t, 0, [$t1]);
7985
$t->send(new RawMessage(''));
@@ -93,6 +99,9 @@ public function testSendOneDeadAndRecoveryWithinRetryPeriod()
9399
$t2->expects($this->at(2))->method('send');
94100
$t2->expects($this->at(3))->method('send')->will($this->throwException(new TransportException()));
95101
$t = new FailoverTransport([$t1, $t2], 6);
102+
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
103+
$p->setAccessible(true);
104+
$p->setValue($t, 0);
96105
$t->send(new RawMessage('')); // t1>fail - t2>sent
97106
$this->assertTransports($t, 0, [$t1]);
98107
sleep(4);
@@ -139,6 +148,9 @@ public function testSendOneDeadButRecover()
139148
$t2->expects($this->at(1))->method('send');
140149
$t2->expects($this->at(2))->method('send')->will($this->throwException(new TransportException()));
141150
$t = new FailoverTransport([$t1, $t2], 1);
151+
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
152+
$p->setAccessible(true);
153+
$p->setValue($t, 0);
142154
$t->send(new RawMessage(''));
143155
sleep(1);
144156
$t->send(new RawMessage(''));

‎src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,16 @@ public function testToString()
4141
public function testSendAlternate()
4242
{
4343
$t1 = $this->createMock(TransportInterface::class);
44-
$t1->expects($this->exactly(2))->method('send');
44+
$t1->expects($this->atLeast(1))->method('send');
4545
$t2 = $this->createMock(TransportInterface::class);
46-
$t2->expects($this->once())->method('send');
46+
$t2->expects($this->atLeast(1))->method('send');
4747
$t = new RoundRobinTransport([$t1, $t2]);
4848
$t->send(new RawMessage(''));
49-
$this->assertTransports($t, 1, []);
49+
$cursor = $this->assertTransports($t, -1, []);
5050
$t->send(new RawMessage(''));
51-
$this->assertTransports($t, 0, []);
51+
$cursor = $this->assertTransports($t, 0 === $cursor ? 1 : 0, []);
5252
$t->send(new RawMessage(''));
53-
$this->assertTransports($t, 1, []);
53+
$this->assertTransports($t, 0 === $cursor ? 1 : 0, []);
5454
}
5555

5656
public function testSendAllDead()
@@ -73,6 +73,9 @@ public function testSendOneDead()
7373
$t2 = $this->createMock(TransportInterface::class);
7474
$t2->expects($this->exactly(3))->method('send');
7575
$t = new RoundRobinTransport([$t1, $t2]);
76+
$p = new \ReflectionProperty($t, 'cursor');
77+
$p->setAccessible(true);
78+
$p->setValue($t, 0);
7679
$t->send(new RawMessage(''));
7780
$this->assertTransports($t, 0, [$t1]);
7881
$t->send(new RawMessage(''));
@@ -88,6 +91,9 @@ public function testSendOneDeadAndRecoveryNotWithinRetryPeriod()
8891
$t2 = $this->createMock(TransportInterface::class);
8992
$t2->expects($this->once())->method('send')->will($this->throwException(new TransportException()));
9093
$t = new RoundRobinTransport([$t1, $t2], 60);
94+
$p = new \ReflectionProperty($t, 'cursor');
95+
$p->setAccessible(true);
96+
$p->setValue($t, 0);
9197
$t->send(new RawMessage(''));
9298
$this->assertTransports($t, 1, []);
9399
$t->send(new RawMessage(''));
@@ -106,6 +112,9 @@ public function testSendOneDeadAndRecoveryWithinRetryPeriod()
106112
$t2->expects($this->at(0))->method('send')->will($this->throwException(new TransportException()));
107113
$t2->expects($this->at(1))->method('send');
108114
$t = new RoundRobinTransport([$t1, $t2], 3);
115+
$p = new \ReflectionProperty($t, 'cursor');
116+
$p->setAccessible(true);
117+
$p->setValue($t, 0);
109118
$t->send(new RawMessage(''));
110119
$this->assertTransports($t, 1, []);
111120
$t->send(new RawMessage(''));
@@ -121,10 +130,15 @@ private function assertTransports(RoundRobinTransport $transport, int $cursor, a
121130
{
122131
$p = new \ReflectionProperty($transport, 'cursor');
123132
$p->setAccessible(true);
124-
$this->assertSame($cursor, $p->getValue($transport));
133+
if (-1 !== $cursor) {
134+
$this->assertSame($cursor, $p->getValue($transport));
135+
}
136+
$cursor = $p->getValue($transport);
125137

126138
$p = new \ReflectionProperty($transport, 'deadTransports');
127139
$p->setAccessible(true);
128140
$this->assertSame($deadTransports, iterator_to_array($p->getValue($transport)));
141+
142+
return $cursor;
129143
}
130144
}

‎src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class RoundRobinTransport implements TransportInterface
2727
private $deadTransports;
2828
private $transports = [];
2929
private $retryPeriod;
30-
private $cursor = 0;
30+
private $cursor = -1;
3131

3232
/**
3333
* @param TransportInterface[] $transports
@@ -66,6 +66,12 @@ public function __toString(): string
6666
*/
6767
protected function getNextTransport(): ?TransportInterface
6868
{
69+
if (-1 === $this->cursor) {
70+
// the cursor initial value is randomized so that
71+
// when are not in a daemon, we are still rotating the transports
72+
$this->cursor = mt_rand(0, \count($this->transports) - 1);
73+
}
74+
6975
$cursor = $this->cursor;
7076
while (true) {
7177
$transport = $this->transports[$cursor];

0 commit comments

Comments
 (0)
0