Skip to content

[Messenger] remove send_and_handle which can be achieved with SyncTransport #31454

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
May 11, 2019
Merged

[Messenger] remove send_and_handle which can be achieved with SyncTransport #31454

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
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ CHANGELOG
`framework.messenger.default_serializer`, which holds the string service
id and `framework.messenger.symfony_serializer`, which configures the
options if you're using Symfony's serializer.
* [BC Break] Removed the `framework.messenger.routing.send_and_handle` configuration.
Instead of setting it to true, configure a `SyncTransport` and route messages to it.
* Added information about deprecated aliases in `debug:autowiring`
* Added php ini session options `sid_length` and `sid_bits_per_character`
to the `session` section of the configuration
Expand Down
3 changes: 0 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,6 @@ private function addMessengerSection(ArrayNodeDefinition $rootNode)
if (!\is_int($k)) {
$newConfig[$k] = [
'senders' => $v['senders'] ?? (\is_array($v) ? array_values($v) : [$v]),
'send_and_handle' => $v['send_and_handle'] ?? false,
];
} else {
$newConfig[$v['message-class']]['senders'] = array_map(
Expand All @@ -1134,7 +1133,6 @@ function ($a) {
},
array_values($v['sender'])
);
$newConfig[$v['message-class']]['send-and-handle'] = $v['send-and-handle'] ?? false;
}
}

Expand All @@ -1147,7 +1145,6 @@ function ($a) {
->requiresAtLeastOneElement()
->prototype('scalar')->end()
->end()
->booleanNode('send_and_handle')->defaultFalse()->end()
->end()
->end()
->end()
Expand Down
3 changes: 0 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1744,7 +1744,6 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
}

$messageToSendersMapping = [];
$messagesToSendAndHandle = [];
foreach ($config['routing'] as $message => $messageConfiguration) {
if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) {
throw new LogicException(sprintf('Invalid Messenger routing configuration: class or interface "%s" not found.', $message));
Expand All @@ -1758,7 +1757,6 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
}

$messageToSendersMapping[$message] = $messageConfiguration['senders'];
$messagesToSendAndHandle[$message] = $messageConfiguration['send_and_handle'];
}

$senderReferences = [];
Expand All @@ -1769,7 +1767,6 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
$container->getDefinition('messenger.senders_locator')
->replaceArgument(0, $messageToSendersMapping)
->replaceArgument(1, ServiceLocatorTagPass::register($container, $senderReferences))
->replaceArgument(2, $messagesToSendAndHandle)
;

$container->getDefinition('messenger.retry_strategy_locator')
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
<service id="messenger.senders_locator" class="Symfony\Component\Messenger\Transport\Sender\SendersLocator">
<argument type="collection" /> <!-- Per message senders map -->
<argument /> <!-- senders locator -->
<argument type="collection" /> <!-- Messages to send and handle -->
</service>
<service id="messenger.middleware.send_message" class="Symfony\Component\Messenger\Middleware\SendMessageMiddleware">
<tag name="monolog.logger" channel="messenger" />
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@
<xsd:element name="sender" type="messenger_routing_sender" />
</xsd:choice>
<xsd:attribute name="message-class" type="xsd:string" use="required"/>
<xsd:attribute name="send-and-handle" type="xsd:boolean" default="false"/>
</xsd:complexType>

<xsd:complexType name="messenger_routing_sender">
Expand Down
1 change: 0 additions & 1 deletion ...mfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage' => ['amqp', 'audit'],
'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage' => [
'senders' => ['amqp', 'audit'],
'send_and_handle' => true,
],
'*' => 'amqp',
],
Expand Down
2 changes: 1 addition & 1 deletion ...mfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<framework:sender service="amqp" />
<framework:sender service="audit" />
</framework:routing>
<framework:routing message-class="Symfony\Component\Messenger\Tests\Fixtures\SecondMessage" send-and-handle="true">
<framework:routing message-class="Symfony\Component\Messenger\Tests\Fixtures\SecondMessage">
<framework:sender service="amqp" />
<framework:sender service="audit" />
</framework:routing>
Expand Down
1 change: 0 additions & 1 deletion ...mfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ framework:
'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage': [amqp, audit]
'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage':
senders: [amqp, audit]
send_and_handle: true
'*': amqp
transports:
amqp: 'amqp://localhost/%2f/messages'
8 changes: 0 additions & 8 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
use Symfony\Component\HttpClient\ScopingHttpClient;
use Symfony\Component\HttpKernel\DependencyInjection\LoggerPass;
use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
use Symfony\Component\Messenger\Tests\Fixtures\SecondMessage;
use Symfony\Component\Messenger\Transport\TransportFactory;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
Expand Down Expand Up @@ -715,13 +714,6 @@ public function testMessengerRouting()
$container = $this->createContainerFromFile('messenger_routing');
$senderLocatorDefinition = $container->getDefinition('messenger.senders_locator');

