- Issue created by @jonathanshaw
- 🇫🇷France abdei Paris, Ile de France
Hello,
I've created an initial patch for this issue, but please note that it's currently missing the required test coverage. I'm actively working on writing the tests and will update the patch once they are ready. - 🇬🇧United Kingdom jonathanshaw Stroud, UK
Great!
-
+++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -14,11 +15,11 @@ + * @var \Drupal\Core\Cache\MemoryCache\MemoryCache @@ -53,12 +54,13 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce + $this->memoryCache = new MemoryCache();
I think we need a service for this. See https://www.drupal.org/node/3409455 → for the correct pattern. Without that the cache won't get properly invalidated when dependencies are updated.
The same service is shared between access handlers, which will make resetCache unecessarily aggressive but that doesn't seem like a big problem.
-
+++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -76,15 +78,13 @@ public function access(EntityInterface $entity, $operation, ?AccountInterface $a $cid .= ':' . $entity->getRevisionId();
I'm not very comfortable with this combination of creating part of the cache key here and part in the helper method. I think it might be best to pass the entity to the helper method and build the whole cache key there, with conditional logic for 'create' bundle where necessary.
-
+++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -76,15 +78,13 @@ public function access(EntityInterface $entity, $operation, ?AccountInterface $a - // It is not possible to delete or revert the default revision.
Looks like an accidental removal.
-
+++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php @@ -221,17 +223,36 @@ protected function setCache($access, $cid, $operation, $langcode, AccountInterfa + protected function generateCacheKey($cid, $operation, $langcode, AccountInterface $account) {
Please pass the full $context array to generaateCacheKey(). This will help other issues that want to use that in the cache key.
-
- 🇬🇧United Kingdom jonathanshaw Stroud, UK
Also, Gitlab MRs are preferred - maybe even required - rather than patches now.
- 🇬🇧United Kingdom jonathanshaw Stroud, UK
On reflection, I think this will be better with 2 helper methods, one for access and one for createAccess. The createAccess one will get enhanced in a follow up issue to better handle $context.
- 🇳🇿New Zealand quietone
Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.