Incorrect bundle/bundle key handling in EntityContentBase::processStubRow()

Looks good to me.

Btw, was wondering that it still uses EntityManager which is deprecated since Drupal 8.0.0.
But I think it should be handled in another issue, right?

In order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

1. https://www.drupal.org/docs/8/testing
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.5.x

I wonder how this has escaped detection.

There are subclasses for some entity types that override EntityContentBase::processStubRow(), but they call the parent.

Do we maybe not have any tests for stubbing at all?

Rerolled for 8.8. This patch also applies to 8.9.

According to #4, uploading two patches for 9.2.x to prove that issue still exists. Also, tagging with "Bug Smash Initiative".

Re-uploading with cs fix.

Ready for review 🙂

I am not familiar with the whole stubbing process so I applied the patch and walked through the test. This works as expected and fixes the problem in the IS and the proposed resolution agrees with the patch. And we have a fail test that shows the problem.

So, this is good to go.

Committed and pushed 9d96600683 to 9.2.x and 027f08e7ed to 9.1.x. Thanks!

Woah! 🙈🤯

Hello,

Could you explain me what I am doing wrong trying to use `dom_migration_lookup` plugin. It breaks on this particular piece of code in EntityContentBase::processStubRow().

Scenario:
- It's migration from D7 to D9
- Migrating several content types that have links in body field to each other
- These links in body field are preprocessed by migration in a way that in the output they look like following pattern: /node/[node-type]/[nid]

The problem:
The problem is that in this condition

if ($bundle_key && empty($row->getDestinationProperty($bundle_key))) {
      if (empty($this->bundles)) {
        throw new MigrateException('Stubbing failed, no bundles available for entity type: ' . $this->storage->getEntityTypeId());
      }
      $row->setDestinationProperty($bundle_key, reset($this->bundles));
    }

To be exact $row->getDestinationProperty($bundle_key) is empty BUT should have proper destination content type machine name from `dom_migration_lookup` therefore following the logic it takes the first content type from the stack:
$row->setDestinationProperty($bundle_key, reset($this->bundles));
and populate wrong default filelds in code below this condition.

Body field migration definition:

  process:
    'body/value':
      - plugin: dom
        method: import
        source: 'body/0/value'
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/quick_guide/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          qg_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/news/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          news_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/article/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          article_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/page/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          page_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/document/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          document_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/f_a_q/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          faq_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/sfc_files/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          sfc_files_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/home/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          dghome_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/sfc2007_content/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          sfc2007_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/training/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          video_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom_migration_lookup
        mode: attribute
        xpath: '//a'
        attribute_options:
          name: href
        search: '@/node/not_indexed_in_searcher/(\d+)@'
        replace: '/node/[mapped-id]'
        migrations:
          page_notindexedinsearcher_nodes:
            replace: '/node/[mapped-id]'
        no_stub: false
      - plugin: dom
        method: export

Workaround:
I have found this workaround, but I believe there is a better solution for my case?

In file: `../core/modules/migrate/src/MigrateStub.php`

protected function doCreateStub(MigrationInterface $migration, array $source_ids, array $default_values = []) {
    $destination = $migration->getDestinationPlugin(TRUE);
-    $process = $migration->getProcess();
    $id_map = $migration->getIdMap();
-    $migrate_executable = new MigrateExecutable($migration);
    $row = new Row($source_ids + $migration->getSourceConfiguration(), $migration->getSourcePlugin()->getIds(), TRUE);
-    $migrate_executable->processRow($row, $process); // If this line is not removed then migration even fails at any Stub occurence.
+    $migrate_destination = $migration->getDestinationConfiguration();
+    if (!empty($migrate_destination['bundle'])) {
+      $default_values['type'] = $migrate_destination['bundle'];
+    }
    foreach ($default_values as $key => $value) {
      $row->setDestinationProperty($key, $value);
    }
    $destination_ids = [];
    try {
      $destination_ids = $destination->import($row);
    }
    catch (\Exception $e) {
      $id_map->saveMessage($row->getSourceIdValues(), $e->getMessage());
    }
    if ($destination_ids) {
      $id_map->saveIdMapping($row, $destination_ids, MigrateIdMapInterface::STATUS_NEEDS_UPDATE);
      return $destination_ids;
    }
    return FALSE;
  }

I'd be appreciated for any tips!

Best regards,
Michał.