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
- Implement a QueueWorker plugin that makes use of a deriver and that uses cron (see code example below).
- Implement the deriver plugin (see code example below).
- Create an item on the queue (see code example below).
- 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
- Confirm that this is a bug (and not a misunderstanding of how derivatives should be implemented).
- Implement the proposed resolution.
- 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
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
Comment #4
acbramley commentedFailing 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()Comment #5
smustgrave commentedThanks @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.
Comment #6
megachrizWhen I run the test without the fix I get indeed the expected test failure:
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++
Comment #7
larowlanComment #9
larowlanFixed on commit, added a pre-condition just to be safe.
Verified it still passed locally