$messageToSendAndHandleMapping = [
DummyMessage::class => false,
SecondMessage::class => true,
'*' => false,
];

$this->assertSame($messageToSendAndHandleMapping, $senderLocatorDefinition->getArgument(2));
$sendersMapping = $senderLocatorDefinition->getArgument(0);
$this->assertEquals([
'amqp',
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Messenger/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* [BC BREAK] `SendersLocatorInterface` has an additional method:
`getSenderByAlias()`.
* Removed argument `?bool &$handle = false` from `SendersLocatorInterface::getSenders`
* A new `ListableReceiverInterface` was added, which a receiver
can implement (when applicable) to enable listing and fetching
individual messages by id (used in the new "Failed Messages" commands).
Expand Down
14 changes: 4 additions & 10 deletions src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
'class' => \get_class($envelope->getMessage()),
];

$handle = false;
$sender = null;

try {
Expand All @@ -65,7 +64,7 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope

// dispatch event unless this is a redelivery
$shouldDispatchEvent = null === $redeliveryStamp;
foreach ($this->getSenders($envelope, $handle, $redeliveryStamp) as $alias => $sender) {
foreach ($this->getSenders($envelope, $redeliveryStamp) as $alias => $sender) {
if (null !== $this->eventDispatcher && $shouldDispatchEvent) {
$event = new SendMessageToTransportsEvent($envelope);
$this->eventDispatcher->dispatch($event);
Expand All @@ -76,14 +75,9 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
$this->logger->info('Sending message "{class}" with "{sender}"', $context + ['sender' => \get_class($sender)]);
$envelope = $sender->send($envelope->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null)));
}

// on a redelivery, only send back to queue: never call local handlers
if (null !== $redeliveryStamp) {
$handle = false;
}
}

if (null === $sender || $handle) {
if (null === $sender) {
return $stack->next()->handle($envelope, $stack);
}
} catch (\Throwable $e) {
Expand All @@ -100,14 +94,14 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
/**
* * @return iterable|SenderInterface[]
*/
private function getSenders(Envelope $envelope, &$handle, ?RedeliveryStamp $redeliveryStamp): iterable
private function getSenders(Envelope $envelope, ?RedeliveryStamp $redeliveryStamp): iterable
{
if (null !== $redeliveryStamp) {
return [
$redeliveryStamp->getSenderClassOrAlias() => $this->sendersLocator->getSenderByAlias($redeliveryStamp->getSenderClassOrAlias()),
];
}

return $this->sendersLocator->getSenders($envelope, $handle);
return $this->sendersLocator->getSenders($envelope);
}
}
50 changes: 12 additions & 38 deletions src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,7 @@ public function testItSendsToOnlyOneSenderOnRedelivery()

$sendersLocator = $this->createSendersLocator(
[DummyMessage::class => ['foo', 'bar']],
['foo' => $sender, 'bar' => $sender2],
[
// normally, this class sends and handles (but not on retry)
DummyMessage::class => true,
]
['foo' => $sender, 'bar' => $sender2]
);

$middleware = new SendMessageMiddleware($sendersLocator);
Expand Down Expand Up @@ -126,68 +122,46 @@ public function testItSendsTheMessageToAssignedSenderWithPreWrappedMessage()
$middleware->handle($envelope, $this->getStackMock(false));
}

public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageClass()
{
$message = new DummyMessage('Hey');
$envelope = new Envelope($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();

$sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
DummyMessage::class => true,
]);
$middleware = new SendMessageMiddleware($sendersLocator);

$sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);

$middleware->handle($envelope, $this->getStackMock());
}

public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageParentClass()
public function testItSendsTheMessageBasedOnTheMessageParentClass()
{
$message = new ChildDummyMessage('Hey');
$envelope = new Envelope($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();

$sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
DummyMessage::class => true,
]);
$sendersLocator = $this->createSendersLocator([DummyMessage::class => ['foo_sender']], ['foo_sender' => $sender]);
$middleware = new SendMessageMiddleware($sendersLocator);

$sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);

