[Messenger] make middlewares truly lazy on a bus #28907
Conversation
fe04974 to
3e71a94
Compare
| } elseif (\is_array($middlewareHandlers)) { | ||
| $this->middlewareAggregate = new \ArrayObject($middlewareHandlers); | ||
| } else { | ||
| $this->middlewareAggregate = new \ArrayObject(iterator_to_array($middlewareHandlers, false)); |
There was a problem hiding this comment.
previously the expansion (iterator_to_array) happened lazily, should we preserve it?
b1539b6 to
c00e896
Compare
| return $this->aggregate = new \ArrayObject(iterator_to_array($this->iterator, false)); | ||
| } | ||
| }; | ||
| $this->middlewareAggregate->aggregate = &$this->middlewareAggregate; |
There was a problem hiding this comment.
Can you move that to be in the constructor of your class instead?
There was a problem hiding this comment.
The class is internal, the public properties are not accessible. Adding a constructor would just add unneeded boilerplate. Better not to me.
| if (null === $this->indexedMiddlewareHandlers) { | ||
| $this->indexedMiddlewareHandlers = \is_array($this->middlewareHandlers) ? array_values($this->middlewareHandlers) : iterator_to_array($this->middlewareHandlers, false); | ||
| $middlewareIterator = $this->middlewareAggregate->getIterator(); | ||
| while ($middlewareIterator instanceof \IteratorAggregate) { |
There was a problem hiding this comment.
Why while? What's the use-case in which it will return an iterator twice (or more)?
There was a problem hiding this comment.
@sroze i suggested that, see #28907 (comment)
There was a problem hiding this comment.
but maybe we should enforce it resolves once, either here or in the constructor 🤔
There was a problem hiding this comment.
That's how "foreach" works internally. We must have it to preserve basic PHP semantics of Traversable.
c00e896 to
4fec7f7
Compare
4fec7f7 to
6a5d7a1
Compare
|
PR rebased, ready. |
|
Thank you @nicolas-grekas. |
…s-grekas) This PR was merged into the 4.2-dev branch. Discussion ---------- [Messenger] make middlewares truly lazy on a bus | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Right now, as soon as a message is dispatched on a bus, all its handlers are instantiated. This PR fixes it by leveraging `IteratorAggregate` when possible (implemented by DI's `RewindableGenerator`). Commits ------- 6a5d7a1 [Messenger] make middlewares truly lazy on a bus
Right now, as soon as a message is dispatched on a bus, all its handlers are instantiated.
This PR fixes it by leveraging
IteratorAggregatewhen possible (implemented by DI'sRewindableGenerator).