Skip to content
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

Include message data in the history #17

Open
SpartakusMd opened this issue Sep 28, 2023 · 7 comments · May be fixed by #46
Open

Include message data in the history #17

SpartakusMd opened this issue Sep 28, 2023 · 7 comments · May be fixed by #46

Comments

@SpartakusMd
Copy link
Contributor

I believe, in some cases it would be helpful to also store the data from the message too in the history. In case the message contains sensitive info, we could use another stamp to indicate that for this message the data should not be recorded.

@kbond
Copy link
Member

kbond commented Sep 28, 2023

Good idea. I'm trying to think how we'd store this - it needs to be json in the db. Simple as running through symfony/serializer?

It'd be nice to reuse the messenger sererializer system but by default, I believe it uses serialize().

@SpartakusMd
Copy link
Contributor Author

I checked and you're right. If we're going to use the messenger serializer, we'll end up with encoded PHP objects in the DB. I would say it should be safe to use the symfony/serializer instead and store the json. For special cases we could emulate the messenger serializer and look for SerializerStamp and SerializedMessageStamp in the envelope.

@Chris53897
Copy link
Contributor

This would be a great feature.

@SpartakusMd Are you up for an PR?

@SpartakusMd
Copy link
Contributor Author

Was thinking to do it, but didn't find the time yet. @Chris53897, if you're willing to do it, feel free.

@Chris53897
Copy link
Contributor

I will probably have some time after 01.10 to crate some PRs. I will decide than, wich one i will pick.

@kbond
Copy link
Member

kbond commented Oct 24, 2023

Thinking about this feature some more. If we're going to add it, it'll need to be before 1.0 as it would require a breaking change (the extra entity column).

Question for you guys though: is making your message \Stringable not enough or do you require the full message context (properties)?

In case the message contains sensitive info, we could use another stamp to indicate that for this message the data should not be recorded.

Could/should this be handled via serializer groups?

@SpartakusMd
Copy link
Contributor Author

@kbond, my use case would benefit from storing the full message in the history. It gives the possibility to process the data later and still have a human view via \Stringable for the message. In the end, I could live without it, but would be something nice to have.

I prepared a sample PR #46 to see if it's a good approach. For implementation, I used symfony/serializer to normalize the message from the Envelope. For now I didn't add serializer groups, if needed we can add them later.

What do you think? Should this land in the package? Is it something which wouldn't be required? Should there be something more added?

@SpartakusMd SpartakusMd linked a pull request Nov 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants