Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUP-6613: Improve caching #40

Merged
merged 4 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions embargo.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -56,7 +56,7 @@ public static function create(
array $configuration,
$plugin_id,
$plugin_definition,
MigrationInterface $migration = NULL
MigrationInterface $migration = NULL,
) {
return new static(
$configuration,
Expand Down
26 changes: 19 additions & 7 deletions src/Access/EmbargoAccessCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}
jordandukart marked this conversation as resolved.
Show resolved Hide resolved

/** @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;
}

}
77 changes: 77 additions & 0 deletions src/Cache/Context/UserExemptedCacheContext.php
Original file line number Diff line number Diff line change
@@ -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;
}

}
13 changes: 5 additions & 8 deletions src/Entity/Embargo.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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),
);
}

Expand Down
14 changes: 13 additions & 1 deletion src/Entity/IpRange.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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),
);
}

}
2 changes: 1 addition & 1 deletion src/Plugin/search_api/processor/EmbargoJoinProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/src/Kernel/FileEmbargoTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function testEmbargoedNodeRelatedMediaFileAccessDenied($operation) {
* @throws \Drupal\Core\Entity\EntityStorageException
*/
public function testDeletedEmbargoedFileRelatedMediaFileAccessAllowed(
$operation
$operation,
) {
$node = $this->createNode();
$file = $this->createFile();
Expand Down
Loading