Merge pull request #40 from discoverygarden/fix/caching

SUP-6613: Improve caching

embargo.services.yml

@@ -62,3 +62,10 @@ services:
     class: Drupal\embargo\EventSubscriber\TaggingEventSubscriber
     tags:
       - { name: 'event_subscriber' }
+  cache_context.user.embargo__has_exemption:
+    class: Drupal\embargo\Cache\Context\UserExemptedCacheContext
+    arguments:
+      - '@current_user'
+      - '@entity_type.manager'
+    tags:
+      - { name: 'cache.context' }

modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php

@@ -40,7 +40,7 @@ public function __construct(
     $plugin_id,
     $plugin_definition,
     MigrationInterface $migration,
-    EntityTypeManagerInterface $entity_type_manager
+    EntityTypeManagerInterface $entity_type_manager,
   ) {
     parent::__construct($configuration, $plugin_id, $plugin_definition, $migration);
 
@@ -56,7 +56,7 @@ public static function create(
     array $configuration,
     $plugin_id,
     $plugin_definition,
-    MigrationInterface $migration = NULL
+    MigrationInterface $migration = NULL,
   ) {
     return new static(
       $configuration,

src/Access/EmbargoAccessCheck.php

@@ -57,28 +57,40 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Req
    * {@inheritdoc}
    */
   public function access(EntityInterface $entity, AccountInterface $user) {
+    $type = $this->entityTypeManager->getDefinition('embargo');
+    $state = AccessResult::neutral();
+
+    if ($user->hasPermission('bypass embargo access')) {
+      return $state->setReason('User has embargo bypass permission.')
+        ->addCacheContexts(['user.permissions']);
+    }
+
     /** @var \Drupal\embargo\EmbargoStorage $storage */
     $storage = $this->entityTypeManager->getStorage('embargo');
+    $state->addCacheTags($type->getListCacheTags())
+      ->addCacheContexts($type->getListCacheContexts());
+    $related_embargoes = $storage->getApplicableEmbargoes($entity);
+    if (empty($related_embargoes)) {
+      return $state->setReason('No embargo statements for the given entity.');
+    }
+
+    array_map([$state, 'addCacheableDependency'], $related_embargoes);
+
     $embargoes = $storage->getApplicableNonExemptNonExpiredEmbargoes(
       $entity,
       $this->request->server->get('REQUEST_TIME'),
       $user,
       $this->request->getClientIp()
     );
-    $state = AccessResult::forbiddenIf(
+    return $state->andIf(AccessResult::forbiddenIf(
       !empty($embargoes),
       $this->formatPlural(
         count($embargoes),
         '1 embargo preventing access.',
         '@count embargoes preventing access.'
       )->render()
-    );
-    array_map([$state, 'addCacheableDependency'], $embargoes);
+    ));
 
-    $type = $this->entityTypeManager->getDefinition('embargo');
-    $state->addCacheTags($type->getListCacheTags())
-      ->addCacheContexts($type->getListCacheContexts());
-    return $state;
   }
 
 }

src/Cache/Context/UserExemptedCacheContext.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace Drupal\embargo\Cache\Context;
+
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\Context\CacheContextInterface;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Session\AccountInterface;
+
+/**
+ * Context based of the given user being exempt from _any_ embargoes.
+ */
+class UserExemptedCacheContext implements CacheContextInterface {
+
+  /**
+   * Memoize if exempted.
+   *
+   * @var bool
+   */
+  protected bool $exempted;
+
+  /**
+   * Constructor.
+   */
+  public function __construct(
+    protected AccountInterface $currentUser,
+    protected EntityTypeManagerInterface $entityTypeManager,
+  ) {
+    // No-op, other than stashing properties.
+  }
+
+  /**
+   * {@inheritDoc}
+   */
+  public static function getLabel() {
+    return \t('User, has any embargo exemptions');
+  }
+
+  /**
+   * {@inheritDoc}
+   */
+  public function getContext() {
+    return $this->isExempted() ? '1' : '0';
+  }
+
+  /**
+   * {@inheritDoc}
+   */
+  public function getCacheableMetadata() {
+    return (new CacheableMetadata())
+      ->addCacheContexts([$this->isExempted() ? 'user' : 'user.permissions'])
+      ->addCacheTags(['embargo_list']);
+  }
+
+  /**
+   * Determine if the current user has _any_ exemptions.
+   *
+   * @return bool
+   *   TRUE if the user is exempt to at least one embargo; otherwise, FALSE.
+   *
+   * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
+   * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
+   */
+  protected function isExempted() : bool {
+    if (!isset($this->exempted)) {
+      $results = $this->entityTypeManager->getStorage('embargo')->getQuery()
+        ->accessCheck(FALSE)
+        ->condition('exempt_users', $this->currentUser->id())
+        ->range(0, 1)
+        ->execute();
+      $this->exempted = !empty($results);
+    }
+
+    return $this->exempted;
+  }
+
+}

src/Entity/Embargo.php

@@ -12,6 +12,7 @@
 use Drupal\Core\TypedData\Exception\MissingDataException;
 use Drupal\datetime\Plugin\Field\FieldType\DateTimeItemInterface;
 use Drupal\embargo\EmbargoInterface;
+use Drupal\embargo\EmbargoStorageInterface;
 use Drupal\embargo\IpRangeInterface;
 use Drupal\node\NodeInterface;
 use Drupal\user\UserInterface;
@@ -43,8 +44,6 @@
  *       "html" = "Drupal\Core\Entity\Routing\AdminHtmlRouteProvider"
  *     },
  *   },
