Skip to content

[Messenger] Proposal: Alternative for the Stack #31185

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
wants to merge 1 commit into from
Closed

[Messenger] Proposal: Alternative for the Stack #31185

wants to merge 1 commit into from

Conversation

volkyeth
Copy link

Draft proposal for replacing the StackInterface

Follow up on #31179


public function handle(Envelope $envelope) : Envelope
{
return $this->currentMiddleware->handle($envelope, $this->nextHandler);
Copy link
Member

@nicolas-grekas nicolas-grekas Apr 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what the current code avoids: this method now becomes a mandatory indirection point between all middleware frames.

@nicolas-grekas
Copy link
Member

There is another alternative: stop passing $next or $stack as argument to handle() and turn middleware into dispatcher decorators:

interface MiddlewareInterface extends MessageBusInterface
{
    /** @return static */
    public function withNextBus(MessageBusInterface $next): MiddlewareInterface;
}

then, a middleware would do: $this->next->dispatch($envelope);

@stof
Copy link
Member

stof commented Apr 19, 2019

I think this would be harder to understand here

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 23, 2019
nicolas-grekas added a commit that referenced this pull request Apr 26, 2019
…are stack (nicolas-grekas)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Messenger] ease testing and allow forking the middleware stack

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

A less radical alternative than #31185 that preserves laziness and addresses the linked issue.

Commits
-------

3bdf4b0 [Messenger] ease testing and allow forking the middleware stack
@volkyeth volkyeth deleted the feature/messenger-remove-stack branch April 26, 2019 10:58
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants