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

Extract getId / setId method from the MessageInterface to a new one #183

Closed
wants to merge 1 commit into from

Conversation

xepozz
Copy link
Member

@xepozz xepozz commented Dec 25, 2023

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

Copy link

what-the-diff bot commented Dec 25, 2023

PR Summary

  • Introduction of ParametrizedMessageInterface and ParametrizedMessageTrait
    These new additions provide functions for assigning and retrieving a unique identification for each message. This enhances our ability to manage individual messages more effectively.

  • Modification of SynchronousAdapter.php, JobFailureException.php, Message.php
    These files have been updated to set and get the ID but only if the message is compatible with the new ParametrizedMessageInterface. These changes align with the introduction of message ID functions, ensuring they are only applied to applicable messages.

  • Update of FailureHandlingRequest.php
    This file has been updated with removal of specific PHPDoc comments for the getMessage() method. This change is in line with our efforts to maintain clean and relevant code documentation.

  • Changes in ExponentialDelayMiddleware.php, SendAgainMiddleware.php, Queue.php, And Worker.php
    These files have been revised to utilize the new getId() method from ParametrizedMessageInterface to extract the message ID. Additionally, for messages that do not use ParametrizedMessageInterface, the Worker.php will log the message data. This change ensures correct logging of all messages.

Copy link

codecov bot commented Dec 25, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (2639357) 94.31% compared to head (f32e7aa) 94.25%.
Report is 2 commits behind head on master.

Files Patch % Lines
src/Worker/Worker.php 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #183      +/-   ##
============================================
- Coverage     94.31%   94.25%   -0.06%     
- Complexity      340      346       +6     
============================================
  Files            40       41       +1     
  Lines           915      923       +8     
============================================
+ Hits            863      870       +7     
- Misses           52       53       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xepozz xepozz added the status:code review The pull request needs review. label Dec 25, 2023

namespace Yiisoft\Yii\Queue\Message;

interface ParametrizedMessageInterface extends MessageInterface
Copy link
Member

Choose a reason for hiding this comment

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

Name is a bit confusing. Parametrized means there are parameters. Here either there aren't or only one. Is there a better name?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was temporary named, thought about moving also getMetadata.

Is there a better name?

Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

IdentifyableMessageInterface?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or MessageIdInterface? :)

Also it might be closed after introducing envelope

@xepozz xepozz closed this Jan 13, 2024
@xepozz xepozz deleted the extract-id-methods branch January 13, 2024 14:56
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.

2 participants