bug #41078 [Notifier] Make FailoverTransport always pick the first transport (jschaedl)
This PR was merged into the 5.2 branch.
Discussion
----------
[Notifier] Make FailoverTransport always pick the first transport
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix #40895 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | - <!-- required for new features -->
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
- Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
-->
Commits
-------
337f828b23
Make FailoverTransport always pick the first transport
This commit is contained in:
commit
960891e756
@ -0,0 +1,166 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
/*
|
||||||
|
* This file is part of the Symfony package.
|
||||||
|
*
|
||||||
|
* (c) Fabien Potencier <fabien@symfony.com>
|
||||||
|
*
|
||||||
|
* For the full copyright and license information, please view the LICENSE
|
||||||
|
* file that was distributed with this source code.
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace Symfony\Component\Notifier\Tests\Transport;
|
||||||
|
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Component\Notifier\Exception\LogicException;
|
||||||
|
use Symfony\Component\Notifier\Exception\RuntimeException;
|
||||||
|
use Symfony\Component\Notifier\Exception\TransportExceptionInterface;
|
||||||
|
use Symfony\Component\Notifier\Message\SentMessage;
|
||||||
|
use Symfony\Component\Notifier\Transport\FailoverTransport;
|
||||||
|
use Symfony\Component\Notifier\Transport\TransportInterface;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @group time-sensitive
|
||||||
|
*/
|
||||||
|
class FailoverTransportTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testSendNoTransports()
|
||||||
|
{
|
||||||
|
$this->expectException(LogicException::class);
|
||||||
|
|
||||||
|
new FailoverTransport([]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testToString()
|
||||||
|
{
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->expects($this->once())->method('__toString')->willReturn('t1://local');
|
||||||
|
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->expects($this->once())->method('__toString')->willReturn('t2://local');
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2]);
|
||||||
|
|
||||||
|
$this->assertEquals('t1://local || t2://local', (string) $t);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendMessageNotSupportedByAnyTransport()
|
||||||
|
{
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2]);
|
||||||
|
|
||||||
|
$this->expectException(LogicException::class);
|
||||||
|
|
||||||
|
$t->send(new DummyMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendFirstWork()
|
||||||
|
{
|
||||||
|
$message = new DummyMessage();
|
||||||
|
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t1->expects($this->exactly(3))->method('send')->with($message)->willReturn(new SentMessage($message, 'test'));
|
||||||
|
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->expects($this->never())->method('send');
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2]);
|
||||||
|
|
||||||
|
$t->send($message);
|
||||||
|
$t->send($message);
|
||||||
|
$t->send($message);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendAllDead()
|
||||||
|
{
|
||||||
|
$message = new DummyMessage();
|
||||||
|
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t1->expects($this->once())->method('send')->with($message)->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
|
||||||
|
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t2->expects($this->once())->method('send')->with($message)->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2]);
|
||||||
|
|
||||||
|
$this->expectException(RuntimeException::class);
|
||||||
|
$this->expectExceptionMessage('All transports failed.');
|
||||||
|
|
||||||
|
$t->send($message);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendOneDead()
|
||||||
|
{
|
||||||
|
$message = new DummyMessage();
|
||||||
|
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t1->expects($this->once())->method('send')->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
|
||||||
|
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t2->expects($this->exactly(1))->method('send')->with($message)->willReturn(new SentMessage($message, 'test'));
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2]);
|
||||||
|
|
||||||
|
$t->send($message);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendAllDeadWithinRetryPeriod()
|
||||||
|
{
|
||||||
|
$message = new DummyMessage();
|
||||||
|
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t1->method('send')->will($this->throwException($this->createMock(TransportExceptionInterface::class)));
|
||||||
|
$t1->expects($this->once())->method('send');
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t2->expects($this->exactly(3))
|
||||||
|
->method('send')
|
||||||
|
->willReturnOnConsecutiveCalls(
|
||||||
|
new SentMessage($message, 't2'),
|
||||||
|
new SentMessage($message, 't2'),
|
||||||
|
$this->throwException($this->createMock(TransportExceptionInterface::class))
|
||||||
|
);
|
||||||
|
$t = new FailoverTransport([$t1, $t2], 40);
|
||||||
|
$t->send($message);
|
||||||
|
sleep(4);
|
||||||
|
$t->send($message);
|
||||||
|
sleep(4);
|
||||||
|
|
||||||
|
$this->expectException(RuntimeException::class);
|
||||||
|
$this->expectExceptionMessage('All transports failed.');
|
||||||
|
|
||||||
|
$t->send($message);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSendOneDeadButRecover()
|
||||||
|
{
|
||||||
|
$message = new DummyMessage();
|
||||||
|
|
||||||
|
$t1 = $this->createMock(TransportInterface::class);
|
||||||
|
$t1->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t1->expects($this->exactly(2))->method('send')->willReturnOnConsecutiveCalls(
|
||||||
|
$this->throwException($this->createMock(TransportExceptionInterface::class)),
|
||||||
|
new SentMessage($message, 't1')
|
||||||
|
);
|
||||||
|
$t2 = $this->createMock(TransportInterface::class);
|
||||||
|
$t2->method('supports')->with($message)->willReturn(true);
|
||||||
|
$t2->expects($this->exactly(2))->method('send')->willReturnOnConsecutiveCalls(
|
||||||
|
new SentMessage($message, 't2'),
|
||||||
|
$this->throwException($this->createMock(TransportExceptionInterface::class))
|
||||||
|
);
|
||||||
|
|
||||||
|
$t = new FailoverTransport([$t1, $t2], 1);
|
||||||
|
|
||||||
|
$t->send($message);
|
||||||
|
sleep(2);
|
||||||
|
$t->send($message);
|
||||||
|
}
|
||||||
|
}
|
@ -33,6 +33,11 @@ class FailoverTransport extends RoundRobinTransport
|
|||||||
return $this->currentTransport;
|
return $this->currentTransport;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getInitialCursor(): int
|
||||||
|
{
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
protected function getNameSymbol(): string
|
protected function getNameSymbol(): string
|
||||||
{
|
{
|
||||||
return '||';
|
return '||';
|
||||||
|
@ -29,7 +29,7 @@ class RoundRobinTransport implements TransportInterface
|
|||||||
private $deadTransports;
|
private $deadTransports;
|
||||||
private $transports = [];
|
private $transports = [];
|
||||||
private $retryPeriod;
|
private $retryPeriod;
|
||||||
private $cursor = 0;
|
private $cursor = -1;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param TransportInterface[] $transports
|
* @param TransportInterface[] $transports
|
||||||
@ -43,9 +43,6 @@ class RoundRobinTransport implements TransportInterface
|
|||||||
$this->transports = $transports;
|
$this->transports = $transports;
|
||||||
$this->deadTransports = new \SplObjectStorage();
|
$this->deadTransports = new \SplObjectStorage();
|
||||||
$this->retryPeriod = $retryPeriod;
|
$this->retryPeriod = $retryPeriod;
|
||||||
// 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($transports) - 1);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function __toString(): string
|
public function __toString(): string
|
||||||
@ -66,6 +63,10 @@ class RoundRobinTransport implements TransportInterface
|
|||||||
|
|
||||||
public function send(MessageInterface $message): SentMessage
|
public function send(MessageInterface $message): SentMessage
|
||||||
{
|
{
|
||||||
|
if (!$this->supports($message)) {
|
||||||
|
throw new LogicException(sprintf('None of the configured Transports of "%s" supports the given message.', static::class));
|
||||||
|
}
|
||||||
|
|
||||||
while ($transport = $this->getNextTransport($message)) {
|
while ($transport = $this->getNextTransport($message)) {
|
||||||
try {
|
try {
|
||||||
return $transport->send($message);
|
return $transport->send($message);
|
||||||
@ -82,12 +83,17 @@ class RoundRobinTransport implements TransportInterface
|
|||||||
*/
|
*/
|
||||||
protected function getNextTransport(MessageInterface $message): ?TransportInterface
|
protected function getNextTransport(MessageInterface $message): ?TransportInterface
|
||||||
{
|
{
|
||||||
|
if (-1 === $this->cursor) {
|
||||||
|
$this->cursor = $this->getInitialCursor();
|
||||||
|
}
|
||||||
|
|
||||||
$cursor = $this->cursor;
|
$cursor = $this->cursor;
|
||||||
while (true) {
|
while (true) {
|
||||||
$transport = $this->transports[$cursor];
|
$transport = $this->transports[$cursor];
|
||||||
|
|
||||||
if (!$transport->supports($message)) {
|
if (!$transport->supports($message)) {
|
||||||
$cursor = $this->moveCursor($cursor);
|
$cursor = $this->moveCursor($cursor);
|
||||||
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -116,6 +122,13 @@ class RoundRobinTransport implements TransportInterface
|
|||||||
return $this->deadTransports->contains($transport);
|
return $this->deadTransports->contains($transport);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getInitialCursor(): int
|
||||||
|
{
|
||||||
|
// the cursor initial value is randomized so that
|
||||||
|
// when are not in a daemon, we are still rotating the transports
|
||||||
|
return mt_rand(0, \count($this->transports) - 1);
|
||||||
|
}
|
||||||
|
|
||||||
protected function getNameSymbol(): string
|
protected function getNameSymbol(): string
|
||||||
{
|
{
|
||||||
return '&&';
|
return '&&';
|
||||||
|
Reference in New Issue
Block a user