Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I am using entity_lookup plugin at the end of a pipeline. Some of my entries do not have a value.
What I'm finding is that even if I use skip_on_empty or any other process plugins NONE of them even bother to run because the error is tripped during the construct phase, before the pipeline has even had a chance to run.
So essentially this plugin tries to set itself up before it even knows if it will be needed or not.
I found the problem is triggerd in the query method. Fortunately I know if the value is empty at the start of the query so I can simply not build the query object in that case. So I am starting the query method in entity_lookup with the following:
if (empty($value)) {
return NULL;
}...and now everything in the pipeline works as expected.
Earlier in my debugging process I found that if I did not run parent::__construct that would get me further along, but the above code seems to be a much better for now in my case.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff_8-10.txt | 814 bytes | danflanagan8 |
| #10 | migrate_plus-lookup-2830058-10.patch | 1.66 KB | danflanagan8 |
| #8 | migrate_plus-lookup-2830058-8-FAIL.patch | 887 bytes | danflanagan8 |
| #7 | 2830058-7.patch | 816 bytes | Thomas Cys |
| #3 | entity_lookup-plugin-fails-with-empty-values-2830058-3.patch | 628 bytes | badrange |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedRyan Weal created an issue. See original summary.
Comment #2
mikeryanAre you sure it's in the constructor? I don't see how the query() would be invoked there... That being said, I think transform() should pass through an empty value instead of calling query().
Comment #3
badrange CreditAttribution: badrange at Digia commentedWould this be one way to resolve this?
We started seeing this when I did some updates to our migration job, and this patch helps preventing blank terms from being created.
Comment #4
heddnI hate to do this, but it also won't be hard to fix. Let's add a quick unit test of this. Setting back to NW for tests.
Comment #5
badrange CreditAttribution: badrange at Digia commentedI have later found out that
skip_on_emptysometimes gets skipped due to multiple being set when you have an empty array as a value: https://www.drupal.org/node/2905929Comment #6
Thomas CysFor PHP 8.1 this results in a deprecation message being thrown when you run a migration that uses entity_lookup with an empty value.
Deprecated: addcslashes(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/html/docroot/core/lib/Drupal/Core/Database/Connection.php on line 1483Comment #7
Thomas CysComment #8
danflanagan8Here's the little test requested in #4. I added a couple assertions to the existing Kernel test.
The first new assertion adds missing coverage for passing an empty array. I think we want to make sure not to modify that behavior. The second new assertion triggers the error reported in #6.
I've also updated the title slightly to remove mention of the constructor.
Comment #10
danflanagan8Failure looks perfect. Here's the fix from #7 on top of the fail patch.
My only concern here is whether
emptyis too strong here. Is there ever a reasonable scenario for passing zero? OrFALSE? I don't think so. The string 'false' is not empty, so that's not a problem.I also did some testing locally without the patch and the query could not find a user with the name
0as is. (That's a string zero.) So using empty is probably ok.