Problem/Motivation
The filter_format_permission process plugin hard codes the migration lookup name. This should be configurable, similar to #2640842: Make related file migration ID configurable in d6_cck_file.
Proposed resolution
Make the configuration configurable.
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration = NULL) {
return new static(
$configuration,
$plugin_id,
$plugin_definition,
$migration,
$container->get('plugin.manager.migrate.process')->createInstance('migration', ['migration' => 'd6_filter_format'], $migration)
);
}
Remaining tasks
code it.review it.- update/add change record
User interface changes
n/a
API changes
n/a
Data model changes
Original report
I get the following error when trying to import users
InvalidArgumentException: Passed variable is not an array or object, using empty array instead in /var/www/docroot/core/modules/migrate/src/Plugin/migrate/process/Flatten.php:30
Stack trace:
#0 /var/www/docroot/core/modules/migrate/src/Plugin/migrate/process/Flatten.php(30): ArrayIterator->__construct(NULL)
#1 /var/www/docroot/core/modules/migrate/src/MigrateExecutable.php(395): Drupal\migrate\Plugin\migrate\process\Flatten->transform(NULL, Object(Drupal\migrate_tools\MigrateExecutable), Object(Drupal\migrate\Row), 'permissions')
#2 /var/www/docroot/core/modules/migrate/src/Plugin/migrate/process/Migration.php(139): Drupal\migrate\MigrateExecutable->processRow(Object(Drupal\migrate\Row), Array)
#3 /var/www/docroot/core/modules/migrate/src/MigrateExecutable.php(381): Drupal\migrate\Plugin\migrate\process\Migration->transform(Array, Object(Drupal\migrate_tools\MigrateExecutable), Object(Drupal\migrate\Row), 'roles')
#4 /var/www/docroot/core/modules/migrate/src/MigrateExecutable.php(218): Drupal\migrate\MigrateExecutable->processRow(Object(Drupal\migrate\Row))
#5 /usr/local/share/drush/includes/drush.inc(720): Drupal\migrate\MigrateExecutable->import()
#6 /usr/local/share/drush/includes/drush.inc(711): drush_call_user_func_array(Array, Array)
#7 /var/www/docroot/modules/contrib/migrate_tools/migrate_tools.drush.inc(280): drush_op(Array)
#8 [internal function]: _drush_migrate_tools_execute_migration(Object(Drupal\migrate\Plugin\Migration), 'awm_users', Array)
#9 /var/www/docroot/modules/contrib/migrate_tools/migrate_tools.drush.inc(246): array_walk(Array, '_drush_migrate_...', Array)
#10 /usr/local/share/drush/includes/command.inc(422): drush_migrate_tools_migrate_import('awm_users')
#11 /usr/local/share/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#12 /usr/local/share/drush/includes/command.inc(199): drush_command('awm_users')
#13 /usr/local/share/drush/lib/Drush/Boot/BaseBoot.php(73): drush_dispatch(Array)
#14 /usr/local/share/drush/includes/preflight.inc(93): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#15 /usr/local/share/drush/drush.php(11): drush_main()
#16 {main}
I've generated this from the migrate-update --config-only drush command
langcode: en
status: true
dependencies: { }
id: awm_users
migration_tags:
- 'Drupal 7'
migration_group: migrate_drupal_7
label: 'User accounts'
source:
plugin: d7_user
process:
uid: uid
name: name
pass: pass
mail: mail
created: created
access: access
login: login
status: status
timezone: timezone
langcode:
plugin: user_langcode
source: language
fallback_to_site_default: false
preferred_langcode:
plugin: user_langcode
source: language
fallback_to_site_default: true
preferred_admin_langcode:
plugin: user_langcode
source: language
fallback_to_site_default: true
init: init
roles:
plugin: migration
migration: awm_users_role
source: roles
user_picture:
-
plugin: default_value
source: picture
default_value: null
-
plugin: migration
migration: upgrade_d7_file
destination:
plugin: 'entity:user'
migration_dependencies:
required:
- awm_users_role
optional:
- upgrade_d7_file
- upgrade_language
- upgrade_default_language
- upgrade_user_picture_field_instance
- upgrade_user_picture_entity_display
- upgrade_user_picture_entity_form_display
- upgrade_d7_field_instance
- upgrade_d7_user_role
Using Drupal 8.2.5. awm_users_role migrated ok.
Tbh I have no idea which of the 9 additional migration modules this would fall under. I'm trying to do a selective migration from drupal 7 to 8, so generating the migration templates through drush, and then running migrate-import.
Comment | File | Size | Author |
---|---|---|---|
#48 | interdiff_28-48.txt | 666 bytes | heddn |
#48 | 2850312-48.patch | 2.59 KB | heddn |
Comments
Comment #2
mikeryanThe problem is arising here:
The migration process plugin is not finding one of your roles in the awm_users_role map table, so it is attempting to create a stub role by passing the unmigrated role ID to the awm_users_role migration. Because a permissions array is not passed (and there is no way to pass more info to the stub: #2835586: Allow customization of stub rows from Migration process plugin - also note #2800279: Document that migrations used for stubbing need to deal with empty source values), that flatten plugin in the permissions mapping craps out.
So, the first thing I would do is figure out what role is causing you the problem, and figure out why it's not migrated in awm_users_role. If you decide it's a role you don't need, the easiest thing to do is to disable stubbing:
Comment #3
Gribnif CreditAttribution: Gribnif commentedI am also running into this problem. In this case I specifically want stubbing, but the flatten plugin doesn't handle the situation gracefully. I'm trying to migrate a custom table that refers to the D7 "role" table by rid:
So I want it to generate a stub row, but since no default value is generated by the migrate plugin for the permissions field, Flatten::transform() dies. I propose a simple solution: have Flatten::transform() return NULL if $value is null. This is also in keeping with #2800279. See attachment.
This solves the uncaught exception experienced by the original poster and me. However, the migration will still not work correctly because roles are config entities, and config entities cannot be used as stubs. I still feel that this patch is needed because instead of getting an uncaught exception, the user gets a caught one ("Config entities can not be stubbed."), which is stored in the migrate_message_* table. This is a much better user experience.
Comment #4
mikeryanYes, I think both d6_user and d7_user need to add
no_stub: true
to the roles mapping.Comment #5
heddnTo be more explicit, a novice would just need to add 'no_stub: true' to d6 & d7 user yml files.
Comment #6
Charlotte17 CreditAttribution: Charlotte17 at MTech, LLC commentedAdding
'no_stub: true'
#4-5Comment #7
Charlotte17 CreditAttribution: Charlotte17 at MTech, LLC commentedThe previous patch is wrong.....this is correct
Comment #8
phenaproximaThank you for the patch, @Charlotte17!
Unfortunately, I need to kick this back because it will need tests to get committed :( I don't think the tests will be novice-level, so I'm removing the Novice tag as well.
Comment #9
ohthehugemanatee CreditAttribution: ohthehugemanatee as a volunteer commentedSorry, I think the approach here is wrong. We do not create stub entities for config entities. See the beginning of Drupal/migrate/src/Plugin/migrate/destination/EntityConfigBase:import(), where we throw an exception for it.
I talked this through with @mikeryan, and we agreed that we should have similar behavior in MigrationLookup::transform(), where it tries to create the entity.
Attached two patches: a failing test, and a combined test and patch.
Comment #11
phenaproximaMoar bikeshedding! Good thing I'm wearing my "Migrate All The Things" t-shirt as I write this.
Anyway, I'm not sure I agree with the approach in #9, because it's utterly opaque. The stubbing will fail, silently, without any kind of reporting, and the user will be left bemused, scratching their heads, with incomplete data. I would actually prefer if EntityConfigBase threw the exception, because at least then the user has a clear lead on what went wrong.
So having said that, in the name keeping things explicit (very important in a system as ridiculously complex as Migrate), I'd be more in favour of @Charlotte17's patch in #7. But I'm certainly open to more argument on the matter :)
Comment #12
mikeryanComment #13
mikeryanEntityConfigBase and EntityContentBase know whether they permit stubbing (no in the former and yes in the latter case), so they should be the ones to determine this - we shouldn't need to remember to explicitly add configuration to particular migrations to handle this. The checking is done in Migration::getDestinationPlugin() based on 'no_stub' being set in the destination configuration. So, how about we simply have EntityConfigBase do
in its constructor?
Comment #14
mikeryanComment #15
kubrt CreditAttribution: kubrt commentedHi,
I've been hitting a simillar issue since upgrade to drupal 8.3 and migration tools 4.x.
The upgrade_d6_user_role migration ignores the roles, the roles are thus not created and the
upgrade_d6_user
results in this error.In desperation I have created the roles myself by exporting/importing their configurations from an older version of the database (where the migration run fine) and also put the values into the
migrate_map_upgrade_d6_user_role
table. Now I getwhich has made me bow. However the
upgrade_d6_user
still fails in the transform function (empty $value).When I hack it like this
So now I can run the
upgrade_d6_user
and carry on to the higher floors of this nightmare, however the roles are not being assigned.I guess I need to figure out why are the roles being ignored, but how do I debug that ?
Thanks M
Comment #16
mikeryan@kubrt: When you say upgrade_d6_user_role ignores the roles, do you mean it specifically says something like "0 imported, 7 ignored" when you run it? Have you checked the message table for any messages (drush mmsg upgrade_d6_user_role)?
I'm not seeing anything in d6_user_role (in the source plugin or in the process plugins) that would cause roles to be ignored.
Comment #17
kubrt CreditAttribution: kubrt commented@mikeryan Yes, that's exactly what hapens. I've tried with a clean d8 database and experience the same behavior.
When I get past the roles import as described in #15 I can import users, but then any attempt to import nodes shows the same - all ignored.
I'm also discussing it here at Drupal Answers and trying out the Migrate devel but hitting a bug.
Becase the nodes are being ignored also I don't think it's specific to the upgrade_d6_user_role migration. There seems to be something more fundamental. The d6 database is huge but I can temporary share the mysql connection string. I am on slack drupal #migration channel.
Thanks for your help
Comment #18
mikeryanI can reproduce it - here's the problem, in the filter_format_permission process plugin:
The migration name 'd6_filter_format' is hard-coded, when the migrations are prefixed it needs to change. We've fixed this issue previously for the d6_cck_file process plugin, we need to make it configurable here as well.
Comment #19
kubrt CreditAttribution: kubrt commentedfor (ever) repeat("Hallelujah !");
Thank you !
BTW, next challenge - how do I prevent d6 text fields being created as plaintext,long in d8 by the migration ?
Comment #20
kubrt CreditAttribution: kubrt commentedFurther down:
Comment #21
drzraf CreditAttribution: drzraf commentedmaybe related #2788139: InvalidArgumentException in Flatten.php
Comment #22
drzraf CreditAttribution: drzraf commentedpatch worked for me
Comment #23
heddnHere's what we discussed in #18. It has tests, so removing that tag. But we need to update the IS now. Adding the novice tag for that. The problem/solution is discussed in #18. No interdiff against #9 because this a new solution.
Comment #26
heddnLet's get a successful fail here.
Comment #28
heddnNamespace was off. Let's see if that fixes things.
Comment #30
heddnThat's more like it. Now we have a proper failure.
Comment #31
joelpittetNot sure how to make the issue summary match the patch, but this looks like a quick fix. The passed in configuration wasn't being unioned before now it is.
Comment #32
heddnUpdated IS. Do we need a change record? We had on for #2640842: Make related file migration ID configurable in d6_cck_file. Back to NW. Maybe we can just update https://www.drupal.org/node/2823856 to reference more migrations where we are making it possible?
Comment #33
heddnComment #34
Matt BI was pointed here from #2788139: InvalidArgumentException in Flatten.php, applied the patches and still get the same problem - see #2788139: InvalidArgumentException in Flatten.php
Comment #35
heddnre #34: Did you modify the migrate configuration to point to the renamed migration? The permission yml should have a 'migration' config item that points to the d6_filter_format.
Comment #36
johnvb CreditAttribution: johnvb commentedre #35. I have the same issue as #34 but I'm not sure what the migrate configuration changes I need to make are, even after reading both threads. I've applied the patch in #28 and have successfully imported my filter formats (via a migration called upgrade_d6_filter_format) but my user roles don't import and are ignored (via a migration called upgrade_d6_user_role). If I hack the FilterFormatPermission.php file to change the expected migration name from d6_filter_format to upgrade_d6_filter_format then I can successfully import all my roles, but when I subsequently import my users I get the error in Flatten.php as described at https://www.drupal.org/node/2788139.
Can you explain exactly what I need to do please? Sorry if I'm missing something obvious.
Comment #37
johnvb CreditAttribution: johnvb commentedPosting in case this helps anyone else. I've managed to get round my problem by replacing the yml for the upgrade_d6_user migration with the one I'd generated from a previous migration effort generated using Drupal 8.2.x rather than 8.3.2. Notably this uses the migration plugin for the roles and user_picture rather than the migration_lookup. Here's a quick diff so you can see the difference:
Comment #38
jofitz CreditAttribution: jofitz at ComputerMinds commented@johnvb The change from "migration_lookup" to "migration" is interesting because the latter simply extends the former - it was renamed to improve clarity (see #2845486: Rename Migration process plugin and add documentation). Would you be able to test whether it works with migration_lookup, please? I would be very surprised if it didn't.
Comment #39
johnvb CreditAttribution: johnvb commentedAh - I see, maybe it is just the migration name changes that fixed this then? That would tie in with my question about the earlier post suggesting I change migration names - the answer is given by my Drupal 8.2.x YML file which has the correct names.
I seem to have broken things though. I rolled back my migration (which seems to work OK) and updated the migration config to use migration_lookup, then tried again and I am now getting lots of errors such as:
Attempt to create a field field_habitat that does not exist on entity type node. [error]
(/../core/modules/field/src/Entity/FieldConfig.php:286)
It seems to occur for both versions of the migration config, so presumably the rollback did not work cleanly. Any ideas appreciated.
Comment #40
johnvb CreditAttribution: johnvb commentedI found I had to rollback both the field instances and the fields themselves, then it worked. I can confirm it was the change to migration names, not switching the plugin from migration_lookup to migration, which fixed things.
Comment #41
jofitz CreditAttribution: jofitz at ComputerMinds commentedI've had a go at writing the change record, I decided it was better to have a whole new one rather than editing [#2823856]. Would someone mind reviewing (and publishing) it, please?
Comment #42
heddnChange record added. I did a review of it and tweaked the "now" section to mention 'filter_format_permission' instead of a custom plugin.
Comment #43
mikeryanUmmm.... I saw this in my change record feed and said "Great, that got committed!". But, it didn't... A little ahead of ourselves here?
Comment #44
jofitz CreditAttribution: jofitz at ComputerMinds commented@mikeryan Sorry, that's my fault. It was my first attempt at a change record. Should they only be created after a patch has been committed? I did wonder when I saw the "Introduced in version" field. Lesson learnt.
Comment #45
tstoeckler@Jo Fitzgerald: You did almost everything correctly, but you should leave the Published checkbox unchecked next time. The committer will then check that checkbox and, thus, publish the change notice upon commit. I removed the checkbox now for this one.
Comment #46
Matt BI can confirm that the changes to upgrade_d6_user in #37 fix this for me.
Comment #47
Gábor HojtsyHow does the issue title relate to the patch? No users or user roles being dealt with in the patch. I tried to make the connection by reading the comments, especially #18 but I did not manage to make the connection. (I don't have a lot of migrate experience). Can we summarize in the issue summary why is this needed?
Also a minor thing:
Tests for... ?
Comment #48
heddnI've re-titled things here. d6_filter_format is required by d6_user_role. That was the trigger for opening this issue. Making that not hard-coded is the solution.
Comment #49
heddnComment #50
joelpittetThanks @heddn, back to RTBC. thanks for updating the title and the comment from @Gábor Hojtsy in #47
Comment #53
catchCommitted/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!
Comment #54
Publishing Future CreditAttribution: Publishing Future commentedI still get the same error with drupal 8.3.5 and can only avoid it when changing
container->get('plugin.manager.migrate.process')->createInstance('migration', $migration_plugin_configuration, $migration)
to
$container->get('plugin.manager.migrate.process')->createInstance('migration', ['migration' => 'upgrade_6_filter_format'], $migration)
in FilterFormatPermission.php located at ...core\modules\filter\src\Plugin\migrate\process\d6
Comment #55
heddnIn the configuration for the filter_format_permission process plugin, set an option of
migration: upgrade_6_filter_format
. And make sure you using 8.3.x HEAD. I don't think this went out with a 8.3 point release yet.