$middleware->handle($envelope, $this->getStackMock());
$middleware->handle($envelope, $this->getStackMock(false));
}

public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageInterface()
public function testItSendsTheMessageBasedOnTheMessageInterface()
{
$message = new DummyMessage('Hey');
$envelope = new Envelope($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();

$sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
DummyMessageInterface::class => true,
]);
$sendersLocator = $this->createSendersLocator([DummyMessageInterface::class => ['foo_sender']], ['foo_sender' => $sender]);
$middleware = new SendMessageMiddleware($sendersLocator);

$sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);

$middleware->handle($envelope, $this->getStackMock());
$middleware->handle($envelope, $this->getStackMock(false));
}

public function testItAlsoCallsTheNextMiddlewareBasedOnWildcard()
public function testItSendsTheMessageBasedOnWildcard()
{
$message = new DummyMessage('Hey');
$envelope = new Envelope($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();

$sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
'*' => true,
]);
$sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender]);
$middleware = new SendMessageMiddleware($sendersLocator);

$sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);

$middleware->handle($envelope, $this->getStackMock());
$middleware->handle($envelope, $this->getStackMock(false));
}

public function testItCallsTheNextMiddlewareWhenNoSenderForThisMessage()
Expand Down Expand Up @@ -267,7 +241,7 @@ public function testItDoesNotDispatchOnRedeliver()
$middleware->handle($envelope, $this->getStackMock(false));
}

private function createSendersLocator(array $sendersMap, array $senders, array $sendAndHandle = [])
private function createSendersLocator(array $sendersMap, array $senders)
{
$container = $this->createMock(ContainerInterface::class);
$container->expects($this->any())
Expand All @@ -281,6 +255,6 @@ private function createSendersLocator(array $sendersMap, array $senders, array $
return $senders[$id];
});

return new SendersLocator($sendersMap, $container, $sendAndHandle);
return new SendersLocator($sendersMap, $container);
}
}
16 changes: 2 additions & 14 deletions src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,36 +30,30 @@ class SendersLocator implements SendersLocatorInterface
private $sendersMap;
private $sendersLocator;
private $useLegacyLookup = false;
private $sendAndHandle;

/**
* @param string[][] $sendersMap An array, keyed by "type", set to an array of sender aliases
* @param ContainerInterface $sendersLocator Locator of senders, keyed by sender alias
* @param bool[] $sendAndHandle
*/
public function __construct(array $sendersMap, /*ContainerInterface*/ $sendersLocator = null, array $sendAndHandle = [])
public function __construct(array $sendersMap, /*ContainerInterface*/ $sendersLocator = null)
{
$this->sendersMap = $sendersMap;

if (\is_array($sendersLocator) || null === $sendersLocator) {
@trigger_error(sprintf('"%s::__construct()" requires a "%s" as 2nd argument. Not doing so is deprecated since Symfony 4.3 and will be required in 5.0.', __CLASS__, ContainerInterface::class), E_USER_DEPRECATED);
// "%s" requires a "%s" as 2nd argument. Not doing so is deprecated since Symfony 4.3 and will be required in 5.0.'
$this->sendersLocator = new ServiceLocator([]);
$this->sendAndHandle = $sendersLocator;
$this->useLegacyLookup = true;
} else {
$this->sendersLocator = $sendersLocator;
$this->sendAndHandle = $sendAndHandle;
}
}

/**
* {@inheritdoc}
*/
public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
public function getSenders(Envelope $envelope): iterable
{
$handle = false;
$sender = null;
$seen = [];

foreach (HandlersLocator::listTypes($envelope) as $type) {
Expand All @@ -71,8 +65,6 @@ public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
}
}

$handle = $handle ?: $this->sendAndHandle[$type] ?? false;

continue;
}

Expand All @@ -87,11 +79,7 @@ public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
yield $senderAlias => $sender;
}
}

$handle = $handle ?: $this->sendAndHandle[$type] ?? false;
}

$handle = $handle || null === $sender;
}

public function getSenderByAlias(string $alias): SenderInterface
Expand Down
5 changes: 1 addition & 4 deletions src/Symfony/Component/Messenger/Transport/Sender/SendersLocatorInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ interface SendersLocatorInterface
/**
* Gets the senders for the given message name.
*
* @param bool|null &$handle True after calling the method when the next middleware
* should also get the message; false otherwise
*
* @return iterable|SenderInterface[] Indexed by sender alias if available
*/
public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable;
public function getSenders(Envelope $envelope): iterable;

/**
* Returns a specific sender by its alias.
Expand Down