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

Issue #2818653 by tim.plunkett, tstoeckler, jibran: Allow object-based plugin definitions to be processed in DefaultPluginManager::findDefinitions()

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

+<?php
+
+namespace Drupal\Component\Plugin\Definition;
+
+/**
+ * Provides object-based plugin definitions.
+ */
+class PluginDefinition implements PluginDefinitionInterface {
+
+  /**
+   * The plugin ID.
+   *
+   * @var string
+   */
+  protected $id;
+
+  /**
+   * A fully qualified class name.
+   *
+   * @var string
+   */
+  protected $class;
+
+  /**
+   * The plugin provider.
+   *
+   * @var string
+   */
+  protected $provider;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function id() {
+    return $this->id;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setClass($class) {
+    $this->class = $class;
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getClass() {
+    return $this->class;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getProvider() {
+    return $this->provider;
+  }
+
+}

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

@@ -11,6 +11,14 @@
  */
 interface PluginDefinitionInterface {
 
+  /**
+   * Gets the unique identifier of the plugin.
+   *
+   * @return string
+   *   The unique identifier of the plugin.
+   */
+  public function id();
+
   /**
    * Sets the class.
    *
@@ -32,4 +40,15 @@ public function setClass($class);
    */
   public function getClass();
 
+  /**
+   * Gets the plugin provider.
+   *
+   * The provider is the name of the module that provides the plugin, or "core',
+   * or "component".
+   *
+   * @return string
+   *   The provider.
+   */
+  public function getProvider();
+
 }

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

@@ -2,6 +2,7 @@
 
 namespace Drupal\Core\Entity;
 
+use Drupal\Component\Plugin\Definition\PluginDefinition;
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\Entity\Exception\EntityTypeIdLengthException;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
@@ -12,7 +13,7 @@
  *
  * @ingroup entity_api
  */
-class EntityType implements EntityTypeInterface {
+class EntityType extends PluginDefinition implements EntityTypeInterface {
 
   use StringTranslationTrait;
 
@@ -51,20 +52,6 @@ class EntityType implements EntityTypeInterface {
    */
   protected $id;
 
-  /**
-   * The name of the provider of this entity type.
-   *
-   * @var string
-   */
-  protected $provider;
-
-  /**
-   * The name of the entity type class.
-   *
-   * @var string
-   */
-  protected $class;
-
   /**
    * The name of the original entity type class.
    *
@@ -388,27 +375,6 @@ public function hasKey($key) {
     return !empty($keys[$key]);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function id() {
-    return $this->id;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getProvider() {
-    return $this->provider;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getClass() {
-    return $this->class;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -425,8 +391,8 @@ public function setClass($class) {
       // class, assume that is the original class name.
       $this->originalClass = $this->class;
     }
-    $this->class = $class;
-    return $this;
+
+    return parent::setClass($class);
   }
 
   /**

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

@@ -47,22 +47,6 @@ public function get($property);
    */
   public function set($property, $value);
 
-  /**
-   * Gets the unique identifier of the entity type.
-   *
-   * @return string
-   *   The unique identifier of the entity type.
-   */
-  public function id();
-
-  /**
-   * Gets the name of the provider of this entity type.
-   *
-   * @return string
-   *   The name of the provider of this entity type.
-   */
-  public function getProvider();
-
   /**
    * Gets the name of the original entity type class.
    *

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

@@ -3,6 +3,7 @@
 namespace Drupal\Core\Layout;
 
 use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
+use Drupal\Component\Plugin\Definition\PluginDefinition;
 
 /**
  * Provides an implementation of a layout definition and its metadata.
@@ -12,23 +13,7 @@
  *   experimental modules and development releases of contributed modules.
  *   See https://www.drupal.org/core/experimental for more information.
  */
-class LayoutDefinition implements PluginDefinitionInterface, DerivablePluginDefinitionInterface {
-
-  /**
-   * The plugin ID.
-   *
-   * @var string
-   */
-  protected $id;
-
-  /**
-   * The name of the provider of this layout definition.
-   *
-   * @todo Make protected after https://www.drupal.org/node/2818653.
-   *
-   * @var string
-   */
-  public $provider;
+class LayoutDefinition extends PluginDefinition implements PluginDefinitionInterface, DerivablePluginDefinitionInterface {
 
   /**
    * The name of the deriver of this layout definition, if any.
@@ -130,13 +115,6 @@ class LayoutDefinition implements PluginDefinitionInterface, DerivablePluginDefi
    */
   protected $default_region;
 
-  /**
-   * The name of the layout class.
-   *
-   * @var string
-   */
-  protected $class;
-
   /**
    * Any additional properties and values.
    *
@@ -195,31 +173,6 @@ public function set($property, $value) {
     return $this;
   }
 
-  /**
-   * Gets the unique identifier of the layout definition.
-   *
-   * @return string
-   *   The unique identifier of the layout definition.
-   */
-  public function id() {
-    return $this->id;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getClass() {
-    return $this->class;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setClass($class) {
-    $this->class = $class;
-    return $this;
-  }
-
   /**
    * Gets the human-readable name of the layout definition.
    *
@@ -499,16 +452,6 @@ public function setDefaultRegion($default_region) {
     return $this;
   }
 
-  /**
-   * Gets the name of the provider of this layout definition.
-   *
-   * @return string
-   *   The name of the provider of this layout definition.
-   */
-  public function getProvider() {
-    return $this->provider;
-  }
-
   /**
    * Gets the config dependencies of this layout definition.
    *

core/lib/Drupal/Core/Plugin/DefaultPluginManager.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Core\Plugin;
 
+use Drupal\Component\Plugin\Definition\PluginDefinitionInterface;
 use Drupal\Component\Plugin\Discovery\CachedDiscoveryInterface;
 use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Cache\CacheBackendInterface;
@@ -285,18 +286,39 @@ protected function findDefinitions() {
     // If this plugin was provided by a module that does not exist, remove the
     // plugin definition.
     foreach ($definitions as $plugin_id => $plugin_definition) {
-      // If the plugin definition is an object, attempt to convert it to an
-      // array, if that is not possible, skip further processing.
-      if (is_object($plugin_definition) && !($plugin_definition = (array) $plugin_definition)) {
-        continue;
-      }
-      if (isset($plugin_definition['provider']) && !in_array($plugin_definition['provider'], array('core', 'component')) && !$this->providerExists($plugin_definition['provider'])) {
+      $provider = $this->extractProviderFromDefinition($plugin_definition);
+      if ($provider && !in_array($provider, array('core', 'component')) && !$this->providerExists($provider)) {
         unset($definitions[$plugin_id]);
       }
     }
     return $definitions;
   }
 
+  /**
+   * Extracts the provider from a plugin definition.
+   *
+   * @param mixed $plugin_definition
+   *   The plugin definition. Usually either an array or an instance of
+   *   \Drupal\Component\Plugin\Definition\PluginDefinitionInterface
+   *
+   * @return string|null
+   *   The provider string, if it exists. NULL otherwise.
+   */
+  protected function extractProviderFromDefinition($plugin_definition) {
+    if ($plugin_definition instanceof PluginDefinitionInterface) {
+      return $plugin_definition->getProvider();
+    }
+
+    // Attempt to convert the plugin definition to an array.
+    if (is_object($plugin_definition)) {
+      $plugin_definition = (array) $plugin_definition;
+    }
+
+    if (isset($plugin_definition['provider'])) {
+      return $plugin_definition['provider'];
+    }
+  }
+
   /**
    * Invokes the hook to alter the definitions if the alter hook is set.
    *

core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\Core\Plugin;
 
+use Drupal\Component\Plugin\Definition\PluginDefinition;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Plugin\PluginFormInterface;
@@ -359,6 +360,32 @@ public function testGetCacheMaxAge() {
     $this->assertInternalType('int', $cache_max_age);
   }
 
+  /**
+   * @covers ::findDefinitions
+   * @covers ::extractProviderFromDefinition
+   */
+  public function testProviderExists() {
+    $definitions = [];
+    $definitions['array_based_found'] = ['provider' => 'module_found'];
+    $definitions['array_based_missing'] = ['provider' => 'module_missing'];
+    $definitions['stdclass_based_found'] = (object) ['provider' => 'module_found'];
+    $definitions['stdclass_based_missing'] = (object) ['provider' => 'module_missing'];
+    $definitions['classed_object_found'] = new ObjectDefinition(['provider' => 'module_found']);
+    $definitions['classed_object_missing'] = new ObjectDefinition(['provider' => 'module_missing']);
+
+    $expected = [];
+    $expected['array_based_found'] = $definitions['array_based_found'];
+    $expected['stdclass_based_found'] = $definitions['stdclass_based_found'];
+    $expected['classed_object_found'] = $definitions['classed_object_found'];
+
+    $module_handler = $this->prophesize(ModuleHandlerInterface::class);
+    $module_handler->moduleExists('module_found')->willReturn(TRUE)->shouldBeCalled();
+    $module_handler->moduleExists('module_missing')->willReturn(FALSE)->shouldBeCalled();
+    $plugin_manager = new TestPluginManager($this->namespaces, $definitions, $module_handler->reveal());
+    $result = $plugin_manager->getDefinitions();
+    $this->assertEquals($expected, $result);
+  }
+
   /**
    * @covers ::processDefinition
    * @dataProvider providerTestProcessDefinition
@@ -453,3 +480,17 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s
   }
 
 }
+class ObjectDefinition extends PluginDefinition {
+
+  /**
+   * ObjectDefinition constructor.
+   *
+   * @param array $definition
+   */
+  public function __construct(array $definition) {
+    foreach ($definition as $property => $value) {
+      $this->{$property} = $value;
+    }
+  }
+
+}