Better document how skip_on_empty works with multiple value sources

I just faced this same issue.

Can someone else confirm that this is the same problem as Why doesn't skip_on_empty work when migrating from a text field??

I've had this same issue and it was driving me crazy.
When there is an empty array, skip_on_empty never runs the checks on the values and so it just keeps on keeping on.
Would it make sense to make a new plugin that checks the value / all values for a source and skips processing if it / they are empty?

Isn't extending skip_on_empty an option, instead of working around it not handling multiple values?

I'm not sure if it should be handling multiple values by default in core.

<?php

namespace Drupal\*\Plugin\migrate\process;

use Drupal\migrate\Plugin\migrate\process\SkipOnEmpty;

/**
 * Skips processing the current row when the input values are empty.
 *
 * @see \Drupal\migrate\Plugin\migrate\process\SkipOnEmpty
 *
 * @MigrateProcessPlugin(
 *   id = "skip_on_empty_multiple",
 *   handle_multiples = TRUE
 * )
 */
class SkipOnEmptyMultiple extends SkipOnEmpty {

}

Related to this #2154209: Process refactor: multiple handling

I was running into this from D7 too, but I guess that's still not a priority. ;)

Here's the full code that I've just added to a project. Just one file at mymodule/src/Plugin/migrate/process/SkipOnEmptyMultiple.php.

I used the suggestion by StefanPr of extending SkipOnEmpty.

namespace Drupal\mymodule\Plugin\migrate\process;

use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\Plugin\migrate\process\SkipOnEmpty;
use Drupal\migrate\Row;

/**
 * Skip on empty, including multivalue fields.
 *
 * This is needed for text fields and many fields on Drupal sources, because
 * even if the cardinality is set to 1, the field data is still available in a
 * single-item array.
 *
 * @MigrateProcessPlugin(
 *   id = "skip_on_empty_multiple",
 *   handle_multiples = TRUE
 * )
 */
class SkipOnEmptyMultiple extends SkipOnEmpty {

  /**
   * Indicate whether there are multiple values for this field.
   *
   * @var bool
   */
  protected $multiple;

  /**
   * {@inheritdoc}
   */
  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
    $value = parent::transform($value, $migrate_executable, $row, $destination_property);

    // Set multiple.
    $this->multiple = is_array($value) && !empty($value);

    return $value;
  }

  /**
   * {@inheritdoc}
   */
  public function multiple() {
    return $this->multiple;
  }

}

With this in place, you can use skip_on_empty_multiple as a process plugin.

skip_on_empty does not work for me when encountering empty arrays (ie, empty field). Say field_offers_is_free is a text field, the following migrate process throws extraction errors when the field is empty for a single node:

field_offers_is_free:
    -
      plugin: skip_on_empty
      method: process
      source: offers
    -
      plugin: extract
      index:
        - und
        - 0
        - value

As of 8.5.6, this does work:

field_offers_is_free:
    -
      plugin: skip_on_empty
      method: process
      source: offers/und/0/value

Similarly, if field_offers_is_free is a multiple-value field, the following works:

field_offers_is_free:
    -
      plugin: skip_on_empty
      method: process
      source: offers/und
    -
      plugin: sub_process
      process:
        value: value

I ran into this today. I had completely forgotten about changing the source value to be an array and then I found this and the proverbial light bulb went off. Thanks to badrange posting this issue and to MakaziMtingwa for the example.

So this actually works as designed but I'm not closing it. Instead setting to NW for documentation, so this can into the handbook. This is also pertinent to skip_row_if_not_set, see https://www.drupal.org/project/drupal/issues/2615602#comment-12618383.

Removing the tag because this issue affects all migrations.

If this works as designed, does that mean that core does not support skipping empty arrays? Or is there another way to do it?

Agreed. We should keep this open until there's a standard way to handle this.

We are migrating body field values (long text) into a paragraph entity, and saw an issue where paragraph entities were being created even when there were empty values.

We were able to solve this issue by using @nadavoid's custom process plugin to check for a value in the body field before processing it for the body field on the paragraph.

This is what our process configuration ended up looking like:

process:
  field_body:
    -
      plugin: skip_on_empty_multiple
      method: row
      source: body
    -
      plugin: iterator
      process:
        value: value
        format:
          plugin: default_value
          default_value: full_html
destination:
  plugin: 'entity_reference_revisions:paragraph'
  default_bundle: text

Here is the custom process plugin (simplified logic in ::transform()):

<?php

