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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michaelmallett created an issue. See original summary.

mikeryan’s picture

Category: Bug report » Support request
Status: Active » Postponed (maintainer needs more info)
Related issues: +#2835586: Allow customization of stub rows from Migration process plugin, +#2800279: Document that migrations used for stubbing need to deal with empty source values

The problem is arising here:

  roles:
    plugin: migration
    migration: awm_users_role
    source: roles

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:

  roles:
    plugin: migration
    migration: awm_users_role
    source: roles
    no_stub: true
Gribnif’s picture

FileSize
860 bytes

I 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:

process:
  rid:
    plugin: migration
    migration: d7_user_role
    source: 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.

mikeryan’s picture

Title: 7 to 8 drupal users migraton: InvalidArgumentException: Passed variable is not an array or object, using empty array » User migrations fail with missing roles
Version: 8.2.5 » 8.3.x-dev
Category: Support request » Bug report
Status: Postponed (maintainer needs more info) » Active
Issue tags: +migrate-d6-d8, +migrate-d7-d8, +Novice

Yes, I think both d6_user and d7_user need to add no_stub: true to the roles mapping.

heddn’s picture

To be more explicit, a novice would just need to add 'no_stub: true' to d6 & d7 user yml files.

Charlotte17’s picture

Status: Active » Needs review
FileSize
2.74 KB

Adding 'no_stub: true' #4-5

Charlotte17’s picture

The previous patch is wrong.....this is correct

phenaproxima’s picture

Status: Needs review » Needs work
Issue tags: -Novice +Baltimore2017, +Needs tests

Thank 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.

ohthehugemanatee’s picture

Status: Needs work » Needs review
FileSize
2.05 KB
3.18 KB

Sorry, 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.

The last submitted patch, 9: 2850312-test.patch, failed testing.

phenaproxima’s picture

Moar 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 :)

mikeryan’s picture

Assigned: Unassigned » mikeryan
mikeryan’s picture

Status: Needs review » Needs work

EntityConfigBase 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

  $this->configuration['no_stub'] = TRUE;

in its constructor?

mikeryan’s picture

Assigned: mikeryan » Unassigned
kubrt’s picture

Hi,
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 get

drush  migrate-status upgrade_d6_user_role
 Group: Import from Drupal 6 (migrate_drupal_6)  Status  Total  Imported  Unprocessed  Last imported       
 upgrade_d6_user_role                            Idle    7      7         0            2017-05-20 15:57:36

which has made me bow. However the upgrade_d6_user still fails in the transform function (empty $value).
When I hack it like this

if (!$value) $value = array();

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

mikeryan’s picture

@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.

kubrt’s picture

@mikeryan Yes, that's exactly what hapens. I've tried with a clean d8 database and experience the same behavior.

drush migrate-rollback upgrade_d6_user_role
Rolled back 7 items - done with 'upgrade_d6_user_role'                                                                                                                                                   
$ drush migrate-import upgrade_d6_user_role
Processed 7 items (0 created, 0 updated, 0 failed, 7 ignored) - done with 'upgrade_d6_user_role'                                                                                                         
$ drush migrate-messages upgrade_d6_user_role

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

mikeryan’s picture

I can reproduce it - here's the problem, in the filter_format_permission process plugin:

  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)
    );
  }

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.

kubrt’s picture

for (ever) repeat("Hallelujah !");

drush migrate-import  upgrade_d6_user_role
Processed 7 items (7 created, 0 updated, 0 failed, 0 ignored) - done with 'upgrade_d6_user_role'

Thank you !

BTW, next challenge - how do I prevent d6 text fields being created as plaintext,long in d8 by the migration ?

kubrt’s picture

Further down:

Processed 194 items (0 created, 0 updated, 0 failed, 194 ignored) -  [status]
done with 'upgrade_d6_node_event'. 
Shall I just remove the prefix from all the migrations ? 
drzraf’s picture

drzraf’s picture

patch worked for me

heddn’s picture

Here'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.

