Skip to content

[Messenger] Add $stamps parameter to HandleTrait::handle #42124

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

Conversation

alexander-schranz
Copy link
Contributor

@alexander-schranz alexander-schranz commented Jul 15, 2021

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets --
License MIT
Doc PR TODO

Like the MessageBusInterface::dispatch it would be great to use stamps.

We are using example a custom middleware to control if a flush should be done.

$this->handle(new CreateUserMessage($data), [new DoctrineFlushStamp()]);

Also a custom middleware for locking:

$this->handle(new OurMessage(), [new LockStamp('lock-key', 300.0, true)]));

@alexander-schranz alexander-schranz requested a review from sroze as a code owner July 15, 2021 08:10
@carsonbot carsonbot changed the title Add stamps to handle trait [Messenger] Add stamps to handle trait Jul 15, 2021
@nicolas-grekas nicolas-grekas added this to the 5.4 milestone Jul 15, 2021
@fabpot fabpot modified the milestones: 5.4, 6.1 Nov 3, 2021
@alexander-schranz alexander-schranz force-pushed the feature/stamps-to-handletrait branch 2 times, most recently from e37a935 to bdaa434 Compare April 11, 2022 18:25
@alexander-schranz alexander-schranz force-pushed the feature/stamps-to-handletrait branch from bdaa434 to 436e506 Compare May 19, 2022 16:55
@alexander-schranz
Copy link
Contributor Author

I rebased this branch. As I think this is a new feature and we are in RC phase I updated the changelog to 6.2.

@chalasr
Copy link
Member

chalasr commented May 19, 2022

While having stamps part of the dispatch() signature works (not fond of it personally), I tend to think handle() should rather be free of this concept.
My reasoning is that HandleTrait is meant to ease implementing regular command/query buses, and doing so would kind of make it deviate from the command bus pattern which aims to be purely about handling domain use cases without leaking any technical detail.
Considering this, and given one can already pass a stamped envelope to handle() (not fond of it either 😅), I would prefer not to merge this.

@alexander-schranz
Copy link
Contributor Author

In our cases it is very important that we are allowed to input stamps. As they control our middlewares. We had very bad experience with "automated" middlewares like DoctrineFlush oder DoctrineTransactionMiddleware and creating for everything seperate message buses was a bad developer experience. So we decided to use stamps for this kind of things so developer had 100% control in the userinterface layer our messages are dispatched how the message is handled by the middlewares. And I don't see much which speak against it as it is still optional and if somebody don't want to use it they don't need it.

@fabpot fabpot modified the milestones: 6.1, 6.2 May 20, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Dec 13, 2024

Is there any chance to rediscuss this? We are using Stamps not only for Doctrine Flush but also for Lock Mechanic:

new YourMessage(), [new LockStamp('lock-key', 300.0, true)])

@alexander-schranz alexander-schranz force-pushed the feature/stamps-to-handletrait branch 2 times, most recently from 2f044ea to a652612 Compare December 13, 2024 13:49
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Works for me, thanks for clarifying.

@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Jan 6, 2025

just rebased, fabbot unrelated: #42124 (comment)

@alexander-schranz alexander-schranz force-pushed the feature/stamps-to-handletrait branch from a61a656 to 9d69f84 Compare March 10, 2025 10:56
@OskarStark OskarStark changed the title [Messenger] Add stamps to handle trait [Messenger] Add $stamps parameter to HandleTrait::handle Mar 10, 2025
@alexander-schranz
Copy link
Contributor Author

Something I can do to push this over the line :)

@alexander-schranz alexander-schranz force-pushed the feature/stamps-to-handletrait branch from c3237a6 to cb10d2e Compare March 29, 2025 14:01
Co-authored-by: Oskar Stark <oskarstark@googlemail.com>
@chalasr chalasr force-pushed the feature/stamps-to-handletrait branch from 7834310 to 1742f67 Compare March 29, 2025 14:45
@chalasr
Copy link
Member

chalasr commented Mar 29, 2025

Thank you @alexander-schranz.

@chalasr chalasr merged commit c6baf1b into symfony:7.3 Mar 29, 2025
4 of 10 checks passed
@alexander-schranz alexander-schranz deleted the feature/stamps-to-handletrait branch March 29, 2025 14:57
@fabpot fabpot mentioned this pull request May 2, 2025
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.

7 participants