Contrib.social

Wrong clean action applied when running imports for multiple feeds in one request

Open on Drupal.org →
Created on 8 July 2025, 7 days ago

Problem/Motivation

Feeds provides an option to clean up items that no longer appear in the source. On the feed type you can for example configure to delete the items on the Drupal site or unpublish these.

If you have two feed types with the setting "Previously imported items" differently configured and then you run imports for both feeds in the same request, for the second feed the wrong clean action can get used.

⚠️ This could potentially lead to data loss: items getting deleted instead of unpublished.

Steps to reproduce

  1. Create two feed types. For feed type 1, set "Previously imported items" to "Delete". For feed type 2, set "Previously imported items" to "Unpublish content item" (or any other action, as long as it is not the same as feed type 1).
  2. Configure mappings for both feed types.
  3. Create feeds for both feed types.
  4. Run imports for both feeds, so that for both of them content exists.
  5. For the second feed, remove one item from the source.
  6. Run imports for both feeds during the same request. The easiest way to do so is by using the drush command "feeds:import-all":
    drush feeds:import-all

Now instead of the item getting unpublished, it gets deleted instead!

Debug information

The bug is in LazySubscriber. A listener for every Feeds plugin that implements \Drupal\feeds\Plugin\Type\CleanableInterface is added for the Feeds clean event. But at the time that listener gets added, the configuration from the first feed's plugin gets passed to the anonymous function:

$dispatcher->addListener(FeedsEvents::CLEAN, function (CleanEvent $event) use ($plugin) {

See use ($plugin) the line above.

This causes that every time the clean stage happens - during the same request - the same configuration is used.

Proposed resolution

Proposed solution is to:

  1. Only add one listener, not a listener per plugin.
  2. Iterate through all plugins in the anonymous function instead, for the feed that is then passed to the function.

Remaining tasks

  1. Test coverage
  2. Fix

User interface changes

None.

API changes

None.

Data model changes

None.

MR with a fix will follow.

🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇳🇱Netherlands megachriz

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @megachriz
  • Merge request !216Resolve #3534622 "Wrong clean action applied" → (Open) created by megachriz
  • Comment 7 days ago →
    🇳🇱Netherlands megachriz
  • Comment 4 days ago →
    🇨🇦Canada joelpittet Vancouver

    I love the simplification here — big +1 from me.

    One thing I’m wondering about: with the new approach, if a plugin throws an exception during clean(), it looks like the remaining plugins won’t be called. Previously, each plugin’s clean was isolated inside its own listener. Is that an intentional change, or should we consider wrapping each plugin’s call in its own try/catch? I think the answer is that the catches happen deeper anyways so not a concern but thought I'd ask anyway.

    Also, is there any risk that one plugin’s clean() call could mutate shared state on the feed in a way that affects how later plugins behave within the same loop?

  • Comment 4 days ago →
    🇳🇱Netherlands megachriz

    @joelpittet
    Good catch about the exception throwing during clean(). I think if one of the plugins throws an exception, the remaining plugins should still be called. I have adjusted this. (In practice, only the processor plugin in Feeds implements CleanableInterface, but in theory, there could exist contributed modules that implement it for the other plugins).

    Also, is there any risk that one plugin’s clean() call could mutate shared state on the feed in a way that affects how later plugins behave within the same loop?

    Yes, each clean() receives the same state object, so they could manipulate that. I'm not sure yet if that is a concern. At least, that behavior doesn't change with the code changes.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024