Issue #3126063 by quietone, Wim Leers, DamienMcKenna, benjifisher, alexpott,...

Issue #3126063 by quietone, Wim Leers, DamienMcKenna, benjifisher, alexpott, heddn: Harden SubProcess process plugin

core/modules/filter/tests/src/Kernel/Migrate/d7/MigrateFilterFormatTest.php

@@ -2,8 +2,11 @@
 
 namespace Drupal\Tests\filter\Kernel\Migrate\d7;
 
+use Drupal\Core\Database\Database;
 use Drupal\filter\Entity\FilterFormat;
 use Drupal\filter\FilterFormatInterface;
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\Tests\migrate\Kernel\MigrateDumpAlterInterface;
 use Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase;
 
 /**
@@ -11,7 +14,7 @@
  *
  * @group filter
  */
-class MigrateFilterFormatTest extends MigrateDrupal7TestBase {
+class MigrateFilterFormatTest extends MigrateDrupal7TestBase implements MigrateDumpAlterInterface {
 
   /**
    * {@inheritdoc}
@@ -27,6 +30,30 @@ protected function setUp(): void {
     $this->executeMigration('d7_filter_format');
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function migrateDumpAlter(KernelTestBase $test) {
+    $db = Database::getConnection('default', 'migrate');
+    $fields = [
+      'format' => 'image_resize_filter',
+      'name' => 'Image resize',
+      'cache' => '1',
+      'status' => '1',
+      'weight' => '0',
+    ];
+    $db->insert('filter_format')->fields($fields)->execute();
+    $fields = [
+      'format' => 'image_resize_filter',
+      'module' => 'filter',
+      'name' => 'image_resize_filter',
+      'weight' => '0',
+      'status' => '1',
+      'settings' => serialize([]),
+    ];
+    $db->insert('filter')->fields($fields)->execute();
+  }
+
   /**
    * Asserts various aspects of a filter format entity.
    *
@@ -80,6 +107,22 @@ public function testFilterFormat() {
     // The disabled php_code format gets migrated, but the php_code filter is
     // changed to filter_null.
     $this->assertEntity('php_code', 'PHP code', ['filter_null' => 0], 11, FALSE);
+
+    // Test a non-existent format.
+    $this->assertEntity('image_resize_filter', 'Image resize', [], 0, TRUE);
+
+    // For each filter that does not exist on the destination, there should be
+    // a log message.
+    $migration = $this->getMigration('d7_filter_format');
+    $errors = array_map(function ($message) {
+      return $message->message;
+    }, iterator_to_array($migration->getIdMap()->getMessages()));
+    $this->assertCount(2, $errors);
+    sort($errors);
+    $message = 'Filter image_resize_filter could not be mapped to an existing filter plugin; omitted since it is a transformation-only filter. Install and configure a successor after the migration.';
+    $this->assertEquals($errors[0], $message);
+    $message = ('Filter php_code could not be mapped to an existing filter plugin; defaulting to filter_null and dropping all settings. Either redo the migration with the module installed that provides an equivalent filter, or modify the text format after the migration to remove this filter if it is no longer necessary.');
+    $this->assertEquals($errors[1], $message);
   }
 
 }

core/modules/migrate/src/Plugin/migrate/process/SubProcess.php

@@ -16,7 +16,8 @@
  * Available configuration keys:
  *   - process: the plugin(s) that will process each element of the source.
  *   - key: runs the process pipeline for the key to determine a new dynamic
- *     name.
+ *     name. If the new dynamic name is NULL then the result of the sub_process
+ *     pipeline is ignored.
  *   - include_source: (optional) If TRUE, all source plugin configuration and
  *     values will be copied into the sub-processed row in a new property named
  *     for the source_key configuration value (see below). Defaults to FALSE.
@@ -143,7 +144,8 @@
  * you need to change the key, it is possible for the returned array to be keyed
  * by one of the transformed values in the sub-array. For the same source data
  * used in the previous example, the migration below would result to keys
- * 'filter_2' and 'filter_0'.
+ * 'filter_2' and 'filter_0'. If the value for 'id' is NULL the result of the
+ * sub_process pipeline is ignored.
  * @code
  * process:
  *   filters:
@@ -207,7 +209,13 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
         if (array_key_exists('key', $this->configuration)) {
           $key = $this->transformKey($key, $migrate_executable, $new_row);
         }
-        $return[$key] = $destination;
+        // Do not save the result if the key is NULL. The configured process
+        // pipeline used in transformKey() will return NULL if a
+        // MigrateSkipProcessException is thrown.
+        // @see \Drupal\filter\Plugin\migrate\process\FilterID
+        if ($key !== NULL) {
+          $return[$key] = $destination;
+        }
       }
     }
     return $return;

core/modules/migrate/tests/src/Unit/process/SubProcessTest.php

@@ -125,4 +125,73 @@ public function providerTestSubProcess() {
     ];
   }
 
+  /**
+   * Tests the sub_process process plugin.
+   *
+   * @dataProvider providerTestNotFoundSubProcess
+   */
+  public function testNotFoundSubProcess($process_configuration, $source_values = []) {
+    $migration = $this->getMigration();
+    // Set up the properties for the sub_process.
+    $plugin = new SubProcess($process_configuration, 'sub_process', []);
+    // Manually create the plugins. Migration::getProcessPlugins does this
+    // normally but the plugin system is not available.
+    foreach ($process_configuration['process'] as $destination => $source) {
+      $sub_process_plugins[$destination][] = new Get(['source' => $source], 'get', []);
+    }
+    $migration->expects($this->at(1))
+      ->method('getProcessPlugins')
+      ->willReturn($sub_process_plugins);
+    // Set up the key plugins.
+    if (array_key_exists('key', $process_configuration)) {
+      $key_plugin['key'][] = new Get(['source' => '@id'], 'get', []);
+      $migration->expects($this->at(2))
+        ->method('getProcessPlugins')
+        ->will($this->returnValue($key_plugin));
+    }
+    $event_dispatcher = $this->createMock(EventDispatcherInterface::class);
+    $migrate_executable = new MigrateExecutable($migration, $this->createMock(MigrateMessageInterface::class), $event_dispatcher);
+
+    // The current value of the pipeline.
+    $current_value = [
+      [
+        'source_foo' => 'test',
+        'source_id' => NULL,
+      ] + $source_values,
+    ];
+    // This is not used but the interface requires it, so create an empty row.
+    $row = new Row($source_values);
+
+    // After transformation, check to make sure that source_foo and source_id's
+    // values ended up in the proper destinations, and that the value of the
+    // key (@id) is the same as the destination ID (42).
+    $new_value = $plugin->transform($current_value, $migrate_executable, $row, 'test');
+    $this->assertArrayEquals([], $new_value);
+  }
+
+  /**
+   * Data provider for testNotFoundSubProcess().
+   */
+  public function providerTestNotFoundSubProcess() {
+    return [
+      'no key' => [
+        'process configuration' => [
+          'process' => [
+            'foo' => 'source_foo',
+          ],
+          'key' => '@id',
+        ],
+      ],
+      'lookup returns NULL' => [
+        'process configuration' => [
+          'process' => [
+            'foo' => 'source_foo',
+            'id' => 'source_id',
+          ],
+          'key' => '@id',
+        ],
+      ],
+    ];
+  }
+
 }