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.

Sign up for GitHub

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

Jump to bottom

DDST-213: Adjust caching for embargo and related access control. #39

Merged
merged 3 commits into from
Jun 10, 2024
Merged

DDST-213: Adjust caching for embargo and related access control. #39

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63d0178
Adjust caching for embargo and related access control.
adam-vessey May 9, 2024
d8c5b3b
Add in the related entity to the mix.
adam-vessey May 9, 2024
ec7eac6
Merge branch 'main' into fix/caching
adam-vessey May 27, 2024
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
4 changes: 4 additions & 0 deletions src/Access/EmbargoAccessCheck.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public function access(EntityInterface $entity, AccountInterface $user) {
)->render()
);
array_map([$state, 'addCacheableDependency'], $embargoes);

$type = $this->entityTypeManager->getDefinition('embargo');
$state->addCacheTags($type->getListCacheTags())
->addCacheContexts($type->getListCacheContexts());
return $state;
}

Expand Down
17 changes: 16 additions & 1 deletion src/Entity/Embargo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
* "html" = "Drupal\Core\Entity\Routing\AdminHtmlRouteProvider"
* },
* },
* list_cache_tags = { "node_list", "media_list", "file_list" },
Copy link
Contributor Author

@adam-vessey adam-vessey May 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per Drupal's EntityTypeInterface::getListCacheTags(), which should be:

The list cache tags associated with this entity type.

Enables code listing entities of this type to ensure that newly created entities show up immediately.

Really could leave unset, as the default is embargo_list, but whatever, we can set explicitly.

Thinking this was originally set as it was in a misguided attempt to have the effects of the embargo applied immediately, but this fits more inline with ::getListCacheTagsToInvalidate(), which is used when invalidating cache tags on entity save/delete, instead of providing the tags/contexts to set where listing of the given embargo entities are used, such as when calculating the access control results.

* list_cache_contexts = { "ip.embargo_range", "user" },
* list_cache_tags = { "embargo_list" },
* base_table = "embargo",
* admin_permission = "administer embargo",
* entity_keys = {
Expand Down Expand Up @@ -450,4 +451,18 @@ public function ipIsExempt(string $ip): bool {
return $exempt_ips && $exempt_ips->withinRanges($ip);
}

/**
* {@inheritdoc}
*/
protected function getListCacheTagsToInvalidate() : array {
return array_merge(
parent::getListCacheTagsToInvalidate(),
[
'node_list',
'media_list',
'file_list',
]
);
}

}
Loading