From b1a99e1c83d35eef6a374c1764cad190f6faedc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20St=C3=B6ckler?= <tobiasstoeckler@googlemail.com>
Date: Sat, 21 May 2016 21:23:26 -0500
Subject: [PATCH] by tstoeckler: Refactor the way that library type plugin IDs
 and class names are retrieved

---
 src/ExternalLibrary/LibraryTrait.php          | 21 ++---------
 .../LibraryType/LibraryTypeManager.php        | 15 ++++++--
 .../LibraryTypeManagerInterface.php           | 29 +++++++++++++++
 src/ExternalLibrary/Local/LocatorManager.php  |  4 ++-
 .../Registry/LibraryRegistry.php              |  2 +-
 .../Utility/IdAccessorTrait.php               | 35 +++++++++++++++++++
 .../Utility/LibraryIdAccessorTrait.php        |  4 +--
 .../LibraryType/AssetLibraryType.php          | 24 ++++++-------
 .../LibraryType/PhpFileLibraryType.php        | 21 ++++++-----
 .../test_asset_library.yml                    |  1 -
 .../test_php_file_library.yml                 |  1 -
 11 files changed, 107 insertions(+), 50 deletions(-)
 create mode 100644 src/ExternalLibrary/LibraryType/LibraryTypeManagerInterface.php
 create mode 100644 src/ExternalLibrary/Utility/IdAccessorTrait.php

diff --git a/src/ExternalLibrary/LibraryTrait.php b/src/ExternalLibrary/LibraryTrait.php
index 58e5036..f28fd91 100644
--- a/src/ExternalLibrary/LibraryTrait.php
+++ b/src/ExternalLibrary/LibraryTrait.php
@@ -7,29 +7,14 @@
 
 namespace Drupal\libraries\ExternalLibrary;
 
+use Drupal\libraries\ExternalLibrary\Utility\IdAccessorTrait;
+
 /**
  * Provides a base external library implementation.
  */
 trait LibraryTrait {
 
-  /**
-   * The library ID.
-   *
-   * @var string
-   */
-  protected $id;
-
-  /**
-   * Returns the ID of the library.
-   *
-   * @return string
-   *   The library ID. This must be unique among all known libraries.
-   *
-   * @see \Drupal\libraries\ExternalLibrary\LibraryInterface::getId()
-   */
-  public function getId() {
-    return $this->id;
-  }
+  use IdAccessorTrait;
 
   /**
    * Returns the currently installed version of the library.
diff --git a/src/ExternalLibrary/LibraryType/LibraryTypeManager.php b/src/ExternalLibrary/LibraryType/LibraryTypeManager.php
index 72378ab..6ca779c 100644
--- a/src/ExternalLibrary/LibraryType/LibraryTypeManager.php
+++ b/src/ExternalLibrary/LibraryType/LibraryTypeManager.php
@@ -10,11 +10,12 @@ namespace Drupal\libraries\ExternalLibrary\LibraryType;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Plugin\DefaultPluginManager;
+use Drupal\libraries\Annotation\LibraryType;
 
 /**
  * Provides a plugin manager for library type plugins.
  */
-class LibraryTypeManager extends DefaultPluginManager {
+class LibraryTypeManager extends DefaultPluginManager implements LibraryTypeManagerInterface {
 
   /**
    * Constructs a locator manager.
@@ -28,9 +29,19 @@ class LibraryTypeManager extends DefaultPluginManager {
    *   The module handler to invoke the alter hook with.
    */
   public function __construct(\Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler) {
-    parent::__construct('Plugin/libraries/LibraryType', $namespaces, $module_handler, 'Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeInterface', 'Drupal\libraries\Annotation\LibraryType');
+    parent::__construct('Plugin/libraries/LibraryType', $namespaces, $module_handler, LibraryTypeInterface::class, LibraryType::class);
+    // @todo Document this hook.
     $this->alterInfo('libraries_library_type_info');
     $this->setCacheBackend($cache_backend, 'libraries_library_type_info');
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getLibraryClass(LibraryTypeInterface $library_type) {
+    // @todo Make this alter-able.
+    return $library_type->getLibraryClass();
+  }
+
+
 }
diff --git a/src/ExternalLibrary/LibraryType/LibraryTypeManagerInterface.php b/src/ExternalLibrary/LibraryType/LibraryTypeManagerInterface.php
new file mode 100644
index 0000000..d3b68a6
--- /dev/null
+++ b/src/ExternalLibrary/LibraryType/LibraryTypeManagerInterface.php
@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeManagerInterface.
+ */
+
+namespace Drupal\libraries\ExternalLibrary\LibraryType;
+
+use Drupal\Component\Plugin\Factory\FactoryInterface;
+
+/**
+ * Provides an interface for library type managers.
+ */
+interface LibraryTypeManagerInterface extends FactoryInterface {
+
+  /**
+   * Gets the library class to use for a given library type.
+   *
+   * @param \Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeInterface $library_type
+   *   The library type to return the library class for.
+   *
+   * @return string
+   *   The library class.
+   */
+  public function getLibraryClass(LibraryTypeInterface $library_type);
+
+}
+
diff --git a/src/ExternalLibrary/Local/LocatorManager.php b/src/ExternalLibrary/Local/LocatorManager.php
index 4979f31..adbae2c 100644
--- a/src/ExternalLibrary/Local/LocatorManager.php
+++ b/src/ExternalLibrary/Local/LocatorManager.php
@@ -10,6 +10,7 @@ namespace Drupal\libraries\ExternalLibrary\Local;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Plugin\DefaultPluginManager;
+use Drupal\libraries\Annotation\Locator;
 
 /**
  * Provides a plugin manager for library locator plugins.
@@ -30,7 +31,8 @@ class LocatorManager extends DefaultPluginManager {
    *   The module handler to invoke the alter hook with.
    */
   public function __construct(\Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler) {
-    parent::__construct('Plugin/libraries/Locator', $namespaces, $module_handler, 'Drupal\libraries\ExternalLibrary\Local\LocatorInterface', 'Drupal\libraries\Annotation\Locator');
+    parent::__construct('Plugin/libraries/Locator', $namespaces, $module_handler, LocatorInterface::class, Locator::class);
+    // @todo Document this hook.
     $this->alterInfo('libraries_locator_info');
     $this->setCacheBackend($cache_backend, 'libraries_locator_info');
   }
diff --git a/src/ExternalLibrary/Registry/LibraryRegistry.php b/src/ExternalLibrary/Registry/LibraryRegistry.php
index 5cb7dd2..2555ffd 100644
--- a/src/ExternalLibrary/Registry/LibraryRegistry.php
+++ b/src/ExternalLibrary/Registry/LibraryRegistry.php
@@ -58,7 +58,7 @@ class LibraryRegistry implements LibraryRegistryInterface {
   public function getLibrary($id) {
     $library_type = $this->getLibraryType($id);
 
-    $class = $library_type->getLibraryClass();
+    $class = $this->libraryTypeFactory->getLibraryClass($library_type);
     // @todo Make sure that the library class implements the correct interface.
     $library = $class::create($id, $this->getDefinition($id));
 
diff --git a/src/ExternalLibrary/Utility/IdAccessorTrait.php b/src/ExternalLibrary/Utility/IdAccessorTrait.php
new file mode 100644
index 0000000..423d3c1
--- /dev/null
+++ b/src/ExternalLibrary/Utility/IdAccessorTrait.php
@@ -0,0 +1,35 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\libraries\ExternalLibrary\Utility\LibraryIdAccessorTrait.
+ */
+
+namespace Drupal\libraries\ExternalLibrary\Utility;
+
+/**
+ * Provides a trait for classes that have a string identifier.
+ */
+trait IdAccessorTrait {
+
+  /**
+   * The ID.
+   *
+   * @var string
+   */
+  protected $id;
+
+  /**
+   * Returns the ID.
+   *
+   * @return string
+   *   The ID.
+   *
+   * @see \Drupal\libraries\ExternalLibrary\LibraryInterface::getId()
+   * @see \Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeInterface::getId()
+   */
+  public function getId() {
+    return $this->id;
+  }
+
+}
diff --git a/src/ExternalLibrary/Utility/LibraryIdAccessorTrait.php b/src/ExternalLibrary/Utility/LibraryIdAccessorTrait.php
index 85afbe7..1794eb8 100644
--- a/src/ExternalLibrary/Utility/LibraryIdAccessorTrait.php
+++ b/src/ExternalLibrary/Utility/LibraryIdAccessorTrait.php
@@ -13,14 +13,14 @@ namespace Drupal\libraries\ExternalLibrary\Utility;
 trait LibraryIdAccessorTrait {
 
   /**
-   * The library ID of the library that caused the exception.
+   * The ID of the library.
    *
    * @var string
    */
   protected $libraryId;
 
   /**
-   * Returns the library ID of the library that caused the exception.
+   * Returns the ID of the library.
    *
    * @return string
    *   The library ID.
diff --git a/src/Plugin/libraries/LibraryType/AssetLibraryType.php b/src/Plugin/libraries/LibraryType/AssetLibraryType.php
index 7bd9150..9c1b262 100644
--- a/src/Plugin/libraries/LibraryType/AssetLibraryType.php
+++ b/src/Plugin/libraries/LibraryType/AssetLibraryType.php
@@ -9,10 +9,12 @@ namespace Drupal\libraries\Plugin\libraries\LibraryType;
 
 use Drupal\Component\Plugin\Factory\FactoryInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
+use Drupal\libraries\ExternalLibrary\Asset\AssetLibrary;
 use Drupal\libraries\ExternalLibrary\LibraryInterface;
 use Drupal\libraries\ExternalLibrary\LibraryType\LibraryCreationListenerInterface;
 use Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeInterface;
 use Drupal\libraries\ExternalLibrary\Local\LocalLibraryInterface;
+use Drupal\libraries\ExternalLibrary\Utility\IdAccessorTrait;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -24,6 +26,8 @@ class AssetLibraryType implements
   ContainerFactoryPluginInterface
 {
 
+  use IdAccessorTrait;
+
   /**
    * The locator factory.
    *
@@ -32,12 +36,15 @@ class AssetLibraryType implements
   protected $locatorFactory;
 
   /**
-   * Constructs the PHP file library type.
+   * Constructs the asset library type.
    *
+   * @param string $plugin_id
+   *   The plugin ID taken from the class annotation.
    * @param \Drupal\Component\Plugin\Factory\FactoryInterface $locator_factory
    *   The locator factory.
    */
-  public function __construct(FactoryInterface $locator_factory) {
+  public function __construct($plugin_id, FactoryInterface $locator_factory) {
+    $this->id = $plugin_id;
     $this->locatorFactory = $locator_factory;
   }
 
@@ -45,23 +52,14 @@ class AssetLibraryType implements
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
-    return new static($container->get('plugin.manager.libraries.locator'));
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getId() {
-    // @todo Remove the duplication with the annotation.
-    return 'asset';
+    return new static($plugin_id, $container->get('plugin.manager.libraries.locator'));
   }
 
   /**
    * {@inheritdoc}
    */
   public function getLibraryClass() {
-    // @todo Make this alter-able.
-    return 'Drupal\libraries\ExternalLibrary\Asset\AssetLibrary';
+    return AssetLibrary::class;
   }
 
   /**
diff --git a/src/Plugin/libraries/LibraryType/PhpFileLibraryType.php b/src/Plugin/libraries/LibraryType/PhpFileLibraryType.php
index d5cd656..dced4c3 100644
--- a/src/Plugin/libraries/LibraryType/PhpFileLibraryType.php
+++ b/src/Plugin/libraries/LibraryType/PhpFileLibraryType.php
@@ -13,7 +13,9 @@ use Drupal\libraries\ExternalLibrary\LibraryInterface;
 use Drupal\libraries\ExternalLibrary\LibraryType\LibraryCreationListenerInterface;
 use Drupal\libraries\ExternalLibrary\LibraryType\LibraryLoadingListenerInterface;
 use Drupal\libraries\ExternalLibrary\LibraryType\LibraryTypeInterface;
+use Drupal\libraries\ExternalLibrary\PhpFile\PhpFileLibrary;
 use Drupal\libraries\ExternalLibrary\PhpFile\PhpFileLoaderInterface;
+use Drupal\libraries\ExternalLibrary\Utility\IdAccessorTrait;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -26,6 +28,8 @@ class PhpFileLibraryType implements
   ContainerFactoryPluginInterface
 {
 
+  use IdAccessorTrait;
+
   /**
    * The locator factory.
    *
@@ -43,12 +47,15 @@ class PhpFileLibraryType implements
   /**
    * Constructs the PHP file library type.
    *
+   * @param string $plugin_id
+   *   The plugin ID taken from the class annotation.
    * @param \Drupal\Component\Plugin\Factory\FactoryInterface $locator_factory
    *   The locator factory.
    * @param \Drupal\libraries\ExternalLibrary\PhpFile\PhpFileLoaderInterface $php_file_loader
    *   The PHP file loader.
    */
-  public function __construct(FactoryInterface $locator_factory, PhpFileLoaderInterface $php_file_loader) {
+  public function __construct($plugin_id, FactoryInterface $locator_factory, PhpFileLoaderInterface $php_file_loader) {
+    $this->id = $plugin_id;
     $this->locatorFactory = $locator_factory;
     $this->phpFileLoader = $php_file_loader;
   }
@@ -58,25 +65,17 @@ class PhpFileLibraryType implements
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
     return new static(
+      $plugin_id,
       $container->get('plugin.manager.libraries.locator'),
       $container->get('libraries.php_file_loader')
     );
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getId() {
-    // @todo Remove the duplication with the annotation.
-    return 'php_file';
-  }
-
   /**
    * {@inheritdoc}
    */
   public function getLibraryClass() {
-    // @todo Make this alter-able.
-    return 'Drupal\libraries\ExternalLibrary\PhpFile\PhpFileLibrary';
+    return PhpFileLibrary::class;
   }
 
   /**
diff --git a/tests/library_definitions/test_asset_library.yml b/tests/library_definitions/test_asset_library.yml
index ac6ee9b..3c6e1a1 100644
--- a/tests/library_definitions/test_asset_library.yml
+++ b/tests/library_definitions/test_asset_library.yml
@@ -1,4 +1,3 @@
-id: test_asset_library
 type: asset
 remote_url: http://example.com
 css:
diff --git a/tests/library_definitions/test_php_file_library.yml b/tests/library_definitions/test_php_file_library.yml
index 0127266..ced08c5 100644
--- a/tests/library_definitions/test_php_file_library.yml
+++ b/tests/library_definitions/test_php_file_library.yml
@@ -1,3 +1,2 @@
-id: test_php_file_library
 type: php_file
 files: [test_php_file_library.php]
-- 
GitLab