Skip to content

Remove QueueInterface::withAdapter() and AdapterFactoryQueueProvider#266

Open
vjik wants to merge 5 commits intomasterfrom
rm-with-adapter
Open

Remove QueueInterface::withAdapter() and AdapterFactoryQueueProvider#266
vjik wants to merge 5 commits intomasterfrom
rm-with-adapter

Conversation

@vjik
Copy link
Copy Markdown
Member

@vjik vjik commented Apr 2, 2026

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

@vjik vjik changed the title Remove QueueInterface::withAdapter() Remove QueueInterface::withAdapter() and AdapterFactoryQueueProvider Apr 2, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.51%. Comparing base (5999193) to head (95045ee).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #266      +/-   ##
============================================
- Coverage     98.59%   98.51%   -0.09%     
+ Complexity      372      354      -18     
============================================
  Files            48       46       -2     
  Lines           998      942      -56     
============================================
- Hits            984      928      -56     
  Misses           14       14              

☔ 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 April 2, 2026 13:42
@vjik vjik added the status:code review The pull request needs review. label Apr 2, 2026
@vjik vjik requested a review from Copilot April 2, 2026 13:43
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

This PR removes the queue “adapter injection” API (QueueInterface::withAdapter()), deletes the AdapterFactoryQueueProvider, and updates queue construction and push flow to assume an adapter is always present.

Changes:

  • Removed QueueInterface::withAdapter() and made Queue require an AdapterInterface via constructor.
  • Removed AdapterFactoryQueueProvider and AdapterNotConfiguredException, simplifying adapter configuration and error handling.
  • Updated middleware/push request flow and unit/integration tests to use non-null adapters and new queue construction patterns.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/QueueInterface.php Removes withAdapter() from the public queue interface.
src/Queue.php Makes adapter mandatory (constructor) and removes adapter-not-configured checks and withAdapter().
src/Middleware/Push/PushRequest.php Makes adapter non-nullable in push requests; updates getter signature.
src/Middleware/Push/AdapterPushHandler.php Removes “no adapter” exception path; always pushes via request adapter.
src/Debug/QueueDecorator.php Drops withAdapter() passthrough now that the interface no longer defines it.
src/Provider/AdapterFactoryQueueProvider.php Deletes the provider that built queues from adapter definitions.
src/Exception/AdapterConfiguration/AdapterNotConfiguredException.php Deletes the exception that was thrown when using a queue without an adapter.
stubs/StubQueue.php Removes withAdapter() support and simplifies stub implementation accordingly.
tests/TestCase.php Updates queue factory helper to construct Queue with an adapter and name.
tests/Unit/QueueTest.php Reworks tests to use createQueue($adapter) instead of withAdapter().
tests/Unit/Stubs/StubQueueTest.php Removes withAdapter() test coverage from stub queue tests.
tests/Unit/Provider/AdapterFactoryQueueProviderTest.php Deletes tests for removed AdapterFactoryQueueProvider.
tests/Unit/Middleware/Push/Implementation/IdMiddlewareTest.php Updates PushRequest instantiation to pass a non-null adapter.
tests/Unit/Middleware/Push/AdapterPushHandlerTest.php Removes test expecting exception when adapter is missing.
tests/Unit/Debug/QueueDecoratorTest.php Removes decorator withAdapter() tests now that method is gone.
tests/Unit/Adapter/SynchronousAdapterTest.php Refactors adapter tests to instantiate SynchronousAdapter directly without queue withAdapter().
tests/Integration/MiddlewareTest.php Updates Queue constructor argument order (adapter before name).
tests/App/DummyQueue.php Removes withAdapter() from dummy queue implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@viktorprogger viktorprogger left a comment

Choose a reason for hiding this comment

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

The functional changes are ok for me, but in config we should provide reasonable defaults for these cases:

  • Super simple: get a default queue with DI and make a push/listen.
  • Named queues: provide a default way to define and get named queues.

? $container->get(SignalLoop::class)
: $container->get(SimpleLoop::class);
},
QueueInterface::class => Queue::class,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why?

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.

It is assumed that the queue or queue provider will be configured in the container configuration in the application.

Comment on lines -25 to -27
'queues' => [
QueueProviderInterface::DEFAULT_QUEUE => AdapterInterface::class,
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How to configure queues with this change?

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.

It is assumed that the queue provider will be configured in the container configuration in the application.

'definitions' => $params['yiisoft/queue']['queues'],
],
],
QueueProviderInterface::class => AdapterFactoryQueueProvider::class,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why? How to get a named queue now?

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.

It is assumed that the queue provider will be configured in the container configuration in the application.

src/Queue.php Outdated
private readonly LoopInterface $loop,
private readonly LoggerInterface $logger,
private readonly PushMiddlewareDispatcher $pushMiddlewareDispatcher,
private readonly AdapterInterface $adapter,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we're moving it, let's move it to the first place.

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

@@ -24,13 +22,12 @@ final class StubQueue implements QueueInterface
*/
public function __construct(
private ?AdapterInterface $adapter = null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since the QueueInterface provides no adapter knowledge, let's completely remove $adapter from this class and the corresponding tests.

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

@vjik vjik requested a review from viktorprogger April 5, 2026 19:02
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.

4 participants