namespace Drupal\mymodule\Plugin\migrate\process;

use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\Plugin\migrate\process\SkipOnEmpty;
use Drupal\migrate\Row;

/**
 * Skip on empty, including multivalue fields.
 *
 * This is needed for text fields and many fields on Drupal sources, because
 * even if the cardinality is set to 1, the field data is still available in a
 * single-item array.
 *
 * @MigrateProcessPlugin(
 *   id = "skip_on_empty_multiple",
 *   handle_multiples = TRUE
 * )
 */
class SkipOnEmptyMultiple extends SkipOnEmpty {

  /**
   * Indicate whether there are multiple values for this field.
   *
   * @var bool
   */
  protected $multiple;

  /**
   * {@inheritdoc}
   */
  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
    // Set multiple.
    $this->multiple = is_array($value) && !empty($value);

    return parent::transform($value, $migrate_executable, $row, $destination_property);
  }

  /**
   * {@inheritdoc}
   */
  public function multiple() {
    return $this->multiple;
  }

}

Hope this helps someone!

FYI I turned the code samples into patches for migrate_plus: #3054494: Alternative solution to skip_on_empty_multiple plugin from 2905929

This definitely fixes the issue I was encountering with empty image fields being processed even though skip_on_empty was set. Same basic issue: if the source is an empty array, it seems to not get considered for whether to skip_on_empty. Thanks everyone!

Had same issue with the normal 'skip_on_empty' plugin processing empty image field and this patch worked great.

Ran into this issue, and I believe the SkipRowIfNotSet (skip_row_if_not_set) plugin should work around this issue if you know what the index should be.

I still think this is a bug and should be looked into.

In my case, it was enough to use "single_value" plugin (migrate_plus) before calling "skip_on_empty". It's a switch to disable multivalue processing. There is also an opposite way available using "multiple_values" plugin.

Inside the example below, "migration_lookup" switches processing to multivalue, so if I need to check the whole lookup result is empty, I need to call "single_value" there.

- plugin: migration_lookup
  source: some_array_of_values
  migration: some_migration
  no_stub: true
- plugin: single_value
- plugin: skip_on_empty
  method: row

Best regards
Vit

I commented on this about 9 months ago when I used the 'skip_on_empty_multiple' process plugin which worked fine and since used it on another migration, but as I bumped into the same issue again on a new migration I re-discovered this issue and decided to try the 'single_value' plugin solution explained above and works perfectly.
Means we don't have to include another patch on the site and make use of what's already available (or at least provided by the 'Migrate Plus' module).

I think I have the same problem, or a similar bug.

I am trying to migrate from a field_test/0/format, which has no value in the source database, but it is somehow getting a value of 'basic_html' in the migration. I don't even understand how, it makes no sense.

If I have this migration:

    -
      plugin: get
      source: field_test/0/format
    -
      plugin: callback
      callable: var_dump

Results in this output:

   1/286 [>---------------------------]   0%string(9) "full_html"
   2/286 [>---------------------------]   0%string(9) "full_html"
   3/286 [>---------------------------]   1%string(9) "full_html"
   4/286 [>---------------------------]   1%string(18) "full_html"
   5/286 [>---------------------------]   1%NULL
   6/286 [>---------------------------]   2%NULL

As you can see, some values are clearly NULL.

When I change the migration to this:

    -
      plugin: get
      source: field_test/0/format
    -
      plugin: skip_on_empty
      method: process
    -
      plugin: callback
      callable: var_dump

The output is this:

  1/286 [>---------------------------]   0%string(9) "full_html"
   2/286 [>---------------------------]   0%string(9) "full_html"
   3/286 [>---------------------------]   1%string(9) "full_html"
   4/286 [>---------------------------]   1%string(18) "full_html"
   7/286 [>---------------------------]   2%string(9) "full_html"
   8/286 [>---------------------------]   2%string(9) "full_html"
  10/286 [>---------------------------]   3%string(9) "full_html"

Everything is getting set to 'full_html'. So the only conclusion I can come to is somehow the skip_on_empty plugin is creating a value out of nowhere, or using the value from the previous row. Seems like a bug to me.

I tried inserting single_value plugin above skip_on_empty but it made no difference.

Rob230... from your output, it appears to be working. The final output has skipped #5 & #6 due to the skip_on_empty and proceeded to #7 & #8.

Yes the migration is actually working. I stepped through it in a debugger and skip_on_empty worked correctly.

My problem was something else. The field settings have a default value of:

