Migration Lookup plugin does not return multiple values when matched

@mathieuhelie - i think test bot will not adjust for it so i have created a new patch for this.

I'm happy to resolve the test failures of the patch in #6, but it's not clear what you are trying to achieve here.

Can you include a test, please? That would probably help clarify things, specifically what you mean by "multiple-valued migration lookups".

Setting back to NW because this needs tests (and local testing suggests this patch will fail).

@mathieuhelie Thank you for your explanation, but I'm afraid it is still not clear to me so I cannot help to progress this much further (hopefully it will help somebody else). I understand what the code currently does and what you want it to do, but not why. Perhaps a real-world example would clarify things? Or, as I suggested, you could add to the Data Provider of the existing test (Drupal\Tests\migrate\Unit\process\MigrationLookupTest::successfulLookupDataProvider).

Please can you remember to include an interdiff (and perhaps a brief description of any changes you have made), It makes it easier to review your patch. I must admit I'm sometimes guilty of not explaining my patches too, but if they are straightforward then just an interdiff is often enough. For completeness/clarity, I have attached the interdiff between the patches in #8 and #10.

It seems you also accidentally removed the changes to the mocks in the tests (which an interdiff would have highlighted too).

I have re-instated the changes to the test mocks and gone all-out (!) with the interdiffs to clarify the workflow so far.

This will take us another small step towards a working solution, but I expect many tests will still fail and this change still requires tests of its own.

...and after all that I forget to set to Needs Review!

A couple more test mock fixes.

Add related issues.

I'm successfully using the patch from #14 for an XML migration. I'll try to explain this real-world example in the hope of clarifying why the patch is needed:

My source data is in two XML files, products.xml and photos.xml. Each product can have 1 or more photos. The photos have a weight that determines the order in which they are displayed on the product page. The photos XML looks like this:

<photos>
  <photo>
    <product_id>123</product_id>
    <path>http://www.example.com/foo.jpg</path>
    <weight>1</weight>
  </photo>
  <photo>
    <product_id>123</product_id>
    <path>http://www.example.com/bar.jpg</path>
    <weight>3</weight>
  </photo>
  <photo>
    <product_id>456</product_id>
    <path>http://www.example.com/baz.jpg</path>
    <weight>1</weight>
  </photo>
  ...

In order to migrate all photos and not just one per product I need to uniquely identify each one, so I use product_id and weight as ids in the migration config, like this:

id: migrate_photos
source:
  plugin: url
  data_fetcher_plugin: http
  data_parser_plugin: xml
  urls:
    - public://products/photos.xml
  item_selector: /photos/photo
  fields:
    -
      name: product_reference
      label: 'Product reference'
      selector: product_id
    -
      name: photo_url
      label: 'Path to file'
      selector: path
    -
      name: weight
      label: 'Order in which to display photo on product page'
      selector: weight
  ids:
    product_reference:
      type: integer
    weight:
      type: integer
process:
  ...

My product migration can't assume anything about photo weights, so it uses just the product id as the source of the migration_lookup process, like this:

id: migrate_products
source:
  plugin: url
  data_fetcher_plugin: http
  data_parser_plugin: xml
  urls:
    - public://products/products.xml
  item_selector: /products/product
  fields:
    -
      name: product_reference
      label: 'Product reference'
      selector: product_id
  ...
    ids:
      product_reference:
        type: integer
process:
  field_property_photos:
    plugin: migration_lookup
    migration: migrate_photos
    source: product_reference
    no_stub: true
...

Without the patch, this configuration results in only one photo being migrated per product. With the patch all photos are successfully migrated.

As for tests, it looks like the existing test cases already cover this scenario. In Drupal\Tests\migrate\Unit\process\MigrationLookupTest::successfulLookupDataProvider the second data set is for scalar to array, i.e. one source id yielding two destination ids. I don't really understand what’s going on in the tests but I think it’s as @mathieuhelie said: the mocked return values for lookupDestinationId were not the same as the actual return values. The mocked values were what should have been returned (two destination ids) but the actual lookupDestinationId returned just one destination id.

