Skip to content

[Messenger] Re-introduce wrapped message configuration (with fix) #27182

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

Merged
merged 3 commits into from
May 9, 2018

Conversation

sroze
Copy link
Contributor

@sroze sroze commented May 7, 2018

Q A
Branch? 4.1
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26945
License MIT
Doc PR ø

The pull request was merged before beta1, but because it introduced a bug, it has been reverted. This adds back the merged PR but pushes a fix for the found bug.

@sroze sroze force-pushed the re-introduce-wrapped-message-configuration branch from 5a9d672 to ad01c29 Compare May 7, 2018 16:08
@sroze sroze added this to the 4.1 milestone May 7, 2018
@sroze sroze added the Messenger label May 7, 2018
@sroze sroze requested a review from ogizanagi May 7, 2018 16:08
@kbond
Copy link
Member

kbond commented May 7, 2018

What exactly was the bug?

@ogizanagi
Copy link
Contributor

The original envelope was not properly passed along the whole middleware stack when calling $next($message), so items were vanished in between.
This fixes it and also allows altering the message and the envelope items through middlewares.

@ogizanagi
Copy link
Contributor

I've pushed another commit to sroze#6 for the TraceableBus which misses a small update.

@sroze sroze force-pushed the re-introduce-wrapped-message-configuration branch from 837c483 to b879e1d Compare May 8, 2018 14:52
sroze and others added 3 commits May 9, 2018 15:46
Ensure that the middlewares can also update the message within the envelope
@sroze sroze force-pushed the re-introduce-wrapped-message-configuration branch from b879e1d to 21e49d2 Compare May 9, 2018 14:48
@sroze
Copy link
Contributor Author

sroze commented May 9, 2018

Merging as we found and fixed the bug that was almost to bug in beta1.

@sroze
Copy link
Contributor Author

sroze commented May 9, 2018

Thank you @ogizanagi.

@sroze sroze merged commit 21e49d2 into symfony:4.1 May 9, 2018
sroze added a commit that referenced this pull request May 9, 2018
… (with fix) (sroze, ogizanagi)

This PR was merged into the 4.1 branch.

Discussion
----------

[Messenger] Re-introduce wrapped message configuration (with fix)

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

The pull request was merged before beta1, but because it introduced a bug, it has been reverted. This adds back the merged PR but pushes a fix for the found bug.

Commits
-------

21e49d2 [Messenger] Fix TraceableBus with envelope
599f32c Ensure the envelope is passed back and can be altered Ensure that the middlewares can also update the message within the envelope
7c33cb2 feature #26945 [Messenger] Support configuring messages when dispatching (ogizanagi)
@sroze sroze deleted the re-introduce-wrapped-message-configuration branch May 9, 2018 18:54
@fabpot fabpot mentioned this pull request May 21, 2018
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