Issue #2821189 by tim.plunkett, tstoeckler: Allow object-based plugin...

Issue #2821189 by tim.plunkett, tstoeckler: Allow object-based plugin definitions to be processed in DerivativeDiscoveryDecorator

core/lib/Drupal/Core/Layout/DerivablePluginDefinitionInterface.php → core/lib/Drupal/Component/Plugin/Definition/DerivablePluginDefinitionInterface.php

 <?php
 
-namespace Drupal\Core\Layout;
-
-use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
+namespace Drupal\Component\Plugin\Definition;
 
 /**
  * Provides an interface for a derivable plugin definition.
  *
  * @see \Drupal\Component\Plugin\Derivative\DeriverInterface
- * @see \Drupal\Core\Layout\ObjectDefinitionContainerDerivativeDiscoveryDecorator
- *
- * @internal
- *   The layout system is currently experimental and should only be leveraged by
- *   experimental modules and development releases of contributed modules.
- *   See https://www.drupal.org/core/experimental for more information.
- *
- * @todo Move into \Drupal\Component\Plugin\Definition in
- *   https://www.drupal.org/node/2821189.
  */
 interface DerivablePluginDefinitionInterface extends PluginDefinitionInterface {
 

core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Component\Plugin\Discovery;
 
+use Drupal\Component\Plugin\Definition\DerivablePluginDefinitionInterface;
 use Drupal\Component\Plugin\Exception\InvalidDeriverException;
 
 /**
@@ -203,12 +204,21 @@ protected function getDeriver($base_plugin_id, $base_definition) {
    */
   protected function getDeriverClass($base_definition) {
     $class = NULL;
-    if ((is_array($base_definition) || ($base_definition = (array) $base_definition)) && (isset($base_definition['deriver']) && $class = $base_definition['deriver'])) {
+    $id = NULL;
+    if ($base_definition instanceof DerivablePluginDefinitionInterface) {
+      $class = $base_definition->getDeriver();
+      $id = $base_definition->id();
+    }
+    if ((is_array($base_definition) || ($base_definition = (array) $base_definition)) && (isset($base_definition['deriver']))) {
+      $class = $base_definition['deriver'];
+      $id = $base_definition['id'];
+    }
+    if ($class) {
       if (!class_exists($class)) {
-        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" does not exist.', $base_definition['id'], $class));
+        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" does not exist.', $id, $class));
       }
       if (!is_subclass_of($class, '\Drupal\Component\Plugin\Derivative\DeriverInterface')) {
-        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" must implement \Drupal\Component\Plugin\Derivative\DeriverInterface.', $base_definition['id'], $class));
+        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" must implement \Drupal\Component\Plugin\Derivative\DeriverInterface.', $id, $class));
       }
     }
     return $class;

core/lib/Drupal/Core/Layout/LayoutDefinition.php

@@ -4,6 +4,7 @@
 
 use Drupal\Component\Plugin\Definition\DependentPluginDefinitionInterface;
 use Drupal\Component\Plugin\Definition\DependentPluginDefinitionTrait;
+use Drupal\Component\Plugin\Definition\DerivablePluginDefinitionInterface;
 use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
 use Drupal\Component\Plugin\Definition\PluginDefinition;
 

core/lib/Drupal/Core/Layout/LayoutPluginManager.php

@@ -3,6 +3,7 @@
 namespace Drupal\Core\Layout;
 
 use Drupal\Component\Annotation\Plugin\Discovery\AnnotationBridgeDecorator;
+use Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator;
 use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -65,7 +66,7 @@ protected function getDiscovery() {
       $discovery = new AnnotatedClassDiscovery($this->subdir, $this->namespaces, $this->pluginDefinitionAnnotationName, $this->additionalAnnotationNamespaces);
       $discovery = new YamlDiscoveryDecorator($discovery, 'layouts', $this->moduleHandler->getModuleDirectories() + $this->themeHandler->getThemeDirectories());
       $discovery = new AnnotationBridgeDecorator($discovery, $this->pluginDefinitionAnnotationName);
-      $discovery = new ObjectDefinitionContainerDerivativeDiscoveryDecorator($discovery);
+      $discovery = new DerivativeDiscoveryDecorator($discovery);
       $this->discovery = $discovery;
     }
     return $this->discovery;

core/lib/Drupal/Core/Layout/ObjectDefinitionContainerDerivativeDiscoveryDecorator.php

-<?php
-
-namespace Drupal\Core\Layout;
-
-use Drupal\Component\Plugin\Exception\InvalidDeriverException;
-use Drupal\Core\Plugin\Discovery\ContainerDerivativeDiscoveryDecorator;
-
-/**
- * Allows object-based definitions to use derivatives.
- *
- * @internal
- *   The layout system is currently experimental and should only be leveraged by
- *   experimental modules and development releases of contributed modules.
- *   See https://www.drupal.org/core/experimental for more information.
- *
- * @todo In https://www.drupal.org/node/2821189 merge into
- *    \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator.
- */
-class ObjectDefinitionContainerDerivativeDiscoveryDecorator extends ContainerDerivativeDiscoveryDecorator {
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function getDeriverClass($base_definition) {
-    $class = NULL;
-    if ($base_definition instanceof DerivablePluginDefinitionInterface && $class = $base_definition->getDeriver()) {
-      if (!class_exists($class)) {
-        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" does not exist.', $base_definition['id'], $class));
-      }
-      if (!is_subclass_of($class, '\Drupal\Component\Plugin\Derivative\DeriverInterface')) {
-        throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" must implement \Drupal\Component\Plugin\Derivative\DeriverInterface.', $base_definition['id'], $class));
-      }
-    }
-    return $class;
-  }
-
-}

core/modules/system/tests/modules/plugin_test/src/Plugin/MockBlockManager.php

@@ -35,6 +35,7 @@ public function __construct() {
 
     // A simple plugin: the user login block.
     $this->discovery->setDefinition('user_login', array(
+      'id' => 'user_login',
       'label' => t('User login'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserLoginBlock',
     ));
@@ -46,11 +47,13 @@ public function __construct() {
     // MockMenuBlockDeriver class ensures that only derivatives, and not the
     // base plugin, are available to the system.
     $this->discovery->setDefinition('menu', array(
+      'id' => 'menu',
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
       'deriver' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlockDeriver',
     ));
     // A plugin defining itself as a derivative.
     $this->discovery->setDefinition('menu:foo', array(
+      'id' => 'menu',
       'label' => t('Base label'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
     ));
@@ -62,6 +65,7 @@ public function __construct() {
     // MockLayoutBlockDeriver class ensures that both the base plugin and the
     // derivatives are available to the system.
     $this->discovery->setDefinition('layout', array(
+      'id' => 'layout',
       'label' => t('Layout'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
       'deriver' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlockDeriver',
@@ -70,6 +74,7 @@ public function __construct() {
     // A block plugin that requires context to function. This block requires a
     // user object in order to return the user name from the getTitle() method.
     $this->discovery->setDefinition('user_name', array(
+      'id' => 'user_name',
       'label' => t('User name'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserNameBlock',
       'context' => array(
@@ -79,6 +84,7 @@ public function __construct() {
 
     // An optional context version of the previous block plugin.
     $this->discovery->setDefinition('user_name_optional', array(
+      'id' => 'user_name_optional',
       'label' => t('User name optional'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserNameBlock',
       'context' => array(
@@ -88,12 +94,14 @@ public function __construct() {
 
     // A block plugin that requires a typed data string context to function.
     $this->discovery->setDefinition('string_context', array(
+      'id' => 'string_context',
       'label' => t('String typed data'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\TypedDataStringBlock',
     ));
 
     // A complex context plugin that requires both a user and node for context.
     $this->discovery->setDefinition('complex_context', array(
+      'id' => 'complex_context',
       'label' => t('Complex context'),
       'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockComplexContextBlock',
       'context' => array(

core/tests/Drupal/KernelTests/Core/Plugin/PluginTestBase.php

@@ -58,31 +58,38 @@ protected function setUp() {
     );
     $this->mockBlockExpectedDefinitions = array(
       'user_login' => array(
+        'id' => 'user_login',
         'label' => 'User login',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserLoginBlock',
       ),
       'menu:main_menu' => array(
+        'id' => 'menu',
         'label' => 'Main menu',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
       ),
       'menu:navigation' => array(
+        'id' => 'menu',
         'label' => 'Navigation',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
       ),
       'menu:foo' => array(
+        'id' => 'menu',
         'label' => 'Base label',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockMenuBlock',
         'setting' => 'default',
       ),
       'layout' => array(
+        'id' => 'layout',
         'label' => 'Layout',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
       ),
       'layout:foo' => array(
+        'id' => 'layout',
         'label' => 'Layout Foo',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockLayoutBlock',
       ),
       'user_name' => array(
+        'id' => 'user_name',
         'label' => 'User name',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserNameBlock',
         'context' => array(
@@ -90,6 +97,7 @@ protected function setUp() {
         ),
       ),
       'user_name_optional' => array(
+        'id' => 'user_name_optional',
         'label' => 'User name optional',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockUserNameBlock',
         'context' => array(
@@ -97,10 +105,12 @@ protected function setUp() {
         ),
       ),
       'string_context' => array(
+        'id' => 'string_context',
         'label' => 'String typed data',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\TypedDataStringBlock',
       ),
       'complex_context' => array(
+        'id' => 'complex_context',
         'label' => 'Complex context',
         'class' => 'Drupal\plugin_test\Plugin\plugin_test\mock_block\MockComplexContextBlock',
         'context' => array(

core/tests/Drupal/Tests/Core/Plugin/Discovery/DerivativeDiscoveryDecoratorTest.php

@@ -2,12 +2,16 @@
 
 namespace Drupal\Tests\Core\Plugin\Discovery;
 
+use Drupal\Component\Plugin\Definition\DerivablePluginDefinitionInterface;
 use Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator;
+use Drupal\Component\Plugin\Exception\InvalidDeriverException;
 use Drupal\Tests\UnitTestCase;
 
 /**
  * Unit tests for the derivative discovery decorator.
  *
+ * @coversDefaultClass \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator
+ *
  * @group Plugin
  */
 class DerivativeDiscoveryDecoratorTest extends UnitTestCase {
@@ -82,6 +86,50 @@ public function testGetDerivativeFetcherWithAnnotationObjects() {
     $this->assertEquals('\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscoveryWithObject', $definitions['non_container_aware_discovery:test_discovery_1']->deriver);
   }
 
+  /**
+   * Tests getDeriverClass with classed objects instead of arrays.
+   *
+   * @covers ::getDeriverClass
+   */
+  public function testGetDeriverClassWithClassedDefinitions() {
+    $definitions = array();
+    $definition = $this->prophesize(DerivablePluginDefinitionInterface::class);
+    $definition->id()->willReturn('non_container_aware_discovery');
+    $definition->getDeriver()->willReturn(TestDerivativeDiscoveryWithObject::class);
+    $definitions['non_container_aware_discovery'] = $definition->reveal();
+
+    $this->discoveryMain->expects($this->any())
+      ->method('getDefinitions')
+      ->will($this->returnValue($definitions));
+
+    $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain);
+    $definitions = $discovery->getDefinitions();
+
+    // Ensure that both test derivatives got added.
+    $this->assertContainsOnlyInstancesOf(DerivablePluginDefinitionInterface::class, $definitions);
+    $this->assertEquals(['non_container_aware_discovery:test_discovery_0', 'non_container_aware_discovery:test_discovery_1'], array_keys($definitions));
+  }
+
+  /**
+   * @covers ::getDeriverClass
+   */
+  public function testGetDeriverClassWithInvalidClassedDefinitions() {
+    $definition = $this->prophesize(DerivablePluginDefinitionInterface::class);
+    $definition->id()->willReturn('non_existent_discovery');
+    $definition->getDeriver()->willReturn('\Drupal\system\Tests\Plugin\NonExistentDeriver');
+
+    $definitions['non_existent_discovery'] = $definition->reveal();
+
+    $this->discoveryMain->expects($this->any())
+      ->method('getDefinitions')
+      ->willReturn($definitions);
+
+    $discovery = new DerivativeDiscoveryDecorator($this->discoveryMain);
+
+    $this->setExpectedException(InvalidDeriverException::class, 'Plugin (non_existent_discovery) deriver "\Drupal\system\Tests\Plugin\NonExistentDeriver" does not exist.');
+    $discovery->getDefinitions();
+  }
+
   /**
    * Tests the getDerivativeFetcher method with a non-existent class.
    *