value: ''
format: 'basic_html'

So when the migration says that field has no value Drupal is setting it to the default.

MR opened but I'm concerned that it does need tests and backwards compatibility might be an issue.

if BC does count as an issue:
- easy fix IMO would be to add optional flag to treat it as handles multiple and otherwise use a foreach loop internally.

Tests are semi-blocked based on whether BC needs to be handled

Changing the title to be clearer; adding parent issue.

One way in which this is a problem is that you can't use it to check that a migration lookup returned something.

E.g.

  mid:
    -
      plugin: migration_lookup
      migration: my_dependent_migration
      no_stub: true
      source:
        - id
        - revision_id
        - content_translation_source/0/value
    -
      plugin: skip_on_empty
      method: row

That won't work, because migration_lookup returns an empty array if nothing is found. And skip_on_empty will carry on going (in fact, it won't even be called).

I managed to import the multi value paragraphs without throwing empty array errors using a callback method - array_filter

Here field_section_two is the paragraph field

combination_1:
    - 
      plugin: skip_on_empty
      source: image_1
      method: process
    -   
      plugin: migration_lookup
      migration: product_image_inset_one
      no_stub: true
      source:
        - ID
        - image_1
  combination_2:
    - 
      plugin: skip_on_empty
      source: image_2
      method: process
    - 
      plugin: migration_lookup
      migration: product_image_inset_two
      no_stub: true
      source:
        - ID
        - image_2
  field_section_two:
    - 
      plugin: callback
      callable: array_filter
      source:
        - '@combination_1'
        - '@combination_2'
    - 
      plugin: skip_on_empty
      method: process
    -
      plugin: sub_process
      process:
          target_id: '0'
          target_revision_id: '1'

As a reminder, and as @quietone pointed out in #13 this is not a bug, and we cannot change skip_on_empty to handle multiple both because of BC reasons, and because it would then not work as it is supposed to. This issue is about improving documentation on how skip_on_empty interacts with arrays and sources that contain multiple values. skip_on_empty is not supposed to skip empty multiple arrays, nor does it need to for any of the standard core migrations. If you think you need that for a custom pipeline, most likely your pipeline is not structured correctly, and in the rare case there might be a legitimate reason to need to do this, there are a number of workarounds using popular contrib plugins, such as single_value from migrate_plus.

One concern I have about adding documentation around this inside the plugin is that none of this information really has anything to do with the plugin itself. The concepts here are with regard to multiple vs single values in pipelines, and how multiple and single values interact with plugins depending on whether the plugin declares handle_multiples: true. If we add documentation along the lines of "Skip on empty does not operate on an entire value if the pipeline is in 'multiple' mode. In this case, skip on empty will be called once for each element in the array. If a 'multiple' value contains no elements, skip on empty won't be called, and the pipeline will continue with the empty multiple array", We are just describing how plugins that don't `handle_multiples`. The exact same documentation would be applicable to all 58 non-multiple plugins in core. I feel like the plugin's documentation should focus on what the plugin does. What it's outputs are given an input and what the configuration options are. With an empty 'multiple' array this plugin and any other single plugin is never expected to be called. I'm not convinced that one random single plugin header is the correct place to add that documentation.

If you think you need that for a custom pipeline, most likely your pipeline is not structured correctly

I think that's a bit harsh :) , but I agree with everything else in the previous two comments from @mikelutz.

In light of comment #40 I would recommend closing this issue in favor of #2961560: Document usage of 'handle_multiples' in process plugins.

Setting to NR to get input on that approach.

For the record, I followed that comment with the correct workaround in the rare cases it's appropriate, but I definitely stand by it. There is almost never a need to halt processing on a empty multiple source value with core or migrate_plus plugins. Any non-mutiple plugin isn't ran and all multiple plugins should handle that case, and core entity destinations can handle empty arrays as easily as nulls, so it's rare you would need to convert an empty array to a null outside of very custom plugins.

I think @vinodhinisureshbabu in #37 poses a nice solution:
```
field_plaintext_author:
- plugin: parse_lcm_author
source: body/0/value
- plugin: default_value
default_value: 0
- plugin: skip_on_value
value: 0
method: row
```
My plugin `parse_lcm_author` returns an array of values (or empty array `[]`), and `default_value` understands an empty array to require a default value. I used 0 but any scalar value will do. (skip row or process as is appropriate to your needs -- I'm building paragraphs in this migration, so it was good to get out without a mapping made)