-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Refactor handlers #182
base: master
Are you sure you want to change the base?
Refactor handlers #182
Conversation
PR Summary
Minor changes and adjustments were made throughout the codebase to align with these changes. Please ask if further clarification is needed. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #182 +/- ##
============================================
- Coverage 83.98% 83.15% -0.84%
+ Complexity 359 345 -14
============================================
Files 46 46
Lines 1049 1021 -28
============================================
- Hits 881 849 -32
- Misses 168 172 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was that handler is not a PHP class string because worker could be implemented in non-PHP.
Already discussed in the internal chat. There're should be routing outside a message to be able to work with different services across the one queue. |
With queue backends such as Redis, there's no external routing. |
Discussed internally. It makes sense if we'll introduce "message type decorator" and "message-based handlers" for messages in a separate PR. |
src/Middleware/FailureHandling/Implementation/ExponentialDelayMiddleware.php
Outdated
Show resolved
Hide resolved
# Conflicts: # README.md # config/di.php # config/params.php # src/Middleware/FailureHandling/Implementation/ExponentialDelayMiddleware.php # src/Middleware/FailureHandling/Implementation/SendAgainMiddleware.php # src/Queue.php # src/Worker/Worker.php # tests/App/FakeHandler.php # tests/Integration/MessageConsumingTest.php # tests/Integration/MiddlewareTest.php # tests/TestCase.php # tests/Unit/QueueTest.php # tests/Unit/SynchronousAdapterTest.php # tests/Unit/WorkerTest.php
# Conflicts: # config/params.php # tests/App/FakeHandler.php # tests/TestCase.php # tests/Unit/WorkerTest.php
@@ -10,8 +10,9 @@ interface MessageInterface | |||
* Returns handler name. | |||
* | |||
* @return string | |||
* @psalm-return class-string<MessageHandlerInterface> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any string can be here (for example, ID for container), but class string only.
@@ -89,21 +92,21 @@ class FileDownloader | |||
$this->absolutePath = $absolutePath; | |||
} | |||
|
|||
public function handle(\Yiisoft\Queue\Message\MessageInterface $downloadMessage): void | |||
public function handle(\Yiisoft\Queue\Message\MessageInterface $downloadMessage): MessageInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public function handle(\Yiisoft\Queue\Message\MessageInterface $downloadMessage): MessageInterface | |
public function handle(\Yiisoft\Queue\Message\MessageInterface $downloadMessage): void |
|
||
return $message; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return $message; |
{ | ||
return $this->message->getHandlerName(); | ||
return $this->message->getHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be a problem here, when handler support only original message and don't support envelope.
Drop support pre-configured handlers, make it compatible with validator/hydrator handlers way
The old way:
handler name
=>handler definition
mapping to config/paramshandler name
to the messagegetHandlerName
methodThe new way:
MessageHandlerInterface
getHandler
methodThe new way is similar with validator's or hydrator's way.