drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.33k stars 1.08k forks source link

migrate:import idlist option fails when the first key is a string #4730

Open marvil07 opened 3 years ago

marvil07 commented 3 years ago

Describe the bug On migration:import's --idlist the ids parsing with : as internal item ids separator fails when the first key is a string.

To Reproduce Use the following example migration test_migration2.yml.txt

Try to run the migration with one of the ids, e.g.

vendor/bin/drush migrate:import --idlist='"i:1":2' test_migration2

Expected behavior --idlist will filter correctly without an error triggering.

Actual behavior Error is produced, e.g.

Workaround If you have control of the source plugin, and one of the keys has integer type, change its getIds() to list first the integer key. E.g. on the example test_migration2 migration, change the source ids order, then remove your map and message tables, and try again with the flipped ids, --idlist='2:"i:1"' will work.

System Configuration

Q A
Drush version? 10.x, dbdb6733655231687d8ab68cdea6bf9fedbd0562
Drupal version? 9.2.x, b6d40d0
PHP version 7.3
OS? Linux

Additional information This was discovered on the context of #4728.

claudiu-cristea commented 3 years ago

@marvil07 what exactly is the error?

marvil07 commented 3 years ago

@claudiu-cristea, here the full output based on the migration YAML from the last comment:

$ vendor/bin/drush migrate:import --idlist='"i:1":2' test_migration2
 [warning] array_combine(): Both parameters should have an equal number of elements MigrateExecutable.php:279
 [error]  TypeError: Argument 1 passed to Drupal\migrate\Plugin\migrate\id_map\Sql::setUpdate() must be of the type array, bool given, called in vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php on line 279 in Drupal\migrate\Plugin\migrate\id_map\Sql->setUpdate() (line 853 of web/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php) #0 vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php(279): Drupal\migrate\Plugin\migrate\id_map\Sql->setUpdate(false)
#1 vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php(244): Drush\Drupal\Migrate\MigrateExecutable->handleMissingSourceRows(Object(Drupal\migrate\Plugin\Migration))
#2 [internal function]: Drush\Drupal\Migrate\MigrateExecutable->onPreImport(Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#3 web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher)
)
#4 web/core/modules/migrate/src/MigrateExecutable.php(158): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...')
#5 vendor/drush/drush/includes/drush.inc(206): Drupal\migrate\MigrateExecutable->import()
#6 vendor/drush/drush/includes/drush.inc(197): drush_call_user_func_array(Array, Array)
#7 vendor/drush/drush/src/Drupal/Commands/core/MigrateRunnerCommands.php(348): drush_op(Array)
#8 [internal function]: Drush\Drupal\Commands\core\MigrateRunnerCommands->executeMigration(Object(Drupal\migrate\Plugin\Migration), 'test_migration2', Array)
#9 vendor/drush/drush/src/Drupal/Commands/core/MigrateRunnerCommands.php(298): array_walk(Array, Array, Array)
#10 [internal function]: Drush\Drupal\Commands\core\MigrateRunnerCommands->import('test_migration2', Array)
#11 vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#12 vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#13 vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#14 vendor/consolidation/annotated-command/src/AnnotatedCommand.php(311): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#15 vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 vendor/symfony/console/Application.php(1027): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#17 vendor/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput)) 
#18 vendor/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#19 vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 vendor/drush/drush/src/Runtime/Runtime.php(48): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#21 vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run(Array)
#22 vendor/drush/drush/drush(4): require('...')
#23 {main}. 
TypeError: Argument 1 passed to Drupal\migrate\Plugin\migrate\id_map\Sql::setUpdate() must be of the type array, bool given, called in vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php on line 279 in web/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php on line 853 #0 vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php(279): Drupal\migrate\Plugin\migrate\id_map\Sql->setUpdate(false)
#1 vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php(244): Drush\Drupal\Migrate\MigrateExecutable->handleMissingSourceRows(Object(Drupal\migrate\Plugin\Migration))
#2 [internal function]: Drush\Drupal\Migrate\MigrateExecutable->onPreImport(Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#3 web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#4 web/core/modules/migrate/src/MigrateExecutable.php(158): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Drupal\migrate\Event\MigrateImportEvent), 'migrate.pre_imp...')
#5 vendor/drush/drush/includes/drush.inc(206): Drupal\migrate\MigrateExecutable->import()
#6 vendor/drush/drush/includes/drush.inc(197): drush_call_user_func_array(Array, Array)
#7 vendor/drush/drush/src/Drupal/Commands/core/MigrateRunnerCommands.php(348): drush_op(Array)
#8 [internal function]: Drush\Drupal\Commands\core\MigrateRunnerCommands->executeMigration(Object(Drupal\migrate\Plugin\Migration), 'test_migration2', Array)
#9 vendor/drush/drush/src/Drupal/Commands/core/MigrateRunnerCommands.php(298): array_walk(Array, Array, Array)
#10 [internal function]: Drush\Drupal\Commands\core\MigrateRunnerCommands->import('test_migration2', Array)
#11 vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#12 vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#13 vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#14 vendor/consolidation/annotated-command/src/AnnotatedCommand.php(311): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#15 vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 vendor/symfony/console/Application.php(1027): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#17 vendor/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#18 vendor/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#19 vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 vendor/drush/drush/src/Runtime/Runtime.php(48): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#21 vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run(Array)
#22 vendor/drush/drush/drush(4): require('...')
#23 {main}
TypeError: Argument 1 passed to Drupal\migrate\Plugin\migrate\id_map\Sql::setUpdate() must be of the type array, bool given, called in vendor/drush/drush/src/Drupal/Migrate/MigrateExecutable.php on line 279 in Drupal\migrate\Plugin\migrate\id_map\Sql->setUpdate() (line 853 of web/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php).
 [warning] Drush command terminated abnormally.

