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
Comment #2
TrevorBradley CreditAttribution: TrevorBradley commentedComment #3
heddnThis could use tests and a patch.
Comment #10
mikelutzFor 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.