Issue #2378263 by Wim Leers: hook_library_alter() must be manually invoked by...

Issue #2378263 by Wim Leers: hook_library_alter() must be manually invoked by users of LibraryDiscovery, and has no test coverage

core/core.services.yml

@@ -1053,7 +1053,7 @@ services:
     class: Drupal\Core\Asset\AssetDumper
   library.discovery:
     class: Drupal\Core\Asset\LibraryDiscovery
-    arguments: ['@library.discovery.collector']
+    arguments: ['@library.discovery.collector', '@module_handler']
   library.discovery.collector:
     class: Drupal\Core\Asset\LibraryDiscoveryCollector
     arguments: ['@cache.discovery', '@lock', '@library.discovery.parser']

core/includes/common.inc

@@ -2023,10 +2023,6 @@ function _drupal_add_library($library_name, $every_page = NULL) {
   // Only process the library if it exists and it was not added already.
   if (!isset($added[$extension][$name])) {
     if ($library = $library_discovery->getLibraryByName($extension, $name)) {
-      // Allow modules and themes to dynamically attach request and context
-      // specific data for this library; e.g., localization.
-      \Drupal::moduleHandler()->alter('library', $library, $library_name);
-
       // Add all components within the library.
       $elements['#attached'] = array(
         'library' => $library['dependencies'],

core/lib/Drupal/Core/Asset/LibraryDiscovery.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\Asset;
 
 use Drupal\Core\Cache\CacheCollectorInterface;
+use Drupal\Core\Extension\ModuleHandlerInterface;
 
 /**
  * Discovers available asset libraries in Drupal.
@@ -21,23 +22,53 @@ class LibraryDiscovery implements LibraryDiscoveryInterface {
    */
   protected $collector;
 
+  /**
+   * The module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
+
+  /**
+   * The final library definitions, statically cached.
+   *
+   * hook_library_alter() allows modules and themes to dynamically alter a
+   * library definition (once per request).
+   *
+   * @var array
+   */
+  protected $libraryDefinitions = [];
+
   /**
    * Constructs a new LibraryDiscovery instance.
    *
-   * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend
-   *   The cache backend.
+   * @param \Drupal\Core\Cache\CacheCollectorInterface $library_discovery_collector
+   *   The library discovery cache collector.
    * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
    *   The module handler.
    */
-  public function __construct(CacheCollectorInterface $library_discovery_collector) {
+  public function __construct(CacheCollectorInterface $library_discovery_collector, ModuleHandlerInterface $module_handler) {
     $this->collector = $library_discovery_collector;
+    $this->moduleHandler = $module_handler;
   }
 
   /**
    * {@inheritdoc}
    */
   public function getLibrariesByExtension($extension) {
-    return $this->collector->get($extension);
+    if (!isset($this->libraryDefinitions[$extension])) {
+      $libraries = $this->collector->get($extension);
+      $this->libraryDefinitions[$extension] = [];
+      foreach ($libraries as $name => $definition) {
+        // Allow modules and themes to dynamically attach request and context
+        // specific data for this library; e.g., localization.
+        $library_name = "$extension/$name";
+        $this->moduleHandler->alter('library', $definition, $library_name);
+        $this->libraryDefinitions[$extension][$name] = $definition;
+      }
+    }
+
+    return $this->libraryDefinitions[$extension];
   }
 
   /**

core/modules/locale/src/Tests/LocaleLibraryInfoAlterTest.php → core/modules/locale/src/Tests/LocaleLibraryAlterTest.php

 <?php
 /**
  * @file
- * Definition of Drupal\locale\Tests\LocaleLibraryInfoAlterTest.
+ * Contains \Drupal\locale\Tests\LocaleLibraryAlterTest.
  */
 
 namespace Drupal\locale\Tests;
@@ -15,7 +15,7 @@
  *
  * @group locale
  */
-class LocaleLibraryInfoAlterTest extends WebTestBase {
+class LocaleLibraryAlterTest extends WebTestBase {
 
   /**
    * Modules to enable.
@@ -27,9 +27,9 @@ class LocaleLibraryInfoAlterTest extends WebTestBase {
   /**
    * Verifies that the datepicker can be localized.
    *
-   * @see locale_library_info_alter()
+   * @see locale_library_alter()
    */
-  public function testLibraryInfoAlter() {
+  public function testLibraryAlter() {
     $attached['#attached']['library'][] = 'core/jquery.ui.datepicker';
     drupal_render($attached);
     drupal_process_attached($attached);

core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php

@@ -2,7 +2,7 @@
 
 /**
  * @file
- * Contains \Drupal\Tests\Core\Asset\LibraryDiscoveryTest.
+ * Contains \Drupal\Tests\Core\Asset\LibraryDiscoveryParserTest.
  */
 
 namespace Drupal\Tests\Core\Asset;
@@ -31,7 +31,7 @@
 class LibraryDiscoveryParserTest extends UnitTestCase {
 
   /**
-   * The tested library provider.
+   * The tested library discovery parser service.
    *
    * @var \Drupal\Core\Asset\LibraryDiscoveryParser|\Drupal\Tests\Core\Asset\TestLibraryDiscoveryParser
    */
@@ -52,7 +52,7 @@ class LibraryDiscoveryParserTest extends UnitTestCase {
   protected $moduleHandler;
 
   /**
-   * The mocked lock backend..
+   * The mocked lock backend.
    *
    * @var \Drupal\Core\Lock\LockBackendInterface|\PHPUnit_Framework_MockObject_MockObject
    */

core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Asset\LibraryDiscoveryTest.
+ */
+
+namespace Drupal\Tests\Core\Asset;
+
+use Drupal\Core\Asset\LibraryDiscovery;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Asset\LibraryDiscovery
+ * @group Asset
+ */
+class LibraryDiscoveryTest extends UnitTestCase {
+
+  /**
+   * The tested library discovery service.
+   *
+   * @var \Drupal\Core\Asset\LibraryDiscovery
+   */
+  protected $libraryDiscovery;
+
+  /**
+   * The mocked library discovery cache collector.
+   *
+   * @var \Drupal\Core\Cache\CacheCollectorInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $libraryDiscoveryCollector;
+
+  /**
+   * The mocked module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $moduleHandler;
+
+  /**
+   * Test library data.
+   *
+   * @var array
+   */
+  protected $libraryData = [
+    'test_1' => [
+      'js' => [],
+      'css' => [
+        'foo.css' => [],
+      ],
+    ],
+    'test_2' => [
+      'js' => [
+        'bar.js' => [],
+      ],
+      'css' => [],
+    ],
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->libraryDiscoveryCollector = $this->getMockBuilder('Drupal\Core\Asset\LibraryDiscoveryCollector')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $this->moduleHandler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface');
+    $this->libraryDiscovery = new LibraryDiscovery($this->libraryDiscoveryCollector, $this->moduleHandler);
+  }
+
+  /**
+   * @covers ::getLibrariesByExtension()
+   */
+  public function testGetLibrariesByExtension() {
+    $this->libraryDiscoveryCollector->expects($this->once())
+      ->method('get')
+      ->with('test')
+      ->willReturn($this->libraryData);
+    $this->moduleHandler->expects($this->exactly(2))
+      ->method('alter')
+      ->with(
+        'library',
+        $this->logicalOr($this->libraryData['test_1'], $this->libraryData['test_2']),
+        $this->logicalOr('test/test_1', 'test/test_2')
+      );
+
+    $this->libraryDiscovery->getLibrariesbyExtension('test');
+    // Verify that subsequent calls don't trigger hook_library_alter()
+    // invocations, nor do they talk to the collector again. This ensures that
+    // the alterations made by hook_library_alter() implementations are
+    // statically cached, as desired.
+    $this->libraryDiscovery->getLibraryByName('test', 'test_1');
+    $this->libraryDiscovery->getLibrariesbyExtension('test');
+  }
+
+}