Skip to content

Commit 2074129

Browse files
jschaedljschaedl
committed
Make FailoverTransport always pick the first transport
1 parent bdb8702 commit 2074129

File tree

3 files changed

+188
-4
lines changed

3 files changed

+188
-4
lines changed

‎Tests/Transport/FailoverTransportTest.php

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Notifier\Tests\Transport;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Notifier\Exception\LogicException;
16+
use Symfony\Component\Notifier\Exception\RuntimeException;
17+
use Symfony\Component\Notifier\Exception\TransportExceptionInterface;
18+
use Symfony\Component\Notifier\Message\SentMessage;
19+
use Symfony\Component\Notifier\Transport\FailoverTransport;
20+
use Symfony\Component\Notifier\Transport\TransportInterface;
21+
22+
/**
23+
* @group time-sensitive
24+
*/
25+
class FailoverTransportTest extends TestCase
26+
{
27+
public function testSendNoTransports()
28+
{
29+
$this->expectException(LogicException::class);
30+
31+
new FailoverTransport([]);
32+
}
33+
34+
public function testToString()
35+
{
36+
$t1 = $this->createMock(TransportInterface::class);
37+
$t1->expects($this->once())->method('__toString')->willReturn('t1://local');
38+
39+
$t2 = $this->createMock(TransportInterface::class);
40+
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
41+
42+
$t = new FailoverTransport([$t1, $t2]);
43+
44+
$this->assertEquals('t1://local || t2://local', (string) $t);
45+
}
46+
47+
public function testSendMessageNotSupportedByAnyTransport()
48+
{
49+
$t1 = $this->createMock(TransportInterface::class);
50+
$t2 = $this->createMock(TransportInterface::class);
51+
52+
$t = new FailoverTransport([$t1, $t2]);
53+
54+
$this->expectException(LogicException::class);
55+
56+
$t->send(new DummyMessage());
57+
}
58+
59+
public function testSendFirstWork()
60+
{
61+
$message = new DummyMessage();
62+
63+
$t1 = $this->createMock(TransportInterface::class);
64+
$t1->method('supports')->with($message)->willReturn(true);
65+
$t1->expects($this->exactly(3))->method('send')->with($message)->willReturn(new SentMessage($message, 'test'));
66+
67+
$t2 = $this->createMock(TransportInterface::class);
68+
$t2->expects($this->never())->method('send');
69+
70+
$t = new FailoverTransport([$t1, $t2]);
71+
72+
$t->send($message);
73+
$t->send($message);
74+
$t->send($message);
75+
}
76+
77+
public function testSendAllDead()
78+
{
79+
$message = new DummyMessage();
80+
81+
$t1 = $this->createMock(TransportInterface::class);
82+
$t1->method('supports')->with($message)->willReturn(true);
83+
$t1->expects($this->once())->method('send')->with($message)->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
84+
85+
$t2 = $this->createMock(TransportInterface::class);
86+
$t2->method('supports')->with($message)->willReturn(true);
87+
$t2->expects($this->once())->method('send')->with($message)->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
88+
89+
$t = new FailoverTransport([$t1, $t2]);
90+
91+
$this->expectException(RuntimeException::class);
92+
$this->expectExceptionMessage('All transports failed.');
93+
94+
$t->send($message);
95+
}
96+
97+
public function testSendOneDead()
98+
{
99+
$message = new DummyMessage();
100+
101+
$t1 = $this->createMock(TransportInterface::class);
102+
$t1->method('supports')->with($message)->willReturn(true);
103+
$t1->expects($this->once())->method('send')->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
104+
105+
$t2 = $this->createMock(TransportInterface::class);
106+
$t2->method('supports')->with($message)->willReturn(true);
107+
$t2->expects($this->exactly(1))->method('send')->with($message)->willReturn(new SentMessage($message, 'test'));
108+
109+
$t = new FailoverTransport([$t1, $t2]);
110+
111+
$t->send($message);
112+
}
113+
114+
public function testSendAllDeadWithinRetryPeriod()
115+
{
116+
$message = new DummyMessage();
117+
118+
$t1 = $this->createMock(TransportInterface::class);
119+
$t1->method('supports')->with($message)->willReturn(true);
120+
$t1->method('send')->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
121+
$t1->expects($this->once())->method('send');
122+
$t2 = $this->createMock(TransportInterface::class);
123+
$t2->method('supports')->with($message)->willReturn(true);
124+
$t2->expects($this->exactly(3))
125+
->method('send')
126+
->willReturnOnConsecutiveCalls(
127+
new SentMessage($message, 't2'),
128+
new SentMessage($message, 't2'),
129+
$this->throwException($this->createMock(TransportExceptionInterface::class))
130+
);
131+
$t = new FailoverTransport([$t1, $t2], 40);
132+
$t->send($message);
133+
sleep(4);
134+
$t->send($message);
135+
sleep(4);
136+
137+
$this->expectException(RuntimeException::class);
138+
$this->expectExceptionMessage('All transports failed.');
139+
140+
$t->send($message);
141+
}
142+
143+
public function testSendOneDeadButRecover()
144+
{
145+
$message = new DummyMessage();
146+
147+
$t1 = $this->createMock(TransportInterface::class);
148+
$t1->method('supports')->with($message)->willReturn(true);
149+
$t1->expects($this->exactly(2))->method('send')->willReturnOnConsecutiveCalls(
150+
$this->throwException($this->createMock(TransportExceptionInterface::class)),
151+
new SentMessage($message, 't1')
152+
);
153+
$t2 = $this->createMock(TransportInterface::class);
154+
$t2->method('supports')->with($message)->willReturn(true);
155+
$t2->expects($this->exactly(2))->method('send')->willReturnOnConsecutiveCalls(
156+
new SentMessage($message, 't2'),
157+
$this->throwException($this->createMock(TransportExceptionInterface::class))
158+
);
159+
160+
$t = new FailoverTransport([$t1, $t2], 1);
161+
162+
$t->send($message);
163+
sleep(2);
164+
$t->send($message);
165+
}
166+
}

