Skip to content

Replace SynchronousAdapter with SynchronousPushHandler - #270

Merged
vjik merged 21 commits into
masterfrom
rm-sync-adapter
Apr 29, 2026
Merged

Replace SynchronousAdapter with SynchronousPushHandler#270
vjik merged 21 commits into
masterfrom
rm-sync-adapter

Conversation

@vjik

@vjikvjik commented Apr 26, 2026

Copy link
Copy Markdown
Member
QA
Is bugfix?
New feature?
Breaks BC?✔️
Tests pass?✔️

@codecov

codecovBot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.82%. Comparing base (5037487) to head (2479f7c).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #270 +/- ##
============================================
- Coverage 98.83% 98.82% -0.01% + Complexity 316 313 -3 
============================================
Files 46 46 Lines 857 853 -4 ============================================
- Hits 847 843 -4 
Misses 10 10 

☔ 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 teamApril 26, 2026 10:12
@vjikvjik added the status:code review The pull request needs review. label Apr 26, 2026
Comment threaddocs/guide/en/synchronous-mode.md Outdated
Comment on lines +38 to +39
- `listen()` throws `BadMethodCallException`.
- `status()` throws `BadMethodCallException` — there is no message storage to track IDs.

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.

Isn't it better to do nothing on listen() and return empty status in status()?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It's better. Implemented.

Comment threadsrc/Queue.php
Comment threadsrc/Queue.php Outdated
Comment on lines +117 to +121
if ($this->adapter === null) {
throw new BadMethodCallException(
'Cannot listen without an adapter. Queue is in synchronous mode.',
);
}

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.

Should it do $this->logger->info and return instead?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

fixed

Comment threadsrc/Queue.php Outdated
) implements MessageHandlerPushInterface {
public function __construct(
private readonly AdapterPushHandler $adapterPushHandler,
private readonly MessageHandlerPushInterface $finishHandler,

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.

What's the name finishHandler means?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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.

Worth adding some phpdoc.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

done

Comment threadREADME.md Outdated
@vjik
vjik requested review from a team and samdarkApril 26, 2026 15:26
Statuses are represented by the `Yiisoft\Queue\MessageStatus` enum:

- `MessageStatus::NOT_FOUND`
The message is not known to the queue, or the adapter doesn't support status tracking.

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.

It is fine but we need a separate way of knowing if the adapter doesn't support status tracking. Probably in a separate PR.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Comment threaddocs/guide/en/synchronous-mode.md Outdated
Comment threaddocs/guide/en/synchronous-mode.md Outdated
Comment threadsrc/Queue.php
Comment threadsrc/Queue.php Outdated
) implements MessageHandlerPushInterface {
public function __construct(
private readonly AdapterPushHandler $adapterPushHandler,
private readonly MessageHandlerPushInterface $finishHandler,

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.

Worth adding some phpdoc.

Comment threadtests/App/InMemoryAdapter.php Outdated
use function count;

final class SynchronousAdapter implements AdapterInterface
final class InMemoryAdapter implements AdapterInterface

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 be shorten a bit:

Suggested change
finalclassInMemoryAdapterimplements AdapterInterface
finalclassMemoryAdapterimplements AdapterInterface

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

done

vjikand others added 2 commits April 29, 2026 13:37
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
@vjik
vjik requested a review from samdarkApril 29, 2026 10:48
@vjik
vjik merged commit 42e0b04 into masterApr 29, 2026
29 of 30 checks passed
@vjik
vjik deleted the rm-sync-adapter branch April 29, 2026 10:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code reviewThe pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vjik@samdark