sourceMigration doesn't work if source migration map uses more than 1 destination key

To make it work in the meantime could you concatenate the keys in prepareRow() to construct a regular string of a single unique key?

Relevant code, the MigrateSQLMap::__construct(): http://drupalcode.org/project/migrate.git/blob/refs/heads/7.x-2.x:/plugi... showing that an array is valid input.

This causes getSourceMigration() to return an array of both the values from the two keys rather than just one, and so loading the entity from migration A fails, because entity_load() gets an array of those values where it expects an ID.

The function comments in http://drupalcode.org/project/migrate.git/blob/refs/heads/7.x-2.x:/inclu... say that an array is a valid return for getSourceMigration().

Seems like the fix is to implode() the array sent to entity_load(). I'm not familiar with the migrate module process past this point...

Ah... yes. HAHA!

Looks like a dupe of a previous issue: #2217909: sourceMigration doesn't work if source migration map uses more than 1 destination key.

Actually, I'm not sure it's the same as the other. At least this one is clearer about what's going on - you have a migration with a multi-column destination key, and when mapping via sourceMigration you want to apply just one of the columns to the mapped field. I'm not quite sure what's being accomplished by this, though - if you're only using part of the key then the migration A object is not going to be uniquely identified. Thus, if there are multiple A objects matching on that column, they will all be mapped to the B field...

Changing to a feature request and postponing - this won't go into Migrate 2.6, and frankly I think it's too obscure a use case to go in at all (but if someone submits a patch I can consider it after 2.6).

Hi,

Ok, I'm mixing it up a bit here with a migration, with multiple source keys and multiple destination keys. And I have it working... with a patch to migration::applyMappings().

The critical thing that does not work (at least not without extending the Migration class) is handling multiple source keys in a sourceMigration.

The tack I took was suggested in this thread (and the other related bug) -- contatenate the source keys. The problem is that this does not match up to the previous migration, and so it creates stubs. After stepping through with a debugger, that's because applyMappings is not as robust as it should be -- it cannot handle array values, at least not before callbacks are applied.

And callbacks are applied immediately after the source migration mapping!

So my fix was simply to move the callbacks ahead of the source migration mapping -- then I could expand the string-ified source keys back into an array, and get the correct value back from the mapping.

That's a muddled mess, I'm sure, so let me get more specific:

Migration 1

Imports a set of retailers. The source data has a key comprised of two fields: CardCode and Address2. The source migration stores these in the map table as source1 and source2, and has a single destination field.

Migration 2

Imports a set of parts carried at each retailer. 3 columns -- CardCode, Address2, SKU.

In Drupal, we already have products populated... I couldn't find a proper destination class for entity references, so I extended MigrateDestination to handle that part of it. This migration has 3 source keys (all 3 fields) and 2 destination keys (nid and delta for an entityreference field).

So for migration 2, I need to retrieve the nid from two of the keys, looked up in the migration map for Migration 1.

Here's some relevant code:

// in my_migration::__construct:

    $this->addFieldMapping('nid','source_key')
        ->separator('~')
        ->callbacks('my_array_callback')
        ->sourceMigration($migration1);
  }
  function prepareRow($row) {
    // find the entity_id of the product we have a sku for...
    // ...
    $row->entity_id = $model_array[$row->SKU]->nid;
    $row->source_key = $row->CardCode . '~' . $row->Address2;
  }
}
function my_array_callback($argument) {
  return array($argument);
}

Ok. This seems a bit more convoluted than should be necessary... the challenge is supporting all the flexibility the Migrate module tries to support. Breaking down applyMappings(), the key issue is this line (1206):

        if ($source && isset($this->sourceValues->{$source})) {

... if you pass in an array of source fields here, it cannot be interpreted as an object property, and so it grabs the default value, and you're done -- the keys get ignored from there.

So in my code I turned it into a string (source_key) which I set in the prepare_row method. Next I had to turn it back into an array to get it to match in the sourceMigration.

The ->separator split my keys back into an array, but the next problem is ->handleSourceMigration... which supports 3 different types of $source_keys: a single scalar, an array of scalars, or an array of arrays for multi-key sources.

If we pass it a string, it matches nothing. If we pass it a single-level array, it tries to find two different rows, one for each key. Thus the only opportunity I had to leverage this was to wrap the exploded key in yet another array.

I saw callbacks in the method, but haven't seen any documentation on them -- seems perfect for the job. The only problem? They get called immediately after the sourceMigrations are applied, not before. Is there a good reason for this?

If not, I would propose the following patch:

diff --git a/includes/migration.inc b/includes/migration.inc
index 5c2a0ad..57664f8 100644
--- a/includes/migration.inc
+++ b/includes/migration.inc
@@ -1218,13 +1218,6 @@ abstract class Migration extends MigrationBase {
           $destination_values = explode($separator, $destination_values);
         }
 
-        // If a source migration is supplied, use the current value for this field
-        // to look up a destination ID from the provided migration
-        $source_migration = $mapping->getSourceMigration();
-        if ($source_migration && isset($destination_values)) {
-          $destination_values = $this->handleSourceMigration($source_migration, $dest
-        }
-
         // Call any designated callbacks
         $callbacks = $mapping->getCallbacks();
         foreach ($callbacks as $callback) {
@@ -1233,6 +1226,13 @@ abstract class Migration extends MigrationBase {
           }
         }
 
+        // If a source migration is supplied, use the current value for this field
+        // to look up a destination ID from the provided migration
+        $source_migration = $mapping->getSourceMigration();
+        if ($source_migration && isset($destination_values)) {
+          $destination_values = $this->handleSourceMigration($source_migration, $dest
+        }
+
         // If specified, assure a unique value for this property.
         $dedupe = $mapping->getDedupe();
         if ($dedupe && isset($destination_values)) {

... seems like the only place this might be a concern is if there are any callbacks in use that assume sourceMigrations have already been applied on a field.

Hmm, I see #1406802: Change to field mapping callback support, perhaps if there's a new callback API being added, it might make sense to add that one before sourceMigrations, if there's any risk of breaking things with this change.

I don't see any mention of Callbacks together with sourceMigrations, so this looks like unspecified behaviour -- the document page on field mappings actually lists callbacks ahead of sourceMigration...

TLDR; for #7 above:

Is it more likely for somebody to want to use a field mapping callback to adjust the keys of a source migration before mapping, or to adjust the resulting destination keys after mapping?

In my case, I needed to adjust the format of source keys to pass in to the map to get the right mapping back -- but the applyMappings() method has the opposite sequence. Documentation is unclear, with no examples using both sourceMigration and callbacks.

So I'd like to propose changing the order these are applied, to solve the real-world problem of handling a sourceMigration with multiple source keys, which is currently not possible without overriding Migration->applyMappings().

I've added more specific steps on how to build a migration with multiple source and destination keys in a blog post here: http://www.freelock.com/blog/john-locke/2014-10/importing-foreign-key-re... ... this depends upon making the change outlined in #7.

I have the same trouble when I'm trying to migrate mi comments from other Drupal 7 instance. I have one message "No node ID provided for comment" my code is the following:

class ForumCommentsMigration extends GeneralMigrationForums {
  public function __construct() {
    parent::__construct();
    $this->description = t('Migration of the Comments');

    //Setup the dependency
    $this->dependencies = array('ForumTerms','ForumNodes');

    //Setup mapping b/w the source and destination
    $this->map = new MigrateSQLMap($this->machineName,
      array(
        'cid' => array(
           'type' => 'int',
           'length' => 11,
           'not null' => TRUE,
           'description' => 'Id of the comment',
           ),
        ),
      MigrateDestinationComment::getKeySchema()
      );

    $forum_columns = array (
      'cid',
      'subject',
      'created',
      'changed',
      'status',
      'hostname',
      'name',
      'mail',
      'homepage',
      'thread');

    $query = Database::getConnection('default', 'source_db')
    ->select('comment','cm')
    ->fields('cm', $forum_columns);

    $query->leftJoin('node', 'n', ' cm.nid = n.nid');
    $query->addField('n', 'nid');

    $this->addSimpleMappings(array('subject', 'created', 'changed', 'status', 'hostname', 'name', 'mail', 'homepage', 'thread'));

    $this->addFieldMapping('nid', 'SOURCE_ID')
    ->sourceMigration('ForumNodes', 'DESTINATION_KEY_NAME')
    ->arguments(array('source_type' => 'nid'));

But the message "No node ID provided for comment" still displayed any idea will be appreciated

As freelock has stated, sourceMigration doesn't seem to support the case when the "source migration map" has more than 1 source key.
For example if the first migration defines a multiple field source key like this

$this->map = new MigrateSQLMap($this->machineName,
      array(
        'menu_name' => array(
          'type' => 'varchar',
          'length' => 255,
          'not null' => TRUE,
          'description' => t('Lunch menu name'),
        ),
        'lunch_menu_date' => array(
          'type' => 'varchar',
          'length' => 255,
          'not null' => TRUE,
          'description' => t('Lunch menu date'),
        ),
      ),
      MigrateDestinationNode::getKeySchema()
    );

In the second migration I can't use

 $this->addFieldMapping('field_lunch_day', array('menu_name', 'lunch_menu_date'))
      ->sourceMigration($source_migration);

Because passing an array of field names as the source key is not supported in Migration::applyMappings(), although the underlying method Migration::handleSourceMigration() can work it out, if you send the values of those fields as an array of an array.

My fix for this was to add an additional conditional, to check in Migration::applyMappings() the case when the source is an array of field names. The relevant patched code looks like this:

        // If there's a separator specified for this destination, then it
        // will be populated as an array exploded from the source value
        $separator = $mapping->getSeparator();
        if ($separator && isset($destination_values)) {
          $destination_values = explode($separator, $destination_values);
        }

        // If a source migration is supplied, use the current value for this
        // field to look up a destination ID from the provided migration.
        $source_migration = $mapping->getSourceMigration();
        if ($source_migration) {
          if (isset($destination_values)) {
            $destination_values = $this->handleSourceMigration($source_migration, $destination_values, $default, $this);
          }
          // Handle case for multi-field source key.
          elseif (is_array($source)) {
            $multi_source_values = array();
            $all_source_values_present = TRUE;

            // Go through each field that is part of the source key.
            foreach ($source as $source_key) {
              // If the value is present, add it to the list.
              if (isset($this->sourceValues->{$source_key})) {
                $multi_source_values[] = $this->sourceValues->{$source_key};
              }
              // One of the source key values is not present,
              // something went wrong, so break out of the foreach.
              else {
                $all_source_values_present = FALSE;
                break;
              }
            }

            // Only if all source key values were found, try to get the
            // source migration destination value.
            if ($all_source_values_present) {
              $destination_values = $this->handleSourceMigration($source_migration, array($multi_source_values), $default, $this);
            }
          }
        }

        // Call any designated callbacks
        $callbacks = $mapping->getCallbacks();
        foreach ($callbacks as $callback) {
          if (isset($destination_values)) {
            $destination_values = call_user_func($callback, $destination_values);
          }
        }

I believe this a better approach than doing the mapping in the callback function, in case the callback function call is moved higher in the execution line.

Attaching patch with the proposed solution for the multi field source key use case.

I can't image a case where that could happen, can you give an example?

If the source migration has a multi-field source key, then you are required to provide all the fields for the source key, to get a valid destination value back.

Or am I misunderstanding your question?

I think there is a misunderstanding.
In the original issue text (post #0) you mention that you have a multi-field "destination key", whereas my example is regarding a multi-field "source key".
Both are currently an issue, but my patch was targeted only for the multi-field source key usecase.

@Placinta: thanks for your patch, great work. This helped me a lot.
One thing I had to add though: I was passing in the separator option also and your patch didn't take that into account.

    $this->addFieldMapping('field_to_recieve', array('key_field1', 'key_field2'))
      ->sourceMigration('SourceMigration')
      ->separator(',');

// If a source migration is supplied, use the current value for this
        // field to look up a destination ID from the provided migration.
        $source_migration = $mapping->getSourceMigration();
        if ($source_migration) {
          if (isset($destination_values)) {
            $destination_values = $this->handleSourceMigration($source_migration, $destination_values, $default, $this);
          }
          // Handle case for multi-field source key.
          elseif (is_array($source)) {
            $multi_source_values = array();
            $all_source_values_present = TRUE;

            // Go through each field that is part of the source key.
            foreach ($source as $source_index => $source_key) {
              // If the value is present, add it to the list.
              if (isset($this->sourceValues->{$source_key})) {
                if ($separator) {
                  $values = explode($separator, $this->sourceValues->{$source_key});
                }
                else {
                  $values = array($this->sourceValues->{$source_key});
                }
                foreach ($values as $value_i => $value_val) {
                  $multi_source_values[$value_i][$source_index] = $value_val;
                }
              }
              // One of the source key values is not present,
              // something went wrong, so break out of the foreach.
              else {
                $all_source_values_present = FALSE;
                break;
              }
            }
            // Only if all source key values were found, try to get the
            // source migration destination value.
            if ($all_source_values_present) {
              $destination_values = $this->handleSourceMigration($source_migration, $multi_source_values, $default, $this);
            }
          }
        }

To solve this without breaking existing migrate behavior, I propose adding an advanced mode to the separator instead, to allow defining both multiple groups and multiple keys at the same time, instead of assuming a single key with multiple lookup values.

This lets the separator expand the value to the full "array of arrays" structure.

Done