Problem/Motivation

In #3198868: Add delay to queue suspend the code for running cron queues has changed. This caused that queue workers that use a deriver cause a fatal error on cron runs.

An error message like the following can occur when running cron:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "feeds_feed_refresh" plugin does not exist. Valid plugin IDs for Drupal\Core\Queue\QueueWorkerManager are: feeds_feed_refresh:article, feeds_feed_refresh:csv, feeds_feed_refresh:issue, feeds_feed_refresh:products, feeds_feed_refresh:variations, feeds_feed_refresh:xml in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 53 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).

This is how a queue definition from the Feeds module looks like:

'feeds_feed_refresh:article' => array (6) [
        'title' => Drupal\Core\StringTranslation\TranslatableMarkup (5) (
            protected 'string' -> string (30) "Feed refresh: @feed_type_label"
            protected 'arguments' -> array (1) [
                '@feed_type_label' => string (7) "Article"
            ]
            protected 'translatedMarkup' -> null
            protected 'options' -> array (0) []
            protected 'stringTranslation' -> null
        )
        'id' => string (18) "feeds_feed_refresh"
        'cron' => array (1) [
            'time' => integer 60
        ]
        'deriver' => string (46) "Drupal\feeds\Plugin\Derivative\FeedQueueWorker"
        'class' => string (43) "Drupal\feeds\Plugin\QueueWorker\FeedRefresh"
        'provider' => string (5) "feeds"
    ]

The issue is that \Drupal\Core\Cron now picks the queue name from $queue_info['id'], while it previously took the key from the queue array. By my understanding 'id' in the example above is the base plugin ID. And the key on the array is the derivative plugin ID.

Steps to reproduce

  1. Implement a QueueWorker plugin that makes use of a deriver and that uses cron (see code example below).
  2. Implement the deriver plugin (see code example below).
  3. Create an item on the queue (see code example below).
  4. Run cron.

Example QueueWorker plugin:

namespace Drupal\mymodule\Plugin\QueueWorker;

use Drupal\Core\Queue\QueueWorkerBase;

/**
 * A queue worker.
 *
 * @QueueWorker(
 *   id = "myqueue",
 *   title = @Translation("My Queue"),
 *   cron = {"time" = 60},
 *   deriver = "Drupal\mymodule\Plugin\Derivative\QueueWorkerDerivative"
 * )
 */
class MyQueueWorker extends QueueWorkerBase {

  /**
   * {@inheritdoc}
   */
  public function processItem($data) {}

}

Example deriver:

namespace Drupal\mymodule\Plugin\Derivative;

use Drupal\Component\Plugin\Derivative\DeriverBase;

/**
 * Provides separate queue works.
 *
 * @see \Drupal\mymodule\Plugin\QueueWorker\MyQueueWorker
 */
class QueueWorkerDerivative extends DeriverBase {

  /**
   * {@inheritdoc}
   */
  public function getDerivativeDefinitions($base_plugin_definition) {
    $example_data = [
      'foo' => 'Foo',
      'bar' => 'Bar',
    ];

    $derivatives = [];
    foreach ($example_data as $key => $label) {
      $derivatives[$key] = [
        'title' => strtr('My Queue: @label', [
          '@label' => $label,
        ]),
      ] + $base_plugin_definition;
    }

    return $derivatives;
  }

}

Create an item on the queue:

\Drupal::service('queue')->get('myqueue:foo')
  ->createItem([]);

Proposed resolution

Change the implementation of Drupal\Core\Cron::processQueues() so that the queue name is taken from the key of the queues array instead of picking the 'id' property directly.

Remaining tasks

  1. Confirm that this is a bug (and not a misunderstanding of how derivatives should be implemented).
  2. Implement the proposed resolution.
  3. Add a test that demonstrates the bug. Probably the example code from above should be put in a test module in core/modules/system/tests/modules.

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3348832

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

MegaChriz created an issue. See original summary.

acbramley made their first commit to this issue’s fork.

acbramley’s picture

Status: Active » Needs review

Failing test + fix pushed as separate commits. We were dropping keys at 2 steps - in the array_filter and the array_map. I've removed all of that and imo made this a bit easier to read through by iterating directly on $this->queueManager->getDefinitions()

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Thanks @acbramley for getting to this so quickly!

Change looks good and the test case appears to cover the scenario exactly.

Verified by install dev branch of the feeds module.
Setup a simple feed importer that reads a csv and creates an Article with the title mapping.
Ran cron and got the error

Applied the patch

Ran cron again, no error, and my page was created.

megachriz’s picture

When I run the test without the fix I get indeed the expected test failure:

Drupal\Tests\system\Kernel\System\CronQueueTest::testQueueWorkerDeriver
Drupal\Component\Plugin\Exception\PluginNotFoundException: The "cron_queue_test_deriver" plugin does not exist. Valid plugin IDs for Drupal\Core\Queue\QueueWorkerManager are: cron_queue_test_requeue_exception, cron_queue_test_deriver:foo, cron_queue_test_deriver:bar, cron_queue_test_lease_time, cron_queue_test_suspend, cron_queue_test_exception, cron_queue_test_memory_delay_exception, cron_queue_test_database_delay_exception

Nice that the test checks how many times an item from the queue was processed.

And the applied fix makes the code of \Drupal\Core\Cron::processQueues() look a lot simpler too.

And in #3351610: Drupal 10.1 compatibility I checked that with the fix to core applied if this makes the Feeds tests pass on Drupal 10.1.x again and they do!

RTBC++

larowlan’s picture

  • larowlan committed f2e1e5c5 on 10.1.x
    Issue #3348832 by acbramley, MegaChriz, smustgrave: Running cron queues...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Fixed on commit, added a pre-condition just to be safe.

diff --git a/core/modules/system/tests/src/Kernel/System/CronQueueTest.php b/core/modules/system/tests/src/Kernel/System/CronQueueTest.php
index 4bec9a7e07f..4e2f1e7858c 100644
--- a/core/modules/system/tests/src/Kernel/System/CronQueueTest.php
+++ b/core/modules/system/tests/src/Kernel/System/CronQueueTest.php
@@ -335,6 +335,7 @@ public function testQueueWorkerManagerSafeguard(): void {
    * Tests that cron queues from derivers work.
    */
   public function testQueueWorkerDeriver(): void {
+    $this->assertEquals(0, \Drupal::state()->get(CronQueueTestDeriverQueue::PLUGIN_ID, 0));
     $queue = \Drupal::queue(sprintf('%s:foo', CronQueueTestDeriverQueue::PLUGIN_ID));
     $queue->createItem('foo');

Verified it still passed locally

phpunit -c app/core app/core/modules/system/tests/src/Kernel/System/CronQueueTest.php --filter=Deriver
⏳️ Bootstrapped tests in 0 seconds.
🐘 PHP Version 8.1.16.
💧 Drupal Version 10.1.0-dev.
🗄️  Database engine mysql.
PHPUnit 9.6.6 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\system\Kernel\System\CronQueueTest
.                                                                   1 / 1 (100%)

Time: 00:00.535, Memory: 10.00 MB

OK (1 test, 4 assertions)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.