Skip to content

Introduce abstract Message class#291

Open
vjik wants to merge 12 commits into
masterfrom
with-metadata
Open

Introduce abstract Message class#291
vjik wants to merge 12 commits into
masterfrom
with-metadata

Conversation

@vjik
Copy link
Copy Markdown
Member

@vjik vjik commented May 28, 2026

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

Example of custom message class:

final class SmsNotification extends Message
{
    private const TYPE = 'sms';

    public function __construct(
        public readonly string $number,
        public readonly string $text,
    ) {}

    public static function fromData(string $type, mixed $data): MessageInterface
    {
        if ($type !== self::TYPE) {
            throw new InvalidArgumentException("Unsupported message type: $type");
        }

        return new self(
            number: $data['number'] ?? throw new InvalidArgumentException('Number is required.'),
            text: $data['text'] ?? throw new InvalidArgumentException('Text is required.'),
        );
    }

    public function getType(): string
    {
        return self::TYPE;
    }

    public function getData(): mixed
    {
        return [
            'number' => $this->number,
            'text' => $this->text,
        ];
    }
}

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (1a8ba57) to head (07e2f83).

Files with missing lines Patch % Lines
src/Message/GenericMessage.php 0.00% 8 Missing ⚠️
src/Message/Message.php 0.00% 6 Missing ⚠️
src/Message/Envelope.php 0.00% 5 Missing ⚠️
src/Message/JsonMessageSerializer.php 0.00% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vjik vjik requested a review from a team May 28, 2026 14:01
@vjik vjik added the status:code review The pull request needs review. label May 28, 2026
@vjik vjik requested a review from Copilot May 28, 2026 15:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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/Message split: fromData(string, mixed) only, plus a new withMetadata(array): static; Message is abstract and owns metadata; GenericMessage replaces the old concrete generic message.
  • JsonMessageSerializer defaults to GenericMessage for restoration and applies metadata via withMetadata(); Envelope::fromData() now throws and Envelope::withMetadata() re-wraps the underlying message.
  • Tests/benchmarks updated from new Message(...) to new 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 MessageGenericMessage; metadata via withMetadata.
tests/Unit/{Queue,Worker,Envelope,Stubs,Debug}*.php MessageGenericMessage updates.
tests/Integration/MiddlewareTest.php MessageGenericMessage; one call still passes a third [] arg.
tests/Integration/MessageConsumingTest.php, Support/TestMiddleware.php MessageGenericMessage.
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.

Comment thread tests/Integration/MiddlewareTest.php Outdated
Comment thread tests/Unit/Support/TestMessage.php Outdated
vjik and others added 2 commits May 28, 2026 20:46
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread docs/guide/en/message-handler-advanced.md Outdated
Comment thread docs/guide/en/messages-and-handlers.md Outdated
Comment thread src/Message/Envelope.php
Comment thread src/Message/GenericMessage.php
Comment thread tests/Unit/Message/DelayEnvelopeTest.php Outdated

$this->assertEquals(
'{"type":"handler","data":"test","meta":{"message-class":"Yiisoft\\\\Queue\\\\Message\\\\Message"}}',
'{"type":"handler","data":"test","meta":{"message-class":"Yiisoft\\\\Queue\\\\Message\\\\GenericMessage"}}',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we avoid having PHP class in the message?

  1. It makes refactoring impossible.
  2. It doesn't make sense for non-PHP or other app consumers.
  3. If there are multiple envelopes... which class should be there?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. I created issue: #292

vjik and others added 4 commits May 29, 2026 15:36
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Comment thread docs/guide/en/messages-and-handlers.md Outdated
Comment on lines 14 to 19
```
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)
```
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could it be mermaid diagram inestad of pseudo-code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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
Loading
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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

{
$message = new Message('test', ['data' => 'value'], [DelayEnvelope::META_DELAY_SECONDS => 150]);
$delayEnvelope = DelayEnvelope::fromMessage($message);
$delayEnvelope = DelayEnvelope::fromMessage(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$delayEnvelope = DelayEnvelope::fromMessage(
$delayEnvelope = DelayEnvelope::wrap(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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,
);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test for fromMessage() method checking.

Copy link
Copy Markdown
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.

Added some ideas.

@vjik vjik requested a review from samdark May 29, 2026 19:38
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