Issue #3370930 by catch, Amber Himes Matz, kevinquillen, mherchel: Aggregation...

Issue #3370930 by catch, Amber Himes Matz, kevinquillen, mherchel: Aggregation URL hashes should be built from normalized list of libraries

(cherry picked from commit 9a66ffce)

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

@@ -101,8 +101,18 @@ public function __construct(LibraryDiscoveryInterface $library_discovery, Librar
    *   loaded, excluding any libraries that have already been loaded.
    */
   protected function getLibrariesToLoad(AttachedAssetsInterface $assets) {
+    // The order of libraries passed in via assets can differ, so to reduce
+    // variation, first normalize the requested libraries to the minimal
+    // representative set before then expanding the list to include all
+    // dependencies.
+    // @see Drupal\FunctionalTests\Core\Asset\AssetOptimizationTestUmami
+    // @todo: https://www.drupal.org/project/drupal/issues/1945262
+    $libraries = $assets->getLibraries();
+    if ($libraries) {
+      $libraries = $this->libraryDependencyResolver->getMinimalRepresentativeSubset($libraries);
+    }
     return array_diff(
-      $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getLibraries()),
+      $this->libraryDependencyResolver->getLibrariesWithDependencies($libraries),
       $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getAlreadyLoadedLibraries())
     );
   }

core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTest.php

@@ -56,6 +56,15 @@ public function testAssetAggregation(): void {
     $this->doTestAggregation($settings);
   }
 
+  /**
+   * Creates a user and requests a page.
+   */
+  protected function requestPage(): void {
+    $user = $this->createUser();
+    $this->drupalLogin($user);
+    $this->drupalGet('');
+  }
+
   /**
    * Helper to test aggregate file URLs.
    *
@@ -73,44 +82,35 @@ protected function doTestAggregation(array $settings): void {
       'preprocess' => TRUE,
       'gzip' => TRUE,
     ])->save();
-    $user = $this->createUser();
-    $this->drupalLogin($user);
-    $this->drupalGet('');
+    $this->requestPage();
     $session = $this->getSession();
     $page = $session->getPage();
 
-    $elements = $page->findAll('xpath', '//link[@rel="stylesheet"]');
-    $urls = [];
-    foreach ($elements as $element) {
+    // Collect all the URLs for all the script and styles prior to making any
+    // more requests.
+    $style_elements = $page->findAll('xpath', '//link[@rel="stylesheet"]');
+    $script_elements = $page->findAll('xpath', '//script');
+    $style_urls = [];
+    foreach ($style_elements as $element) {
       if ($element->hasAttribute('href')) {
-        $urls[] = $element->getAttribute('href');
+        $style_urls[] = $element->getAttribute('href');
       }
     }
-    foreach ($urls as $url) {
-      $this->assertAggregate($url);
+    $script_urls = [];
+    foreach ($script_elements as $element) {
+      if ($element->hasAttribute('src')) {
+        $script_urls[] = $element->getAttribute('src');
+      }
     }
-    foreach ($urls as $url) {
+    foreach ($style_urls as $url) {
+      $this->assertAggregate($url);
       $this->assertAggregate($url, FALSE);
-    }
-
-    foreach ($urls as $url) {
       $this->assertInvalidAggregates($url);
     }
 
-    $elements = $page->findAll('xpath', '//script');
-    $urls = [];
-    foreach ($elements as $element) {
-      if ($element->hasAttribute('src')) {
-        $urls[] = $element->getAttribute('src');
-      }
-    }
-    foreach ($urls as $url) {
+    foreach ($script_urls as $url) {
       $this->assertAggregate($url);
-    }
-    foreach ($urls as $url) {
       $this->assertAggregate($url, FALSE);
-    }
-    foreach ($urls as $url) {
       $this->assertInvalidAggregates($url);
     }
   }
@@ -125,16 +125,20 @@ protected function doTestAggregation(array $settings): void {
    */
   protected function assertAggregate(string $url, bool $from_php = TRUE): void {
     $url = $this->getAbsoluteUrl($url);
-    $this->assertStringContainsString($this->fileAssetsPath, $url);
+    // Not every script or style on a page is aggregated.
+    if (!str_contains($url, $this->fileAssetsPath)) {
+      return;
+    }
     $session = $this->getSession();
     $session->visit($url);
     $this->assertSession()->statusCodeEquals(200);
     $headers = $session->getResponseHeaders();
     if ($from_php) {
       $this->assertEquals(['no-store, private'], $headers['Cache-Control']);
+      $this->assertArrayHasKey('X-Generator', $headers);
     }
     else {
-      $this->assertArrayNotHasKey('Cache-Control', $headers);
+      $this->assertArrayNotHasKey('X-Generator', $headers);
     }
   }
 