Notice that this does not happen when the first key is numeric, e.g. Use the same migration YAML, change the source.ids order to have the revision field first, remove the map and message tables from the database, and try to run a similar command, vendor/bin/drush migrate:import --idlist='2:"i:1"' test_migration2.

Sounds like the problem comes from the following line https://github.com/drush-ops/drush/blob/e436955b5556113dda0af8a3c4225f0b209ed329/src/Drupal/Migrate/MigrateExecutable.php#L279

claudiu-cristea commented 3 years ago

@marvil07, thank you for report. I've just merged #4778 into 10.x, which refactors that part of code. I wonder if you can test the above scenario with the 10.x branch and check if the error is still there. Thank you for patience.

marvil07 commented 3 years ago

@claudiu-cristea, thanks for the feedback!

I have re-tested with drush at current 10.x, i.e. at 89abfb443c77479c33ef3f0fdfa0c3a072c54acf, which includes the mentioned changes at #4778. I am using the same migration example YAML mentioned above.

The results are the same, i.e. --idlist is failing to import a row from a migration with two identifiers when the first one is a string.

Here a full log of the testing for convenience.

# 1. Fails when 1st id is a string.

$ grep -A4 ids: web/modules/testcallback/migrations/test_migration2.yml
  ids:
    identifier:
      type: string
    revision:
      type: integer
$ vendor/bin/drush cr
 [success] Cache rebuild complete.
$ vendor/bin/drush migrate:status test_migration2
 ----------------- -------- ------- ---------- ------------- --------------- 
  Migration ID      Status   Total   Imported   Unprocessed   Last Imported  
 ----------------- -------- ------- ---------- ------------- --------------- 
  test_migration2   Idle     2       0          2                            
 ----------------- -------- ------- ---------- ------------- --------------- 
$ vendor/bin/drush migrate:import test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Processed 2 items (2 created, 0 updated, 0 failed, 0 ignored) in 0.3 seconds (377.5/min) - done with 'test_migration2'
$ vendor/bin/drush sql:query 'select sourceid1, sourceid2 from migrate_map_test_migration2'
i:1     2
i:2     1

$ vendor/bin/drush migrate:rollback test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Rolled back 2 items - done with 'test_migration2'
$ vendor/bin/drush migrate:import --idlist='"i:1":2' test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Processed 0 items (0 created, 0 updated, 0 failed, 0 ignored) in 0 seconds (0/min) - done with 'test_migration2'

# Some cleanup and change the identifiers order.

$ vendor/bin/drush migrate:rollback test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Rolled back 0 items - done with 'test_migration2'
$ vendor/bin/drush sql:query 'drop table if exists migrate_map_test_migration2; drop table if exists migrate_message_test_migration2;'

$ vi web/modules/testcallback/migrations/test_migration2.yml

# 2. Works OK 1st id is an integer.

$ grep -A4 ids: web/modules/testcallback/migrations/test_migration2.yml
  ids:
    revision:
      type: integer
    identifier:
      type: string
$ vendor/bin/drush cr
 [success] Cache rebuild complete.
$ vendor/bin/drush migrate:status test_migration2
 ----------------- -------- ------- ---------- ------------- --------------- 
  Migration ID      Status   Total   Imported   Unprocessed   Last Imported  
 ----------------- -------- ------- ---------- ------------- --------------- 
  test_migration2   Idle     2       0          2                            
 ----------------- -------- ------- ---------- ------------- --------------- 
$ vendor/bin/drush migrate:import test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Processed 2 items (2 created, 0 updated, 0 failed, 0 ignored) in 0.2 seconds (578.2/min) - done with 'test_migration2'
$ vendor/bin/drush sql:query 'select sourceid1, sourceid2 from migrate_map_test_migration2'
1       i:2
2       i:1

$ vendor/bin/drush migrate:rollback test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Rolled back 2 items - done with 'test_migration2'
$ vendor/bin/drush migrate:status test_migration2
 ----------------- -------- ------- ---------- ------------- --------------- 
  Migration ID      Status   Total   Imported   Unprocessed   Last Imported  
 ----------------- -------- ------- ---------- ------------- --------------- 
  test_migration2   Idle     2       0          2                            
 ----------------- -------- ------- ---------- ------------- --------------- 
$ vendor/bin/drush migrate:import --idlist='2:"i:1"' test_migration2
 2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) in 0.1 seconds (811/min) - done with 'test_migration2'
marvil07 commented 3 years ago

@claudiu-cristea, I have opened pull request #4783 with a test that captures the error. No fix on the source yet, so it is expected to fail.