From 2bdbb0297acf3bce2f30d12830061268e8736fe3 Mon Sep 17 00:00:00 2001 From: Adam G-H <32250-phenaproxima@users.noreply.drupalcode.org> Date: Tue, 25 Feb 2025 20:41:14 +0000 Subject: [PATCH] Issue #3507468: EnabledSourceHandler's query result caching should also consider the contents of composer.lock --- src/EnabledSourceHandler.php | 24 ++++++++++++++++++- tests/src/Functional/ClearStorageTest.php | 16 ++++--------- .../Functional/EnabledSourceHandlerTest.php | 19 +++++++++------ 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/EnabledSourceHandler.php b/src/EnabledSourceHandler.php index 2c61552fd..c223bde84 100644 --- a/src/EnabledSourceHandler.php +++ b/src/EnabledSourceHandler.php @@ -2,6 +2,7 @@ namespace Drupal\project_browser; +use Composer\InstalledVersions; use Drupal\Component\Serialization\Json; use Drupal\Core\Config\ConfigCrudEvent; use Drupal\Core\Config\ConfigEvents; @@ -109,7 +110,7 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter */ public function getProjects(string $source_id, array $query = []): ProjectsResultsPage { // Cache only exact query, down to the page number. - $cache_key = 'query:' . md5(Json::encode($query)); + $cache_key = $this->getQueryCacheKey($query); $storage = $this->keyValue($source_id); @@ -144,6 +145,27 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter return $results; } + /** + * Generates a cache key for a specific query. + * + * @param array $query + * The query. + * + * @return string + * A cache key for the given query. + */ + private function getQueryCacheKey(array $query): string { + // Include a quick hash of the top-level `composer.lock` file in the hash, + // so that sources which base their queries on the state of the local site + // will be refreshed when the local site changes. + ['install_path' => $project_root] = InstalledVersions::getRootPackage(); + $lock_file = $project_root . DIRECTORY_SEPARATOR . 'composer.lock'; + $lock_file_hash = file_exists($lock_file) + ? hash_file('xxh64', $lock_file) + : ''; + return 'query:' . md5(Json::encode($query) . $lock_file_hash); + } + /** * Queries the specified source. * diff --git a/tests/src/Functional/ClearStorageTest.php b/tests/src/Functional/ClearStorageTest.php index d0be6f3f6..a1209bc38 100644 --- a/tests/src/Functional/ClearStorageTest.php +++ b/tests/src/Functional/ClearStorageTest.php @@ -28,13 +28,6 @@ class ClearStorageTest extends BrowserTestBase { */ protected $defaultTheme = 'stark'; - /** - * The cache key under which project data is stored in key-value. - * - * @var string - */ - private readonly string $cacheKey; - /** * The key-value storage. * @@ -52,12 +45,11 @@ class ClearStorageTest extends BrowserTestBase { ->set('enabled_sources', ['project_browser_test_mock']) ->save(); - $this->cacheKey = 'query:' . md5('[]'); $this->keyValue = \Drupal::service('keyvalue')->get('project_browser:project_browser_test_mock'); // Warm the project cache and confirm it is populated. \Drupal::service(EnabledSourceHandler::class)->getProjects('project_browser_test_mock'); - $this->assertNotEmpty($this->keyValue->get($this->cacheKey)); + $this->assertNotEmpty($this->keyValue->getAll()); } /** @@ -67,7 +59,7 @@ class ClearStorageTest extends BrowserTestBase { */ public function testClearCacheDirectly(): void { \Drupal::service(EnabledSourceHandler::class)->clearStorage(); - $this->assertEmpty($this->keyValue->get($this->cacheKey)); + $this->assertEmpty($this->keyValue->getAll()); } /** @@ -81,7 +73,7 @@ class ClearStorageTest extends BrowserTestBase { */ public function testClearCacheWithDrush(string $command): void { $this->drush($command); - $this->assertEmpty($this->keyValue->get($this->cacheKey)); + $this->assertEmpty($this->keyValue->getAll()); } /** @@ -95,7 +87,7 @@ class ClearStorageTest extends BrowserTestBase { $assert_session = $this->assertSession(); $assert_session->buttonExists('Clear storage')->press(); $assert_session->statusMessageContains('Storage cleared.'); - $this->assertEmpty($this->keyValue->get($this->cacheKey)); + $this->assertEmpty($this->keyValue->getAll()); } } diff --git a/tests/src/Functional/EnabledSourceHandlerTest.php b/tests/src/Functional/EnabledSourceHandlerTest.php index f0b9ce8b4..aea25f80f 100644 --- a/tests/src/Functional/EnabledSourceHandlerTest.php +++ b/tests/src/Functional/EnabledSourceHandlerTest.php @@ -65,21 +65,26 @@ class EnabledSourceHandlerTest extends BrowserTestBase { public function testErrorsAreNotStored(): void { /** @var \Drupal\project_browser\EnabledSourceHandler $handler */ $handler = $this->container->get(EnabledSourceHandler::class); - $handler->getProjects('project_browser_test_mock'); - /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $storage */ - $storage = $this->container->get('keyvalue') - ->get('project_browser:project_browser_test_mock'); + + $has_cached_queries = function (): bool { + $items = $this->container->get('keyvalue') + ->get('project_browser:project_browser_test_mock') + ->getAll(); + return (bool) array_filter( + array_keys($items), + fn (string $key): bool => str_starts_with($key, 'query:'), + ); + }; // Query results should have been stored. - $query_cache_key = 'query:' . md5('[]'); - $this->assertTrue($storage->has($query_cache_key)); + $this->assertTrue($has_cached_queries()); $handler->clearStorage(); ProjectBrowserTestMock::$resultsError = 'Nope!'; $handler->getProjects('project_browser_test_mock'); // No query results should have been stored. - $this->assertFalse($storage->has($query_cache_key)); + $this->assertFalse($has_cached_queries()); } /** -- GitLab