IdMapFilter::getRowBySouce() will throw a TypeError on failed lookup

Created on 20 January 2024, 11 months ago

Problem/Motivation

According to MigrateIdMapInterface::getRowBySource() must always return an array.

Accordingly, in migrate_tools's IdMapFilter class, we define the function like so:

public function getRowBySource(array $source_id_values): array {}

However, our function wraps core Migrate module's Sql::getRowBySourceId(), which also implements the same interface but does not follow the contract, and may in fact return a boolean, in cases where a lookup fails.

  public function getRowBySource(array $source_id_values) {
...
...
...
return $result->fetchAssoc();
}

fetchAssoc, as we know, can return FALSE or an array.

Which means that when IdMapFilter::getRowBySource() does this:

// ($map is under some circumstances, Drupal\migrate\Plugin\migrate\id_map\Sql)
return $map->getRowBySource($source_id_values); 

our migrate_tools IdMapFilter::getRowBySource() can attempt to return a boolean, which it is not allowed to do, thus the TypeError.

While the bug actually lies in the migrate module (and I will open an issue there), since migrate is a core module, and it might take a long while if ever for a patch to be approved there, we might do well to make our implementation handle the devilry itself.

Propose something like:

  public function getRowBySource(array $source_id_values): array {
    $map = $this->getInnerIterator();
    \assert($map instanceof MigrateIdMapInterface);
    // Handle the fact that currently 
    // Drupal\migrate\Plugin\migrate\id_map\Sql:: getRowBySource()
    // may possibly return FALSE.
-    return $map->getRowBySource($source_id_values);
+   return $map->getRowBySource($source_id_values) ?? [];
  }

Steps to reproduce

Call IdMapFilter::getRowBySource() with a known non-existent source id:

$migrate_executable->getIdMap()->getRowBySource($key);

I wrapped this call in a try{} catch (\TypeError) {} which helped.

Proposed resolution

Convert any returned FALSE from Drupal\migrate\Plugin\migrate\id_map\Sql::getRowById() to an array before returning.

return $map->getRowBySource($source_id_values) ?? [];

Remaining tasks

Open a merge request with the suggested change.

User interface changes

None

API changes

None

Data model changes

None

πŸ› Bug report
Status

Needs review

Version

6.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States apotek

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Production build 0.71.5 2024