MembershipManager still returns a user's memberships after the user is deleted #480
Conversation
|
Started by adding a failing test, this demonstrates that |
pfrenssen
force-pushed
the
invalidate-cache-on-membership-deletion
branch
from
c4c37dd
to
5bde52d
Compare
This ensures that the static cache is correctly invalidated whenever one of the cached items is updated.
…he tags are invalidated.
pfrenssen
force-pushed
the
invalidate-cache-on-membership-deletion
branch
from
5bde52d
to
eddd176
Compare
|
OK so using the memory backend that supports invalidation based on cache tags fixes the issue without requiring any changes to the code base. I'm now removing all calls to |
pfrenssen
force-pushed
the
invalidate-cache-on-membership-deletion
branch
from
7614817
to
739ec60
Compare
It is no longer needed. The service is now using a cache backend that gets cleared automatically whenever related cache tags are invalidated.
pfrenssen
force-pushed
the
invalidate-cache-on-membership-deletion
branch
from
739ec60
to
d6baff9
Compare
|
Great! Working fine, and the bug is fixed :) |
| */ | ||
| protected $cache; | ||
| protected $staticCache; |
There was a problem hiding this comment.
@pfrenssen, I wonder why we're only doing memory caching here. This looks to me a good candidate to double the memory caching with persistent caching (database, Redis, etc.). All we need here is to declare a \Drupal\Core\Cache\BackendChain service and add a memory and a cache bin backends to it. Then replace the static cache service with this one. That would improve dramatically the performance.
There was a problem hiding this comment.
I've opened #485 to fix this.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function reset() { |
There was a problem hiding this comment.
IMO, This should have been deprecated instead of fully removed. I know we're not offering yet BC but still, given the fact that so many sites are already using OG...
There was a problem hiding this comment.
You're right, we could have deprecated this.
It broke tests in #181.
MembershipManager::getGroupMembershipsByRoleNames()andMembershipManager::getGroupMembershipIdsByRoleNames()still return the memberships of a user from the static cache after a user is deleted.There are two possible solutions for this:
MembershipManager->reset()inOgMembership::delete()just as it is now being done inOgMembership::save().$cacheproperty with core's staticMemoryBackendcache backend which has full support for cache tag invalidation.In this PR I am going to explore the second solution. By using
MemoryBackendwe can add the correct cache tags to each of our cache entries, and they will be invalidated automatically when needed. This should allow us to remove all calls toMembershipManager::reset()and clean up some code that does unnecessary cache clears.