Problem/Motivation

Got the following error while running feeds importer via cronjob:
WD php: TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in FeedsProcessor->clean() (line 624 of /app/web/sites/all/modules/contrib/feeds/plugins/FeedsProcessor.inc).

Issue fork feeds-3337811

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

tormi created an issue. See original summary.

megachriz’s picture

Status: Active » Postponed (maintainer needs more info)

Can you give more information about the source you were trying to import and how the the feeds importer looks like?

Can it be that the source was completely empty? That would be my first guess. Because assuming it is about $state->removeList, that variable gets set in FeedsProcessor::process(), but maybe there is a situation where FeedsProcessor::process() never gets called?

tormi’s picture

MegaChriz, you're correct - the source was completely empty :)

Should I close the issue or should we check if $state->removeList is countable (and give some feedback if not)?

megachriz’s picture

Status: Postponed (maintainer needs more info) » Active

I think this is worth to fix. What I think we need to do is:

  1. Add a test that imports an empty source. The test should fail with the error given. The test would ensure that the error doesn't accidentally come back in the future.
  2. In FeedsProcessor::clean() we should probably check if $state->removeList is not null before proceeding further.

I thought too about that the clean stage should perhaps be entirely skipped (so no calls to clean() would happen) when the source is empty, but there there is also a feature request for cleaning data in that case: #2333667: Add an option to delete all nodes when the feed source is empty.

dsnopek’s picture

Status: Active » Needs review
StatusFileSize
new487 bytes

Here's a patch to fix this error

megachriz’s picture

StatusFileSize
new2.46 KB
new2.94 KB

Here is a patch with tests.

cboyden’s picture

StatusFileSize
new3.39 KB

There's another instance of the potential PHP error in FeedsNodeProcessor.inc. I've attached an updated patch that includes this.

Status: Needs review » Needs work

The last submitted patch, 7: feeds-clean-empty-3337811-7.patch, failed testing. View results

nathan tsai’s picture

#7 Worked for me (although I don't know why the tests are not passing)

megachriz’s picture

The patch in #7 fails tests because the file "empty.csv" is missing:

diff --git a/tests/feeds/empty.csv b/tests/feeds/empty.csv
new file mode 100644
index 00000000..e69de29b

I hope to finalize this issue soon!

bharath-kondeti made their first commit to this issue’s fork.

bharath-kondeti’s picture

StatusFileSize
new1.42 KB

Addressing #10.

nathan tsai’s picture

@bharath-kondeti, did you intentionally leave out the tests that were in #6 and #7?

nathan tsai’s picture

Also, on a side note: When using Feeds Extensible Parsers, an empty response still results in no nodes being deleted or unpublished.

E.g. I have 10 nodes with the feed importer setting "Delete missing nodes". If I then delete all of them (and thus, the response is empty), none of the nodes are deleted.

Don't know if this issue belongs here or on the Feeds Extensible Parsers module.

bharath-kondeti’s picture

StatusFileSize
new3.4 KB

@Nathan Tsai
I have missed them, while creating the patch. I will hide the file from #12. Here is the updated patch.

bharath-kondeti’s picture

The patch seems to be not failing more test cases. Let me re-work.

  • dsnopek authored 4300af94 on 7.x-2.x
    Issue #3337811 by bharath-kondeti, MegaChriz, dsnopek, cboyden, Nathan...
megachriz’s picture

Status: Needs work » Fixed

I committed #6 with the additional change from #7 included.

megachriz’s picture

@Nathan Tsai

Also, on a side note: When using Feeds Extensible Parsers, an empty response still results in no nodes being deleted or unpublished.

E.g. I have 10 nodes with the feed importer setting "Delete missing nodes". If I then delete all of them (and thus, the response is empty), none of the nodes are deleted.

For your issue there exist an other open issue: #2333667: Add an option to delete all nodes when the feed source is empty

It is intentional that an empty source doesn't delete everything - to account for the situation where the source was temporary unavailable. The issue there is marked RTBC, but it still needs tests to be accepted.

Status: Fixed » Closed (fixed)

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

nathan tsai’s picture

@MegaChriz, thank you for pointing that out. I did not realize that feeds do not import nodes when the response is empty or there are no items.

For future reference, I added the following code for my REST Export to ensure there is always at least one item in my response.

use Drupal\views\ViewExecutable;
use Drupal\views\ResultRow;
use Drupal\node\Entity\Node;

/**
 * Implements hook_views_post_execute().
 */
function HOOK_views_post_execute(ViewExecutable $view) {
  if ($view->id() == 'MY_VIEW_ID') {
    // Feeds does not import an empty response. Therefore, add a dummy feeds item.
    // @see https://www.drupal.org/project/feeds/issues/2333667#comment-15145873
    if ($view->total_rows === 0) {
      $dummyNode = Node::create([
        'type' => 'job_posting',
        'nid'  => -1, // Our GUID. This is valid because we never save the node
        'title' => 'PLACEHOLDER NODE',
        'status' => 0, // Be unpublished, so it doesn't appear on the importing website
      ]);
      $view->result[] = new ResultRow([
        '_entity' => $dummyNode,
      ]);
    }
  }
}
nathan tsai’s picture

nathan tsai’s picture

Oh wait, my issue was technically different. Removing the "related" issue.