Skip to content

Commit

Permalink
minor #27841 [Messenger] Envelope-aware middleware is never called wi…
Browse files Browse the repository at this point in the history
…th a message (Cydonia7)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] Envelope-aware middleware is never called with a message

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | no
| License       | MIT
| Doc PR        | no

Messenger components middlewares implementing `Symfony\Component\Messenger\EnvelopeAwareInterface` receive in their handle method an instance of `Symfony\Component\Messenger\Envelope`, not a message.

To better reflect the expected usage, I've updated the unit tests for the message middleware and fixed the code accordingly.

Commits
-------

9488e2a [Messenger] Envelope-aware middleware is never called with a message
  • Loading branch information
sroze committed Jul 19, 2018
2 parents c1f23c8 + 9488e2a commit 4b92b96
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 30 deletions.
5 changes: 5 additions & 0 deletions UPGRADE-4.2.md
Expand Up @@ -83,6 +83,11 @@ FrameworkBundle
this will generate 404s for non-UTF-8 URLs, which are incompatible with you app anyway,
and will allow dumping optimized routers and using Unicode classes in requirements.

Messenger
---------

* The `handle` method of the `Symfony\Component\Messenger\Middleware\ValidationMiddleware` and `Symfony\Component\Messenger\Asynchronous\Middleware\SendMessageMiddleware` middlewares now requires an `Envelope` object to be given (because they implement the `EnvelopeAwareInterface`). When using these middleware with the provided `MessageBus`, you will not have to do anything. If you use the middlewares any other way, you can use `Envelope::wrap($message)` to create an envelope for your message.

Security
--------

Expand Down
Expand Up @@ -34,14 +34,15 @@ public function __construct(SenderLocatorInterface $senderLocator, array $messag
}

/**
* @param Envelope $envelope
*
* {@inheritdoc}
*/
public function handle($message, callable $next)
public function handle($envelope, callable $next)
{
$envelope = Envelope::wrap($message);
if ($envelope->get(ReceivedMessage::class)) {
// It's a received message. Do not send it back:
return $next($message);
return $next($envelope);
}

$sender = $this->senderLocator->getSenderForMessage($envelope->getMessage());
Expand All @@ -54,7 +55,7 @@ public function handle($message, callable $next)
}
}

return $next($message);
return $next($envelope);
}

private function mustSendAndHandle($message): bool
Expand Down
Expand Up @@ -37,12 +37,12 @@ public function __construct(MiddlewareInterface $inner, $activated)
/**
* @param Envelope $message
*/
public function handle($message, callable $next)
public function handle($envelope, callable $next)
{
if (\is_callable($this->activated) ? ($this->activated)($message) : $this->activated) {
return $this->inner->handle($message->getMessageFor($this->inner), $next);
if (\is_callable($this->activated) ? ($this->activated)($envelope) : $this->activated) {
return $this->inner->handle($envelope->getMessageFor($this->inner), $next);
}

return $next($message);
return $next($envelope);
}
}
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\Messenger\Middleware;

use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\EnvelopeAwareInterface;
use Symfony\Component\Messenger\Exception\ValidationFailedException;
use Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration;
Expand All @@ -29,21 +28,20 @@ public function __construct(ValidatorInterface $validator)
$this->validator = $validator;
}

public function handle($message, callable $next)
public function handle($envelope, callable $next)
{
$envelope = Envelope::wrap($message);
$subject = $envelope->getMessage();
$message = $envelope->getMessage();
$groups = null;
/** @var ValidationConfiguration|null $validationConfig */
if ($validationConfig = $envelope->get(ValidationConfiguration::class)) {
$groups = $validationConfig->getGroups();
}

$violations = $this->validator->validate($subject, null, $groups);
$violations = $this->validator->validate($message, null, $groups);
if (\count($violations)) {
throw new ValidationFailedException($subject, $violations);
throw new ValidationFailedException($message, $violations);
}

return $next($message);
return $next($envelope);
}
}
Expand Up @@ -26,15 +26,16 @@ class SendMessageMiddlewareTest extends TestCase
public function testItSendsTheMessageToAssignedSender()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator($sender));

$sender->expects($this->once())->method('send')->with(Envelope::wrap($message));
$sender->expects($this->once())->method('send')->with($envelope);
$next->expects($this->never())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItSendsTheMessageToAssignedSenderWithPreWrappedMessage()
Expand All @@ -54,77 +55,82 @@ public function testItSendsTheMessageToAssignedSenderWithPreWrappedMessage()
public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageClass()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator($sender), array(
DummyMessage::class => true,
));

$sender->expects($this->once())->method('send')->with(Envelope::wrap($message));
$sender->expects($this->once())->method('send')->with($envelope);
$next->expects($this->once())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageParentClass()
{
$message = new ChildDummyMessage('Hey');
$envelope = Envelope::wrap($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator($sender), array(
DummyMessage::class => true,
));

$sender->expects($this->once())->method('send')->with(Envelope::wrap($message));
$sender->expects($this->once())->method('send')->with($envelope);
$next->expects($this->once())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageInterface()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator($sender), array(
DummyMessageInterface::class => true,
));

$sender->expects($this->once())->method('send')->with(Envelope::wrap($message));
$sender->expects($this->once())->method('send')->with($envelope);
$next->expects($this->once())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItAlsoCallsTheNextMiddlewareBasedOnWildcard()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);
$sender = $this->getMockBuilder(SenderInterface::class)->getMock();
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator($sender), array(
'*' => true,
));

$sender->expects($this->once())->method('send')->with(Envelope::wrap($message));
$sender->expects($this->once())->method('send')->with($envelope);
$next->expects($this->once())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItCallsTheNextMiddlewareWhenNoSenderForThisMessage()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);
$next = $this->createPartialMock(\stdClass::class, array('__invoke'));

$middleware = new SendMessageMiddleware(new InMemorySenderLocator(null));

$next->expects($this->once())->method($this->anything());

$middleware->handle($message, $next);
$middleware->handle($envelope, $next);
}

public function testItSkipsReceivedMessages()
Expand Down
Expand Up @@ -24,6 +24,7 @@ class ValidationMiddlewareTest extends TestCase
public function testValidateAndNextMiddleware()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);

$validator = $this->createMock(ValidatorInterface::class);
$validator
Expand All @@ -36,19 +37,18 @@ public function testValidateAndNextMiddleware()
$next
->expects($this->once())
->method('__invoke')
->with($message)
->with($envelope)
->willReturn('Hello')
;

$result = (new ValidationMiddleware($validator))->handle($message, $next);
$result = (new ValidationMiddleware($validator))->handle($envelope, $next);

$this->assertSame('Hello', $result);
}

public function testValidateWithConfigurationAndNextMiddleware()
{
$envelope = Envelope::wrap($message = new DummyMessage('Hey'))->with(new ValidationConfiguration($groups = array('Default', 'Extra')));

$validator = $this->createMock(ValidatorInterface::class);
$validator
->expects($this->once())
Expand Down Expand Up @@ -76,6 +76,7 @@ public function testValidateWithConfigurationAndNextMiddleware()
public function testValidationFailedException()
{
$message = new DummyMessage('Hey');
$envelope = Envelope::wrap($message);

$violationList = $this->createMock(ConstraintViolationListInterface::class);
$violationList
Expand All @@ -96,6 +97,6 @@ public function testValidationFailedException()
->method('__invoke')
;

(new ValidationMiddleware($validator))->handle($message, $next);
(new ValidationMiddleware($validator))->handle($envelope, $next);
}
}

0 comments on commit 4b92b96

Please sign in to comment.