Skip to content

[Messenger] Input for HandlerDescriptor should cause container rebuild #31409

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.

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

Closed
weaverryan opened this issue May 7, 2019 · 0 comments
Closed

Comments

@weaverryan
Copy link
Member

Symfony version(s) affected: master (4.3)

Description
When creating a handler that implements MessageSubscriberInterface, which passes custom options about the handler (e.g. from_transport from #30958), if the user changes those options, they are not visible in the app. The problem is that this input is used to create an internal service (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php#L168), but the files these options were sourced from (the handler itself) is not added as a container resource.

How to reproduce

namespace App\MessageHandler;

use App\Message\SendEmail;
use Symfony\Component\Messenger\Handler\MessageSubscriberInterface;

class SendEmailHandler implements MessageSubscriberInterface
{
    public function __invoke(SendEmail $email)
    {
        //
    }

    public static function getHandledMessages(): iterable
    {
        yield SendEmail::class => [
            'from_transport' => 'async',
        ];
    }
}

Completely clear the cache. You can see that this handler is only called when coming from some async transport. Now, change to some other transport name and try again. It will still try to consume only from the original, async transport.

Possible Solution
If the config comes from the handler (i.e. if it implements MessageSubscriberInterface), it should be added as a resource. Doesn't this also affect normal MessageHandlerInterface handlers - if you changed the type-hint to __invoke(), would that cause a container rebuild?

@weaverryan weaverryan added the Bug label May 7, 2019
@weaverryan weaverryan added this to the 4.3 milestone May 7, 2019
@weaverryan weaverryan removed this from the 4.3 milestone May 9, 2019
nicolas-grekas added a commit that referenced this issue May 11, 2019
…bscribers change (weaverryan)

This PR was merged into the 4.2 branch.

Discussion
----------

[Messenger] Making cache rebuild correctly when message subscribers change

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31409
| License       | MIT
| Doc PR        | not needed

An edge-case that's identical to `EventSubscriberInterface`: when the return value of `getHandledMessages()` changes, the container needs to be rebuilt.

If you're wondering why these checks aren't in their own resource class, see #25984 - it's something we probably should do, but haven't done yet.

Commits
-------

d88446b Making cache rebuild correctly with MessageSubscriberInterface return values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants