Issue #2485513 by Xano, tstoeckler, lauriii: DefaultFactory cannot deal with...

Issue #2485513 by Xano, tstoeckler, lauriii: DefaultFactory cannot deal with objects as plugin definitions

core/lib/Drupal/Component/Plugin/Definition/PluginDefinitionInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Component\Plugin\PluginDefinitionInterface.
+ */
+
+namespace Drupal\Component\Plugin\Definition;
+
+/**
+ * Defines a plugin definition.
+ *
+ * Object-based plugin definitions MUST implement this interface.
+ *
+ * @ingroup Plugin
+ */
+interface PluginDefinitionInterface {
+
+  /**
+   * Sets the class.
+   *
+   * @param string $class
+   *   A fully qualified class name.
+   *
+   * @return static
+   *
+   * @throws \InvalidArgumentException
+   *   If the class is invalid.
+   */
+  public function setClass($class);
+
+  /**
+   * Gets the class.
+   *
+   * @return string
+   *   A fully qualified class name.
+   */
+  public function getClass();
+
+}

core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php

@@ -6,6 +6,7 @@
 
 namespace Drupal\Component\Plugin\Factory;
 
+use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
 use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
 use Drupal\Component\Plugin\Exception\PluginException;
 
@@ -63,7 +64,7 @@ public function createInstance($plugin_id, array $configuration = array()) {
    *
    * @param string $plugin_id
    *   The id of a plugin.
-   * @param mixed $plugin_definition
+   * @param \Drupal\Component\Plugin\Definition\PluginDefinitionInterface|mixed[] $plugin_definition
    *   The plugin definition associated with the plugin ID.
    * @param string $required_interface
    *   (optional) THe required plugin interface.
@@ -77,18 +78,32 @@ public function createInstance($plugin_id, array $configuration = array()) {
    *
    */
   public static function getPluginClass($plugin_id, $plugin_definition = NULL, $required_interface = NULL) {
-    if (empty($plugin_definition['class'])) {
-      throw new PluginException(sprintf('The plugin (%s) did not specify an instance class.', $plugin_id));
+    $missing_class_message = sprintf('The plugin (%s) did not specify an instance class.', $plugin_id);
+    if (is_array($plugin_definition)) {
+      if (empty($plugin_definition['class'])) {
+        throw new PluginException($missing_class_message);
+      }
+
+      $class = $plugin_definition['class'];
     }
+    elseif ($plugin_definition instanceof PluginDefinitionInterface) {
+      if (!$plugin_definition->getClass()) {
+        throw new PluginException($missing_class_message);
+      }
 
-    $class = $plugin_definition['class'];
+      $class = $plugin_definition->getClass();
+    }
+    else {
+      $plugin_definition_type = is_object($plugin_definition) ? get_class($plugin_definition) : gettype($plugin_definition);
+      throw new PluginException(sprintf('%s can only handle plugin definitions that are arrays or that implement %s, but %s given.', __CLASS__, PluginDefinitionInterface::class, $plugin_definition_type));
+    }
 
     if (!class_exists($class)) {
       throw new PluginException(sprintf('Plugin (%s) instance class "%s" does not exist.', $plugin_id, $class));
     }
 
-    if ($required_interface && !is_subclass_of($plugin_definition['class'], $required_interface)) {
-      throw new PluginException(sprintf('Plugin "%s" (%s) must implement interface %s.', $plugin_id, $plugin_definition['class'], $required_interface));
+    if ($required_interface && !is_subclass_of($class, $required_interface)) {
+      throw new PluginException(sprintf('Plugin "%s" (%s) must implement interface %s.', $plugin_id, $class, $required_interface));
     }
 
     return $class;

core/lib/Drupal/Core/Entity/EntityTypeInterface.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\Core\Entity;
 
+use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
+
 /**
  * Provides an interface for an entity type and its metadata.
  *
@@ -15,7 +17,7 @@
  * implemented to alter existing data and fill-in defaults. Module-specific
  * properties should be documented in the hook implementations defining them.
  */
-interface EntityTypeInterface {
+interface EntityTypeInterface extends PluginDefinitionInterface {
 
   /**
    * The maximum length of ID, in characters.
@@ -66,14 +68,6 @@ public function id();
    */
   public function getProvider();
 
-  /**
-   * Gets the name of the entity type class.
-   *
-   * @return string
-   *   The name of the entity type class.
-   */
-  public function getClass();
-
   /**
    * Gets the name of the original entity type class.
    *
@@ -170,16 +164,6 @@ public function isRenderCacheable();
    */
   public function isPersistentlyCacheable();
 
-  /**
-   * Sets the name of the entity type class.
-   *
-   * @param string $class
-   *   The name of the entity type class.
-   *
-   * @return $this
-   */
-  public function setClass($class);
-
   /**
    * Determines if there is a handler for a given type.
    *

core/tests/Drupal/Tests/Component/Plugin/DefaultFactoryTest.php

@@ -7,7 +7,11 @@
 
 namespace Drupal\Tests\Component\Plugin;
 
+use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
 use Drupal\Component\Plugin\Factory\DefaultFactory;
+use Drupal\plugin_test\Plugin\plugin_test\fruit\Cherry;
+use Drupal\plugin_test\Plugin\plugin_test\fruit\FruitInterface;
+use Drupal\plugin_test\Plugin\plugin_test\fruit\Kale;
 use Drupal\Tests\UnitTestCase;
 
 /**
@@ -17,41 +21,110 @@
 class DefaultFactoryTest extends UnitTestCase {
 
   /**
-   * Tests getPluginClass() with a valid plugin.
+   * Tests getPluginClass() with a valid array plugin definition.
+   *
+   * @covers ::getPluginClass
    */
-  public function testGetPluginClassWithValidPlugin() {
-    $plugin_class = 'Drupal\plugin_test\Plugin\plugin_test\fruit\Cherry';
+  public function testGetPluginClassWithValidArrayPluginDefinition() {
+    $plugin_class = Cherry::class;
     $class = DefaultFactory::getPluginClass('cherry', ['class' => $plugin_class]);
 
     $this->assertEquals($plugin_class, $class);
   }
 
+  /**
+   * Tests getPluginClass() with a valid object plugin definition.
+   *
+   * @covers ::getPluginClass
+   */
+  public function testGetPluginClassWithValidObjectPluginDefinition() {
+    $plugin_class = Cherry::class;
+    $plugin_definition = $this->getMock(PluginDefinitionInterface::class);
+    $plugin_definition->expects($this->atLeastOnce())
+      ->method('getClass')
+      ->willReturn($plugin_class);
+    $class = DefaultFactory::getPluginClass('cherry', $plugin_definition);
+
+    $this->assertEquals($plugin_class, $class);
+  }
+
   /**
    * Tests getPluginClass() with a missing class definition.
    *
+   * @covers ::getPluginClass
+   *
    * @expectedException \Drupal\Component\Plugin\Exception\PluginException
    * @expectedExceptionMessage The plugin (cherry) did not specify an instance class.
    */
-  public function testGetPluginClassWithMissingClass() {
+  public function testGetPluginClassWithMissingClassWithArrayPluginDefinition() {
     DefaultFactory::getPluginClass('cherry', []);
   }
 
+  /**
+   * Tests getPluginClass() with a missing class definition.
+   *
+   * @covers ::getPluginClass
+   *
+   * @expectedException \Drupal\Component\Plugin\Exception\PluginException
+   * @expectedExceptionMessage The plugin (cherry) did not specify an instance class.
+   */
+  public function testGetPluginClassWithMissingClassWithObjectPluginDefinition() {
+    $plugin_definition = $this->getMock(PluginDefinitionInterface::class);
+    DefaultFactory::getPluginClass('cherry', $plugin_definition);
+  }
+
   /**
    * Tests getPluginClass() with a not existing class definition.
    *
+   * @covers ::getPluginClass
+   *
    * @expectedException \Drupal\Component\Plugin\Exception\PluginException
    * @expectedExceptionMessage Plugin (kiwifruit) instance class "\Drupal\plugin_test\Plugin\plugin_test\fruit\Kiwifruit" does not exist.
    */
-  public function testGetPluginClassWithNotExistingClass() {
+  public function testGetPluginClassWithNotExistingClassWithArrayPluginDefinition() {
     DefaultFactory::getPluginClass('kiwifruit', ['class' => '\Drupal\plugin_test\Plugin\plugin_test\fruit\Kiwifruit']);
   }
 
+  /**
+   * Tests getPluginClass() with a not existing class definition.
+   *
+   * @covers ::getPluginClass
+   *
+   * @expectedException \Drupal\Component\Plugin\Exception\PluginException
+   */
+  public function testGetPluginClassWithNotExistingClassWithObjectPluginDefinition() {
+    $plugin_class = '\Drupal\plugin_test\Plugin\plugin_test\fruit\Kiwifruit';
+    $plugin_definition = $this->getMock(PluginDefinitionInterface::class);
+    $plugin_definition->expects($this->atLeastOnce())
+      ->method('getClass')
+      ->willReturn($plugin_class);
+    DefaultFactory::getPluginClass('kiwifruit', $plugin_definition);
+  }
+
+  /**
+   * Tests getPluginClass() with a required interface.
+   *
+   * @covers ::getPluginClass
+   */
+  public function testGetPluginClassWithInterfaceWithArrayPluginDefinition() {
+    $plugin_class = Cherry::class;
+    $class = DefaultFactory::getPluginClass('cherry', ['class' => $plugin_class], FruitInterface::class);
+
+    $this->assertEquals($plugin_class, $class);
+  }
+
   /**
    * Tests getPluginClass() with a required interface.
+   *
+   * @covers ::getPluginClass
    */
-  public function testGetPluginClassWithInterface() {
-    $plugin_class = 'Drupal\plugin_test\Plugin\plugin_test\fruit\Cherry';
-    $class = DefaultFactory::getPluginClass('cherry', ['class' => $plugin_class], '\Drupal\plugin_test\Plugin\plugin_test\fruit\FruitInterface');
+  public function testGetPluginClassWithInterfaceWithObjectPluginDefinition() {
+    $plugin_class = Cherry::class;
+    $plugin_definition = $this->getMock(PluginDefinitionInterface::class);
+    $plugin_definition->expects($this->atLeastOnce())
+      ->method('getClass')
+      ->willReturn($plugin_class);
+    $class = DefaultFactory::getPluginClass('cherry', $plugin_definition, FruitInterface::class);
 
     $this->assertEquals($plugin_class, $class);
   }
@@ -59,12 +132,30 @@ public function testGetPluginClassWithInterface() {
   /**
    * Tests getPluginClass() with a required interface but no implementation.
    *
+   * @covers ::getPluginClass
+   *
+   * @expectedException \Drupal\Component\Plugin\Exception\PluginException
+   * @expectedExceptionMessage Plugin "cherry" (Drupal\plugin_test\Plugin\plugin_test\fruit\Kale) must implement interface Drupal\plugin_test\Plugin\plugin_test\fruit\FruitInterface.
+   */
+  public function testGetPluginClassWithInterfaceAndInvalidClassWithArrayPluginDefinition() {
+    $plugin_class = Kale::class;
+    DefaultFactory::getPluginClass('cherry', ['class' => $plugin_class, 'provider' => 'core'], FruitInterface::class);
+  }
+
+  /**
+   * Tests getPluginClass() with a required interface but no implementation.
+   *
+   * @covers ::getPluginClass
+   *
    * @expectedException \Drupal\Component\Plugin\Exception\PluginException
-   * @expectedExceptionMessage Plugin "cherry" (Drupal\plugin_test\Plugin\plugin_test\fruit\Kale) must implement interface \Drupal\plugin_test\Plugin\plugin_test\fruit\FruitInterface.
    */
-  public function testGetPluginClassWithInterfaceAndInvalidClass() {
-    $plugin_class = 'Drupal\plugin_test\Plugin\plugin_test\fruit\Kale';
-    DefaultFactory::getPluginClass('cherry', ['class' => $plugin_class, 'provider' => 'core'], '\Drupal\plugin_test\Plugin\plugin_test\fruit\FruitInterface');
+  public function testGetPluginClassWithInterfaceAndInvalidClassWithObjectPluginDefinition() {
+    $plugin_class = Kale::class;
+    $plugin_definition = $this->getMock(PluginDefinitionInterface::class);
+    $plugin_definition->expects($this->atLeastOnce())
+      ->method('getClass')
+      ->willReturn($plugin_class);
+    DefaultFactory::getPluginClass('cherry', $plugin_definition, FruitInterface::class);
   }
 
 }