-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DoctrineBridge][Messenger] Add MessageRecordingEntity functionality #34310
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
Conversation
I used Consider a classic DDD example. |
src/Symfony/Bridge/Doctrine/Messenger/MessageRecordingEntitySubscriber.php
Outdated
Show resolved
Hide resolved
77cb43d
to
a704101
Compare
After a discussion with @weaverryan , I added an event hook |
@Nyholm , @sroze , @weaverryan , looking forward to your reviews :-) |
@Nyholm Even though I'm maintainer of SimpleBus I never used this functionality with SimpleBus. It was already there when I joined. Therefore I cannot really review this functionality. |
@sroze , any news about this PR? |
Perhaps @weaverryan can help us figure out what to do here? |
I’m using this in a project and just noticed something: when an entity is deleted, its messages won’t be recorded |
I tried collecting the message recording entities on the preFlush events but I’m having trouble accessing entities that are scheduled for removal because they became orphaned I’ve moved the message recording into the entities' collection owner for now |
|
||
use Symfony\Component\Messenger\Envelope; | ||
|
||
final class EntityMessagePreDispatchEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand the naming of this class. Can you explain? This seems to me like a ... EntityMessageEvent
- but I don't understand the "Pre" part - it happens postFlush()
.
{ | ||
use MessageRecordingEntityTrait; | ||
|
||
public function doRecordMessage(object $message): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a public method (by default) in the entity?
interface MessageRecordingEntityInterface | ||
{ | ||
/** | ||
* @param callable $dispatcher callable(object[] $messages): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably need some phpdoc explanation on the purpose / usage of this method.
@vudaltsov can you check out the comment from @mathroc about the delete problem? I don't know the internals, but possibly we need to grab the entity map before flush, store it on a property, then in |
We've just moved away from |
This is an alternative to #28850, thanks @Nyholm for the initial implementation and idea.
The interface I propose does not leak out the messages explicitly.