The last submitted patch, 23: 2850312-23.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 23: 2850312-23_tests-only.patch, failed testing.

heddn’s picture

Status: Needs work » Needs review
FileSize
1.44 KB
1.16 KB

Let's get a successful fail here.

Status: Needs review » Needs work

The last submitted patch, 26: 2850312-26_test-only.patch, failed testing.

heddn’s picture

Status: Needs work » Needs review
FileSize
2.56 KB
1.43 KB
615 bytes

Namespace was off. Let's see if that fixes things.

Status: Needs review » Needs work

The last submitted patch, 28: 2850312-28_tests_only.patch, failed testing.

heddn’s picture

Status: Needs work » Needs review

That's more like it. Now we have a proper failure.

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'d6_filter_format'
+'custom_filter_format'

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Not 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.

heddn’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs issue summary update, -Novice +Needs change record

Updated 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?

heddn’s picture

Issue summary: View changes
Matt B’s picture

I was pointed here from #2788139: InvalidArgumentException in Flatten.php, applied the patches and still get the same problem - see #2788139: InvalidArgumentException in Flatten.php

heddn’s picture

re #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.

johnvb’s picture

re #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.

johnvb’s picture

Posting 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:

langcode: en
 status: true
 dependencies: {  }
 id: upgrade_d6_user
-class: null
-field_plugin_method: null
-cck_plugin_method: null
 migration_tags:
   - 'Drupal 6'
 migration_group: migrate_drupal_6
@@ -38,12 +35,12 @@ process:
     fallback_to_site_default: true
   init: init
   roles:
-    plugin: migration_lookup
-    migration: d6_user_role
+    plugin: migration
+    migration: upgrade_d6_user_role
     source: roles
   user_picture:
-    plugin: migration_lookup
-    migration: d6_user_picture_file
+    plugin: migration
+    migration: upgrade_d6_user_picture_file
     source: uid
     no_stub: true
 destination:
@@ -56,3 +53,4 @@ migration_dependencies:
     - upgrade_d6_user_picture_file
     - upgrade_user_picture_entity_display
     - upgrade_user_picture_entity_form_display
+    - upgrade_d6_user_role
jofitz’s picture

@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.

johnvb’s picture

Ah - 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.

johnvb’s picture

I 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.

jofitz’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Needs work » Needs review
Issue tags: -Needs change record

I'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?

heddn’s picture

Status: Needs review » Reviewed & tested by the community

Change record added. I did a review of it and tweaked the "now" section to mention 'filter_format_permission' instead of a custom plugin.

mikeryan’s picture

Ummm.... I saw this in my change record feed and said "Great, that got committed!". But, it didn't... A little ahead of ourselves here?

jofitz’s picture

@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.

tstoeckler’s picture

@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.

Matt B’s picture

I can confirm that the changes to upgrade_d6_user in #37 fix this for me.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

How 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:

+++ b/core/modules/filter/tests/src/Kernel/Migrate/d6/FilterFormatPermissionTest.php
@@ -0,0 +1,32 @@
+ * Filter format permission migration.

Tests for... ?

heddn’s picture

Title: User migrations fail with missing roles » d6/d7_filter_format is hard coded, causing dependent migration to fail
Status: Needs work » Needs review
FileSize
2.59 KB
666 bytes

I'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.

heddn’s picture

Title: d6/d7_filter_format is hard coded, causing dependent migration to fail » d6_filter_format is hard coded, causing dependent migration to fail
Issue tags: -migrate-d7-d8
joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @heddn, back to RTBC. thanks for updating the title and the comment from @Gábor Hojtsy in #47

  • catch committed 77043e4 on 8.4.x
    Issue #2850312 by heddn, Charlotte17, ohthehugemanatee, Gribnif,...

  • catch committed f0be200 on 8.3.x
    Issue #2850312 by heddn, Charlotte17, ohthehugemanatee, Gribnif,...
catch’s picture

Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!

Publishing Future’s picture

I 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

heddn’s picture

In 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.