Error running d6_block migration

A test should be quite easy, we just need to add a block to our dumps that is provided by the block module and then assert it is migrated.

Settings to NR for the bot although it's pretty obvious this code path isn't run anyway :)

@benjy,

A block provided by the block module? That's just a custom block, right? If so, there are already four of them in the Drupal6Block dump and we're testing all of them in MigrateBlockTest.

Or - am I missing something?

-mike

Wow, so it was another PHP WTF :)

This is the current code, but when we have a numeric id, it still works because PHP dereferences the first character of the string for us, which is "4".

$block_ids = "4";
$bid = $block_ids[0];

So we would have issues with collisions. Eg, block id 1 would end up with the same block id as block 12. Patch attached shows the issue. Also, the block ids are generated using the block delta and not the block id which I also think is wrong.

EDIT: $delta is unique once we've filtered on module so the last part of my comment is wrong.

So I added a test that causes a collisisson but it seems that core doesn't care if $block->plugin have identical values across blocks. So, not sure at this point what the value of $block_ids was which caused the issue.

1. The fix is obviously necessary and good
2. I doubt we want to add more than ten blocks just to test this
3. The likely cause of the error: you migrate custom block 14 and custom block 1 has been deleted some time in the ancient past and so there's no block going by block id 1.

    if ($destination_ids) {
      if ($scalar) {
        if (count($destination_ids) == 1) {
          return reset($destination_ids);
        }
      }
      else {
        return $destination_ids;
      }
    }

From Drupal\migrate\Plugin\migrate\process\Migration::transform(). This looks a little iffy - if we just returned $destination_ids it'd always be an array and we'd be more consistent.

waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

I had that originally and then people were pushing back at that because 99% of the time you have single-to-single mapping and so they wanted this back from D7.

@chx ho hum... I don't know every bit of migration code history :)

Just noting that it is this type of magic that just gets us into problems since it is runtime determined.

Ah, I know: multiple handling is enabled by this. Yes, more magic, but very useful magic.

Don't we need to just create one block with an id greater than > 9 and have no other blocks?

I think we should have a test here.

Hmm, I tried this at the weekend and the problem is, if we create a block with an id > than 9 say, 11, but a box with an id of 1 doesn't exist, we get a fatal error in the test.

However, if we create the box with id 1, we end up with two blocks (one for block 1 and another for block 11) that both have $block->plugin values that are equal. And core has no problem with that.

We could assert specifically that they should have separate $block->plugin values which would work for this bug, but it's very specific?

I'm also open to other approaches to testing this bug if someone has a better idea.

Ah yes, of course, we can just add an id 99 and then it'll blow up cos there's no 9.

Not sure about #13, when I did that as I mentioned in #12, I get a fatal error regardless of this patch.

The patch worked for me. I am using beta4. Earlier I was getting following error message:
Running d6_block [ok]

Fatal error: Call to a member function uuid() on a non-object in C:\wamp\www\bet
a4\core\modules\migrate_drupal\src\Plugin\migrate\process\d6\BlockPluginId.php o
n line 87

