diff --git a/core/modules/migrate/src/Plugin/Exception/BadPluginDefinitionException.php b/core/modules/migrate/src/Plugin/Exception/BadPluginDefinitionException.php index d0dfe01..5387f97 100644 --- a/core/modules/migrate/src/Plugin/Exception/BadPluginDefinitionException.php +++ b/core/modules/migrate/src/Plugin/Exception/BadPluginDefinitionException.php @@ -22,7 +22,7 @@ class BadPluginDefinitionException extends InvalidPluginDefinitionException { * @see \Exception */ public function __construct($plugin_id, $property, $code = 0, \Exception $previous = NULL) { - $message = sprintf('The %s plugin should define the %s property.', $plugin_id, $property); + $message = sprintf('The %s plugin must define the %s property.', $plugin_id, $property); parent::__construct($plugin_id, $message, $code, $previous); } diff --git a/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php index 11f32cf..f386a00 100644 --- a/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php +++ b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php @@ -38,7 +38,8 @@ * @see \Drupal\migrate\Plugin\MigrateSourceInterface * * @MigrateSource( - * id = "embedded_data" + * id = "embedded_data", + * source_module = "migrate" * ) */ class EmbeddedDataSource extends SourcePluginBase { diff --git a/core/modules/migrate/tests/src/Kernel/Plugin/MigrationProvidersExistTest.php b/core/modules/migrate/tests/src/Kernel/Plugin/MigrationProvidersExistTest.php index 19a8b24..c3fef36 100644 --- a/core/modules/migrate/tests/src/Kernel/Plugin/MigrationProvidersExistTest.php +++ b/core/modules/migrate/tests/src/Kernel/Plugin/MigrationProvidersExistTest.php @@ -2,7 +2,6 @@ namespace Drupal\Tests\migrate\Kernel\Plugin; -use Drupal\Component\Render\FormattableMarkup; use Drupal\KernelTests\FileSystemModuleDiscoveryDataProviderTrait; use Drupal\migrate\Plugin\Exception\BadPluginDefinitionException; use Drupal\migrate_drupal\Plugin\MigrateFieldPluginManager; @@ -18,53 +17,46 @@ class MigrationProvidersExistTest extends MigrateDrupalTestBase { use FileSystemModuleDiscoveryDataProviderTrait; /** - * {@inheritdoc} + * Tests that a missing source_module property raises an exception. */ - public static $modules = ['migration_provider_test']; + public function testSourceProvider() { + $this->enableModules(['migration_provider_test']); + $this->setExpectedException(BadPluginDefinitionException::class, 'The no_source_module plugin must define the source_module property.'); + $this->container->get('plugin.manager.migration')->getDefinition('migration_provider_no_annotation'); + } /** - * Tests that modules exist for all source and destination plugins. + * Tests that modules exist for all source plugins. */ public function testProvidersExist() { - // Install all available modules. - $module_handler = $this->container->get('module_handler'); - $modules = $this->coreModuleListDataProvider(); - $modules_enabled = $module_handler->getModuleList(); - $modules_to_enable = array_keys(array_diff_key($modules, $modules_enabled)); - $this->enableModules($modules_to_enable); + $this->enableAllModules(); /** @var \Drupal\migrate\Plugin\MigrationPluginManager $plugin_manager */ $plugin_manager = $this->container->get('plugin.manager.migration'); - // Get all the migrations. - $migrations = $plugin_manager->createInstances(array_keys($plugin_manager->getDefinitions())); - // Ensure the test module was enabled. - $this->assertTrue(array_key_exists('migration_provider_test', $migrations)); - $this->assertTrue(array_key_exists('migration_provider_no_annotation', $migrations)); - /** @var \Drupal\migrate\Plugin\Migration $migration */ + + // Instantiate all migrations. + $migrations = array_keys($plugin_manager->getDefinitions()); + $migrations = $plugin_manager->createInstances($migrations); + + /** @var \Drupal\migrate\Plugin\MigrationInterface $migration */ foreach ($migrations as $migration) { - $source_module = $migration->getSourcePlugin()->getSourceModule(); - $destination_module = $migration->getDestinationPlugin()->getDestinationModule(); - $migration_id = $migration->getPluginId(); - if ($migration_id == 'migration_provider_test') { - $this->assertFalse($source_module, new FormattableMarkup('Source module not found for @migration_id.', ['@migration_id' => $migration_id])); - $this->assertFalse($destination_module, new FormattableMarkup('Destination module not found for @migration_id.', ['@migration_id' => $migration_id])); - } - elseif ($migration_id == 'migration_provider_no_annotation') { - $this->assertFalse($source_module, new FormattableMarkup('Source module not found for @migration_id.', ['@migration_id' => $migration_id])); - $this->assertTrue($destination_module, new FormattableMarkup('Destination module found for @migration_id.', ['@migration_id' => $migration_id])); - } - else { - $this->assertTrue($source_module, new FormattableMarkup('Source module found for @migration_id.', ['@migration_id' => $migration_id])); - $this->assertTrue($destination_module, new FormattableMarkup('Destination module found for @migration_id.', ['@migration_id' => $migration_id])); - } - // Destination module can't be migrate or migrate_drupal or - // migrate_drupal_ui. - $invalid_destinations = ['migrate', 'migrate_drupal', 'migrate_drupal_ui']; - $this->assertNotContains($destination_module, $invalid_destinations, new FormattableMarkup('Invalid destination for @migration_id.', ['@migration_id' => $migration_id])); + $this->assertInternalType('string', $migration->getSourcePlugin()->getSourceModule()); } } /** + * Enable all available modules. + */ + protected function enableAllModules() { + // Install all available modules. + $module_handler = $this->container->get('module_handler'); + $modules = $this->coreModuleListDataProvider(); + $modules_enabled = $module_handler->getModuleList(); + $modules_to_enable = array_keys(array_diff_key($modules, $modules_enabled)); + $this->enableModules($modules_to_enable); + } + + /** * Tests that modules exist for all field plugins. */ public function testFieldProvidersExist() { @@ -150,16 +142,9 @@ public function testFieldProvidersExist() { 'destination_module' => 'core', ], ]; - // Install all available modules. - $module_handler = $this->container->get('module_handler'); - $modules = $this->coreModuleListDataProvider(); - $modules_enabled = $module_handler->getModuleList(); - $modules_to_enable = array_keys(array_diff_key($modules, $modules_enabled)); - $this->enableModules($modules_to_enable); + $this->enableAllModules(); - /** @var \Drupal\migrate_drupal\Plugin\MigrateFieldPluginManager $plugin_manager */ - $plugin_manager = $this->container->get('plugin.manager.migrate.field'); - $definitions = $plugin_manager->getDefinitions(); + $definitions = $this->container->get('plugin.manager.migrate.field')->getDefinitions(); foreach ($definitions as $key => $definition) { $this->assertArrayHasKey($key, $expected_mappings); $this->assertEquals($expected_mappings[$key]['source_module'], $definition['source_module']); @@ -192,7 +177,7 @@ public function testFieldProviderMissingRequiredProperty(array $definitions, $mi $plugin_manager->method('getDiscovery') ->willReturn($discovery); - $this->setExpectedException(BadPluginDefinitionException::class, "The missing_{$missing_property} plugin should define the $missing_property property."); + $this->setExpectedException(BadPluginDefinitionException::class, "The missing_{$missing_property} plugin must define the $missing_property property."); $plugin_manager->getDefinitions(); } diff --git a/core/modules/migrate_drupal/config/install/migrate_drupal.settings.yml b/core/modules/migrate_drupal/config/install/migrate_drupal.settings.yml new file mode 100644 index 0000000..d5a464e --- /dev/null +++ b/core/modules/migrate_drupal/config/install/migrate_drupal.settings.yml @@ -0,0 +1,5 @@ +# Migrations with any of these tags will raise an exception if their source +# plugin is missing the source_module property in their annotation. +enforce_source_module_tags: + - Drupal 6 + - Drupal 7 diff --git a/core/modules/migrate_drupal/config/schema/migrate_drupal.schema.yml b/core/modules/migrate_drupal/config/schema/migrate_drupal.schema.yml new file mode 100644 index 0000000..e980081 --- /dev/null +++ b/core/modules/migrate_drupal/config/schema/migrate_drupal.schema.yml @@ -0,0 +1,10 @@ +migrate_drupal.settings: + type: config_object + label: 'Migrate Drupal settings' + mapping: + enforce_source_module_tags: + type: sequence + label: 'source_module enforcement tags' + sequence: + type: string + label: 'Tag' diff --git a/core/modules/migrate_drupal/migrate_drupal.install b/core/modules/migrate_drupal/migrate_drupal.install new file mode 100644 index 0000000..0ef88af --- /dev/null +++ b/core/modules/migrate_drupal/migrate_drupal.install @@ -0,0 +1,16 @@ +getEditable('migrate_drupal.settings') + ->set('enforce_source_module_tags', ['Drupal 6', 'Drupal 7']) + ->save(); +} diff --git a/core/modules/migrate_drupal/src/MigrateDrupalServiceProvider.php b/core/modules/migrate_drupal/src/MigrateDrupalServiceProvider.php new file mode 100644 index 0000000..2ea8c23 --- /dev/null +++ b/core/modules/migrate_drupal/src/MigrateDrupalServiceProvider.php @@ -0,0 +1,26 @@ +getDefinition('plugin.manager.migration') + ->setClass(MigrationPluginManager::class) + ->addArgument(new Reference('plugin.manager.migrate.source')) + ->addArgument(new Reference('config.factory')); + } + +} diff --git a/core/modules/migrate_drupal/src/MigrationPluginManager.php b/core/modules/migrate_drupal/src/MigrationPluginManager.php new file mode 100644 index 0000000..0233de2 --- /dev/null +++ b/core/modules/migrate_drupal/src/MigrationPluginManager.php @@ -0,0 +1,109 @@ +sourceManager = $source_manager; + $this->configFactory = $config_factory; + } + + /** + * Returns the migration tags that trigger source_module enforcement. + * + * @return string[] + */ + protected function getEnforcedSourceModuleTags() { + if ($this->enforcedSourceModuleTags === NULL) { + $this->enforcedSourceModuleTags = $this->configFactory + ->get('migrate_drupal.settings') + ->get('enforce_source_module_tags') ?: []; + } + return $this->enforcedSourceModuleTags; + } + + /** + * {@inheritdoc} + */ + public function processDefinition(&$definition, $plugin_id) { + parent::processDefinition($definition, $plugin_id); + + // If the migration has no tags, we don't need to enforce the source_module + // annotation property. + if (empty($definition['migration_tags'])) { + return; + } + + // Check if the migration has any of the tags that trigger source_module + // enforcement. + $applied_tags = array_intersect($this->getEnforcedSourceModuleTags(), $definition['migration_tags']); + if ($applied_tags) { + // Throw an exception if the source plugin definition does not define a + // source_module. + $source_id = $definition['source']['plugin']; + $source_definition = $this->sourceManager->getDefinition($source_id); + if (empty($source_definition['source_module'])) { + throw new BadPluginDefinitionException($source_id, 'source_module'); + } + } + } + +} diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/EmptySource.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/EmptySource.php index e0acc4b..0c49d04 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/EmptySource.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/EmptySource.php @@ -14,7 +14,8 @@ * Source returning an empty row with Drupal specific config dependencies. * * @MigrateSource( - * id = "md_empty" + * id = "md_empty", + * source_module = "system", * ) */ class EmptySource extends BaseEmptySource implements ContainerFactoryPluginInterface, DependentPluginInterface { diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/Variable.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/Variable.php index e127dfb..a585510 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/Variable.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/Variable.php @@ -13,7 +13,8 @@ * example for any normal source class returning multiple rows. * * @MigrateSource( - * id = "variable" + * id = "variable", + * source_module = "system", * ) */ class Variable extends DrupalSqlBase { diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/VariableMultiRow.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/VariableMultiRow.php index 61da5eb..47d575c 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/VariableMultiRow.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/VariableMultiRow.php @@ -11,7 +11,8 @@ * variable. * * @MigrateSource( - * id = "variable_multirow" + * id = "variable_multirow", + * source_module = "system", * ) */ class VariableMultiRow extends DrupalSqlBase { diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/VariableTranslation.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/VariableTranslation.php index f2ca8b7..ce57716 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/VariableTranslation.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/VariableTranslation.php @@ -11,7 +11,8 @@ * Drupal i18n_variable source from database. * * @MigrateSource( - * id = "variable_translation" + * id = "variable_translation", + * source_module = "system", * ) */ class VariableTranslation extends DrupalSqlBase { diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php index dd8d0f0..f1c338a 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php @@ -8,7 +8,8 @@ * Drupal i18n_variable source from database. * * @MigrateSource( - * id = "i18n_variable" + * id = "i18n_variable", + * source_module = "system", * ) * * @deprecated in Drupal 8.4.x and will be removed in Drupal 9.0.x. Use diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/source/d8/Config.php b/core/modules/migrate_drupal/src/Plugin/migrate/source/d8/Config.php index 8d83387..c396b76 100644 --- a/core/modules/migrate_drupal/src/Plugin/migrate/source/d8/Config.php +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d8/Config.php @@ -9,7 +9,8 @@ * Drupal config source from database. * * @MigrateSource( - * id = "d8_config" + * id = "d8_config", + * source_module = "system", * ) */ class Config extends DrupalSqlBase { diff --git a/core/modules/migrate_drupal_ui/tests/modules/migration_provider_test/migrations/migration_provider_test.yml b/core/modules/migrate_drupal_ui/tests/modules/migration_provider_test/migrations/migration_provider_test.yml deleted file mode 100644 index d3afc66..0000000 --- a/core/modules/migrate_drupal_ui/tests/modules/migration_provider_test/migrations/migration_provider_test.yml +++ /dev/null @@ -1,16 +0,0 @@ -id: migration_provider_test -label: Missing source and destination provider -migration_tags: - - Drupal 6 - - Drupal 7 -source: - plugin: variable - variables: - - site_offline_message -# Do not add a provider for the test. -process: - message: site_offline_message -destination: - plugin: config -# An empty config_name will not have a destination provider. - config_name: \ No newline at end of file diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php index 98c05aa..01297a9 100644 --- a/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php @@ -126,8 +126,8 @@ public function testMigrateUpgrade() { $session->responseContains('Upgrade a site by importing its files and the data from its database into a clean and empty new install of Drupal 8.'); $this->drupalPostForm(NULL, [], t('Continue')); - $this->assertText('Provide credentials for the database of the Drupal site you want to upgrade.'); - $this->assertFieldByName('mysql[host]'); + $session->pageTextContains('Provide credentials for the database of the Drupal site you want to upgrade.'); + $session->fieldExists('mysql[host]'); $driver = $connection_options['driver']; $connection_options['prefix'] = $connection_options['prefix']['default']; @@ -157,7 +157,23 @@ public function testMigrateUpgrade() { // Ensure submitting the form with invalid database credentials gives us a // nice warning. $this->drupalPostForm(NULL, [$driver . '[database]' => 'wrong'] + $edits, t('Review upgrade')); - $this->assertText('Resolve the issue below to continue the upgrade.'); + $session->pageTextContains('Resolve the issue below to continue the upgrade.'); + + $this->drupalPostForm(NULL, $edits, t('Review upgrade')); + // Ensure we get errors about missing modules. + $session->pageTextContains(t('Resolve the issue below to continue the upgrade')); + $session->pageTextContains(t('The no_source_module plugin must define the source_module property.')); + + // Uninstall the module causing the missing module error messages. + $this->container->get('module_installer')->uninstall(['migration_provider_test'], TRUE); + + // Restart the upgrade process. + $this->drupalGet('/upgrade'); + $session->responseContains('Upgrade a site by importing its files and the data from its database into a clean and empty new install of Drupal 8.'); + + $this->drupalPostForm(NULL, [], t('Continue')); + $session->pageTextContains('Provide credentials for the database of the Drupal site you want to upgrade.'); + $session->fieldExists('mysql[host]'); $this->drupalPostForm(NULL, $edits, t('Review upgrade')); $session->pageTextContains('WARNING: Content may be overwritten on your new site.'); @@ -174,33 +190,11 @@ public function testMigrateUpgrade() { $session->pageTextContains('content items'); $session->pageTextContains('There is translated content of these types:'); $this->drupalPostForm(NULL, [], t('I acknowledge I may lose data. Continue anyway.')); - $this->assertResponse(200); - $this->assertText('Upgrade analysis report'); - // Ensure we get errors about missing modules. - $session->pageTextContains(t('Source module not found for migration_provider_no_annotation.')); - $session->pageTextContains(t('Source module not found for migration_provider_test.')); - $session->pageTextContains(t('Destination module not found for migration_provider_test')); - - // Uninstall the module causing the missing module error messages. - $this->container->get('module_installer')->uninstall(['migration_provider_test'], TRUE); - - // Restart the upgrade process. - $this->drupalGet('/upgrade'); - $session->responseContains('Upgrade a site by importing its files and the data from its database into a clean and empty new install of Drupal 8.'); - - $this->drupalPostForm(NULL, [], t('Continue')); - $session->pageTextContains('Provide credentials for the database of the Drupal site you want to upgrade.'); - $session->fieldExists('mysql[host]'); - - $this->drupalPostForm(NULL, $edits, t('Review upgrade')); - $session->pageTextContains('WARNING: Content may be overwritten on your new site.'); - $this->drupalPostForm(NULL, [], t('I acknowledge I may lose data. Continue anyway.')); $session->statusCodeEquals(200); $session->pageTextContains('Upgrade analysis report'); // Ensure there are no errors about the missing modules from the test module. $session->pageTextNotContains(t('Source module not found for migration_provider_no_annotation.')); $session->pageTextNotContains(t('Source module not found for migration_provider_test.')); - $session->pageTextNotContains(t('Destination module not found for migration_provider_test')); // Ensure there are no errors about any other missing migration providers. $session->pageTextNotContains(t('module not found')); diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7Test.php b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7Test.php index 21210dd..b927c2c 100644 --- a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7Test.php +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7Test.php @@ -97,7 +97,6 @@ protected function getAvailablePaths() { 'email', 'entityreference', 'field', - 'field_sql_storage', 'file', 'filefield', 'filter', diff --git a/core/modules/node/tests/src/Kernel/Migrate/d6/MigrateNodeDeriverTest.php b/core/modules/node/tests/src/Kernel/Migrate/d6/MigrateNodeDeriverTest.php index abce689..855209e 100644 --- a/core/modules/node/tests/src/Kernel/Migrate/d6/MigrateNodeDeriverTest.php +++ b/core/modules/node/tests/src/Kernel/Migrate/d6/MigrateNodeDeriverTest.php @@ -42,9 +42,7 @@ public function testTranslations() { // With content_translation, there should be translation migrations for // each content type. $this->enableModules(['language', 'content_translation']); - $migrations = $this->pluginManager->createInstances('d6_node_translation'); - $this->assertArrayHasKey('d6_node_translation:story', $migrations, - "Node translation migrations exist after content_translation installed"); + $this->assertTrue($this->container->get('plugin.manager.migration')->hasDefinition('d6_node_translation:story'), "Node translation migrations exist after content_translation installed"); } } diff --git a/core/modules/node/tests/src/Kernel/Migrate/d7/MigrateNodeDeriverTest.php b/core/modules/node/tests/src/Kernel/Migrate/d7/MigrateNodeDeriverTest.php index 2b7169a..4371268 100644 --- a/core/modules/node/tests/src/Kernel/Migrate/d7/MigrateNodeDeriverTest.php +++ b/core/modules/node/tests/src/Kernel/Migrate/d7/MigrateNodeDeriverTest.php @@ -12,37 +12,16 @@ class MigrateNodeDeriverTest extends MigrateDrupal7TestBase { /** - * The migration plugin manager. - * - * @var \Drupal\migrate\Plugin\MigrationPluginManagerInterface - */ - protected $pluginManager; - - /** - * The module handler service. - * - * @var \Drupal\Core\Extension\ModuleHandlerInterface - */ - protected $moduleHandler; - - /** * {@inheritdoc} */ - public function setUp() { - parent::setUp(); - $this->pluginManager = $this->container->get('plugin.manager.migration'); - $this->moduleHandler = $this->container->get('module_handler'); - } + public static $modules = ['node']; /** * Test node translation migrations with translation disabled. */ public function testNoTranslations() { - // Enabling node module for this test. - $this->enableModules(['node']); // Without content_translation, there should be no translation migrations. - $migrations = $this->pluginManager->createInstances('d7_node_translation'); - $this->assertTrue($this->moduleHandler->moduleExists('node')); + $migrations = $this->container->get('plugin.manager.migration')->createInstances('d7_node_translation'); $this->assertEmpty($migrations); } @@ -52,10 +31,8 @@ public function testNoTranslations() { public function testTranslations() { // With content_translation, there should be translation migrations for // each content type. - $this->enableModules(['language', 'content_translation', 'node', 'filter']); - $migrations = $this->pluginManager->createInstances('d7_node_translation'); - $this->assertArrayHasKey('d7_node_translation:article', $migrations, - "Node translation migrations exist after content_translation installed"); + $this->enableModules(['language', 'content_translation', 'filter']); + $this->assertTrue($this->container->get('plugin.manager.migration')->hasDefinition('d7_node_translation:article'), "Node translation migrations exist after content_translation installed"); } }