Issue #3443108 by catch: Optimize AssetResolver caching

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

@@ -125,13 +125,24 @@ public function getCssAssets(AttachedAssetsInterface $assets, $optimize, Languag
     if (!$assets->getLibraries()) {
       return [];
     }
+    $libraries_to_load = $this->getLibrariesToLoad($assets);
+    foreach ($libraries_to_load as $key => $library) {
+      [$extension, $name] = explode('/', $library, 2);
+      $definition = $this->libraryDiscovery->getLibraryByName($extension, $name);
+      if (empty($definition['css'])) {
+        unset($libraries_to_load[$key]);
+      }
+    }
+    $libraries_to_load = array_values($libraries_to_load);
+    if (!$libraries_to_load) {
+      return [];
+    }
     if (!isset($language)) {
       $language = $this->languageManager->getCurrentLanguage();
     }
     $theme_info = $this->themeManager->getActiveTheme();
     // Add the theme name to the cache key since themes may implement
     // hook_library_info_alter().
-    $libraries_to_load = $this->getLibrariesToLoad($assets);
     $cid = 'css:' . $theme_info->getName() . ':' . $language->getId() . Crypt::hashBase64(serialize($libraries_to_load)) . (int) $optimize;
     if ($cached = $this->cache->get($cid)) {
       return $cached->data;
@@ -149,11 +160,6 @@ public function getCssAssets(AttachedAssetsInterface $assets, $optimize, Languag
     foreach ($libraries_to_load as $key => $library) {
       [$extension, $name] = explode('/', $library, 2);
       $definition = $this->libraryDiscovery->getLibraryByName($extension, $name);
-      if (empty($definition['css'])) {
-        unset($libraries_to_load[$key]);
-        continue;
-      }
-
       foreach ($definition['css'] as $options) {
         $options += $default_options;
         // Copy the asset library license information to each file.
@@ -227,10 +233,34 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize, Language
       $language = $this->languageManager->getCurrentLanguage();
     }
     $theme_info = $this->themeManager->getActiveTheme();
+    $libraries_to_load = $this->getLibrariesToLoad($assets);
+
+    // Collect all libraries that contain JS assets and are in the header.
+    // Also remove any libraries with no JavaScript from the libraries to
+    // load.
+    $header_js_libraries = [];
+    foreach ($libraries_to_load as $key => $library) {
+      [$extension, $name] = explode('/', $library, 2);
+      $definition = $this->libraryDiscovery->getLibraryByName($extension, $name);
+      if (empty($definition['js'])) {
+        unset($libraries_to_load[$key]);
+        continue;
+      }
+      if (!empty($definition['header'])) {
+        $header_js_libraries[] = $library;
+      }
+    }
+    $libraries_to_load = array_values($libraries_to_load);
+
+    // If all the libraries to load contained only CSS, there is nothing further
+    // to do here, so return early.
+    if (!$libraries_to_load && !$assets->getSettings()) {
+      return [[], []];
+    }
+
     // Add the theme name to the cache key since themes may implement
     // hook_library_info_alter(). Additionally add the current language to
     // support translation of JavaScript files via hook_js_alter().
-    $libraries_to_load = $this->getLibrariesToLoad($assets);
     $cid = 'js:' . $theme_info->getName() . ':' . $language->getId() . ':' . Crypt::hashBase64(serialize($libraries_to_load)) . (int) (count($assets->getSettings()) > 0) . (int) $optimize;
 
     if ($cached = $this->cache->get($cid)) {
@@ -248,22 +278,6 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize, Language
         'version' => NULL,
       ];
 
-      // Collect all libraries that contain JS assets and are in the header.
-      // Also remove any libraries with no JavaScript from the libraries to
-      // load.
-      $header_js_libraries = [];
-      foreach ($libraries_to_load as $key => $library) {
-        [$extension, $name] = explode('/', $library, 2);
-        $definition = $this->libraryDiscovery->getLibraryByName($extension, $name);
-        if (empty($definition['js'])) {
-          unset($libraries_to_load[$key]);
-          continue;
-        }
-        if (!empty($definition['header'])) {
-          $header_js_libraries[] = $library;
-        }
-      }
-      $libraries_to_load = array_values($libraries_to_load);
       // The current list of header JS libraries are only those libraries that
       // are in the header, but their dependencies must also be loaded for them
       // to function correctly, so update the list with those.

core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php

@@ -111,10 +111,10 @@ public function testAnonymous() {
     $this->assertSame($expected_queries, $recorded_queries);
     $this->assertSame(34, $performance_data->getQueryCount());
     $this->assertSame(124, $performance_data->getCacheGetCount());
-    $this->assertSame(46, $performance_data->getCacheSetCount());
+    $this->assertSame(45, $performance_data->getCacheSetCount());
     $this->assertSame(0, $performance_data->getCacheDeleteCount());
-    $this->assertSame(37, $performance_data->getCacheTagChecksumCount());
-    $this->assertSame(42, $performance_data->getCacheTagIsValidCount());
+    $this->assertSame(36, $performance_data->getCacheTagChecksumCount());
+    $this->assertSame(43, $performance_data->getCacheTagIsValidCount());
     $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());
 
     // Test node page.

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

@@ -76,6 +76,10 @@ class AssetResolverTest extends UnitTestCase {
    * A mocked Japanese language object.
    */
   protected LanguageInterface $japanese;
+  /**
+   * An array of library definitions.
+   */
+  protected $libraries = [];
 
   /**
    * {@inheritdoc}
@@ -86,6 +90,44 @@ protected function setUp(): void {
     $this->libraryDiscovery = $this->getMockBuilder('Drupal\Core\Asset\LibraryDiscovery')
       ->disableOriginalConstructor()
       ->getMock();
+    $this->libraries = [
+      'drupal' => [
+        'version' => '1.0.0',
+        'css' => [],
+        'js' =>
+        [
+          'core/misc/drupal.js' => ['data' => 'core/misc/drupal.js', 'preprocess' => TRUE],
+        ],
+        'license' => '',
+      ],
+      'jquery' => [
+        'version' => '1.0.0',
+        'css' => [],
+        'js' =>
+        [
+          'core/misc/jquery.js' => ['data' => 'core/misc/jquery.js', 'minified' => TRUE],
+        ],
+        'license' => '',
+      ],
+      'llama' => [
+        'version' => '1.0.0',
+        'css' =>
+        [
+          'core/misc/llama.css' => ['data' => 'core/misc/llama.css'],
+        ],
+        'js' => [],
+        'license' => '',
+      ],
+      'piggy' => [
+        'version' => '1.0.0',
+        'css' =>
+        [
+          'core/misc/piggy.css' => ['data' => 'core/misc/piggy.css'],
+        ],
+        'js' => [],
+        'license' => '',
+      ],
+    ];
     $this->libraryDependencyResolver = $this->createMock('\Drupal\Core\Asset\LibraryDependencyResolverInterface');
     $this->libraryDependencyResolver->expects($this->any())
       ->method('getLibrariesWithDependencies')
@@ -124,40 +166,77 @@ protected function setUp(): void {
 
   /**
    * @covers ::getCssAssets
-   * @dataProvider providerAttachedAssets
+   * @dataProvider providerAttachedCssAssets
    */
-  public function testGetCssAssets(AttachedAssetsInterface $assets_a, AttachedAssetsInterface $assets_b, $expected_cache_item_count) {
+  public function testGetCssAssets(AttachedAssetsInterface $assets_a, AttachedAssetsInterface $assets_b, $expected_css_cache_item_count) {
+    $this->libraryDiscovery->expects($this->any())
+      ->method('getLibraryByName')
+      ->willReturnOnConsecutiveCalls(
+        $this->libraries['drupal'],
+        $this->libraries['llama'],
+        $this->libraries['llama'],
+        $this->libraries['piggy'],
+        $this->libraries['piggy'],
+      );
     $this->assetResolver->getCssAssets($assets_a, FALSE, $this->english);
     $this->assetResolver->getCssAssets($assets_b, FALSE, $this->english);
-    $this->assertCount($expected_cache_item_count, $this->cache->getAllCids());
+    $this->assertCount($expected_css_cache_item_count, $this->cache->getAllCids());
+  }
+
+  public static function providerAttachedCssAssets() {
+    $time = time();
+    return [
+      'one js only library and one css only library' => [
+        (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal']),
+        (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['llama/css']),
+        1,
+      ],
+      'two different css libraries' => [
+        (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal', 'llama/css']),
+        (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['piggy/css']),
+        2,
+      ],
+    ];
   }
 
   /**
    * @covers ::getJsAssets
-   * @dataProvider providerAttachedAssets
+   * @dataProvider providerAttachedJsAssets
    */
-  public function testGetJsAssets(AttachedAssetsInterface $assets_a, AttachedAssetsInterface $assets_b, $expected_cache_item_count) {
+  public function testGetJsAssets(AttachedAssetsInterface $assets_a, AttachedAssetsInterface $assets_b, $expected_js_cache_item_count, $expected_multilingual_js_cache_item_count) {
+    $this->libraryDiscovery->expects($this->any())
+      ->method('getLibraryByName')
+      ->willReturnOnConsecutiveCalls(
+        $this->libraries['drupal'],
+        $this->libraries['drupal'],
+        $this->libraries['jquery'],
+        $this->libraries['drupal'],
+        $this->libraries['drupal'],
+        $this->libraries['jquery'],
+      );
     $this->assetResolver->getJsAssets($assets_a, FALSE, $this->english);
     $this->assetResolver->getJsAssets($assets_b, FALSE, $this->english);
-    $this->assertCount($expected_cache_item_count, $this->cache->getAllCids());
+    $this->assertCount($expected_js_cache_item_count, $this->cache->getAllCids());
 
     $this->assetResolver->getJsAssets($assets_a, FALSE, $this->japanese);
     $this->assetResolver->getJsAssets($assets_b, FALSE, $this->japanese);
-    $this->assertCount($expected_cache_item_count * 2, $this->cache->getAllCids());
+    $this->assertCount($expected_multilingual_js_cache_item_count, $this->cache->getAllCids());
   }
 
-  public static function providerAttachedAssets() {
+  public static function providerAttachedJsAssets() {
     $time = time();
     return [
       'same libraries, different timestamps' => [
         (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal'])->setSettings(['currentTime' => $time]),
         (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal'])->setSettings(['currentTime' => $time + 100]),
         1,
+        2,
       ],
       'different libraries, same timestamps' => [
         (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal'])->setSettings(['currentTime' => $time]),
         (new AttachedAssets())->setAlreadyLoadedLibraries([])->setLibraries(['core/drupal', 'core/jquery'])->setSettings(['currentTime' => $time]),
         2,
+        3,
       ],
     ];
   }