diff --git a/project_browser.services.yml b/project_browser.services.yml index 1b89f7e7bb3b35b404a7e13507d2f93589503f1b..53c79727684ad2d0755d098ba07610b2a535245e 100644 --- a/project_browser.services.yml +++ b/project_browser.services.yml @@ -9,7 +9,7 @@ services: parent: default_plugin_manager Drupal\project_browser\EnabledSourceHandler: arguments: - $keyValueFactory: '@keyvalue.expirable' + $keyValueFactory: '@keyvalue' calls: - [setLogger, ['@logger.channel.project_browser']] tags: diff --git a/src/EnabledSourceHandler.php b/src/EnabledSourceHandler.php index 8cc8aee9bdbc7244ae4ef77833c536c51d020c18..d1372f36af551be06e2fcfc5eed62a9b25bc40ee 100644 --- a/src/EnabledSourceHandler.php +++ b/src/EnabledSourceHandler.php @@ -6,9 +6,8 @@ use Drupal\Component\Serialization\Json; use Drupal\Core\Config\ConfigCrudEvent; use Drupal\Core\Config\ConfigEvents; use Drupal\Core\Config\ConfigFactoryInterface; -use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; -use Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface; -use Drupal\project_browser\Plugin\ProjectBrowserSourceInterface; +use Drupal\Core\KeyValueStore\KeyValueFactoryInterface; +use Drupal\Core\KeyValueStore\KeyValueStoreInterface; use Drupal\project_browser\Plugin\ProjectBrowserSourceManager; use Drupal\project_browser\ProjectBrowser\Project; use Drupal\project_browser\ProjectBrowser\ProjectsResultsPage; @@ -23,20 +22,24 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter use LoggerAwareTrait; - /** - * The key-value storage. - * - * @var \Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface - */ - private readonly KeyValueStoreExpirableInterface $keyValue; - public function __construct( private readonly ConfigFactoryInterface $configFactory, private readonly ProjectBrowserSourceManager $pluginManager, private readonly ActivatorInterface $activator, - KeyValueExpirableFactoryInterface $keyValueFactory, - ) { - $this->keyValue = $keyValueFactory->get('project_browser'); + private readonly KeyValueFactoryInterface $keyValueFactory, + ) {} + + /** + * Returns a key-value store for a particular source plugin. + * + * @param string $source_id + * The ID of a source plugin. + * + * @return \Drupal\Core\KeyValueStore\KeyValueStoreInterface + * A key-value store for the specified source plugin. + */ + private function keyValue(string $source_id): KeyValueStoreInterface { + return $this->keyValueFactory->get("project_browser:$source_id"); } /** @@ -55,8 +58,18 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter * The event object. */ public function onConfigSave(ConfigCrudEvent $event): void { - if ($event->getConfig()->getName() === 'project_browser.admin_settings' && $event->isChanged('enabled_sources')) { - $this->keyValue->deleteAll(); + $config = $event->getConfig(); + if ($config->getName() === 'project_browser.admin_settings' && $event->isChanged('enabled_sources')) { + // Ensure that the cached plugin definitions stay in sync with the + // enabled sources. + $this->pluginManager->clearCachedDefinitions(); + + // Clear stored data for the sources that have been disabled. + $disabled_sources = array_diff( + $config->getOriginal('enabled_sources') ?? [], + $config->get('enabled_sources'), + ); + array_walk($disabled_sources, $this->clearStorage(...)); } } @@ -99,48 +112,39 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter // Cache only exact query, down to the page number. $cache_key = 'query:' . md5(Json::encode($query)); - $stored = $this->keyValue->get($cache_key); - if (is_array($stored)) { - // We store query results as a set of arguments to ProjectsResultsPage, - // although the list of projects is a list of project IDs, all of which - // we expect to be in the data store. - $arguments = $stored[$source_id]; - $arguments[1] = array_map($this->getStoredProject(...), $arguments[1]); - $projects = [ - $source_id => new ProjectsResultsPage(...$arguments), - ]; + $storage = $this->keyValue($source_id); + + $results = $storage->get($cache_key); + // If $results is an array, it's a set of arguments to ProjectsResultsPage, + // with a list of project IDs that we expect to be in the data store. + if (is_array($results)) { + $results[1] = array_map($this->getStoredProject(...), $results[1]); + $results = new ProjectsResultsPage(...$results); } else { - $projects = $this->doQuery($source_id, $query); - - if ($projects) { - $results = $projects[$source_id]; - - foreach ($results->list as $project) { - // Prefix the local project ID with the source plugin ID, so we can - // look it up unambiguously. - $project->id = $source_id . '/' . $project->id; - - $this->keyValue->setIfNotExists($project->id, $project); - // Add activation data to the project. This is volatile and should not - // be changed. - $this->getActivationData($project); - } - // Store each source's results for this query as a set of arguments to - // ProjectsResultsPage. - $stored = [ - $source_id => [ - $results->totalResults, - array_column($results->list, 'id'), - $results->pluginLabel, - $source_id, - $results->error, - ], - ]; - $this->keyValue->set($cache_key, $stored); + $results = $this->doQuery($source_id, $query); + + foreach ($results->list as $project) { + // Prefix the local project ID with the source plugin ID, so we can + // look it up unambiguously. + $project->id = $source_id . '/' . $project->id; + + $storage->setIfNotExists($project->id, $project); + // Add activation data to the project. This is volatile, which is why we + // never store it. + $this->getActivationData($project); } + // Store the results for this query as a set of arguments to + // ProjectsResultsPage. + $storage->set($cache_key, [ + $results->totalResults, + array_column($results->list, 'id'), + $results->pluginLabel, + $source_id, + $results->error, + ]); } - return $projects; + return [$source_id => $results]; } /** @@ -151,21 +155,21 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter * @param array $query * (optional) The query to pass to the specified source. * - * @return \Drupal\project_browser\ProjectBrowser\ProjectsResultsPage[] - * The results of the query, keyed by source plugin ID. + * @return \Drupal\project_browser\ProjectBrowser\ProjectsResultsPage + * The results of the query. * * @see \Drupal\project_browser\Plugin\ProjectBrowserSourceInterface::getProjects() */ - private function doQuery(string $source_id, array $query = []): array { + private function doQuery(string $source_id, array $query = []): ProjectsResultsPage { $query['categories'] ??= ''; $tabwise_categories = Json::decode($query['tabwise_categories'] ?? '[]'); unset($query['tabwise_categories']); - - $source = $this->getCurrentSources()[$source_id]; $query['categories'] = implode(", ", $tabwise_categories[$source_id] ?? []); - return [$source_id => $source->getProjects($query)]; + $enabled_sources = $this->getCurrentSources(); + assert(array_key_exists($source_id, $enabled_sources)); + return $enabled_sources[$source_id]->getProjects($query); } /** @@ -175,15 +179,12 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter * The available categories, keyed by source plugin ID. */ public function getCategories(): array { - $cache_key = 'categories'; - $categories = $this->keyValue->get($cache_key); + $categories = []; - if ($categories === NULL) { - $categories = array_map( - fn (ProjectBrowserSourceInterface $source) => $source->getCategories(), - $this->getCurrentSources(), - ); - $this->keyValue->set($cache_key, $categories); + foreach ($this->getCurrentSources() as $id => $source) { + $storage = $this->keyValue($id); + $categories[$id] = $storage->get('categories', $source->getCategories()); + $storage->setIfNotExists('categories', $categories[$id]); } return $categories; } @@ -201,7 +202,8 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter * Thrown if the project is not found in the non-volatile data store. */ public function getStoredProject(string $id): Project { - $project = $this->keyValue->get($id) ?? throw new \RuntimeException("Project '$id' was not found in non-volatile storage."); + [$source_id] = explode('/', $id, 2); + $project = $this->keyValue($source_id)->get($id) ?? throw new \RuntimeException("Project '$id' was not found in non-volatile storage."); $this->getActivationData($project); return $project; } @@ -224,9 +226,20 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter /** * Clears the key-value store so it can be re-fetched. + * + * @param string|null $source_id + * (optional) The ID of the source for which data should be cleared. If + * NULL, stored data is cleared for all enabled sources. Defaults to NULL. */ - public function clearStorage(): void { - $this->keyValue->deleteAll(); + public function clearStorage(?string $source_id = NULL): void { + if ($source_id) { + $this->keyValue($source_id)->deleteAll(); + } + else { + foreach ($this->getCurrentSources() as $source) { + $this->clearStorage($source->getPluginId()); + } + } } } diff --git a/tests/src/Functional/ClearStorageTest.php b/tests/src/Functional/ClearStorageTest.php index 85e3599493fd631155e9f004808a620392176576..47c893922c019cb60590991f0a84ff7be8522151 100644 --- a/tests/src/Functional/ClearStorageTest.php +++ b/tests/src/Functional/ClearStorageTest.php @@ -2,7 +2,7 @@ namespace Drupal\Tests\project_browser\Functional; -use Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface; +use Drupal\Core\KeyValueStore\KeyValueStoreInterface; use Drupal\project_browser\EnabledSourceHandler; use Drupal\Tests\BrowserTestBase; use Drush\TestTraits\DrushTestTrait; @@ -36,9 +36,9 @@ class ClearStorageTest extends BrowserTestBase { /** * The key-value storage. * - * @var \Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface + * @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface */ - private readonly KeyValueStoreExpirableInterface $keyValue; + private readonly KeyValueStoreInterface $keyValue; /** * {@inheritdoc} @@ -51,7 +51,7 @@ class ClearStorageTest extends BrowserTestBase { ->save(); $this->cacheKey = 'query:' . md5('[]'); - $this->keyValue = \Drupal::service('keyvalue.expirable')->get('project_browser'); + $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'); diff --git a/tests/src/Kernel/EnabledSourceHandlerTest.php b/tests/src/Functional/EnabledSourceHandlerTest.php similarity index 73% rename from tests/src/Kernel/EnabledSourceHandlerTest.php rename to tests/src/Functional/EnabledSourceHandlerTest.php index e20e6b0a727e8b6aadbb0f9d4905954de412278e..1b587717038728659b24b6af1dfcb53a6b400d99 100644 --- a/tests/src/Kernel/EnabledSourceHandlerTest.php +++ b/tests/src/Functional/EnabledSourceHandlerTest.php @@ -2,48 +2,37 @@ declare(strict_types=1); -namespace Drupal\Tests\project_browser\Kernel; +namespace Drupal\Tests\project_browser\Functional; -use Drupal\Core\Extension\ModuleInstallerInterface; -use Drupal\KernelTests\KernelTestBase; use Drupal\project_browser\EnabledSourceHandler; use Drupal\project_browser\ProjectBrowser\Project; +use Drupal\Tests\BrowserTestBase; /** * @covers \Drupal\project_browser\EnabledSourceHandler * @group project_browser */ -class EnabledSourceHandlerTest extends KernelTestBase { +class EnabledSourceHandlerTest extends BrowserTestBase { /** * {@inheritdoc} */ - protected static $modules = [ - 'project_browser', - 'project_browser_test', - 'system', - ]; + protected static $modules = ['project_browser_test']; + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; /** * {@inheritdoc} */ protected function setUp(): void { parent::setUp(); - $this->installSchema('project_browser_test', [ - 'project_browser_projects', - 'project_browser_categories', - ]); - $this->installConfig('project_browser_test'); - $this->config('project_browser.admin_settings')->set('enabled_sources', ['project_browser_test_mock'])->save(TRUE); - - $this->container->get(ModuleInstallerInterface::class)->install([ - 'project_browser_test', - 'project_browser', - ]); - - $this->container->get('module_handler')->loadInclude('project_browser_test', 'install'); - // Hooks are not ran on kernel tests, so trigger it. - project_browser_test_install(); + + $this->config('project_browser.admin_settings') + ->set('enabled_sources', ['project_browser_test_mock']) + ->save(TRUE); } /** @@ -91,8 +80,8 @@ class EnabledSourceHandlerTest extends KernelTestBase { // But if we pull the project directly from the data store, the `status` and // `commands` properties should be uninitialized. - $project = $this->container->get('keyvalue.expirable') - ->get('project_browser') + $project = $this->container->get('keyvalue') + ->get('project_browser:project_browser_test_mock') ->get($project->id); $this->assertInstanceOf(Project::class, $project); $this->assertFalse(self::hasActivationData($project));