From 244ccb28632ed87155d8b22a147fb362a058ffaa Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 4 Aug 2020 10:27:12 +0100 Subject: [PATCH] Issue #2845485 by quietone, jofitz, phenaproxima, masipila, vacho, dww, heddn, alexpott: Refactor and document the MenuLinkParent process plugin --- .../Plugin/migrate/process/MenuLinkParent.php | 71 +++++-- .../src/Unit/process/MenuLinkParentTest.php | 174 +++++++++++++++--- 2 files changed, 208 insertions(+), 37 deletions(-) diff --git a/core/modules/migrate/src/Plugin/migrate/process/MenuLinkParent.php b/core/modules/migrate/src/Plugin/migrate/process/MenuLinkParent.php index b13d222d9a5c..cf4f2a163569 100644 --- a/core/modules/migrate/src/Plugin/migrate/process/MenuLinkParent.php +++ b/core/modules/migrate/src/Plugin/migrate/process/MenuLinkParent.php @@ -7,16 +7,50 @@ use Drupal\Core\Menu\MenuLinkManagerInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Url; -use Drupal\migrate\MigrateLookupInterface; -use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\MigrateExecutableInterface; +use Drupal\migrate\MigrateLookupInterface; use Drupal\migrate\MigrateSkipRowException; +use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\ProcessPluginBase; use Drupal\migrate\Row; use Symfony\Component\DependencyInjection\ContainerInterface; /** - * This plugin figures out menu link parent plugin IDs. + * Determines the parent of a menu link. + * + * Menu link item belongs to a menu such as 'Navigation' or 'Administration'. + * Menu link item also has a parent item unless it is the root element of the + * menu. + * + * This process plugin determines the parent item of a menu link. If the parent + * item can't be determined by ID, we try to determine it by a combination of + * menu name and parent link path. + * + * The source is an array of three values: + * - parent_id: The numeric ID of the parent menu link, or 0 if the link is the + * root element of the menu. + * - menu_name: The name of the menu the menu link item belongs to. + * - parent_link_path: The Drupal path or external URL the parent of this menu + * link points to. + * + * Example: + * + * @code + * process: + * parent: + * plugin: menu_link_parent + * source: + * - 20 + * - management + * - admin/structure + * @endcode + * In this example we first try to look up a menu link item that had an ID '20' + * in the source. If a parent menu item can't be found with this ID, we try to + * determine the parent by a combination of 'management' menu name and + * 'admin/structure' menu link path. + * + * @see https://www.drupal.org/docs/8/api/menu-api + * @see \Drupal\migrate\Plugin\MigrateProcessInterface * * @MigrateProcessPlugin( * id = "menu_link_parent" @@ -25,6 +59,8 @@ class MenuLinkParent extends ProcessPluginBase implements ContainerFactoryPluginInterface { /** + * The menu link plugin manager. + * * @var \Drupal\Core\Menu\MenuLinkManagerInterface */ protected $menuLinkManager; @@ -44,6 +80,8 @@ class MenuLinkParent extends ProcessPluginBase implements ContainerFactoryPlugin protected $migrateLookup; /** + * The menu link entity storage handler. + * * @var \Drupal\Core\Entity\EntityStorageInterface */ protected $menuLinkStorage; @@ -80,7 +118,6 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition * {@inheritdoc} */ public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration = NULL) { - $migration_configuration['migration'][] = $migration->id(); return new static( $configuration, $plugin_id, @@ -99,10 +136,12 @@ public static function create(ContainerInterface $container, array $configuratio */ public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) { $parent_id = array_shift($value); + + // Handle root elements of a menu. if (!$parent_id) { - // Top level item. return ''; } + $lookup_result = $this->migrateLookup->lookup($this->migration->id(), [$parent_id]); if ($lookup_result) { $already_migrated_id = $lookup_result[0]['id']; @@ -112,25 +151,31 @@ public function transform($value, MigrateExecutableInterface $migrate_executable return $link->getPluginId(); } + // Parent could not be determined by ID, so we try to determine by the + // combination of the menu name and parent link path. if (isset($value[1])) { - list($menu_name, $parent_link_path) = $value; + [$menu_name, $parent_link_path] = $value; - $links = []; + // If the parent link path is external, URL will be useless because the + // link will definitely not correspond to a Drupal route. if (UrlHelper::isExternal($parent_link_path)) { - $links = $this->menuLinkStorage->loadByProperties(['link__uri' => $parent_link_path]); + $links = $this->menuLinkStorage->loadByProperties([ + 'menu_name' => $menu_name, + 'link.uri' => $parent_link_path, + ]); } else { - $url = Url::fromUserInput("/$parent_link_path"); + $url = Url::fromUserInput('/' . ltrim($parent_link_path, '/')); if ($url->isRouted()) { $links = $this->menuLinkManager->loadLinksByRoute($url->getRouteName(), $url->getRouteParameters(), $menu_name); } } - if (count($links) == 1) { - /** @var \Drupal\Core\Menu\MenuLinkInterface $link */ - $link = reset($links); - return $link->getPluginId(); + if (!empty($links)) { + return reset($links)->getPluginId(); } } + + // Parent could not be determined. throw new MigrateSkipRowException(sprintf("No parent link found for plid '%d' in menu '%s'.", $parent_id, $value[0])); } diff --git a/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php b/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php index 0e0a84693d0b..416d24c11281 100644 --- a/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php +++ b/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php @@ -2,15 +2,18 @@ namespace Drupal\Tests\migrate\Unit\process; -use Drupal\Component\Plugin\PluginInspectionInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\EntityStorageInterface; +use Drupal\Core\Menu\MenuLinkDefault; use Drupal\Core\Menu\MenuLinkManagerInterface; -use Drupal\menu_link_content\MenuLinkContentInterface; +use Drupal\Core\Menu\StaticMenuLinkOverridesInterface; +use Drupal\Core\Path\PathValidatorInterface; +use Drupal\Core\Url; +use Drupal\menu_link_content\Entity\MenuLinkContent; use Drupal\migrate\MigrateLookupInterface; use Drupal\migrate\MigrateSkipRowException; -use Drupal\migrate\Plugin\migrate\process\MenuLinkParent; use Drupal\migrate\Plugin\MigrationInterface; +use Drupal\migrate\Plugin\migrate\process\MenuLinkParent; /** * Tests the menu link parent process plugin. @@ -35,19 +38,26 @@ class MenuLinkParentTest extends MigrateProcessTestCase { protected $migrateLookup; /** - * A MigrationInterface prophecy. + * A Path validator prophecy. * * @var \Prophecy\Prophecy\ObjectProphecy */ - protected $menuLinkManager; + protected $pathValidator; /** - * A MigrationInterface prophecy. + * The menu link entity storage handler. * * @var \Prophecy\Prophecy\ObjectProphecy */ protected $menuLinkStorage; + /** + * The menu link plugin manager. + * + * @var \Prophecy\Prophecy\ObjectProphecy + */ + protected $menuLinkManager; + /** * {@inheritdoc} */ @@ -55,44 +65,160 @@ protected function setUp(): void { parent::setUp(); $this->migration = $this->prophesize(MigrationInterface::class); $this->migrateLookup = $this->prophesize(MigrateLookupInterface::class); - $this->migrateLookup->lookup(NULL, [1])->willReturn([]); $this->menuLinkManager = $this->prophesize(MenuLinkManagerInterface::class); $this->menuLinkStorage = $this->prophesize(EntityStorageInterface::class); $container = new ContainerBuilder(); $container->set('migrate.lookup', $this->migrateLookup->reveal()); - \Drupal::setContainer($container); + $this->pathValidator = $this->prophesize(PathValidatorInterface::class); + $container->set('path.validator', $this->pathValidator->reveal()); + \Drupal::setContainer($container); } /** - * @covers ::transform + * Tests that an exception is thrown when the parent menu link is not found. + * + * @param string[] $source_value + * The source value(s) for the migration process plugin. + * + * @throws \Drupal\Component\Plugin\Exception\PluginException + * @throws \Drupal\migrate\MigrateException + * @throws \Drupal\migrate\MigrateSkipRowException + * + * @dataProvider providerTransformException */ - public function testTransformException() { + public function testTransformException(array $source_value) { + [$parent_id, $menu_name, $parent_link_path] = $source_value; + $this->migrateLookup->lookup(NULL, [1])->willReturn([]); $plugin = new MenuLinkParent([], 'map', [], $this->migrateLookup->reveal(), $this->menuLinkManager->reveal(), $this->menuLinkStorage->reveal(), $this->migration->reveal()); $this->expectException(MigrateSkipRowException::class); - $this->expectExceptionMessage("No parent link found for plid '1' in menu 'admin'."); - $plugin->transform([1, 'admin', NULL], $this->migrateExecutable, $this->row, 'destination_property'); + $this->expectExceptionMessage("No parent link found for plid '$parent_id' in menu '$menu_name'."); + $plugin->transform($source_value, $this->migrateExecutable, $this->row, 'destination'); } /** - * Tests the plugin when the parent is an external link. + * Provides data for testTransformException(). + */ + public function providerTransformException() { + // The parent ID does not for the following tests. + return [ + 'parent link external and could not be loaded' => [ + 'source_value' => [1, 'admin', 'http://drupal.org'], + ], + 'parent link path/menu name not passed' => [ + 'source_value' => [1, NULL, 'http://drupal.org'], + ], + 'parent link is an internal URI that does not exist' => [ + 'source_value' => [1, NULL, 'admin/structure'], + ], + ]; + } + + /** + * Tests the menu link content process plugin. + * + * @param string[] $source_value + * The source value(s) for the migration process plugin. + * @param string $lookup_result + * The ID value to be returned from migration_lookup. + * @param string $plugin_id + * The menu link plugin ID. + * @param string $route_name + * A route to create. + * @param string $expected_result + * The expected value(s) of the migration process plugin. * - * @covers ::transform + * @throws \Drupal\Component\Plugin\Exception\PluginException + * @throws \Drupal\migrate\MigrateException + * @throws \Drupal\migrate\MigrateSkipRowException + * + * @dataProvider providerMenuLinkParent + */ + public function testMenuLinkParent(array $source_value, $lookup_result, $plugin_id, $route_name, $expected_result) { + [$parent_id, $menu_name, $parent_link_path] = $source_value; + $this->migrateLookup->lookup(NULL, [$parent_id]) + ->willReturn([['id' => $lookup_result]]); + if ($route_name) { + $plugin_definition = ['menu_name' => $menu_name]; + $static_override = $this->prophesize(StaticMenuLinkOverridesInterface::class); + $static_override = $static_override->reveal(); + $menu_link = new MenuLinkDefault([], $plugin_id, $plugin_definition, $static_override); + $this->menuLinkManager->loadLinksByRoute($route_name, [], 'admin') + ->willReturn([$plugin_id => $menu_link]); + + $url = new Url($route_name, [], []); + $this->pathValidator->getUrlIfValidWithoutAccessCheck($parent_link_path) + ->willReturn($url); + } + $result = $this->doTransform($source_value, $plugin_id); + $this->assertSame($expected_result, $result); + } + + /** + * Provides data for testMenuLinkParent(). */ - public function testTransformExternal() { - $menu_link_content = $this->prophesize(MenuLinkContentInterface::class); - $menu_link_content->getPluginId()->willReturn('menu_link_content:fe151460-dfa2-4133-8864-c1746f28ab27'); + public function providerMenuLinkParent() { + return [ + 'menu link is route item' => [ + 'source_value' => [0, NULL, NULL], + 'lookup_result' => NULL, + 'plugin_id' => NULL, + 'route_name' => NULL, + 'expected_result' => '', + ], + 'parent id exists' => [ + 'source_value' => [1, NULL, NULL], + 'lookup_result' => 1, + 'plugin_id' => 'menu_link_content:abc', + 'route_name' => NULL, + 'expected_result' => 'menu_link_content:abc', + ], + 'no parent id internal route' => [ + 'source_value' => [20, 'admin', 'admin/content'], + 'lookup_result' => NULL, + 'plugin_id' => 'system.admin_structure', + 'route_name' => 'system.admin_content', + 'expected_result' => 'system.admin_structure', + ], + 'external' => [ + 'source_value' => [9054, 'admin', 'http://example.com'], + 'lookup_result' => 9054, + 'plugin_id' => 'menu_link_content:fe151460-dfa2-4133-8864-c1746f28ab27', + 'route_name' => NULL, + 'expected_result' => 'menu_link_content:fe151460-dfa2-4133-8864-c1746f28ab27', + ], + ]; + } + + /** + * Helper to finish setup and run the test. + * + * @param string[] $source_value + * The source value(s) for the migration process plugin. + * @param string $plugin_id + * The menu link plugin ID. + * + * @return string + * The transformed menu link. + * + * @throws \Drupal\migrate\MigrateSkipRowException + */ + public function doTransform(array $source_value, $plugin_id) { + [$parent_id, $menu_name, $parent_link_path] = $source_value; + + $menu_link_content = $this->prophesize(MenuLinkContent::class); + $menu_link_content->getPluginId()->willReturn($plugin_id); + + $this->menuLinkStorage->load($parent_id)->willReturn($menu_link_content); $this->menuLinkStorage->loadByProperties([ - 'link__uri' => 'http://example.com', + 'menu_name' => $menu_name, + 'link.uri' => $parent_link_path, ])->willReturn([ - 9054 => $menu_link_content, + $parent_id => $menu_link_content, ]); - $plugin = $this->prophesize(PluginInspectionInterface::class); - $this->menuLinkManager->createInstance('menu_link_content:fe151460-dfa2-4133-8864-c1746f28ab27')->willReturn($plugin->reveal()); - $plugin = new MenuLinkParent([], 'map', [], $this->migrateLookup->reveal(), $this->menuLinkManager->reveal(), $this->menuLinkStorage->reveal(), $this->migration->reveal()); - $result = $plugin->transform([1, 'admin', 'http://example.com'], $this->migrateExecutable, $this->row, 'destination_property'); - $this->assertEquals('menu_link_content:fe151460-dfa2-4133-8864-c1746f28ab27', $result); + $plugin = new MenuLinkParent([], 'menu_link', [], $this->migrateLookup->reveal(), $this->menuLinkManager->reveal(), $this->menuLinkStorage->reveal(), $this->migration->reveal()); + return $plugin->transform($source_value, $this->migrateExecutable, $this->row, 'destination'); } } -- GitLab