From 7d36a7c3d8bdf3b98c1884ac50fdc24b3fa479db Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Fri, 21 Nov 2014 09:53:12 +0000 Subject: [PATCH] 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 | 2 +- core/includes/common.inc | 4 - .../Drupal/Core/Asset/LibraryDiscovery.php | 39 +++++++- ...terTest.php => LocaleLibraryAlterTest.php} | 8 +- .../Core/Asset/LibraryDiscoveryParserTest.php | 6 +- .../Tests/Core/Asset/LibraryDiscoveryTest.php | 98 +++++++++++++++++++ 6 files changed, 141 insertions(+), 16 deletions(-) rename core/modules/locale/src/Tests/{LocaleLibraryInfoAlterTest.php => LocaleLibraryAlterTest.php} (78%) create mode 100644 core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryTest.php diff --git a/core/core.services.yml b/core/core.services.yml index 99ae337fc60..ee9882013eb 100644 --- a/core/core.services.yml +++ b/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'] diff --git a/core/includes/common.inc b/core/includes/common.inc index d694df10a32..e7d19edb25e 100644 --- a/core/includes/common.inc +++ b/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'], diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscovery.php b/core/lib/Drupal/Core/Asset/LibraryDiscovery.php index b0c3696b28b..9c28e337d79 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscovery.php +++ b/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]; } /** diff --git a/core/modules/locale/src/Tests/LocaleLibraryInfoAlterTest.php b/core/modules/locale/src/Tests/LocaleLibraryAlterTest.php similarity index 78% rename from core/modules/locale/src/Tests/LocaleLibraryInfoAlterTest.php rename to core/modules/locale/src/Tests/LocaleLibraryAlterTest.php index 0430bbdb87a..7b04c69f2dc 100644 --- a/core/modules/locale/src/Tests/LocaleLibraryInfoAlterTest.php +++ b/core/modules/locale/src/Tests/LocaleLibraryAlterTest.php @@ -1,7 +1,7 @@ [ + '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'); + } + +}