Issue #2845485 by quietone, jofitz, phenaproxima, masipila, vacho, dww, heddn,...

Issue #2845485 by quietone, jofitz, phenaproxima, masipila, vacho, dww, heddn, alexpott: Refactor and document the MenuLinkParent process plugin

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]));
   }
 

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');
   }
 
 }