@@ -147,6 +151,11 @@ protected function assertAggregate(string $url, bool $from_php = TRUE): void {
    * @throws \Behat\Mink\Exception\ExpectationException
    */
   protected function assertInvalidAggregates(string $url): void {
+    $url = $this->getAbsoluteUrl($url);
+    // Not every script or style on a page is aggregated.
+    if (!str_contains($url, $this->fileAssetsPath)) {
+      return;
+    }
     $session = $this->getSession();
     $session->visit($this->replaceGroupDelta($url));
     $this->assertSession()->statusCodeEquals(200);

core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTestUmami.php

+<?php
+
+namespace Drupal\FunctionalTests\Asset;
+
+/**
+ * Tests asset aggregation with the Umami install profile.
+ *
+ * Umami includes several core modules as well as the Claro theme, this
+ * results in a more complex asset dependency tree to test than the testing
+ * profile.
+ *
+ * @group asset
+ */
+class AssetOptimizationTestUmami extends AssetOptimizationTest {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $profile = 'demo_umami';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function requestPage(): void {
+    $user = $this->createUser([], NULL, TRUE);
+    $this->drupalLogin($user);
+    $this->drupalGet('node/add/article');
+  }
+
+}

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

@@ -92,6 +92,9 @@ protected function setUp(): void {
     $this->libraryDependencyResolver->expects($this->any())
       ->method('getLibrariesWithDependencies')
       ->willReturnArgument(0);
+    $this->libraryDependencyResolver->expects($this->any())
+      ->method('getMinimalRepresentativeSubset')
+      ->willReturnArgument(0);
     $this->moduleHandler = $this->createMock('\Drupal\Core\Extension\ModuleHandlerInterface');
     $this->themeManager = $this->createMock('\Drupal\Core\Theme\ThemeManagerInterface');
     $active_theme = $this->getMockBuilder('\Drupal\Core\Theme\ActiveTheme')

core/themes/olivero/olivero.libraries.yml

@@ -52,12 +52,12 @@ global-styling:
 
   js:
     js/checkbox.js: {}
-    js/navigation-utils.js: {}
 
   dependencies:
     - core/drupal
     - core/once
     - core/tabbable
+    - olivero/navigation-base
 
 book:
   version: VERSION
@@ -213,6 +213,15 @@ messages:
   dependencies:
     - olivero/global-styling
 
+navigation-base:
+  version: VERSION
+  js:
+    js/navigation-utils.js: {}
+  dependencies:
+    - core/drupal
+    - core/once
+    - core/tabbable
+
 navigation-primary:
   version: VERSION
   css:
@@ -225,8 +234,7 @@ navigation-primary:
     js/second-level-navigation.js: {}
     js/nav-resize.js: {}
   dependencies:
-    - core/drupal
-    - core/once
+    - olivero/navigation-base
 
 navigation-secondary:
   version: VERSION
@@ -265,6 +273,8 @@ search-wide:
       css/components/header-search-wide.css: {}
   js:
     js/search.js: {}
+  dependencies:
+    - olivero/navigation-primary
 
 sidebar:
   version: VERSION