Introduce abstract Message class#291
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
========================================
Coverage 0.00% 0.00%
- Complexity 318 320 +2
========================================
Files 48 49 +1
Lines 872 879 +7
========================================
- Misses 872 879 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors the Message API: Message becomes an abstract base class that only handles metadata, while the previous concrete generic implementation is extracted into a new GenericMessage class. Metadata is no longer a fromData()/constructor parameter — it is applied separately via a new withMetadata() method on MessageInterface. Envelopes can no longer be created via fromData() (they now throw LogicException and must be created from an existing message). The serializer, tests, benchmarks, and documentation are updated to use the new API and to encourage defining dedicated message subclasses.
Changes:
MessageInterface/Messagesplit:fromData(string, mixed)only, plus a newwithMetadata(array): static;Messageis abstract and owns metadata;GenericMessagereplaces the old concrete generic message.JsonMessageSerializerdefaults toGenericMessagefor restoration and applies metadata viawithMetadata();Envelope::fromData()now throws andEnvelope::withMetadata()re-wraps the underlying message.- Tests/benchmarks updated from
new Message(...)tonew GenericMessage(...)/->withMetadata(...); README and several guides rewritten to promote per-message subclasses.
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Message/MessageInterface.php | Drops $metadata from fromData; adds withMetadata(). |
| src/Message/Message.php | Becomes abstract metadata base class. |
| src/Message/GenericMessage.php | New concrete generic message. |
| src/Message/JsonMessageSerializer.php | Defaults to GenericMessage, applies metadata via withMetadata. |
| src/Message/Envelope.php | fromData now throws; adds withMetadata that re-wraps. |
| tests/Unit/Support/TestMessage.php | Extends Message; retains stale $metadata param in fromData. |
| tests/Unit/Message/EnvelopeTest.php | Asserts Envelope::fromData() throws. |
| tests/Unit/Message/JsonMessageSerializerTest.php | Updated default class assertions to GenericMessage. |
| tests/Unit/Message/IdEnvelopeTest.php, DelayEnvelopeTest.php | Use GenericMessage + withMetadata; remove fromData tests. |
| tests/Unit/Middleware/.../*.php | Switch Message → GenericMessage; metadata via withMetadata. |
| tests/Unit/{Queue,Worker,Envelope,Stubs,Debug}*.php | Message → GenericMessage updates. |
| tests/Integration/MiddlewareTest.php | Message → GenericMessage; one call still passes a third [] arg. |
| tests/Integration/MessageConsumingTest.php, Support/TestMiddleware.php | Message → GenericMessage. |
| tests/Benchmark/{QueueBench,MetadataBench}.php | Updated to GenericMessage/withMetadata. |
| README.md, docs/guide/en/*.md | Rewritten examples to use dedicated Message subclasses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
||
| $this->assertEquals( | ||
| '{"type":"handler","data":"test","meta":{"message-class":"Yiisoft\\\\Queue\\\\Message\\\\Message"}}', | ||
| '{"type":"handler","data":"test","meta":{"message-class":"Yiisoft\\\\Queue\\\\Message\\\\GenericMessage"}}', |
There was a problem hiding this comment.
Can we avoid having PHP class in the message?
- It makes refactoring impossible.
- It doesn't make sense for non-PHP or other app consumers.
- If there are multiple envelopes... which class should be there?
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
| ``` | ||
| Producer side Consumer side | ||
| ───────────────────────────── ────────────────────────────────── | ||
| new Message('send-email', …) →→→ Worker resolves handler → handles | ||
| (payload only) (logic only) | ||
| Producer side Consumer side | ||
| ────────────────────────────── ────────────────────────────────── | ||
| new SendEmailMessage(…) →→→ Worker resolves handler → handles | ||
| (payload only) (logic only) | ||
| ``` |
There was a problem hiding this comment.
Could it be mermaid diagram inestad of pseudo-code?
There was a problem hiding this comment.
flowchart LR
subgraph Producer["Producer side"]
Message["new SendEmailMessage(...)\n(payload only)"]
end
subgraph Consumer["Consumer side"]
Worker["Worker resolves handler"]
Handler["Handler handles\n(logic only)"]
end
Message --> Worker --> Handler
flowchart LR
subgraph Producer["Producer side"]
Message["new SendEmailMessage(...)\n(payload only)"]
end
subgraph Consumer["Consumer side"]
Worker["Worker resolves handler"]
Handler["Handler handles\n(logic only)"]
end
Message --> Worker --> Handler
| { | ||
| $message = new Message('test', ['data' => 'value'], [DelayEnvelope::META_DELAY_SECONDS => 150]); | ||
| $delayEnvelope = DelayEnvelope::fromMessage($message); | ||
| $delayEnvelope = DelayEnvelope::fromMessage( |
There was a problem hiding this comment.
| $delayEnvelope = DelayEnvelope::fromMessage( | |
| $delayEnvelope = DelayEnvelope::wrap( |
There was a problem hiding this comment.
For message wrapping we use constructor. fromMessage() creates envelope class from message and its metadata, this is not wrapping.
| $delayEnvelope = DelayEnvelope::fromMessage($message); | ||
| $delayEnvelope = DelayEnvelope::fromMessage( | ||
| (new GenericMessage('test', ['data' => 'value'])) | ||
| ->withMetadata([DelayEnvelope::META_DELAY_SECONDS => 150]) |
There was a problem hiding this comment.
If we have a concrete DelayEnvelope class, would it be better to have a special method rather than crafting metadata ourselves?
$delayEnvelope = DelayEnvelope::fromMessage(
(new GenericMessage('test', ['data' => 'value'])
)->delay(150);Or even:
$delayedMessage = new DelayEnvelope(
message: (new GenericMessage('test', ['data' => 'value']),
delay: 150,
);There was a problem hiding this comment.
This test for fromMessage() method checking.
Example of custom message class: