Skip to content

[#601] Added 'QueueTrait' with Drupal core queue management and assertion steps.#604

Merged
AlexSkrypnyk merged 4 commits intomainfrom
feature/add-queue-trait
Apr 9, 2026
Merged

[#601] Added 'QueueTrait' with Drupal core queue management and assertion steps.#604
AlexSkrypnyk merged 4 commits intomainfrom
feature/add-queue-trait

Conversation

@AlexSkrypnyk
Copy link
Copy Markdown
Member

@AlexSkrypnyk AlexSkrypnyk commented Apr 9, 2026

Closes #601

Summary

  • Added QueueTrait providing Behat steps for managing Drupal core queues: clearing, processing items (specific count or all), and asserting item counts.
  • Includes @AfterScenario cleanup hook (triggered by @queue tag) to delete queues used during tests.
  • Overridable queueGetProcessLimit() and queueGetLeaseTime() methods for customizing processing behavior.

Changes

  • src/Drupal/QueueTrait.php - New trait with five steps: Given the :queue queue is empty, When I process :count item(s) from the :queue queue, When I process all items from the :queue queue, Then the :queue queue should have :count item(s), Then the :queue queue should be empty.
  • tests/behat/features/drupal_queue.feature - BDD tests covering all steps including positive assertions and negative @trait scenarios.
  • tests/behat/bootstrap/FeatureContext.php - Registered QueueTrait.
  • tests/behat/bootstrap/FeatureContextTrait.php - Added helper step I add :count items to the :queue queue for test setup.
  • README.md, STEPS.md - Updated documentation.

Added QueueTrait for Drupal Core Queue Management

Overview

This PR introduces a new QueueTrait that provides Behat step definitions for managing and asserting Drupal core queues during BDD tests, plus tests and documentation.

Key Changes

  • New trait: src/Drupal/QueueTrait.php

    • Steps:
      • Given the :queue queue is empty
      • When I process :count item(s) from the :queue queue
      • When I process all items from the :queue queue
      • Then the :queue queue should have :count item(s)
      • Then the :queue queue should be empty
    • Tracks touched queues for scenario cleanup and registers #[AfterScenario('@Queue')] hook
    • Configurable protected methods: queueGetProcessLimit() (default 1000) and queueGetLeaseTime() (default 30)
    • Implements processing using Drupal core Queue API and the queue worker plugin manager
    • Throws RuntimeException for processing failures and Behat\Mink\Exception\ExpectationException for assertion failures
  • Tests and test helpers:

    • tests/behat/features/drupal_queue.feature — positive scenarios and negative @trait scenarios validating error messages and pluralization
    • tests/behat/bootstrap/FeatureContextTrait.php — added step to populate queues for tests
    • tests/behat/bootstrap/FeatureContext.php — registers QueueTrait
  • Documentation:

    • README.md — added QueueTrait entry to Drupal steps index
    • STEPS.md — new section documenting all queue-related steps
  • Misc:

    • PHPStan-related fix for claimItem() return type noted in commits

Compliance with CONTRIBUTING.md (steps format)

Critical violation:

  • tests/behat/bootstrap/FeatureContextTrait.php adds a Given step annotated as "I add :count items to the :queue queue". CONTRIBUTING.md requires Given steps to define prerequisites and to refrain from using "Given I" (reserved for actions). This step is an action and should be a When step (e.g., "When I add :count items to the :queue queue") or be reworded to a proper prerequisite phrasing (e.g., "Given the :queue queue has :count items").

Other checks:

  • Step wording and trait step method names (prefixed with queue) in QueueTrait follow the documented naming conventions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

Warning

Rate limit exceeded

@AlexSkrypnyk has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 21 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 21 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 984fb2cb-2538-41ef-bd42-3b0469d65747

📥 Commits

Reviewing files that changed from the base of the PR and between 8c1829e and 1ab890d.

📒 Files selected for processing (2)
  • tests/behat/bootstrap/FeatureContextTrait.php
  • tests/behat/features/drupal_queue.feature

Walkthrough

Adds a new Drupal QueueTrait implementing Behat steps to empty queues, process a fixed number or all items, assert queue counts, track & clean up queues after scenarios, plus docs, tests, and a helper to enqueue test items.

Changes

Cohort / File(s) Summary
Documentation
README.md, STEPS.md
Add Drupal\QueueTrait to README index and add a new ## Drupal\QueueTrait section in STEPS.md documenting Given/When/Then steps for emptying, processing, and asserting Drupal core queues.
Trait Implementation
src/Drupal/QueueTrait.php
New trait providing steps: queueEmpty, queueProcessItems, queueProcessAll, queueAssertItemCount, queueAssertEmpty; tracks queue names ($queueNames), includes #[AfterScenario('@queue')] cleanup, and protected config methods queueGetProcessLimit() and queueGetLeaseTime().
Test Helpers
tests/behat/bootstrap/FeatureContext.php, tests/behat/bootstrap/FeatureContextTrait.php
FeatureContext now use QueueTrait;. Added helper step I add :count items to the :queue queue to enqueue test items in FeatureContextTrait.
Feature Tests
tests/behat/features/drupal_queue.feature
New Behat feature exercising queue steps: positive scenarios for emptying, processing, and asserting counts; negative scenarios asserting exact failure output for mismatched expectations.

Sequence Diagram

sequenceDiagram
    participant Behat as Behat Engine
    participant FC as FeatureContext
    participant QT as QueueTrait
    participant DrupalQueue as Drupal Queue API
    participant QueueWorker as Queue Worker

    Behat->>FC: Given the "X" queue is empty
    FC->>QT: queueEmpty("X")
    QT->>QT: queueTrackName("X")
    QT->>DrupalQueue: deleteQueue("X")
    QT->>DrupalQueue: createQueue("X")

    Behat->>FC: When I process 5 items from "X"
    FC->>QT: queueProcessItems(5, "X")
    loop up to 5 times
        QT->>DrupalQueue: claimItem(leaseTime)
        DrupalQueue-->>QT: item or FALSE
        alt item claimed
            QT->>QueueWorker: processItem(item.data)
            QueueWorker-->>QT: processed
            QT->>DrupalQueue: deleteItem(item)
        else no item
            QT-->>FC: throw RuntimeException
        end
    end

    Behat->>FC: Then the "X" queue should have 2 items
    FC->>QT: queueAssertItemCount("X", 2)
    QT->>DrupalQueue: numberOfItems("X")
    DrupalQueue-->>QT: count
    QT-->>FC: assert / throw ExpectationException

    Note over QT,DrupalQueue: AfterScenario('@queue') -> delete tracked queues and reset list
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

PR: Requires more work

Poem

🐰 I hopped into queues with a curious twitch,
Claiming and processing each little stitch.
I emptied, I counted, then cleaned with delight,
Tests passed (I hope) in the soft evening light.
A rabbit's small hop keeps the queues tucked tight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: adding a QueueTrait for Drupal queue management and assertions, directly matching the changeset.
Linked Issues check ✅ Passed The PR implements all coding requirements from issue #601: QueueTrait with five steps, configurable limits via protected methods, @AfterScenario hook, feature tests, and proper registration in FeatureContext.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing QueueTrait as specified in #601: trait implementation, documentation, tests, and FeatureContext registration—no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/add-queue-trait

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Drupal/QueueTrait.php`:
- Around line 175-182: Rename the parameter of the protected helper method
queueTrackName from $queue_name to camelCase $queueName and update all uses
inside the method to match (e.g., the in_array check and array push against
$this->queueNames). Ensure the method signature reads queueTrackName(string
$queueName): void and that any callers (if any within the class) are updated to
pass the same variable name to avoid mismatches.
- Around line 57-72: In queueProcessItems, add defensive handling around the
plugin manager call that creates the worker (the createInstance($queue) call) so
a missing queue worker (PluginNotFoundException) is caught and replaced with a
clear, actionable error: wrap the createInstance($queue) invocation in a
try/catch for PluginNotFoundException and either throw a RuntimeException with a
friendly message including the $queue name (or log and return) so callers see
the real problem; also update the method docblock to state that a queue worker
plugin with an ID matching $queue is required.

In `@tests/behat/features/drupal_queue.feature`:
- Around line 24-28: Add a singular-friendly Behat step alias so the existing
step definition in FeatureContextTrait.php accepts both "item" and "items": add
a Given annotation that matches "I add :count item(s) to the :queue queue" (or
equivalent regex) alongside the current "I add :count items" definition, keeping
the same handler method name (the existing step handler in FeatureContextTrait)
so behavior is unchanged but the feature file can use the grammatically correct
"I add 1 item to the :queue queue".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68ce7dbc-a32f-4c19-8bd8-c85dac028a2a

📥 Commits

Reviewing files that changed from the base of the PR and between 7c6188d and 75996fa.

📒 Files selected for processing (6)
  • README.md
  • STEPS.md
  • src/Drupal/QueueTrait.php
  • tests/behat/bootstrap/FeatureContext.php
  • tests/behat/bootstrap/FeatureContextTrait.php
  • tests/behat/features/drupal_queue.feature

Comment thread src/Drupal/QueueTrait.php
Comment on lines +57 to +72
public function queueProcessItems(int $count, string $queue): void {
$this->queueTrackName($queue);
$queue_instance = \Drupal::service('queue')->get($queue);
$worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
$lease_time = $this->queueGetLeaseTime();

$processed = 0;
while ($processed < $count) {
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
throw new \RuntimeException(sprintf('Queue "%s" has no more items to process. Processed %d of %d requested items.', $queue, $processed, $count));
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider handling missing queue worker gracefully.

The method assumes a queue worker plugin exists with an ID matching the queue name. If no such worker exists, createInstance($queue) throws a PluginNotFoundException with a potentially confusing message.

Consider adding explicit error handling or documenting this requirement in the docblock.

♻️ Proposed defensive handling
   #[When('I process :count item(s) from the :queue queue')]
   public function queueProcessItems(int $count, string $queue): void {
     $this->queueTrackName($queue);
     $queue_instance = \Drupal::service('queue')->get($queue);
-    $worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
+    $worker_manager = \Drupal::service('plugin.manager.queue_worker');
+    if (!$worker_manager->hasDefinition($queue)) {
+      throw new \RuntimeException(sprintf('No queue worker plugin found for queue "%s". Ensure a queue worker with this ID exists.', $queue));
+    }
+    $worker = $worker_manager->createInstance($queue);
     $lease_time = $this->queueGetLeaseTime();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function queueProcessItems(int $count, string $queue): void {
$this->queueTrackName($queue);
$queue_instance = \Drupal::service('queue')->get($queue);
$worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
$lease_time = $this->queueGetLeaseTime();
$processed = 0;
while ($processed < $count) {
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
throw new \RuntimeException(sprintf('Queue "%s" has no more items to process. Processed %d of %d requested items.', $queue, $processed, $count));
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}
public function queueProcessItems(int $count, string $queue): void {
$this->queueTrackName($queue);
$queue_instance = \Drupal::service('queue')->get($queue);
$worker_manager = \Drupal::service('plugin.manager.queue_worker');
if (!$worker_manager->hasDefinition($queue)) {
throw new \RuntimeException(sprintf('No queue worker plugin found for queue "%s". Ensure a queue worker with this ID exists.', $queue));
}
$worker = $worker_manager->createInstance($queue);
$lease_time = $this->queueGetLeaseTime();
$processed = 0;
while ($processed < $count) {
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
throw new \RuntimeException(sprintf('Queue "%s" has no more items to process. Processed %d of %d requested items.', $queue, $processed, $count));
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}
🧰 Tools
🪛 PHPMD (2.15.0)

[error] 67-67: Missing class import via use statement (line '67', column '19'). (undefined)

(MissingImport)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Drupal/QueueTrait.php` around lines 57 - 72, In queueProcessItems, add
defensive handling around the plugin manager call that creates the worker (the
createInstance($queue) call) so a missing queue worker (PluginNotFoundException)
is caught and replaced with a clear, actionable error: wrap the
createInstance($queue) invocation in a try/catch for PluginNotFoundException and
either throw a RuntimeException with a friendly message including the $queue
name (or log and return) so callers see the real problem; also update the method
docblock to state that a queue worker plugin with an ID matching $queue is
required.

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.

Fixed in 1ab890d. Changed test helper annotation to tuple format 'I add :count item(s) to the :queue queue' and updated feature file to use singular form.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@AlexSkrypnyk, it looks like the fix you described — changing the test helper annotation to tuple format 'I add :count item(s) to the :queue queue' and updating the feature file for singular form — addresses a different concern (step annotation format), rather than this specific comment.

This comment was about adding defensive handling for a missing queue worker plugin in queueProcessItems() (and queueProcessAll()). Specifically, adding a hasDefinition($queue) check before calling createInstance($queue) so that a missing worker results in a clear, actionable error instead of a raw PluginNotFoundException.

Could you confirm whether that defensive handling was also included in commit 1ab890d, or if it's still pending?


🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/behat-steps PR: 592
File: tests/behat/features/table.feature:6-7
Timestamp: 2026-04-09T12:24:19.008Z
Learning: In drevops/behat-steps Behat feature files (tests/behat/features/**/*.feature), the `trait:TraitName` tag is used ONLY for negative/failure scenarios that run behat-within-behat via BehatCliContext (i.e., scenarios that invoke `behat --no-colors` to assert expected failure output). Positive "works as expected" scenarios that run directly against the Drupal site use only the `api` tag. Do NOT suggest adding `trait:TraitName` to positive scenarios — it is intentionally absent from them. This pattern is consistent across all trait test files (e.g., element.feature, drupal_content.feature, table.feature).

Learnt from: CR
Repo: drevops/behat-steps PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:14:22.972Z
Learning: Consolidate related tests into existing feature files rather than creating new ones

Learnt from: AlexSkrypnyk
Repo: drevops/behat-steps PR: 592
File: tests/behat/features/table.feature:38-40
Timestamp: 2026-04-09T12:25:05.364Z
Learning: In drevops/behat-steps, within `trait:*` negative scenarios in `tests/behat/features/**/*.feature`, `page content:` (and other DrupalContext content-creation steps like `landing_page content:`) are available and work correctly when the nested steps are written under `scenario steps tagged with "api":`. This is because BehatCliContext generates a feature file tagged with `api`, which makes the full DrupalContext (including `createNodes()`) available. The guideline "avoid custom steps in negative tests" does NOT apply to content-creation steps when `scenario steps tagged with "api":` is used.

Learnt from: AlexSkrypnyk
Repo: drevops/behat-steps PR: 555
File: tests/behat/features/metatag.feature:52-60
Timestamp: 2026-04-08T07:00:13.988Z
Learning: Repository convention (tests/behat/features/**/*.feature): Positive scenarios that exercise static fixtures (no Drupal API), such as visiting tests/behat/fixtures/*.html, are intentionally left untagged. Tags like api or trait:TraitName are used for integration/negative or trait-scoped scenarios only (e.g., in element.feature and metatag.feature).

Learnt from: AlexSkrypnyk
Repo: drevops/behat-steps PR: 560
File: src/ResponsiveTrait.php:173-173
Timestamp: 2026-04-09T08:32:59.254Z
Learning: In this project (drevops/behat-steps), Behat hook methods annotated with attributes like #[BeforeStep], #[BeforeScenario] (and similar Behat hook attributes) must keep their required scope parameter in the method signature (e.g., `BeforeStepScope $scope`, `BeforeScenarioScope $scope`) as required by the Behat framework contract. Even if the parameter is not referenced inside the method body, do not rename it to an unused placeholder like `$_scope` or otherwise change its type/name—treat it as structurally required. PHPMD `UnusedFormalParameter` warnings on these specific hook-method scope parameters are expected false positives and should be ignored.

Comment thread src/Drupal/QueueTrait.php
Comment on lines +175 to +182
/**
* Track a queue name for cleanup.
*/
protected function queueTrackName(string $queue_name): void {
if (!in_array($queue_name, $this->queueNames)) {
$this->queueNames[] = $queue_name;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Rename parameter to follow camelCase convention for non-step methods.

Per coding guidelines, local variables in Behat step methods should use snake_case, but method parameters in protected helper methods should use camelCase.

✨ Proposed fix
-  protected function queueTrackName(string $queue_name): void {
-    if (!in_array($queue_name, $this->queueNames)) {
-      $this->queueNames[] = $queue_name;
+  protected function queueTrackName(string $queueName): void {
+    if (!in_array($queueName, $this->queueNames)) {
+      $this->queueNames[] = $queueName;
     }
   }

As per coding guidelines: "Local variables and method arguments in Behat steps should use snake_case, while method names and class properties should use camelCase."

🧰 Tools
🪛 PHPMD (2.15.0)

[error] 178-182: The parameter $queue_name is not named in camelCase. (undefined)

(CamelCaseParameterName)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Drupal/QueueTrait.php` around lines 175 - 182, Rename the parameter of
the protected helper method queueTrackName from $queue_name to camelCase
$queueName and update all uses inside the method to match (e.g., the in_array
check and array push against $this->queueNames). Ensure the method signature
reads queueTrackName(string $queueName): void and that any callers (if any
within the class) are updated to pass the same variable name to avoid
mismatches.

Comment thread tests/behat/features/drupal_queue.feature
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 41.17647% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.44%. Comparing base (7c6188d) to head (1ab890d).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/Drupal/QueueTrait.php 41.17% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #604      +/-   ##
==========================================
- Coverage   99.57%   98.44%   -1.13%     
==========================================
  Files          38       40       +2     
  Lines        2573     2636      +63     
==========================================
+ Hits         2562     2595      +33     
- Misses         11       41      +30     

☔ 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.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/Drupal/QueueTrait.php (2)

180-183: ⚠️ Potential issue | 🟡 Minor

Resolve the helper parameter naming warning.

Line 180 uses $queue_name in a protected helper method, leaving the CamelCaseParameterName warning unresolved. Rename to camelCase and update references.

♻️ Suggested fix
-  protected function queueTrackName(string $queue_name): void {
-    if (!in_array($queue_name, $this->queueNames)) {
-      $this->queueNames[] = $queue_name;
+  protected function queueTrackName(string $queueName): void {
+    if (!in_array($queueName, $this->queueNames, TRUE)) {
+      $this->queueNames[] = $queueName;
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Drupal/QueueTrait.php` around lines 180 - 183, The helper method
queueTrackName uses a snake_case parameter $queue_name which triggers a
CamelCaseParameterName warning; rename the parameter to $queueName in the
queueTrackName method signature and update all internal references in that
method (and any callers) to use $queueName so the code compiles and the
$this->queueNames usage remains unchanged.

57-61: ⚠️ Potential issue | 🟠 Major

Handle missing queue-worker plugins with an actionable error.

Line 60 and Line 87 call createInstance($queue) without guarding missing plugin definitions. A missing worker ID currently bubbles a low-level plugin exception; wrap it and throw a clear runtime error with the queue name.

♻️ Suggested fix
+use Drupal\Component\Plugin\Exception\PluginNotFoundException;
...
   public function queueProcessItems(int $count, string $queue): void {
     $this->queueTrackName($queue);
     $queue_instance = \Drupal::service('queue')->get($queue);
-    $worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
+    try {
+      $worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
+    }
+    catch (PluginNotFoundException $e) {
+      throw new \RuntimeException(sprintf('No queue worker plugin found for queue "%s".', $queue), 0, $e);
+    }
...
   public function queueProcessAll(string $queue): void {
     $this->queueTrackName($queue);
     $queue_instance = \Drupal::service('queue')->get($queue);
-    $worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
+    try {
+      $worker = \Drupal::service('plugin.manager.queue_worker')->createInstance($queue);
+    }
+    catch (PluginNotFoundException $e) {
+      throw new \RuntimeException(sprintf('No queue worker plugin found for queue "%s".', $queue), 0, $e);
+    }

Also applies to: 84-88

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Drupal/QueueTrait.php` around lines 57 - 61, When calling the plugin
manager's createInstance($queue) in queueProcessItems, wrap the
createInstance(...) call in a try/catch and throw a clear RuntimeException that
includes the queue name (e.g. "No queue worker defined for queue '{$queue}'")
instead of letting the low-level plugin exception bubble; catch the specific
plugin exception if available (or \Exception) and rethrow a \RuntimeException
with the queue name and the caught exception as previous. Apply the same guard
to the other createInstance($queue) usage in this trait so both places produce
an actionable error mentioning the queue ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Drupal/QueueTrait.php`:
- Around line 91-105: The safety check currently throws whenever $processed >=
$limit even if the queue was fully drained; update the post-loop check so we
only throw when the limit was hit and there are still items remaining: after the
loop, if ($processed >= $limit) attempt to claim one more item from
$queue_instance using the same lease_time to detect remaining work (use
claimItem), and if that call returns an item then cleanup that claim
(deleteItem) and throw the RuntimeException; if no item is returned, do not
throw. This change touches the $processed/$limit check and uses
$queue_instance->claimItem($lease_time) and $queue_instance->deleteItem($item)
to determine and clean up remaining items.

---

Duplicate comments:
In `@src/Drupal/QueueTrait.php`:
- Around line 180-183: The helper method queueTrackName uses a snake_case
parameter $queue_name which triggers a CamelCaseParameterName warning; rename
the parameter to $queueName in the queueTrackName method signature and update
all internal references in that method (and any callers) to use $queueName so
the code compiles and the $this->queueNames usage remains unchanged.
- Around line 57-61: When calling the plugin manager's createInstance($queue) in
queueProcessItems, wrap the createInstance(...) call in a try/catch and throw a
clear RuntimeException that includes the queue name (e.g. "No queue worker
defined for queue '{$queue}'") instead of letting the low-level plugin exception
bubble; catch the specific plugin exception if available (or \Exception) and
rethrow a \RuntimeException with the queue name and the caught exception as
previous. Apply the same guard to the other createInstance($queue) usage in this
trait so both places produce an actionable error mentioning the queue ID.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d1725a58-bebc-4d69-b8e6-cb0a380aba1c

📥 Commits

Reviewing files that changed from the base of the PR and between cd4cd29 and 8c1829e.

📒 Files selected for processing (1)
  • src/Drupal/QueueTrait.php

Comment thread src/Drupal/QueueTrait.php
Comment on lines +91 to +105
$processed = 0;
while ($processed < $limit) {
/** @var \stdClass|false $item */
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
break;
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}

if ($processed >= $limit) {
throw new \RuntimeException(sprintf('Queue "%s" processing reached the safety limit of %d items.', $queue, $limit));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Safety-limit check can fail valid “process all” runs at the exact limit.

Line 103 throws whenever $processed >= $limit. If the queue contains exactly $limit items and is fully drained, this still throws incorrectly. Only fail when the limit is reached and items remain.

♻️ Suggested fix
-    if ($processed >= $limit) {
+    if ($processed === $limit && $queue_instance->numberOfItems() > 0) {
       throw new \RuntimeException(sprintf('Queue "%s" processing reached the safety limit of %d items.', $queue, $limit));
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$processed = 0;
while ($processed < $limit) {
/** @var \stdClass|false $item */
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
break;
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}
if ($processed >= $limit) {
throw new \RuntimeException(sprintf('Queue "%s" processing reached the safety limit of %d items.', $queue, $limit));
}
$processed = 0;
while ($processed < $limit) {
/** `@var` \stdClass|false $item */
$item = $queue_instance->claimItem($lease_time);
if (!$item) {
break;
}
$worker->processItem($item->data);
$queue_instance->deleteItem($item);
$processed++;
}
if ($processed === $limit && $queue_instance->numberOfItems() > 0) {
throw new \RuntimeException(sprintf('Queue "%s" processing reached the safety limit of %d items.', $queue, $limit));
}
🧰 Tools
🪛 PHPMD (2.15.0)

[error] 104-104: Missing class import via use statement (line '104', column '17'). (undefined)

(MissingImport)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Drupal/QueueTrait.php` around lines 91 - 105, The safety check currently
throws whenever $processed >= $limit even if the queue was fully drained; update
the post-loop check so we only throw when the limit was hit and there are still
items remaining: after the loop, if ($processed >= $limit) attempt to claim one
more item from $queue_instance using the same lease_time to detect remaining
work (use claimItem), and if that call returns an item then cleanup that claim
(deleteItem) and throw the RuntimeException; if no item is returned, do not
throw. This change touches the $processed/$limit check and uses
$queue_instance->claimItem($lease_time) and $queue_instance->deleteItem($item)
to determine and clean up remaining items.

@AlexSkrypnyk AlexSkrypnyk merged commit ef68857 into main Apr 9, 2026
14 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/add-queue-trait branch April 9, 2026 17:03
@AlexSkrypnyk AlexSkrypnyk added this to the v3.7 milestone Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Added 'QueueTrait' with Drupal core queue management and assertion steps.

1 participant