From 28ae2978b14f2138f6d55396efe05a8f2eaf38c4 Mon Sep 17 00:00:00 2001 From: tuutti Date: Tue, 26 Sep 2023 09:23:47 +0300 Subject: [PATCH 1/3] Revert "Revert "UHF-8526: Use PubSub to invalidate caches"" --- README.md | 8 +- helfi_navigation.module | 42 +++------ helfi_navigation.services.yml | 11 +-- src/ApiManager.php | 39 ++++---- src/CacheWarmer.php | 90 ------------------ src/ExternalMenuTreeBuilder.php | 6 +- src/MainMenuManager.php | 4 +- src/Menu/MenuTreeBuilder.php | 10 +- src/Plugin/QueueWorker/MenuQueue.php | 42 +++++++-- tests/src/Functional/MenuBlockTest.php | 1 + tests/src/Kernel/CacheWarmerTest.php | 91 ------------------- tests/src/Kernel/KernelTestBase.php | 2 + tests/src/Kernel/MenuSyncTest.php | 13 +-- tests/src/Unit/ApiManagerTest.php | 21 +++-- .../Unit/Plugin/QueueWorker/MenuQueueTest.php | 9 +- 15 files changed, 114 insertions(+), 275 deletions(-) delete mode 100644 src/CacheWarmer.php delete mode 100644 tests/src/Kernel/CacheWarmerTest.php diff --git a/README.md b/README.md index 0c346d6..cfd29f8 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,13 @@ Only main-navigation has syncing option. Other navigations are created in Etusiv - run `drush upwd helfi-admin 123` to update admin password. it is used as api key in other instances. - Setup any other instance with helfi_navigation module enabled. - Add following line to local.settings.php. Otherwise syncing global navigation won't work - - `$config['helfi_navigation.api']['key'] = base64_encode('helfi-admin:123');` + ```php + $config['helfi_api_base.api_accounts']['vault'][] = [ + 'id' => 'helfi_navigation', + 'plugin' => 'authorization_token', + 'data' => base64_encode('helfi-admin:123'), + ]; + ``` ### Steps after both instances are up and running. 1. Edit and save menu on any instance with helfi_navigation module enabled. diff --git a/helfi_navigation.module b/helfi_navigation.module index 3b18003..5a5f714 100644 --- a/helfi_navigation.module +++ b/helfi_navigation.module @@ -93,62 +93,48 @@ function helfi_navigation_preprocess_menu__external_menu__fallback(&$variables) * Implements hook_ENTITY_TYPE_update(). */ function helfi_navigation_menu_update(MenuInterface $entity) : void { - if ($entity->id() === 'main') { - _helfi_navigation_queue_item(); - } + _helfi_navigation_queue_item($entity->id(), $entity->language()->getId(), 'update'); } /** * Implements hook_ENTITY_TYPE_update(). */ function helfi_navigation_menu_link_content_update(MenuLinkContentInterface $entity) : void { - if ($entity->getMenuName() === 'main') { - _helfi_navigation_queue_item(); - } + _helfi_navigation_queue_item($entity->getMenuName(), $entity->language()->getId(), 'update'); } /** * Implements hook_ENTITY_TYPE_insert(). */ function helfi_navigation_menu_link_content_insert(MenuLinkContentInterface $entity) : void { - if ($entity->getMenuName() === 'main') { - _helfi_navigation_queue_item(); - } + _helfi_navigation_queue_item($entity->getMenuName(), $entity->language()->getId(), 'insert'); } /** * Implements hook_ENTITY_TYPE_delete(). */ function helfi_navigation_menu_link_content_delete(MenuLinkContentInterface $entity) : void { - if ($entity->getMenuName() === 'main') { - _helfi_navigation_queue_item(); - } + _helfi_navigation_queue_item($entity->getMenuName(), $entity->language()->getId(), 'delete'); } /** * Create menu update queue item. */ -function _helfi_navigation_queue_item() : void { - if (!\Drupal::config('helfi_navigation.api')->get('key')) { +function _helfi_navigation_queue_item(string $menuName, string $langcode, string $action) : void { + if (!\Drupal::service('helfi_navigation.api_authorization')->getAuthorization()) { return; } $queue = Drupal::queue('helfi_navigation_menu_queue'); - // Queue items only when queue is empty. - if ($queue->numberOfItems() === 0) { - foreach (\Drupal::languageManager()->getLanguages() as $language) { - $queue->createItem($language->getId()); - } - } -} + static $items = []; -/** - * Implements hook_cron(). - */ -function helfi_navigation_cron() : void { - /** @var \Drupal\helfi_navigation\CacheWarmer $warmer */ - $warmer = Drupal::service('helfi_navigation.cache_warmer'); - $warmer->warm(); + $key = sprintf('%s:%s:%s', $menuName, $langcode, $action); + + // Queue item once per request. + if (!isset($items[$key])) { + $queue->createItem(['menu' => $menuName, 'language' => $langcode]); + $items[$key] = $key; + } } /** diff --git a/helfi_navigation.services.yml b/helfi_navigation.services.yml index 5be6fd6..d3cc6d8 100644 --- a/helfi_navigation.services.yml +++ b/helfi_navigation.services.yml @@ -1,3 +1,5 @@ +parameters: + helfi_navigation.request_timeout: 15 services: logger.channel.helfi_navigation: parent: logger.channel_base @@ -16,6 +18,7 @@ services: - '@helfi_api_base.environment_resolver' - '@logger.channel.helfi_navigation' - '@helfi_navigation.api_authorization' + - '%helfi_navigation.request_timeout%' helfi_navigation.menu_manager: class: Drupal\helfi_navigation\MainMenuManager arguments: @@ -32,14 +35,6 @@ services: - '@menu.link_tree' - '@plugin.manager.menu.link' - '@event_dispatcher' - helfi_navigation.cache_warmer: - class: Drupal\helfi_navigation\CacheWarmer - arguments: - - '@tempstore.shared' - - '@language_manager' - - '@cache_tags.invalidator' - - '@helfi_navigation.api_manager' - helfi_navigation.api_authorization: class: Drupal\helfi_navigation\ApiAuthorization arguments: diff --git a/src/ApiManager.php b/src/ApiManager.php index e838cc6..9e77606 100644 --- a/src/ApiManager.php +++ b/src/ApiManager.php @@ -20,7 +20,7 @@ use Psr\Log\LoggerInterface; /** - * Service class for global navigation related functions. + * Service class for global navigation-related functions. */ class ApiManager { @@ -58,6 +58,8 @@ class ApiManager { * Logger channel. * @param \Drupal\helfi_navigation\ApiAuthorization $apiAuthorization * The API authorization service. + * @param int $requestTimeout + * The request timeout. */ public function __construct( private readonly TimeInterface $time, @@ -65,7 +67,8 @@ public function __construct( private readonly ClientInterface $httpClient, private readonly EnvironmentResolverInterface $environmentResolver, private readonly LoggerInterface $logger, - private readonly ApiAuthorization $apiAuthorization + private readonly ApiAuthorization $apiAuthorization, + private readonly int $requestTimeout, ) { } @@ -99,7 +102,7 @@ private function cache(string $key, callable $callback) : ? CacheValue { $value = ($cache = $this->cache->get($key)) ? $cache->data : NULL; // Attempt to re-fetch the data in case cache does not exist, cache has - // expired or bypass cache is set to true. + // expired, or bypass cache is set to true. if ( ($value instanceof CacheValue && $value->hasExpired($this->time->getRequestTime())) || $this->bypassCache || @@ -128,7 +131,7 @@ private function cache(string $key, callable $callback) : ? CacheValue { } /** - * Makes a request to fetch external menu from Etusivu instance. + * Makes a request to fetch an external menu from Etusivu instance. * * @param string $langcode * The langcode. @@ -138,7 +141,7 @@ private function cache(string $key, callable $callback) : ? CacheValue { * The request options. * * @return \Drupal\helfi_navigation\ApiResponse - * The JSON object representing external menu. + * The JSON object representing the external menu. * * @throws \GuzzleHttp\Exception\GuzzleException */ @@ -159,13 +162,13 @@ public function get( new CacheValue( $this->makeRequest('GET', $endpoint, $langcode, $options), $this->time->getRequestTime(), - ['external_menu:%s:%s', $menuId, $langcode], + [sprintf('external_menu:%s:%s', $menuId, $langcode)], ) )->value; } /** - * Updates the main menu for currently active project. + * Updates the main menu for the currently active project. * * @param string $langcode * The langcode. @@ -194,25 +197,23 @@ public function update(string $langcode, array $data) : ApiResponse { * * @param string $environmentName * Environment name. + * @param array $options + * The optional options. * * @return array * The request options. */ - private function getDefaultRequestOptions(string $environmentName) : array { - $options = ['timeout' => 15]; - $options['curl'] = [CURLOPT_TCP_KEEPALIVE => TRUE]; - - if (drupal_valid_test_ua()) { - // Speed up mock tests by using very low request timeout value when - // running tests. - $options['timeout'] = 1; - } + private function getRequestOptions(string $environmentName, array $options = []) : array { + $default = [ + 'timeout' => $this->requestTimeout, + 'curl' => [CURLOPT_TCP_KEEPALIVE => TRUE], + ]; if ($environmentName === 'local') { // Disable SSL verification in local environment. - $options['verify'] = FALSE; + $default['verify'] = FALSE; } - return $options; + return array_merge_recursive($options, $default); } /** @@ -296,7 +297,7 @@ private function makeRequest( $url = $this->getUrl('api', $langcode, ['endpoint' => $endpoint]); - $options = array_merge_recursive($options, $this->getDefaultRequestOptions($activeEnvironmentName)); + $options = $this->getRequestOptions($activeEnvironmentName, $options); try { if ($this->previousException instanceof \Exception) { diff --git a/src/CacheWarmer.php b/src/CacheWarmer.php deleted file mode 100644 index 34612df..0000000 --- a/src/CacheWarmer.php +++ /dev/null @@ -1,90 +0,0 @@ -tempStoreFactory->get(self::STORAGE_KEY); - - $currentHash = $storage->get($key); - $hash = hash('sha256', serialize($data)); - - // Only invalidate tags if content has actually changed. - if ($currentHash === $hash) { - return; - } - $storage->set($key, $hash); - // Invalidate menu block instances. - $this->cacheTagsInvalidator->invalidateTags(['config:system.menu.' . $menuName]); - } - - /** - * Warm caches for all available external menus. - */ - public function warm() : void { - $plugin = new ExternalMenuBlock(); - $derives = array_keys($plugin->getDerivativeDefinitions([])); - $derives[] = 'main'; - - foreach ($this->languageManager->getLanguages() as $language) { - foreach ($derives as $name) { - try { - $response = $this->apiManager - ->withBypassCache() - ->get($language->getId(), $name); - $this->invalidateTags($response, $language->getId(), $name); - } - catch (\Exception) { - } - } - } - } - -} diff --git a/src/ExternalMenuTreeBuilder.php b/src/ExternalMenuTreeBuilder.php index d48952c..4fa8277 100644 --- a/src/ExternalMenuTreeBuilder.php +++ b/src/ExternalMenuTreeBuilder.php @@ -141,10 +141,6 @@ private function createLink( // Parse the URL. $item->url = !empty($item->url) ? UrlHelper::parse($item->url) : new Url(''); - - if (!isset($item->parentId)) { - $item->parentId = NULL; - } $item->external = $this->domainResolver->isExternal($item->url); if (isset($item->weight)) { @@ -162,7 +158,7 @@ private function createLink( 'attributes' => new Attribute($item->attributes ?? []), 'title' => $item->name, 'id' => $item->id, - 'parent_id' => $item->parentId, + 'parent_id' => $item->parentId ?? NULL, 'is_expanded' => $expand_all_items || !empty($item->expanded), 'in_active_trail' => $inActiveTrail, 'is_currentPage' => $inActiveTrail, diff --git a/src/MainMenuManager.php b/src/MainMenuManager.php index 9a0ee41..eb92f0f 100644 --- a/src/MainMenuManager.php +++ b/src/MainMenuManager.php @@ -40,7 +40,7 @@ public function __construct( } /** - * Sends main menu tree to frontpage instance. + * Sends the main menu tree to Etusivu instance. * * @param string $langcode * The langcode. @@ -49,7 +49,7 @@ public function __construct( * @throws \InvalidArgumentException */ public function sync(string $langcode): bool { - // Sync menu as an anonymous user to make sure no sensitive + // Sync the menu as an anonymous user to make sure no sensitive // links are synced. $this->accountSwitcher->switchTo(new AnonymousUserSession()); $response = $this->apiManager->update( diff --git a/src/Menu/MenuTreeBuilder.php b/src/Menu/MenuTreeBuilder.php index faeb81c..4685e57 100644 --- a/src/Menu/MenuTreeBuilder.php +++ b/src/Menu/MenuTreeBuilder.php @@ -30,7 +30,7 @@ final class MenuTreeBuilder { * @param \Drupal\helfi_api_base\Link\InternalDomainResolver $domainResolver * The internal domain resolver. * @param \Drupal\Core\Menu\MenuLinkTreeInterface $menuTree - * The menu link tree builder service. + * The 'menu link tree builder' service. * @param \Drupal\Core\Menu\MenuLinkManagerInterface $menuLinkManager * The menu link manager. * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $eventDispatcher @@ -157,7 +157,7 @@ private function transform(array $menuItems, string $langcode, string $rootId = $urlObject = $link->getUrlObject(); - // Make sure url object retains the language information. + // Make sure the url object retains the language information. if (!$urlObject->getOption('language')) { $urlObject->setOptions(['language' => $link->language()]); } @@ -269,9 +269,9 @@ private function getEntity(MenuLinkInterface $link, string $langcode): ? MenuLin * The element to check entity access for. */ private function evaluateEntityAccess(MenuLinkTreeElement $element) : void { - // Attempt to fetch the entity type and id from link's route parameters. - // The route parameters should be an array containing entity type => id - // like: ['node' => '1']. + // Attempt to fetch the entity type, and id from link's route parameters. + // The route parameters should be an array containing an entity type => id + // key pairs, like: ['node' => '1']. $routeParameters = $element->link->getRouteParameters(); $entityType = key($routeParameters); diff --git a/src/Plugin/QueueWorker/MenuQueue.php b/src/Plugin/QueueWorker/MenuQueue.php index f29be38..184a379 100644 --- a/src/Plugin/QueueWorker/MenuQueue.php +++ b/src/Plugin/QueueWorker/MenuQueue.php @@ -6,6 +6,7 @@ use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Queue\QueueWorkerBase; +use Drupal\helfi_api_base\Cache\CacheTagInvalidator; use Drupal\helfi_navigation\MainMenuManager; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -27,34 +28,57 @@ final class MenuQueue extends QueueWorkerBase implements ContainerFactoryPluginI */ private MainMenuManager $mainMenuManager; + /** + * The cache tag invalidator service. + * + * @var \Drupal\helfi_api_base\Cache\CacheTagInvalidator + */ + private CacheTagInvalidator $cacheTagInvalidator; + /** * {@inheritdoc} */ public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : self { $instance = new self($configuration, $plugin_id, $plugin_definition); $instance->mainMenuManager = $container->get('helfi_navigation.menu_manager'); + $instance->cacheTagInvalidator = $container->get('helfi_api_base.cache_tag_invalidator'); return $instance; } /** * Process queue item. * - * @param string $data - * Data of the processable language code. + * @param array|mixed $data + * The queue data. Should contain 'menu' and 'language'. * * @throws \Exception * Throws exception if language code is not set. */ public function processItem($data) : void { - // Data is a langcode, like 'fi' or 'en'. - if (!is_string($data)) { + if (!isset($data['menu'], $data['language'])) { return; } - try { - $this->mainMenuManager->sync($data); - } - catch (\Throwable) { - // The failed sync will be logged by ApiManager. + ['menu' => $menuName, 'language' => $language] = $data; + + $this->cacheTagInvalidator->invalidateTags([ + // These are used by the menu block itself and local Global mobile menu + // REST API endpoint. + sprintf('config:system.menu.%s', $menuName), + sprintf('external_menu_block:%s', $menuName), + // This is used by ApiManager service to cache the API response + // locally. + sprintf('external_menu:%s:%s', $menuName, $language), + // This is used by REST API collection endpoint on Etusivu. + 'config:rest.resource.helfi_global_menu_collection', + ]); + + if ($menuName === 'main') { + try { + $this->mainMenuManager->sync($language); + } + catch (\Throwable) { + // The failed sync will be logged by ApiManager. + } } } diff --git a/tests/src/Functional/MenuBlockTest.php b/tests/src/Functional/MenuBlockTest.php index 9589f2e..6d5a6bc 100644 --- a/tests/src/Functional/MenuBlockTest.php +++ b/tests/src/Functional/MenuBlockTest.php @@ -65,6 +65,7 @@ public function setUp() : void { $this->setActiveProject(Project::ASUMINEN, EnvironmentEnum::Local); _helfi_navigation_generate_blocks('stark', 'content', TRUE); + $this->setContainerParameter('helfi_navigation.request_timeout', 1); } /** diff --git a/tests/src/Kernel/CacheWarmerTest.php b/tests/src/Kernel/CacheWarmerTest.php deleted file mode 100644 index 90ebb2a..0000000 --- a/tests/src/Kernel/CacheWarmerTest.php +++ /dev/null @@ -1,91 +0,0 @@ -populateConfiguration('Test'); - $this->setActiveProject(Project::ASUMINEN, EnvironmentEnum::Local); - $this->sharedTempStore = $this->container->get('tempstore.shared'); - } - - /** - * Constructs a new cache warmer instance. - * - * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $invalidator - * The cache invalidator service. - * - * @return \Drupal\helfi_navigation\CacheWarmer - * The cache warmer service. - */ - private function getCacheWarmer(CacheTagsInvalidatorInterface $invalidator) : CacheWarmer { - return new CacheWarmer( - $this->sharedTempStore, - $this->container->get('language_manager'), - $invalidator, - $this->container->get('helfi_navigation.api_manager'), - ); - } - - /** - * Tests cache warmer. - */ - public function testCacheWarmer() : void { - $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); - $invalidator->invalidateTags(Argument::cetera()) - ->shouldBeCalled(); - - // Warm caches and make sure all tags are invalidated when nothing is - // populated yet. - $this->getCacheWarmer($invalidator->reveal())->warm(); - - // Warming caches again should not invalidate anything since hash - // hasn't changed. - $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); - $invalidator->invalidateTags(Argument::cetera()) - ->shouldNotBeCalled(); - $this->getCacheWarmer($invalidator->reveal())->warm(); - - // Reset hash for finnish main menu and make sure main menu block is - // invalidated (once). - $this->sharedTempStore->get(CacheWarmer::STORAGE_KEY)->set('fi:main', ''); - $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); - $invalidator->invalidateTags(['config:system.menu.main']) - ->shouldBeCalledTimes(1); - $this->getCacheWarmer($invalidator->reveal())->warm(); - } - -} diff --git a/tests/src/Kernel/KernelTestBase.php b/tests/src/Kernel/KernelTestBase.php index 7dac83b..1cd1a2e 100644 --- a/tests/src/Kernel/KernelTestBase.php +++ b/tests/src/Kernel/KernelTestBase.php @@ -63,6 +63,8 @@ protected function setUp() : void { * {@inheritdoc} */ public function register(ContainerBuilder $container) { + $container->setParameter('helfi_navigation.request_timeout', 1); + parent::register($container); // Core's KernelTestBase removes service_collector tags from diff --git a/tests/src/Kernel/MenuSyncTest.php b/tests/src/Kernel/MenuSyncTest.php index 8f2b9f9..0c422be 100644 --- a/tests/src/Kernel/MenuSyncTest.php +++ b/tests/src/Kernel/MenuSyncTest.php @@ -48,7 +48,7 @@ private function getMenuManager() : MainMenuManager { public function testQueueNoApiKey() : void { $queue = $this->getQueue(); - _helfi_navigation_queue_item(); + _helfi_navigation_queue_item('main', 'fi', 'insert'); $this->assertEquals(0, $queue->numberOfItems()); } @@ -59,8 +59,9 @@ public function testQueue() : void { $this->config('helfi_navigation.api')->set('key', '123')->save(); $queue = $this->getQueue(); - _helfi_navigation_queue_item(); - $this->assertEquals(3, $queue->numberOfItems()); + _helfi_navigation_queue_item('main', 'fi', 'insert'); + _helfi_navigation_queue_item('main', 'fi', 'insert'); + $this->assertEquals(1, $queue->numberOfItems()); } /** @@ -89,15 +90,15 @@ public function testEntityHooks() : void { // Make sure entity insert queues items. $link = $this->createTestLink(['link' => ['uri' => 'internal:/']]); - $this->assertQueueCount(3); + $this->assertQueueCount(1); // Make sure entity update queues items. $link->save(); - $this->assertQueueCount(3); + $this->assertQueueCount(1); // Make sure entity delete. $link->delete(); - $this->assertQueueCount(3); + $this->assertQueueCount(1); } /** diff --git a/tests/src/Unit/ApiManagerTest.php b/tests/src/Unit/ApiManagerTest.php index bbe3254..aeb38d1 100644 --- a/tests/src/Unit/ApiManagerTest.php +++ b/tests/src/Unit/ApiManagerTest.php @@ -132,6 +132,7 @@ private function getSut( new AuthorizationToken(ApiAuthorization::VAULT_MANAGER_KEY, '123'), ]) ), + 1, ); } @@ -141,7 +142,7 @@ private function getSut( * @covers ::update * @covers ::__construct * @covers ::makeRequest - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers ::hasAuthorization * @covers ::getAuthorization @@ -174,7 +175,7 @@ public function testUpdateMainMenu() : void { * @covers ::__construct * @covers ::makeRequest * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::hasExpired * @covers \Drupal\helfi_navigation\CacheValue::__construct @@ -207,7 +208,7 @@ public function testGetExternalMenu() : void { * @covers ::__construct * @covers ::makeRequest * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::hasExpired * @covers \Drupal\helfi_navigation\CacheValue::__construct @@ -239,7 +240,7 @@ public function testGetMainMenu() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::hasExpired * @covers \Drupal\helfi_navigation\CacheValue::__construct @@ -277,7 +278,7 @@ public function testStaleCacheOnRequestFailure() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::hasExpired * @covers \Drupal\helfi_navigation\CacheValue::__construct @@ -322,7 +323,7 @@ public function testStaleCacheUpdate() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\ApiAuthorization::__construct * @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization @@ -348,7 +349,7 @@ public function testRequestLoggingException() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\ApiAuthorization::__construct * @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization @@ -376,7 +377,7 @@ public function testMockFallbackException() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::__construct * @covers \Drupal\helfi_navigation\ApiResponse::__construct @@ -409,7 +410,7 @@ public function testMockFallback() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::getUrl * @covers \Drupal\helfi_navigation\ApiAuthorization::__construct * @covers \Drupal\helfi_navigation\ApiAuthorization::getAuthorization @@ -448,7 +449,7 @@ public function testFastRequestFailure() : void { * @covers ::get * @covers ::__construct * @covers ::cache - * @covers ::getDefaultRequestOptions + * @covers ::getRequestOptions * @covers ::withBypassCache * @covers ::getUrl * @covers \Drupal\helfi_navigation\CacheValue::hasExpired diff --git a/tests/src/Unit/Plugin/QueueWorker/MenuQueueTest.php b/tests/src/Unit/Plugin/QueueWorker/MenuQueueTest.php index 886e87c..fed6014 100644 --- a/tests/src/Unit/Plugin/QueueWorker/MenuQueueTest.php +++ b/tests/src/Unit/Plugin/QueueWorker/MenuQueueTest.php @@ -4,6 +4,8 @@ namespace Drupal\Tests\helfi_navigation\Unit\Plugin\QueueWorker; +use Drupal\helfi_api_base\Azure\PubSub\PubSubManagerInterface; +use Drupal\helfi_api_base\Cache\CacheTagInvalidator; use Drupal\helfi_navigation\MainMenuManager; use Drupal\helfi_navigation\Plugin\QueueWorker\MenuQueue; use Drupal\Tests\UnitTestCase; @@ -32,6 +34,10 @@ class MenuQueueTest extends UnitTestCase { public function getSut(ObjectProphecy $menuManager) : MenuQueue { $container = new ContainerBuilder(); $container->set('helfi_navigation.menu_manager', $menuManager->reveal()); + $pubSubManager = $this->prophesize(PubSubManagerInterface::class); + $pubSubManager->sendMessage(Argument::any())->willReturn($pubSubManager->reveal()); + $cacheTagInvalidator = new CacheTagInvalidator($pubSubManager->reveal()); + $container->set('helfi_api_base.cache_tag_invalidator', $cacheTagInvalidator); return MenuQueue::create($container, [], '', []); } @@ -57,7 +63,8 @@ public function testQueueException() : void { $menuManager->sync(Argument::any()) ->shouldBeCalled() ->willThrow(new \InvalidArgumentException()); - $this->getSut($menuManager)->processItem('fi'); + $this->getSut($menuManager) + ->processItem(['menu' => 'main', 'language' => 'fi']); } } From d507958d2784afa97f3d2db4e168e66d9e7d46eb Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 5 Oct 2023 16:06:10 +0300 Subject: [PATCH 2/3] revert-43-revert-42-UHF-8526: Readded cache warmer --- helfi_navigation.services.yml | 7 +++ src/CacheWarmer.php | 90 +++++++++++++++++++++++++++ src/Plugin/QueueWorker/MenuQueue.php | 24 +++++--- tests/src/Kernel/CacheWarmerTest.php | 91 ++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 9 deletions(-) create mode 100644 src/CacheWarmer.php create mode 100644 tests/src/Kernel/CacheWarmerTest.php diff --git a/helfi_navigation.services.yml b/helfi_navigation.services.yml index d3cc6d8..16bc032 100644 --- a/helfi_navigation.services.yml +++ b/helfi_navigation.services.yml @@ -35,6 +35,13 @@ services: - '@menu.link_tree' - '@plugin.manager.menu.link' - '@event_dispatcher' + helfi_navigation.cache_warmer: + class: Drupal\helfi_navigation\CacheWarmer + arguments: + - '@tempstore.shared' + - '@language_manager' + - '@cache_tags.invalidator' + - '@helfi_navigation.api_manager' helfi_navigation.api_authorization: class: Drupal\helfi_navigation\ApiAuthorization arguments: diff --git a/src/CacheWarmer.php b/src/CacheWarmer.php new file mode 100644 index 0000000..34612df --- /dev/null +++ b/src/CacheWarmer.php @@ -0,0 +1,90 @@ +tempStoreFactory->get(self::STORAGE_KEY); + + $currentHash = $storage->get($key); + $hash = hash('sha256', serialize($data)); + + // Only invalidate tags if content has actually changed. + if ($currentHash === $hash) { + return; + } + $storage->set($key, $hash); + // Invalidate menu block instances. + $this->cacheTagsInvalidator->invalidateTags(['config:system.menu.' . $menuName]); + } + + /** + * Warm caches for all available external menus. + */ + public function warm() : void { + $plugin = new ExternalMenuBlock(); + $derives = array_keys($plugin->getDerivativeDefinitions([])); + $derives[] = 'main'; + + foreach ($this->languageManager->getLanguages() as $language) { + foreach ($derives as $name) { + try { + $response = $this->apiManager + ->withBypassCache() + ->get($language->getId(), $name); + $this->invalidateTags($response, $language->getId(), $name); + } + catch (\Exception) { + } + } + } + } + +} diff --git a/src/Plugin/QueueWorker/MenuQueue.php b/src/Plugin/QueueWorker/MenuQueue.php index 184a379..8591c44 100644 --- a/src/Plugin/QueueWorker/MenuQueue.php +++ b/src/Plugin/QueueWorker/MenuQueue.php @@ -7,6 +7,7 @@ use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Queue\QueueWorkerBase; use Drupal\helfi_api_base\Cache\CacheTagInvalidator; +use Drupal\helfi_api_base\Environment\Project; use Drupal\helfi_navigation\MainMenuManager; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -58,8 +59,19 @@ public function processItem($data) : void { if (!isset($data['menu'], $data['language'])) { return; } + sleep(10); + ['menu' => $menuName, 'language' => $language] = $data; + if ($menuName === 'main') { + try { + $this->mainMenuManager->sync($language); + } + catch (\Throwable) { + // The failed sync will be logged by ApiManager. + } + } + $this->cacheTagInvalidator->invalidateTags([ // These are used by the menu block itself and local Global mobile menu // REST API endpoint. @@ -68,18 +80,12 @@ public function processItem($data) : void { // This is used by ApiManager service to cache the API response // locally. sprintf('external_menu:%s:%s', $menuName, $language), + ]); + $this->cacheTagInvalidator->invalidateTags([ // This is used by REST API collection endpoint on Etusivu. 'config:rest.resource.helfi_global_menu_collection', - ]); + ], [Project::ETUSIVU]); - if ($menuName === 'main') { - try { - $this->mainMenuManager->sync($language); - } - catch (\Throwable) { - // The failed sync will be logged by ApiManager. - } - } } } diff --git a/tests/src/Kernel/CacheWarmerTest.php b/tests/src/Kernel/CacheWarmerTest.php new file mode 100644 index 0000000..90ebb2a --- /dev/null +++ b/tests/src/Kernel/CacheWarmerTest.php @@ -0,0 +1,91 @@ +populateConfiguration('Test'); + $this->setActiveProject(Project::ASUMINEN, EnvironmentEnum::Local); + $this->sharedTempStore = $this->container->get('tempstore.shared'); + } + + /** + * Constructs a new cache warmer instance. + * + * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $invalidator + * The cache invalidator service. + * + * @return \Drupal\helfi_navigation\CacheWarmer + * The cache warmer service. + */ + private function getCacheWarmer(CacheTagsInvalidatorInterface $invalidator) : CacheWarmer { + return new CacheWarmer( + $this->sharedTempStore, + $this->container->get('language_manager'), + $invalidator, + $this->container->get('helfi_navigation.api_manager'), + ); + } + + /** + * Tests cache warmer. + */ + public function testCacheWarmer() : void { + $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); + $invalidator->invalidateTags(Argument::cetera()) + ->shouldBeCalled(); + + // Warm caches and make sure all tags are invalidated when nothing is + // populated yet. + $this->getCacheWarmer($invalidator->reveal())->warm(); + + // Warming caches again should not invalidate anything since hash + // hasn't changed. + $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); + $invalidator->invalidateTags(Argument::cetera()) + ->shouldNotBeCalled(); + $this->getCacheWarmer($invalidator->reveal())->warm(); + + // Reset hash for finnish main menu and make sure main menu block is + // invalidated (once). + $this->sharedTempStore->get(CacheWarmer::STORAGE_KEY)->set('fi:main', ''); + $invalidator = $this->prophesize(CacheTagsInvalidatorInterface::class); + $invalidator->invalidateTags(['config:system.menu.main']) + ->shouldBeCalledTimes(1); + $this->getCacheWarmer($invalidator->reveal())->warm(); + } + +} From f3aa50919305370c16374576bcd125f1923a1586 Mon Sep 17 00:00:00 2001 From: tuutti Date: Thu, 5 Oct 2023 16:11:49 +0300 Subject: [PATCH 3/3] revert-43-revert-42-UHF-8526: Removed debug sleep() --- src/Plugin/QueueWorker/MenuQueue.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Plugin/QueueWorker/MenuQueue.php b/src/Plugin/QueueWorker/MenuQueue.php index 8591c44..fb3a21d 100644 --- a/src/Plugin/QueueWorker/MenuQueue.php +++ b/src/Plugin/QueueWorker/MenuQueue.php @@ -59,7 +59,6 @@ public function processItem($data) : void { if (!isset($data['menu'], $data['language'])) { return; } - sleep(10); ['menu' => $menuName, 'language' => $language] = $data;