This is probably more a warning to others, rather than an actual feature request. It's almost certainly an edge user case. But I'm not sure.

I just found a glitch in my code where I was importing paragraphs, with an explode, migration, iterator process chain.

My source CSV had an error in it, where my value had a trailing comma, like so:

key1,key2
"my_key_value_1,","my_key_value_2,"

The effect of this is that explode splits "my_key_value_1," up into two strings: "my_key_value_1" and "".

Migration::transform() comes along and checks:

$this->skipOnEmpty($value);

Which throws a MigrateSkipProcessException, causing both values of the exploded field transform to be discarded.

Back in MigrateExecutable::processRow(), I'd argue this exception isn't being handled correctly:

        if ($multiple && !$definition['handle_multiples']) {
          $new_value = array();
          if (!is_array($value)) {
            throw new MigrateException(sprintf('Pipeline failed for destination %s: %s got instead of an array,', $destination, $value));
          }
          $break = FALSE;
          foreach ($value as $scalar_value) {
            try {
              $new_value[] = $plugin->transform($scalar_value, $this, $row, $destination);
            }
            catch (MigrateSkipProcessException $e) {
              $new_value[] = NULL;
              $break = TRUE;
            }
          }
          $value = $new_value;
          if ($break) {
            break;
          }
        }

Is this the right way to iterate through the plugins for a non-scalar $value? What if one value in the $value array throws an exception, but the other doesn't? Shouldn't those individual values be discarded, rather than the entire array?

Wouldn't something like this be more appropriate?

          if (!is_array($value)) {
            throw new MigrateException(sprintf('Pipeline failed for destination %s: %s got instead of an array,', $destination, $value));
          }
          $break = FALSE;
          foreach ($value as $scalar_value) {
            try {
              $new_value[] = $plugin->transform($scalar_value, $this, $row, $destination);
            }
            catch (MigrateSkipProcessException $e) {
              // Do Nothing.  Don't add the scalar value to the $new_value array, just discard it.
              //$new_value[] = NULL;
              //$break = TRUE;
            }
          }
          $value = $new_value;
          //Check to see if $new_value was populated with any data.  If not, discard $value.
          if (count($value) == 0) {
            break;
          }
        }

Not suggesting any code changes or offering up any patches here, but I'm thinking this should be discussed. This also occurs without throwing up errors, so I had a vent I needed to focus into something practical. I'm content if this ends up as "works as designed".

Comments

TrevorBradley created an issue. See original summary.

TrevorBradley’s picture

Issue summary: View changes
heddn’s picture

Issue tags: +Needs tests

This could use tests and a patch.

Version: 8.2.6 » 8.2.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.

Version: 8.2.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mikelutz’s picture

Status: Active » Closed (works as designed)

For better or worse, this is the intention and skip_on_empty is working as designed. There's definitely other ways you may want to handle this in different situations, but there are other options, like using array_filter in a callback plugin for example, but this is what skip_on_empty does.