Issue #2078879 by pwolanin, dawehner, Xano: Fixed DerivativeDiscoveryDecorator...

Issue #2078879 by pwolanin, dawehner, Xano: Fixed DerivativeDiscoveryDecorator doesn't check/enforce the proposed derivative fetchers implement interface DerivativeInterface.

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

@@ -2,11 +2,13 @@
 
 /**
  * @file
- * Definition of Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator.
+ * Contains \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator.
  */
 
 namespace Drupal\Component\Plugin\Discovery;
 
+use Drupal\Component\Plugin\Exception\InvalidDerivativeClassException;
+
 /**
  * Base class providing the tools for a plugin discovery to be derivative aware.
  *
@@ -31,7 +33,11 @@ public function __construct(DiscoveryInterface $decorated) {
   }
 
   /**
-   * Implements Drupal\Component\Plugin\Discovery\DiscoveryInterface::getDefinition().
+   * {@inheritdoc}
+   *
+   * @throws \Drupal\Component\Plugin\Exception\InvalidDerivativeClassException
+   *   Thrown if the 'derivative' class specified in the plugin definition does
+   *   not implement \Drupal\Component\Plugin\Derivative\DerivativeInterface.
    */
   public function getDefinition($plugin_id) {
     list($base_plugin_id, $derivative_id) = $this->decodePluginId($plugin_id);
@@ -48,7 +54,11 @@ public function getDefinition($plugin_id) {
   }
 
   /**
-   * Implements Drupal\Component\Plugin\Discovery\DiscoveryInterface::getDefinitions().
+   * {@inheritdoc}
+   *
+   * @throws \Drupal\Component\Plugin\Exception\InvalidDerivativeClassException
+   *   Thrown if the 'derivative' class specified in the plugin definition does
+   *   not implement \Drupal\Component\Plugin\Derivative\DerivativeInterface.
    */
   public function getDefinitions() {
     $plugin_definitions = $this->decorated->getDefinitions();
@@ -133,20 +143,48 @@ protected function encodePluginId($base_plugin_id, $derivative_id) {
    * @param array $base_definition
    *   The base plugin definition to build derivatives.
    *
-   * @return \Drupal\Component\Plugin\Discovery\DerivativeInterface|null
-   *   A DerivativeInterface or null if none exists for the plugin.
+   * @return \Drupal\Component\Plugin\Derivative\DerivativeInterface|null
++   *   A DerivativeInterface or NULL if none exists for the plugin.
++   *
++   * @throws \Drupal\Component\Plugin\Exception\InvalidDerivativeClassException
++   *   Thrown if the 'derivative' class specified in the plugin definition does
++   *   not implement \Drupal\Component\Plugin\Derivative\DerivativeInterface.
    */
   protected function getDerivativeFetcher($base_plugin_id, array $base_definition) {
     if (!isset($this->derivativeFetchers[$base_plugin_id])) {
       $this->derivativeFetchers[$base_plugin_id] = FALSE;
-      if (isset($base_definition['derivative'])) {
-        $class = $base_definition['derivative'];
+      $class = $this->getDerivativeClass($base_definition);
+      if ($class) {
         $this->derivativeFetchers[$base_plugin_id] = new $class($base_plugin_id);
       }
     }
     return $this->derivativeFetchers[$base_plugin_id] ?: NULL;
   }
 
+  /**
+   * Get the derivative class name from the base plugin definition.
+   *
+   * @param array $base_definition
+   *   The base plugin definition to build derivatives.
+   *
+   * @return string|NULL
+   *   The name of a class implementing \Drupal\Component\Plugin\Derivative\DerivativeInterface.
+   *
+   * @throws \Drupal\Component\Plugin\Exception\InvalidDerivativeClassException
+   *   Thrown if the 'derivative' class specified in the plugin definition does
+   *   not implement \Drupal\Component\Plugin\Derivative\DerivativeInterface.
+   */
+  protected function getDerivativeClass($base_definition) {
+    $class = NULL;
+    if (isset($base_definition['derivative'])) {
+      $class = $base_definition['derivative'];
+      if (!is_subclass_of($class, '\Drupal\Component\Plugin\Derivative\DerivativeInterface')) {
+        throw new InvalidDerivativeClassException(sprintf('Plugin (%s) derivative class "%s" has to implement interface \Drupal\Component\Plugin\Derivative\DerivativeInterface', $base_definition['id'], $class));
+      }
+    }
+    return $class;
+  }
+
   /**
    * Passes through all unknown calls onto the decorated object.
    */

core/lib/Drupal/Component/Plugin/Exception/InvalidDerivativeClassException.php

+<?php
+/**
+ * @file
+ * Definition of \Drupal\Component\Plugin\Exception\InvalidDerivativeClassException.
+ */
+
+namespace Drupal\Component\Plugin\Exception;
+
+/**
+ * Exception to be thrown if a plugin tries to use an invalid derivative class.
+ */
+class InvalidDerivativeClassException extends PluginException { }

core/lib/Drupal/Core/Plugin/Discovery/ContainerDerivativeDiscoveryDecorator.php

@@ -17,8 +17,8 @@ class ContainerDerivativeDiscoveryDecorator extends DerivativeDiscoveryDecorator
   protected function getDerivativeFetcher($base_plugin_id, array $base_definition) {
     if (!isset($this->derivativeFetchers[$base_plugin_id])) {
       $this->derivativeFetchers[$base_plugin_id] = FALSE;
-      if (isset($base_definition['derivative'])) {
-        $class = $base_definition['derivative'];
+      $class = $this->getDerivativeClass($base_definition);
+      if ($class) {
         // If the derivative class provides a factory method, pass the container
         // to it.
         if (is_subclass_of($class, 'Drupal\Core\Plugin\Discovery\ContainerDerivativeInterface')) {

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Plugin\Discovery\DerivativeDiscoveryDecoratorTest.
+ */
+
+namespace Drupal\Tests\Core\Plugin\Discovery;
+
+use Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator;
+use Drupal\Component\Plugin\Exception\InvalidDerivativeClassException;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * Unit tests for the derivative discovery decorator.
+ */
+class DerivativeDiscoveryDecoratorTest extends UnitTestCase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getInfo() {
+    return array(
+      'name' => 'Derivative discovery decorator.',
+      'description' => 'Tests the derivative discovery decorator.',
+      'group' => 'Plugin',
+    );
+  }
+
+  /**
+   * Tests the getDerivativeFetcher method.
+   *
+   * @see  \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator::getDerivativeFetcher().
+   */
+  public function testGetDerivativeFetcher() {
+    $definitions = array();
+    $definitions['non_container_aware_discovery'] = array(
+      'id' => 'non_container_aware_discovery',
+      'derivative' => '\Drupal\Tests\Core\Plugin\Discovery\TestDerivativeDiscovery',
+    );
+
+    $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface');
+    $discovery_main->expects($this->any())
+      ->method('getDefinitions')
+      ->will($this->returnValue($definitions));
+
+    $discovery = new DerivativeDiscoveryDecorator($discovery_main);
+    $definitions = $discovery->getDefinitions();
+
+    // Ensure that both test derivatives got added.
+    $this->assertEquals(2, count($definitions));
+  }
+
+  /**
+   * Tests the getDerivativeFetcher method with an invalid class.
+   *
+   * @see \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator::getDerivativeFetcher().\
+   *
+   * @expectedException Drupal\Component\Plugin\Exception\InvalidDerivativeClassException
+   */
+  public function testInvalidDerivativeFetcher() {
+    $definitions = array();
+    // Do this with a class that doesn't implement the interface.
+    $definitions['invalid_discovery'] = array(
+      'id' => 'invalid_discovery',
+      'derivative' => '\Drupal\system\Tests\Plugin\DerivativeTest',
+    );
+    $discovery_main = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface');
+    $discovery_main->expects($this->any())
+      ->method('getDefinitions')
+      ->will($this->returnValue($definitions));
+
+    $discovery = new DerivativeDiscoveryDecorator($discovery_main);
+    $definitions = $discovery->getDefinitions();
+  }
+}