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

Allow to restore original pushed message class on consume #214

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

viktorprogger
Copy link
Contributor

@viktorprogger viktorprogger commented Sep 15, 2024

Q A
Is bugfix?
New feature? ✔️
Breaks BC?

This PR allows you to push your own message class and then get it back after it was consumed.
See new tests for details.

@viktorprogger viktorprogger added the status:code review The pull request needs review. label Sep 15, 2024
@viktorprogger viktorprogger requested review from samdark, s1lver, xepozz and a team September 15, 2024 17:28
@viktorprogger viktorprogger self-assigned this Sep 15, 2024
Copy link
Member

@samdark samdark left a comment

Choose a reason for hiding this comment

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

Also, docs should be updated to include info about envelopes.

src/Message/EnvelopeTrait.php Show resolved Hide resolved
@@ -18,6 +18,7 @@ public function serialize(MessageInterface $message): string
'name' => $message->getHandlerName(),
'data' => $message->getData(),
'meta' => $message->getMetadata(),
'class' => $message instanceof EnvelopeInterface ? $message->getMessage()::class : $message::class,
Copy link
Member

Choose a reason for hiding this comment

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

Am I correct that class name is in the data that is sent? If so, I'd avoid that. Is that possible to use a plain envelope name, not a class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a message class. It may be used only by an application which pushed this message to the queue. I'll move this data to the "meta" key. Anyway this data is not usable by another services.

src/Message/JsonMessageSerializer.php Outdated Show resolved Hide resolved
src/Message/JsonMessageSerializer.php Outdated Show resolved Hide resolved
@@ -113,7 +115,7 @@ public function testSerialize(): void
$json = $serializer->serialize($message);

$this->assertEquals(
'{"name":"handler","data":"test","meta":[]}',
'{"name":"handler","data":"test","meta":[],"class":"Yiisoft\\\\Queue\\\\Message\\\\Message"}',
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a map of name => class somewhere (probably hardcoded in the envelope itself)? I'd avoid PHP-specific FQNs at all costs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class is specific to an application which pushes a message to a queue. This class list is not usable by any other service. I'll move this class to the "meta" key.

@viktorprogger
Copy link
Contributor Author

@samdark issue for the docs improvement: #215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants