8000 feature #35525 [Mailer] Randomize the first transport used by the Rou… · symfony/symfony@09bdaf5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 09bdaf5

Browse files
fabpotfabpot
committed
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 ------- 6ebe83c [Mailer] Randomize the first transport used by the RoundRobin transport
2 parents 1bb485b + 6ebe83c commit 09bdaf5

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