- *   list_cache_contexts = { "ip.embargo_range", "user" },
- *   list_cache_tags = { "embargo_list", "embargo_ip_range_list" },
  *   base_table = "embargo",
  *   admin_permission = "administer embargo",
  *   entity_keys = {
@@ -413,7 +412,7 @@ public function getCacheContexts() {
     $contexts = Cache::mergeContexts(
       parent::getCacheContexts(),
       $this->getEmbargoedNode()->getCacheContexts(),
-      [$this->getExemptUsers() ? 'user' : 'user.permissions'],
+      ['user.embargo__has_exemption'],
     );
 
     if ($this->getExemptIps()) {
@@ -458,11 +457,9 @@ public function ipIsExempt(string $ip): bool {
   protected function getListCacheTagsToInvalidate() : array {
     return array_merge(
       parent::getListCacheTagsToInvalidate(),
-      [
-        'node_list',
-        'media_list',
-        'file_list',
-      ]
+      array_map(function (string $type) {
+        return "{$type}_list";
+      }, EmbargoStorageInterface::APPLICABLE_ENTITY_TYPES),
     );
   }
 

src/Entity/IpRange.php

@@ -10,6 +10,7 @@
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
+use Drupal\embargo\EmbargoStorageInterface;
 use Drupal\embargo\IpRangeInterface;
 use Symfony\Component\HttpFoundation\IpUtils;
 
@@ -40,7 +41,6 @@
  *       "html" = "Drupal\Core\Entity\Routing\AdminHtmlRouteProvider"
  *     },
  *   },
- *   list_cache_tags = { "node_list", "media_list", "file_list" },
  *   base_table = "embargo_ip_range",
  *   admin_permission = "administer embargo",
  *   entity_keys = {
@@ -250,4 +250,16 @@ public function getCacheContexts() {
     ]);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getListCacheTagsToInvalidate() : array {
+    return array_merge(
+      parent::getListCacheTagsToInvalidate(),
+      array_map(function (string $type) {
+        return "{$type}_list";
+      }, EmbargoStorageInterface::APPLICABLE_ENTITY_TYPES),
+    );
+  }
+
 }

src/Plugin/search_api/processor/EmbargoJoinProcessor.php

@@ -289,7 +289,7 @@ public function preprocessSearchQuery(QueryInterface $query) : void {
       // cacheability.
       'ip.embargo_range',
       // Exemptable users, so need to deal with them.
-      'user',
+      'user.embargo__has_exemption',
     ]);
     // Embargo dates deal with granularity to the day.
     $query->mergeCacheMaxAge(24 * 3600);

tests/src/Kernel/FileEmbargoTest.php

@@ -73,7 +73,7 @@ public function testEmbargoedNodeRelatedMediaFileAccessDenied($operation) {
    * @throws \Drupal\Core\Entity\EntityStorageException
    */
   public function testDeletedEmbargoedFileRelatedMediaFileAccessAllowed(
-    $operation
+    $operation,
   ) {
     $node = $this->createNode();
     $file = $this->createFile();