We were attempting to migrate data into paragraphs and used a migration similar to the example in #20. We needed to match multiple paragraphs from another migration, so we applied the patch in #14. However, since lookupDestinationIDs() returns an array of arrays, and the nested array contains the target_id and target_revision_id, we needed to preserve that data. This updates the patch to keep the array structure unless the nested array only contains a single item. If there is only one ID in total, it will be returned as a scalar. It works for our use case, but the patch hasn't been tested for other migrations.

I'm reasonably certain that all your regressions are due to assuming that at the end of the function you are transforming an array of destinations ids, but if the lookup failed, then the returned stub is not in an array, which breaks everything.

I think the big problem with this is that what the plugin returns is so darn confusing. It can be a scalar, array, or an array of arrays. IF you get back an array, it might be an array of ids for one record, or an array of single ids for multiple records. Any migration expecting a single record back could have problems.

I would propose a new configuration key, 'allow_multiple' or something like that, which would force the process to return an array of Ids rather than just a single row. This way, you easily preserve BC, while giving the option to use the new behavior moving forward.

The attached patch removes the deprecated function call, and updates the idMap prophecies in the tests to the new function, but doesn't change the behavior (returns the first result if multiple are found). Assuming it passes tests, I think the next step would be to implement the new key with logic to return all results if set (stubs would have to be managed and tests for this would need to be written). All the existing tests should still pass with this behavior, and a new one will have to be written.

Finally, with respect to stub management, #2890690: MigrateLookup plugin has inconsistent return values. should probably be resolved and incorporated into this too.

Thanks for the feedback, @mikelutz! Here's a patch that adds support for the "allow_multiple" configuration you describe. It should behave itself when it's off, too. Still needs tests, but it is working for our project.

I'm running 8.4.5 and the patch in #14 worked for me, while the patch in #27 did not.

My situation is that I have a file migration that brings in 0 or more files for each node. When I run the node migration, the patch in #14 correctly brings in the proper files while the patch in #27 doesn't bring in any files for any nodes.

-mike

Thank you for providing this patch #27, @byrond! I'm excited to get it working. However, you did not provide an example of how to navigate the resulting data structure, and I'm not sure how to go about it.

I'm trying to pull paragraph entities from multiple migrations, so I need to extract the target_id (index 0) and the target_revision_id (index 1) from each of the looked-up items. Here is my process configuration for the field:

  field_body/target_id:
    -
      plugin: migration_lookup
      allow_multiple: true
      migration:
        - aaas_report_body
        - aaas_report_document
        - aaas_report_link
      source: nid
      no_stub: true
    -
      plugin: extract
      index:
        - '0'
  field_body/target_revision_id:
    -
      plugin: migration_lookup
      allow_multiple: true
      migration:
        - aaas_report_body
        - aaas_report_document
        - aaas_report_link
      source: nid
      no_stub: true
    -
      plugin: extract
      index:
        - '1'

I suspect that I need to specify another level of index in those extract plugins, so that the target_id matches [0][0], [1][0], and [2][0], and the target_revision_id matches [1][0], [1][1], and [1][2]. But I don't know how to do that with the extract plugin, and more to the point, I don't know that that's even the structure of the array I'm working with. Can you confirm what structure your function will return in this situation?

Thank you again for your work on this.

Upon consideration, I've revised my configuration above to use the sub_process plugin so that the migration_lookup occurs only once, as follows:

  field_body:
    -
      plugin: migration_lookup
      allow_multiple: true
      migration:
        - aaas_report_body
        - aaas_report_document
        - aaas_report_link
      source: nid
      no_stub: true
    -
      plugin: sub_process
      process:
        target_id:
          plugin: extract
          index:
            - '0'
        target_revision_id:
          plugin: extract
          index:
            - '1'

Unfortunately, this gives me an error that "Input should be an array". Any thoughts?

Your sub_process plugin starts a fresh set of processes, and you have supplied no source into your extract plugins. In this case, your sub_process is working with the values 0 and 1 already, so you would be looking at something like

  field_body:
    -
      plugin: migration_lookup
      allow_multiple: true
      migration:
        - aaas_report_body
        - aaas_report_document
        - aaas_report_link
      source: nid
      no_stub: true
    -
      plugin: sub_process
      process:
        target_id: '0'
        target_revision_id: '1'

