Merge pull request #42 from City-of-Helsinki/UHF-8526

UHF-8526: Use PubSub to invalidate caches
City-of-Helsinki · Sep 16, 2023 · 668049d · 668049d
2 parents c2497e8 + 17abfd7
commit 668049d
Show file tree

Hide file tree

Showing 15 changed files with 114 additions and 275 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,14 +58,17 @@ 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,
     private readonly CacheBackendInterface $cache,
     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
diff --git 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('<nolink>');
-
-    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
@@ -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
@@ -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);