Call Stack:
0.0000 134424 1. {main}() C:\Users\idsa\AppData\Roaming\Composer\vendo
r\drush\drush\drush.php:0
0.0312 1689896 2. drush_main() C:\Users\idsa\AppData\Roaming\Composer\v
endor\drush\drush\drush.php:16
0.4680 5047456 3. Drush\Boot\DrupalBoot->bootstrap_and_dispatch() C:\Us
ers\idsa\AppData\Roaming\Composer\vendor\drush\drush\drush.php:76
1.4820 14364056 4. drush_dispatch() C:\Users\idsa\AppData\Roaming\Compos
er\vendor\drush\drush\lib\Drush\Boot\DrupalBoot.php:46
1.8564 15818528 5. call_user_func_array:{C:\Users\idsa\AppData\Roaming\C
omposer\vendor\drush\drush\includes\command.inc:178}() C:\Users\idsa\AppData\Roa
ming\Composer\vendor\drush\drush\includes\command.inc:178
1.8564 15818792 6. drush_command() C:\Users\idsa\AppData\Roaming\Compose
r\vendor\drush\drush\includes\command.inc:178
1.8564 15819424 7. _drush_invoke_hooks() C:\Users\idsa\AppData\Roaming\C
omposer\vendor\drush\drush\includes\command.inc:210
1.8720 15841248 8. call_user_func_array:{C:\Users\idsa\AppData\Roaming\C
omposer\vendor\drush\drush\includes\command.inc:359}() C:\Users\idsa\AppData\Roa
ming\Composer\vendor\drush\drush\includes\command.inc:359
1.8720 15841528 9. drush_migrate_manifest() C:\Users\idsa\AppData\Roamin
g\Composer\vendor\drush\drush\includes\command.inc:359
139.1053 34161384 10. Drupal\migrate\MigrateExecutable->import() C:\Users\i
dsa\AppData\Roaming\Composer\vendor\drush\drush\commands\core\migrate.d8.drush.i
nc:51
142.8533 39375656 11. Drupal\migrate\MigrateExecutable->processRow() C:\wam
p\www\beta4\core\modules\migrate\src\MigrateExecutable.php:275
142.8689 39397392 12. Drupal\migrate_drupal\Plugin\migrate\process\d6\Block
PluginId->transform() C:\wamp\www\beta4\core\modules\migrate\src\MigrateExecutab
le.php:398

Drush command terminated abnormally due to an unrecoverable error. [error]

Error: Call to a member function uuid() on a non-object in
C:\wamp\www\beta4\core\modules\migrate_drupal\src\Plugin\migrate\process\d6\Bloc
kPluginId.php,
line 87

So what needs to be done to get a test created for this? Add a bunch of custom blocks so that we ensure that block_id=10 doesn't mess with block_id=1?

-mike

Since we don't want to do #7, then the plan in #11 sounds simple and I think covers the problem. Should we update the interface to document the possibility for array $value and return types? It's documented in the params, but not the description. Also, the $value could be int|string|array instead of mixed, I think. Plus the @throws is not documented.

Regarding the fatal error encountered in #12, we could throw an exception if $destination_ids is empty.

i've got data produces this bug that looks like below, and applying this patch fixes things.

2
4
618
620
622
624
630
...

as mentioned in #14 MigrateBlockTest::testBlockMigration() produces a fatal when adding a new test custom block row, with a higher block id (99). not sure how to get started on adding to the tests for this but happy to try, with guidance.

Bump - this fell off the radar, but it totally breaks migrations for sites with a significant number of (perhaps historical) blocks, we need to address it.

Hi
Using the very last dev version of D8 core, had that same error.

Importing d6_block
Calling Drupal\migrate\MigrateExecutable::import() [14.18 sec, 49.8 MB] [debug]
PHP Fatal error: Call to a member function uuid() on null in /var/www/html/D8/core/modules/block/src/Plugin/migrate/process/d6/BlockPluginId.php on line 87
Drush command terminated abnormally due to an unrecoverable error. [error]
Error: Call to a member function uuid() on null in /var/www/html/D8/core/modules/block/src/Plugin/migrate/process/d6/BlockPluginId.php, line 87 [16 sec, 56.7 MB]

But the patch #19 can't be apply anymore, because the core/modules/migrate_drupal/src/Plugin/migrate/process/ doesnt look a valid path anymore.

Best regards
D.

Needs work is the correct status for an issue with a patch that needs additional work. Setting back to NW and adding the re-roll tag.

Just the reroll

Test for 11 blocks, not 10. Add tests for the eleventh block.

Hi
Patch #28 solve the error for me, many many thanks.
D.

@quietone thanks for writing a test. Can you upload a FAIL version of the patch, eg just the test without the fix?

@benjy, I can't produce a failure. I removed the fix and it ran happily. From #13, I tried removing the block with bid 1 and that just results in the migration never ending.

On one attempt I checked the $block->plugin values, and curiously they were alternating between 2 values. I had 5 blocks 3 with one of the values and 2 with the other.

This is addressed separately in #2408165: Migration Files for Drupal 7 Blocks; credited @quietone, @eliza411 and @bdone there.