But I'm not sure off the top of my head if that is legal. It should be, as those values are array keys and not variable names in the code, but you would have to try it.

+++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
@@ -184,7 +184,10 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+        if (empty($this->configuration['allow_multiple'])) {

This is inventing a new process plugin property but not documenting it.

I've formulated my multi-value ERR migration field per @mikelutz's suggestion in #31 and this is working for me with patch #27:

  field_committee_membership:
    -
      plugin: migration_lookup
      allow_multiple: true
      migration:
        - committee_membership
      source: com_mem_id
      no_stub: true
    -
      plugin: sub_process
      process:
        target_id: '0'
        target_revision_id: '1'

I really appreciate the patch here.

Moving to review to run tests again over core 8.6

Back to needs work for a re-roll and tests.

Patch in commit #27 is hard to reroll because this is not apply over last commit before this implemented:

commit fe4f831ef335ec767628fef5a5e4778f738b24f8 (HEAD -> test-branch-2)
Author: Nathaniel Catchpole
Date: Fri Mar 2 13:05:51 2018 +0000
Issue #2946041 by neerajsingh: Theme development link should point to the version specific documentation

So I think that is needed to implement the enhancements of this (#27) over the last patch that work. maybe #25 an re-test all

Re-rolled patch from #27 for 8.7.x.

Testing should be done against 8.7.x

Back to needs work for the tests.

This patch has a tiny fix so existing tests are passing (still require new tests and change request a new option is being introduced)

New patch attached. Changes:

• Patch should apply :)
• Fixed small issue in existing tests
• Added tests for the new behavior

Please review

Setting this back to Needs review as test-only is failing, but full patch does pass

1. +++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
   @@ -92,6 +94,18 @@
   + * Migration lookup might found multiple results. By default only the first
   

   s/found/find/

2. +++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
   @@ -182,7 +196,10 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
   +        if (empty($this->configuration['allow_multiple'])) {
   

   More proof that process plugins need to extend ConfigurablePluginInterface.. The default value of allow_multiple is technically NULL, not FALSE. Still, this is correct for now.

3. +++ b/core/modules/migrate/tests/src/Unit/process/MigrationLookupTest.php
   @@ -263,12 +267,38 @@ public function successfulLookupDataProvider() {
   +      // Test data for multiple lookup results and multiple expected results.
   +      [
   +        // Source IDs of the migration map.
   +        [1, 'foo'],
   +        // Destination IDs of the migration map.
   +        [[2, 'bar'], [3, 'baz']],
   +        // Input values for the migration plugin.
   +        [1, 'foo'],
   +        // Expected output values of the migration plugin.
   +        [[2, 'bar'], [3, 'baz']],
   +        // Allow returning multiple lookup results.
   +        TRUE,
   +      ],
   

   Can we have a test case with a single destination id? I'd like to see what that looks like. Also, this test is mapping a complete set of source ids to multiple destination ids, which is not a valid use case. a complete set of source ids will always map to a single destination id set. We need to set up a test where we map [1,foo] to [2,bar] and [1,baz] to [2, qux] and then do a migration lookup on [1, NULL]

Also setting to NW for the needed CR.

Is this supposed to work also for single values when the field cardinality is unlimited? Without patching my migration works only when one id is returned for migration_lookup, but it breaks with more than one, instead of importing only the first item.

I can't apply the patch to 8.6.5 but I applied the changes manually. When I configure "allow_multiple: true" the migration fails on all items, with multiple destination_ids or not. I tried debugging the destination_ids variable in MigrationLookup.php and it seems to spit out an array of arrays:

drush mim user_personnel --limit 1
Array
(
    [0] => Array
        (
            [0] => 17
        )
)
 [error]  Value is not a valid entity. (/app/web/core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php:106)

Without "allow_multiple: true" everything works okay and the destinations_ids looks like this:

Array
(
  [0] => 17
)

Edit: I also debugged the EntityReference.php and it looks like nested array of values is passed there, when it should be a scalar, meaning a single entity id:

Array
(
    [0] => Array
        (
            [0] => Array
                (
                    [0] => 18
                )
        )
    [1] => Array
        (
            [0] => Array
                (
                    [0] => 19
                )

        )
)
 [error]  Value is not a valid entity. (/app/web/core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php:110)

Even if I edit the code and do some extra reset() calls to get rid of the wrapping arrays, EntityReference.php is still complaining about the value because it gets an array from migration_lookup:

Array
(
    [0] => 18
    [1] => 19
)
 [error]  Value is not a valid entity. (/app/web/core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php:110)

In the end everything seems to work without patching if I just use the multiple_values process plugin https://cgit.drupalcode.org/migrate_plus/tree/src/Plugin/migrate/process... from migrate_plus at the of the processing pipeline:

field_contact_card:
    -
      plugin: migration_lookup
      migration: user_contact_staff
      no_stub: true
      source: contact_cards
    -
      plugin: multiple_values

Edit:
Actually I don't even need the multiple_values plugin. When I removed the target_id from the mapping everything started working okay and I managed to attach two contact cards from another migration into the user entity.

Hello,

Which work remain to merge this patch into core ?

Thanks for continuing to work on this. A use case that was covered in the patch from #14 has fallen through the cracks, and the current patch no longer supports the scenario I described in #20, whereby the migration being looked up has two source ids but you are passing just one of the ids to the lookup process.

I've added that functionality back in this patch.

Sorry, that last patch wasn't quite right. Let's try this one.

Applies cleanly for me on 8.7.2, thank you!

Thanks for all of your work on this, everyone. I discovered an issue with the changes between #43 and #52 -- see
https://www.drupal.org/files/issues/2019-05-14/interdiff-2890844-43-52.txt

With an entity_reference_revisions lookup the destination_ids values need to be arrays ([target_id, target_revision_id]).

With the patch in #52 these lookups do not work. With the patch in #43 these lookups do work.

For what it's worth, the following patches are playing well together after going from Drupal 8.5 to 8.7.3.
It took me some time to find a working combination.

"drupal/core": {
  "Correct migration lookup to work with multiple values": "https://www.drupal.org/files/issues/2018-12-31/drupal-migrate_lookup_multiple_ids-2890844-43.patch",
},
"drupal/migrate_plus": {
  "Handle XML child nodes as XML in xml-based migrations": "https://www.drupal.org/files/issues/migrate_plus_xml_return_asxml.patch"
},
"drupal/migrate_tools": {
  "Introduce drush switch for rolling back missing migration items": "https://www.drupal.org/files/issues/2019-01-22/migrate-tools-rollback-missing-items-from-source-2863426-20.patch",
  "Migrate support for deleting items no longer in the incoming data": "https://www.drupal.org/files/issues/2019-04-22/2809433-136.patch"
}

I applied this combination of #58 (on the project that initiated this ticket) after the migrations failed since the upgrade to 8.7.

Even the combination of #58 did not fix it for the project. After the first attempt, I added allow_multiple in the migration config, but that made it worse (in stead of 1 reference, there were now 0).

Some more information:

With the patch of 43 o only had 1 ref imported. Then I realized I forgot to add allow_multiple in the migration config. Once added, I ended up with 0 references.

Checked with a debugger, and it seems that the lookup is properly returning the correct amount of ID's, but after that the get plugin is run (although not configured). It seems to go wrong there. As I haven't got a config for the get plugin, I could not configure the 'multiple' setting. Maybe that is the missing piece for me, didn't have time yet to test the migration with both migration_lookup and get plugin configured... Will let you know when I had a chance to test that.

Any other suggestions are welcome!

Update:

no luck with the setup in #58, but it does seem to work with #54

Pieter

+++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
@@ -233,9 +250,14 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+      elseif (!empty($this->configuration['allow_multiple']) && count($destination_ids) > 1) {
+        return array_map(function ($a) {
+          return reset($a);
+        }, $destination_ids);
+      }

As mentioned above, this is incorrect. destinations_ids can contain multiple keys and this throws them away.

We likely need to set return multiple to true when we return more than one value so that the result can be iterated more easily with sub_process

This patch does not work for Drupal 8.8.x and I doubt it works for 8.9.x..

Here's a rework for 8.8. The deprecated method has been removed already, but here is a patch to allow multiple values to be returned. It still needs to set the multiple flag though.

One minor comment typo:

+ * Migration lookup might found multiple results. By default only the first

should be find not found
That's so minor it could probably be fixed on commit?

This issue needs a change record to document the new setting. I also think the issue summary and title could do with an update to reflect the solution. Also this feels like a task rather than a bug since the "returning first match" has always been the case and the new option allows migration developers to configure something more complex.

The work here looks very useful and has test coverage and good code docs.

Yes the patch I posted needs a little more work, and the issue needs cleanup before this is ready. I didn't realize this issue got pushed to review already. I'll try and get the cleanup in this week.

+++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
@@ -216,7 +230,11 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+        if (empty($this->configuration['allow_multiple'])) {
+          $destination_ids = array_values(reset($destination_id_array));
+        } else {
+          $destination_ids = $destination_id_array;
+        }

Yeah, this seems to not make it consistent. If the allow_multiple is set to true. It returns something like:

[ 0 => ['fid' => 1], 1 => ['fid' => 2]]

Otherwise it's like:

[0 => 1]

.

Unfortunately, that is necessary to preserve backwards compatibility. We would prefer the former, but the plugin was originally designed to return the later if there is only one destination id in the results.

I am running v 8.9.1 of the migrate module and have been stuck on this error while trying to migrate d7_field_instance for quite a while now.

It looks to me like when MigrateException's import() method is invoked, it executes it's processRow method at line 203:

try {
        $this->processRow($row);
        $save = TRUE;

but it does not pass any value for $value to that method.

Later, if processRow invokes the Extract plugin, there will be no $value and the Extract plugin will complain that its input should be an array. I realize that as plugins are executed they set $value, but apparently at the time that Extract is invoked, $value is NULL, and hence the errors.

I have 480 d7_field_instances, and as best I can tell, 347 of them are returning with the "Input should be an array" and the remaining 133 are returning with the MigrationSkipRowexception and are marked STATUS_IGNORED.

9/3/2020: I crossed out some of the discussion about $value because I figured out that it is the "result" of all of the plugins that operate on a given field. It starts out as null, of course, because no plugins have operated yet.

@wdseelig That does not appear to be related to this issue, but come find us in the #migration channel in slack for help with specific migration issues.

hi all,

I have been busy again upgrading the project that kicked off this ticket a few years back.
Patch #65 didn't work. I had to add this bit from patch #54 to make it work for me (the elseif part in the snippet below)

if (empty($this->configuration['allow_multiple'])) {

          $destination_ids = array_values(reset($destination_id_array));
        } elseif (!empty($this->configuration['allow_multiple']) && count($destination_id_array) > 1) {
                  $destination_ids = array_map(function ($a) {
                      return reset($a);
                    }, $destination_id_array);
        } else {
          $destination_ids = $destination_id_array;
        }

I'm not familiar enough to properly understand if this was deliberately removed from #54 for some reason or accidentally. I'll leave it here for future reference.

I too wasn't able to get patch #65 to work on Drupal 8.9.x I had to make some altercations to get it to work properly. Attached is the updated patch that solved my problem.

I have a data structure similar to #20, where I'm trying to associate multiple paragraphs to a node. There are 800 paragraphs and 35 nodes. Drupal 8.9.8. Without any patches the below migration (less the allow_multiple: field) will import 1 paragraph per node. I have tried #65, #75 and #76 with the allow_multiple: true set in either/or/both pseudo_add_mag_op_paragraph and in field_content, as well as trying to consolidate the pseudo-field directly under source, which results in a transform error. I can't 100% eliminate syntax as the error, but it seems like allow_multiple may not be working?

With #76 applied and allow_multiple not specified, during migration it shows
[warning] reset() expects parameter 1 to be array, string given MigrationLookup.php:286
importing the nodes but without paragraphs.

The below migration with allow_multiple: true on the pseudofield doesn't throw any errors, the nodes are imported but with no paragraphs (versus 1 paragraph when no patches are applied & allow_multiple is not used).

id: mag_opening_node
source:
  plugin: csv
  path: files/migration/pages-UTF8.csv
  ids: [post_id]
process:
  type:
    plugin: default_value
    default_value: news_magazine
  pseudo_add_mag_op_paragraph:
    plugin: migration_lookup
    migration: mag_opening_paragraph
    # the below should be the tying id-- this post name, or the parent id of post for WP to tie together
    # this is using the id of the paragraph, which is set to post_id from WP. there are multiple rows
    # returned for each ID-- how to get those included?
    source: id
    allow_multiple: true
  title: post_title
  field_mag_migr_prior_id: id
  field_mag_content_type: post_type
  field_mag_issue: post_taxonomy
  #field_mag_issue_taxonomy: post_taxonomy
  #    plugin: entity_generate
  #      source: post_taxonomy
  #      entity_type: taxonomy_term
  #      bundle: magazine
  field_teaser: teaser_text
  field_image:
      plugin: image_import
      source: article_main_image
      title: post_title
      alt: post_title
      destination: 'public://magazine'
  # this is paragraph revision field, it should have multiple paragraphs per node
  field_content:
    plugin: sub_process
    include_source: true
    source:
      - '@pseudo_add_mag_op_paragraph'
    process:
      target_id: '0'
      target_revision_id: '1'
destination:
  plugin: 'entity:node'
  default_bundle: news_magazine
migration_dependencies:
  required:
    - mag_opening_paragraph
  optional: []

Patch #72 doesn't work for me. but a temporary solution is the method proposed in https://pitago.com.vn/tutorial/117/drupal-8-migration-multiple-paragraph... may be helpful to someone.

So as per #62 the proposals in #75 and #76 are incorrect.

EDIT: Remove wrong assumption

The patch from #65 helped me a lot to finally reference the items of a multi-value paragraphs field. Thanks!

Rerolled patch.

I need this patch, but my need is to return results from all migrations and not just from the first one with matches.

I am attaching a new patch with the interdiff

this is an example:

  field_paragraphs:
    -
      plugin: migration_lookup
      migration:
        - paragraphs_migration_1
        - paragraphs_migration_2
        - paragraphs_migration_3
      allow_multiple: true
      no_stub: true
      source: id
    -
      plugin: sub_process
      process:
        target_id: id
        target_revision_id: revision_id

This still needs work, and the current thinking is to replace the migrate_lookup plugin entirely with a new version that handles this in a more natural way.

In case this helps anyone at all, I figured out a fairly elegant solution to this issue without any patches or custom plugins.

I had a multi-value paragraphs reference field to populate with previously-migrated paragraph entities (constructed from D7 field collection entities). I made the migration_lookup part of the pipeline (see new_id) within the sub_process block, then extracted the value I needed for each ID from the resulting new_id source array.

  source:
    # All the source code for this migration.

  process:
    field_my_multi_value_paragraph_ref_field:
      -
        plugin: skip_on_empty
        method: process
        source: field_my_d7_field_collection_field
      - 
        plugin: sub_process
        process:
          new_id:
            plugin: migration_lookup
            migration: name_of_my_paragraph_migration
            source: value # The key of the array containing the original field value.

          target_id:
            plugin: extract
            source: '@new_id'
            index:
              - 0

          target_revision_id:
            plugin: extract
            source: '@new_id'
            index:
              - 1

  destination:
    # All the destination code for this migration.

I also faced the same problem the results were returned from the first migration loopkup.
After debugging I noticed in foreach loop the break statement was called even if we don't have any matching records from the first migration lookup thus it was not searching in remaining migration lookup. It was coming empty array as shown below.

Array
(
    [0] => Array
        (
            [id] => 
            [revision_id] => 
        )
)

I also tried code as per #91, but it is not working if we have multiple migration lookups.

I am attaching patch, it is without "allow_multiple: true".
Below is example for multiple migration lookups

field_paragraphs:
    -
      plugin: skip_on_empty
      method: process
      source: field_your_collection_field_name
    -
      plugin: migration_lookup
      migration:
        - migration_1
        - migration_2
        - migration_3
      no_stub: true

@jumpsuitgreen

I'm trying to figure out how to use the code you provided here for multiple paragraph migration: https://www.drupal.org/project/drupal/issues/2890844#comment-14670512

I am running Drupal 9.4.5 and assume this will work as you said without patches to core.

  source:
    # All the source code for this migration.

  process:
    field_my_multi_value_paragraph_ref_field:
      -
        plugin: skip_on_empty
        method: process
        source: field_my_d7_field_collection_field
      - 
        plugin: sub_process
        process:
          new_id:
            plugin: migration_lookup
            migration: name_of_my_paragraph_migration
            source: value # The key of the array containing the original field value.

          target_id:
            plugin: extract
            source: '@new_id'
            index:
              - 0

          target_revision_id:
            plugin: extract
            source: '@new_id'
            index:
              - 1

  destination:
    # All the destination code for this migration.

I have a D9 multi paragrraph reference field I'm trying to populate with paragraphs that have already been migrated with name_of_my_paragraph_migration from D7 custom SQL source.

My source plugin creates a field which contains an array of ids like this:

Array
(
    [0] => 3187
    [1] => 3191
)

My question is: If my source array field = field_my_d7_field_collection_field (used in your code above), will the migration_lookup in the above code loop through this array to lookup every matching id from the name_of_my_paragraph_migration migration? If not, what am I missing in the format of the source array field?

Note: The array field is formatted using this code:

I am using hook_migrate_prepare_row() in my migration module to add the child paragraph ids to each nested paragraph row.

/**
 * Implements hook_migrate_prepare_row().
 */
function sbn_migrations_migrate_prepare_row(Row $row, MigrateSourceInterface $source, MigrationInterface $migration) {
  switch ($migration->id()) {
    case 'nested': // Get the child paragraphs of each nested paragraph
      $mlids = array();
	  $mlid = $row->getSourceProperty('mlid'); // Get this id
	  $mlids = sbn_migrations_get_child_mlids($mlid); // Get all the child ids associated with this paragraph as an array
      $row->setSourceProperty('mlids', $mlids); // Add source column to this row called 'mlids' containing the child ids 
      break;
  }
}

/**
 * Get all the mlids to which an entiry is parent
 *
 * Requires: parent mlid
 *
 * Returns array of mlid:
 *
 *  [0] => mlid 
 *  [1] => mlid 
 *  [2] => mlid 
 *
 */
function sbn_migrations_get_child_mlids($mlid) {
 
$mlids = array();
 
// As per https://www.drupal.org/docs/drupal-apis/database-api/instantiating-a-dat...
// \Drupal\Core\Database\Database::setActiveConnection('migrate');
// $database = \Drupal\Core\Database\Database::getConnection();
// Or, as per: https://drupal.stackexchange.com/a/210574/50789 
$database = \Drupal\Core\Database\Database::getConnection('default', 'migrate');
 
// $database = \Drupal::database();
      $query = $database->select('node', 'n');
  $query->innerjoin('book', 'b', 'n.nid = b.nid');
  $query->innerjoin('menu_links', 'ml', 'b.mlid = ml.mlid');
      $query->condition('ml.plid', $mlid, '=');
      $query->fields('ml', ['mlid']);
      $result = $query->execute();
      foreach ($result as $record) {
        $mlids[] = $record->mlid;
      }
 
return $mlids;
}

@SomebodySysop, based on what your source is providing, maybe what you need to do first is use the multiple_values plugin so that the sub_process plugin sees your array as multiple, individual values instead of a single array value.

For anyone else having trouble, I have used this method several different times for two different clients. The source plugin for my node migrations is typcially d7_node and I perform the migrations using Drush commands.

Using the callback plugin at the very beginning of the pipeline reveals the incoming value from the source:

    field_my_multi_value_paragraph_ref_field:
      -
        plugin: callback
        callable: var_dump
        source: field_my_d7_field_collection_field

    // The incoming source values logged to the console. 
    // array {
    //   ["value"]=> "8423"
    //   ["revision_id"]=> "8651"
    // }
    // array {
    //   ["value"]=> "8424"
    //   ["revision_id"]=> "8652"
    // }

The D7 source field is a multi-value field containing two values on this particular node. Each array represents one value coming from the source.

Now move the callback plugin inside the subprocess pipeline as so:

  process:
    field_my_multi_value_paragraph_ref_field:
      - 
        plugin: sub_process
        source: field_my_d7_field_collection_field
        process:
          new_id:
            plugin: callback
            callable: var_dump
            source: value 
            // 'value' is the key of each incoming array that we want to act on.

    // The individual field values logged to the console.
    // "8423"
    // "8424"

Now insert the migrate_lookup plugin before the callbackplugin:

    field_my_multi_value_paragraph_ref_field:
      - 
        plugin: sub_process
        source: field_my_d7_field_collection_field
        process:
          new_id:
            -
              plugin: migration_lookup
              migration: name_of_my_paragraph_migration
              source: value
            -
              plugin: callback
              callable: var_dump

    // Lookup results logged to the console.
    // array {
    //   [0]=> "14703"
    //   [1]=> "20263"
    // }
    // array {
    //   [0]=> "14704"
    //   [1]=> "20264"
    // }

Because there are two values coming from the source field, the migration will iterate over each value applying the subprocess in which we grab the 'value' array key (in the incoming array of this example) and use it to look up the new value in the appropriate migrate_map table.

Now remove the callbackplugin (because var_dump returns NULL, effectively stopping the process) and add the processing for the target_id and target_revision_id fields:

    field_my_multi_value_paragraph_ref_field:
      - 
        plugin: sub_process
        source: field_my_d7_field_collection_field
        process:
          new_id:
            plugin: migration_lookup
            migration: name_of_my_paragraph_migration
            source: value

          target_id:
            plugin: extract
            source: '@new_id'
            index:
              - 0

          target_revision_id:
            plugin: extract
            source: '@new_id'
            index:
              - 1

For each iteration of the incoming source value, the sub_process pipeline will grab the value from the 'value' key, lookup the new ID which comes back as an indexed array (0,1) and holds that array in the new_id dummy field. Then the target_id and target_revision_id fields use the extractplugin to pull the appropriate value from the array stored in the new_id field.

This is how I got around the issue of migration_lookup only returning a single value. The array keys and values in your situation will be different so you'll need to adjust the code as necessary. For example, the incoming array key containing the value you want to use in your lookup may be different. In my situation, the array key is value. In another situation, the array key may be target_id.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Was previously tagged for IS update and change record which still appear the case.

Did not review or test.

Rerolled #87 on to the MR and rebase against latest 11.x, Updated IS. Hiding all patches.

For what it's worth, I came across this issue and found the patch in #92 worked perfectly for me.

Thanks @shashikant_chauhan!

For added context (see below), I'm processing a JSON file with the url source plugin from the migrate_plus module. As the paragraph mappings come from a `paragraphs` subkey in the JSON structure, I had to do the following in a sub_process plugin. The issue was that only migration lookups only worked for the first migration listed: `blog_paragraphs_copy`. I know this because I tried swapping the order and the `blog_paragraphs_copy_media` then worked but `blog_paragraphs_copy` didn't.

With the patch in #92 both migration lookups are now working successfully. If an item in one migration is found, then it's used, otherwise the other is used.

field_structured_content:
    plugin: sub_process
    source: paragraphs
    process:
      target_id:
        -
          plugin: migration_lookup
          migration:
            - blog_paragraphs_copy
            - blog_paragraphs_copy_media
          source: id
        -
          plugin: extract
          index:
            - 0
      target_revision_id:
        -
          plugin: migration_lookup
          migration:
            - blog_paragraphs_copy
            - blog_paragraphs_copy_media
          source: id
        -
          plugin: extract
          index:
            - 1

Thanks @ckng for the reroll - this was based on the 2890844-65.drupal.patch and now applies cleanly to 10.2.x

Rebased and added the CR.

Patch for Drupal 10.3

Is there a working 11.3.x patch?

Patch for 11.3.3

Hello,
I am using Drupal 11.3.3, Drush 13.7.1.0, PHP 8.3.
Patch mentioned in #109 is not working for me.

Thanks!