‎Transport/FailoverTransport.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ protected function getNextTransport(MessageInterface $message): ?TransportInterf
3333
return $this->currentTransport;
3434
}
3535

36+
protected function getInitialCursor(): int
37+
{
38+
return 0;
39+
}
40+
3641
protected function getNameSymbol(): string
3742
{
3843
return '||';

‎Transport/RoundRobinTransport.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class RoundRobinTransport implements TransportInterface
2929
private $deadTransports;
3030
private $transports = [];
3131
private $retryPeriod;
32-
private $cursor = 0;
32+
private $cursor = -1;
3333

3434
/**
3535
* @param TransportInterface[] $transports
@@ -43,9 +43,6 @@ public function __construct(array $transports, int $retryPeriod = 60)
4343
$this->transports = $transports;
4444
$this->deadTransports = new \SplObjectStorage();
4545
$this->retryPeriod = $retryPeriod;
46-
// the cursor initial value is randomized so that
47-
// when are not in a daemon, we are still rotating the transports
48-
$this->cursor = mt_rand(0, \count($transports) - 1);
4946
}
5047

5148
public function __toString(): string
@@ -66,6 +63,10 @@ public function supports(MessageInterface $message): bool
6663

6764
public function send(MessageInterface $message): SentMessage
6865
{
66+
if (!$this->supports($message)) {
67+
throw new LogicException(sprintf('None of the configured Transports of "%s" supports the given message.', static::class));
68+
}
69+
6970
while ($transport = $this->getNextTransport($message)) {
7071
try {
7172
return $transport->send($message);
@@ -82,12 +83,17 @@ public function send(MessageInterface $message): SentMessage
8283
*/
8384
protected function getNextTransport(MessageInterface $message): ?TransportInterface
8485
{
86+
if (-1 === $this->cursor) {
87+
$this->cursor = $this->getInitialCursor();
88+
}
89+
8590
$cursor = $this->cursor;
8691
while (true) {
8792
$transport = $this->transports[$cursor];
8893

8994
if (!$transport->supports($message)) {
9095
$cursor = $this->moveCursor($cursor);
96+
9197
continue;
9298
}
9399

@@ -116,6 +122,13 @@ protected function isTransportDead(TransportInterface $transport): bool
116122
return $this->deadTransports->contains($transport);
117123
}
118124

125+
protected function getInitialCursor(): int
126+
{
127+
// the cursor initial value is randomized so that
128+
// when are not in a daemon, we are still rotating the transports
129+
return mt_rand(0, \count($this->transports) - 1);
130+
}
131+
119132
protected function getNameSymbol(): string
120133
{
121134
return '&&';

0 commit comments

Comments
 (0)