Problem/Motivation

Made a new issue . similar to #3214639: "Undefined array key 'migration'" during comment processing / "Failed to connect to your database server" and #3131951: Undefined index in findMigrationDependencies.

This probably happens when comments importing config entity is created. Thanks to djassie for reporting it is still an issue. It may be only to do with comments - around line 247 a mistake in building attributes of the migration plugin.

I think it best to extract this L249-262 in createContentMigration into a new function then return the plugin to save it.

First apply this: #3390755: remove createPluginEntityFromPlugin in WordPressMigrationGenerator. To remove redundant createEntityFromPlugin.

Got this warning message - during - writing custom migration
id: custom_migration
label: Custom Migration
but label should be quoted, like `label: 'Custom Migration'

Also: Mykola Dolynskyi :

same problem when calling
\Drupal::service('plugin.manager.migration')->getDefinitions();

It appears also we should remove createPluginEntityFromPlugin in WordPressMigrationGenerator. There is now a same function in migrate_plus: Migration.php (L 84). New issue: #3390755: remove createPluginEntityFromPlugin in WordPressMigrationGenerator. there is an inconsistency . we do not have: $plugin_definition = $migration_plugin->getPluginDefinition();

Also this seems odd - line 246 vs 268
$migration->set('migration_dependencies', ['required' => $dependencies]);
$migration->set('migration_dependencies', ['required' => [$content_id]]);

Steps to reproduce

Seems to happen importing comments.

Warning: Undefined array key "migration" in Drupal\migrate\Plugin\Migration->findMigrationDependencies() (line 719 of /var/www/html/web/core/modules/migrate/src/Plugin/Migration.php)

#0 /var/www/html/web/core/includes/bootstrap.inc(158): _drupal_error_handler_real(2, 'Undefined array...', '/var/www/html/w...', 719)
#1 /var/www/html/web/core/modules/migrate/src/Plugin/Migration.php(719): _drupal_error_handler(2, 'Undefined array...', '/var/www/html/w...', 719)
#2 /var/www/html/web/core/modules/migrate/src/Plugin/Migration.php(692): Drupal\migrate\Plugin\Migration->findMigrationDependencies(Array)
#3 /var/www/html/web/modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php(330): Drupal\migrate\Plugin\Migration->getMigrationDependencies()
#4 /var/www/html/web/modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php(256): Drupal\wordpress_migrate\WordPressMigrationGenerator::createEntityFromPlugin('wordpress_comme...', 'my_wordpress_co...')
#5 /var/www/html/web/modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php(180): Drupal\wordpress_migrate\WordPressMigrationGenerator->createContentMigration('post')
#6 /var/www/html/web/modules/contrib/wordpress_migrate/wordpress_migrate_ui/src/Wizard/ImportWizard.php(85): Drupal\wordpress_migrate\WordPressMigrationGenerator->createMigrations()
#7 [internal function]: Drupal\wordpress_migrate_ui\Wizard\ImportWizard->finish(Array, Object(Drupal\Core\Form\FormState))
#8 /var/www/html/web/core/lib/Drupal/Core/Form/FormSubmitter.php(114): call_user_func_array(Array, Array)
#9 /var/www/html/web/core/lib/Drupal/Core/Form/FormSubmitter.php(52): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState))
#10 /var/www/html/web/core/lib/Drupal/Core/Form/FormBuilder.php(595): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState))
#11 /var/www/html/web/core/lib/Drupal/Core/Form/FormBuilder.php(323): Drupal\Core\Form\FormBuilder->processForm('wordpress_migra...', Array, Object(Drupal\Core\Form\FormState))
#12 /var/www/html/web/modules/contrib/ctools/src/Wizard/WizardFactory.php(57): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\wordpress_migrate_ui\Wizard\ImportWizard), Object(Drupal\Core\Form\FormState))
#13 /var/www/html/web/modules/contrib/ctools/src/Controller/WizardFormController.php(85): Drupal\ctools\Wizard\WizardFactory->getWizardForm(Object(Drupal\wordpress_migrate_ui\Wizard\ImportWizard), Array, false)
#14 [internal function]: Drupal\ctools\Controller\WizardFormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#15 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#16 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(580): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#17 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#18 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#19 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(163): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#20 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(74): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#21 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(686): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#30 {main}

Issue fork wordpress_migrate-3390754

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

HongPong created an issue. See original summary.

hongpong’s picture

hongpong commented
Issue summary: View changes
hongpong’s picture

hongpong commented
Issue summary: View changes
hongpong’s picture

hongpong commented
Issue summary: View changes
hongpong’s picture

hongpong commented
Status: Active » Needs work

WIP added to issue fork with extracted comment entity generator function. https://git.drupalcode.org/issue/wordpress_migrate-3390754 . then reroll after #3390755: remove createPluginEntityFromPlugin in WordPressMigrationGenerator is committed.

That does not fix the problem but does isolate it within a much smaller function.

hongpong’s picture

hongpong commented
Issue summary: View changes
hongpong’s picture

hongpong commented

For anyone else debugging such stuff. with devel and symfony dumper turned on:

Adding the following to web/core/modules/migrate/src/Plugin/Migration.php circa line 745:

        if (in_array($plugin_configuration['plugin'], ['migration', 'migration_lookup'], TRUE)) {
          if (!isset ($plugin_configuration['migration'])) {
            ddm('---- migration is not set on this plugin_configuration!');
            ddm($plugin_configuration, 'findmigrationdeps $plugin_configuration');
            ddm($process, 'findmigrationdeps $process');
            ddm('--end---');
          } else {
            ddm('---- OK: migration is set on this plugin_configuration');
            ddm($plugin_configuration, 'findmigrationdeps $plugin_configuration');
            ddm($process, 'findmigrationdeps $process');
            ddm('--OK end--- \n');
          }
          $return = array_merge($return, (array) $plugin_configuration['migration']);

Getting closer to this!

hongpong’s picture

hongpong commented

Brought this to slack drupal #migrate and mikelutz gave useful info. i think this is the 'plugin derivatives' / plugin derivers' idea. Thus we should transition these migration config entities to 'derivers' when possible. (I think thats the same as derivatives.)

related examples: https://www.drupal.org/project/entity_timeline/issues/3281990
https://api.drupal.org/api/drupal/core!lib!Drupal!Component!Plugin!Deriv...
https://www.sitepoint.com/tutorial-on-using-drupal-8-plugin-derivatives-...
https://drupal.stackexchange.com/questions/293667/how-do-i-create-a-deri...
https://api.druphelp.com/api/drupal/core%21lib%21Drupal%21Component%21Pl...
https://www.drupal.org/docs/drupal-apis/plugin-api/plugin-derivatives
https://manifesto.co.uk/drupal-plugin-api-examples-tutorial/
https://www.hashbangcode.com/article/drupal-9-creating-category-menu-usi...
https://www.adamfranco.com/2018/11/13/creating-a-drupal-plugin-system/

mikelutz: The quick and dirty fix here, without refactoring to derivatives is to put something in the template plugin for that key like migration: wordpress_comment so the plugin can load and pass it’s checks. It doesn’t quite matter much, it’s getting overwritten in the dynamically generated config entities anyway, but something needs to be there in the plugin template itself so it can be loaded and used to create the config entities.

The problem isn’t in the importer. You are generating valid config entities there. The module is trying to dynamically generate migrate_plus config entities using the migration plugins in the /migration folder as templates. It’s really not the right approach. If you want to derive multiple migrations from a template, you should use a plugin deriver similar to the way core derives node plugins by content type. The issue here is that your template migrations has https://git.drupalcode.org/project/wordpress_migrate/-/blob/8.x-3.x/migr...

  pid:
    plugin: migration_lookup
    source: comment_parent
    # migration ID generated dynamically

Despite the fact that this migration is only supposed to be a template, it’s a real valid plugin. And any version of createEntityFromPlugin creates an instance of this plugin to read the template from. Creating an instance of the plugin checks the dependencies. Checking the dependencies of the plugin scans it for migration lookup processes and adds the migration that you are looking up against to the dependencies. And it can’t because there is an undefined offset ‘migration’.

dinarcon made their first commit to this issue’s fork.

dinarcon opened merge request !23

dinarcon’s picture

dinarcon commented
Status: Needs work » Needs review

When using the migration_lookup process plugin, it is assumed that the migration configuration key is set. That defines the list of migrations used for the lookup operation. In migrations/wordpress_comment.yml there are currently two usages of migration_lookup that do not have the migration configuration defined (because they will be generated dynamically).

process:
  entity_id:
    plugin: migration_lookup
    source: post_id
    # migration generated dynamically
  pid:
    plugin: migration_lookup
    source: comment_parent
    # migration ID generated dynamically

In WordPressMigrationGenerator::createEntityFromPlugin there is a call to $plugin_manager->createInstance($plugin_id). In \Drupal\migrate\Plugin\MigrationPluginManager::createInstances there is a call to \Drupal\migrate\Plugin\Migration::getMigrationDependencies where the system tries to infer the required/optional dependencies for the migration plugin being created. Eventually \Drupal\migrate\Plugin\Migration::findMigrationDependencies is called when attempting to set a list of optional dependencies. In there, the each process plugins is analyzed. If usages of migration_lookup are found, the corresponding migration configuration key is added to the list of dependencies. When not set, the warning described in this issue is triggered.

  protected function findMigrationDependencies($process) {
	...
        if (in_array($plugin_configuration['plugin'], ['migration', 'migration_lookup'], TRUE)) {
          $return = array_merge($return, (array) $plugin_configuration['migration']);
        }
	...
  }

Back in \Drupal\wordpress_migrate\WordPressMigrationGenerator::createEntityFromPlugin there is also an explicit call to $migration_plugin->getMigrationDependencies(); which makes the warning to be triggered again.

Later on when processing the generated comment migration plugin, the missing migration keys are injected into the corresponding migration_lookup process definitions. Also, the migration dependencies are overridden.

        $process                                     = $migration->get('process');
        $process['entity_id'][0]['migration']        = $content_id;
        $process['comment_type'][0]['default_value'] = $storage->getSetting('comment_type');
        $process['pid'][0]['migration']              = $id;
        $process['field_name'][0]['default_value']   = $field_name;
        $migration->set('process', $process);
        $migration->set('migration_dependencies', ['required' => [$content_id]]);

At the end generating the configuration entities, the migration plugins have valid process sections. But at that point it is too late. The warnings have been triggered.

The current MR proposes to add the missing migration configuration in migrations/wordpress_comment.yml and set it to a _PLACEHOLDER_ which is later replaced by the proper value.

baltowen’s picture

baltowen commented
Assigned: Unassigned » baltowen
Status: Needs review » Needs work

I tested the MR #23, and the migration warning is gone. But there is another warning still present.

Warning: Undefined array key "default_author" in Drupal\wordpress_migrate\WordPressMigrationGenerator->createMigrations() (line 112 of modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php).

I will work on this.

baltowen’s picture

baltowen commented
Assigned: baltowen » Unassigned
Status: Needs work » Needs review

An array key was being access before checking that it exists. I pushed a commit to fix this. It might be worth to create a follow up issue to make sure that all expected configurations exist or define a default value for them. Back to needs review.

hongpong’s picture

hongpong commented

thank you dinarcon and baltowen for unraveling that and catching the default_author problem. For similar catching problem entries, may want to see also #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry and #3159477: Better exception handling for if username does not exist #2849663: Exceptions + error catching; does not catch missing file uri, or other duplicate migration plugins gracefully. (wip code here)

Really appreciate the help on both this and the new issue for adding config link.

i have also updated the "plan" today #2904990: Plan for wordpress_migrate 8.x-3.x beta release.

dinarcon’s picture

dinarcon commented

I had another look at the module to see how using derivatives would work. I think that should be addressed in a separate issue. For example, we could one "base migration" for all taxonomy migrations and that seems out of scope for this issue.

While investigating, I noticed the module is creating instances out of the "base migrations" shipped with the module. That seems unnecessary. Getting the plugin definition would suffice to accomplish what the \Drupal\wordpress_migrate\WordPressMigrationGenerator::createEntityFromPlugin method is expected to do. I guess this could be considered out of scope for this issue as well.

@hongpong what do you think about these remarks? And how would you like to proceed with this issue?

dinarcon opened merge request !25

dinarcon’s picture

dinarcon commented

Took a step back. The source of the issue is having incomplete definitions for process plugins that leverage migration_lookup. An alternative solution to providing a placeholder for the missing migration configuration is to extract those process plugins out of the base migrations, as there are multiple examples already. MR 25 follows this approach. I also cherry-picked @baltowen's fix to default_author error.

Looking forward to feedback on both MRs. If we decide to go with #25, the topic of not creating base migration instances might be discussed in a follow up.

ressa’s picture

ressa
he/him
commented
Priority: Normal » Major
Parent issue: » #2904990: Plan for wordpress_migrate 8.x-3.x beta release

Increasing Priority, since it is placed under "Major issues" header in #2904990: Plan for wordpress_migrate 8.x-3.x beta release, so I am also adding that as parent issue.

  • hongpong committed 9fcc369c on 8.x-3.x authored by dinarcon 
    git commit -m 'Issue #3390754 by dinarcon, hongpong, baltowen, ressa:...
hongpong’s picture

hongpong commented
Status: Needs review » Fixed

Thank you dinarcon ressa baltowen. I would say we could open another issue about refactoring the migration entities - whether that is a deriver, placeholder values, base migrations or similar. However not sure how it should be titled.

I swear, I will get the merge request interface correctly for the commit messages someday (facepalm)!

I would note that I still get this issue, but I think it is not because of this issue: #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry.

hongpong closed merge request !23

ressa’s picture

ressa
he/him
commented

It's so great to see all the activity and cooperation currently, and a steady move towards a 8.x-3.x beta release. So thank you all for that!

About crediting, don't give it another thought @hongpong. Like we agree, it was unclear ... I am not a coder, so what you are all doing, I could not, so please keep on doing that -- then I'll attempt to improve the documentation of WordPress Migrate -- and crediting :)

Status: Fixed » Closed (fixed)

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