From 3cfe0f0428abf6ff6431cafbbcdd9ae5958a1d87 Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 16 Sep 2019 12:16:34 +0100 Subject: [PATCH] Issue #2815895 by littlepixiez, NickWilde, heddn, quietone, rachel_norfolk, vacho: Malformed migration_dependencies breaks all migrations --- core/modules/migrate/src/Plugin/Migration.php | 4 + .../migrate/tests/src/Unit/MigrationTest.php | 80 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/core/modules/migrate/src/Plugin/Migration.php b/core/modules/migrate/src/Plugin/Migration.php index a946033ffd1d..7b1e6769cafe 100644 --- a/core/modules/migrate/src/Plugin/Migration.php +++ b/core/modules/migrate/src/Plugin/Migration.php @@ -2,6 +2,7 @@ namespace Drupal\migrate\Plugin; +use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Plugin\PluginBase; use Drupal\migrate\Exception\RequirementsException; @@ -618,6 +619,9 @@ class Migration extends PluginBase implements MigrationInterface, RequirementsIn */ public function getMigrationDependencies() { $this->migration_dependencies = ($this->migration_dependencies ?: []) + ['required' => [], 'optional' => []]; + if (count($this->migration_dependencies) !== 2 || !is_array($this->migration_dependencies['required']) || !is_array($this->migration_dependencies['optional'])) { + throw new InvalidPluginDefinitionException($this->id(), "Invalid migration dependencies configuration for migration {$this->id()}"); + } $this->migration_dependencies['optional'] = array_unique(array_merge($this->migration_dependencies['optional'], $this->findMigrationDependencies($this->process))); return $this->migration_dependencies; } diff --git a/core/modules/migrate/tests/src/Unit/MigrationTest.php b/core/modules/migrate/tests/src/Unit/MigrationTest.php index 85935674f668..27e9a803b3ed 100644 --- a/core/modules/migrate/tests/src/Unit/MigrationTest.php +++ b/core/modules/migrate/tests/src/Unit/MigrationTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\migrate\Unit; +use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException; use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Plugin\Migration; use Drupal\migrate\Exception\RequirementsException; @@ -112,6 +113,75 @@ class MigrationTest extends UnitTestCase { $migration->checkRequirements(); } + /** + * Tests valid migration dependencies configuration returns expected values. + * + * @param array|null $source + * The migration dependencies configuration being tested. + * @param array $expected_value + * The migration dependencies configuration array expected. + * + * @covers ::getMigrationDependencies + * @dataProvider getValidMigrationDependenciesProvider + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + */ + public function testMigrationDependenciesWithValidConfig($source, array $expected_value) { + $migration = new TestMigration(); + if (!is_null($source)) { + $migration->set('migration_dependencies', $source); + } + $this->assertSame($migration->getMigrationDependencies(), $expected_value); + } + + /** + * Tests that getting migration dependencies fails with invalid configuration. + * + * @covers ::getMigrationDependencies + */ + public function testMigrationDependenciesWithInvalidConfig() { + $migration = new TestMigration(); + + // Set the plugin ID to test the returned message. + $plugin_id = 'test_migration'; + $migration->setPluginId($plugin_id); + + // Migration dependencies expects ['optional' => []] or ['required' => []]]. + $migration->set('migration_dependencies', ['test_migration_dependency']); + + $this->expectException(InvalidPluginDefinitionException::class); + $this->expectExceptionMessage("Invalid migration dependencies configuration for migration {$plugin_id}"); + $migration->getMigrationDependencies(); + } + + /** + * Provides data for valid migration configuration test. + */ + public function getValidMigrationDependenciesProvider() { + return [ + [ + 'source' => NULL, + 'expected_value' => ['required' => [], 'optional' => []], + ], + [ + 'source' => [], + 'expected_value' => ['required' => [], 'optional' => []], + ], + [ + 'source' => ['required' => ['test_migration']], + 'expected_value' => ['required' => ['test_migration'], 'optional' => []], + ], + [ + 'source' => ['optional' => ['test_migration']], + 'expected_value' => ['optional' => ['test_migration'], 'required' => []], + ], + [ + 'source' => ['required' => ['req_test_migration'], 'optional' => ['opt_test_migration']], + 'expected_value' => ['required' => ['req_test_migration'], 'optional' => ['opt_test_migration']], + ], + ]; + } + } /** @@ -125,6 +195,16 @@ class TestMigration extends Migration { public function __construct() { } + /** + * Sets the migration ID (machine name). + * + * @param string $plugin_id + * The plugin_id of the plugin instance. + */ + public function setPluginId($plugin_id) { + $this->pluginId = $plugin_id; + } + /** * Sets the requirements values. *