diff --git a/og.module b/og.module index a4cba3e1c..05d8a66c9 100755 --- a/og.module +++ b/og.module @@ -332,7 +332,6 @@ function og_invalidate_group_content_cache_tags(EntityInterface $entity) { if ($is_group_content && $original) { /** @var \Drupal\og\OgGroupAudienceHelperInterface $group_audience_helper */ $group_audience_helper = \Drupal::service('og.group_audience_helper'); - $audience_has_changed = FALSE; /** @var \Drupal\Core\Entity\FieldableEntityInterface $original */ foreach ($group_audience_helper->getAllGroupAudienceFields($entity->getEntityTypeId(), $entity->bundle()) as $field) { $field_name = $field->getName(); @@ -342,15 +341,8 @@ function og_invalidate_group_content_cache_tags(EntityInterface $entity) { foreach ($original_field_item_list->referencedEntities() as $old_group) { $tags = Cache::mergeTags($tags, $old_group->getCacheTagsToInvalidate()); } - $audience_has_changed = TRUE; } } - if ($audience_has_changed) { - // If at least a group has changed, we clear the static cache of the - // membership manager which stores such relations. This is important - // when the membership manager was used previously in the same request. - $membership_manager->reset(); - } } foreach ($membership_manager->getGroups($entity) as $group_entity_type_id => $groups) { diff --git a/og.services.yml b/og.services.yml index fae318a62..1add5d435 100644 --- a/og.services.yml +++ b/og.services.yml @@ -45,7 +45,7 @@ services: arguments: ['@config.factory', '@entity_type.manager', '@entity_type.bundle.info', '@event_dispatcher', '@cache.data', '@og.permission_manager', '@og.role_manager', '@router.builder', '@og.group_audience_helper'] og.membership_manager: class: Drupal\og\MembershipManager - arguments: ['@entity_type.manager', '@og.group_audience_helper'] + arguments: ['@entity_type.manager', '@og.group_audience_helper', '@cache.static'] og.permission_manager: class: Drupal\og\PermissionManager arguments: ['@event_dispatcher'] diff --git a/src/Entity/OgMembership.php b/src/Entity/OgMembership.php index 668340c7e..15352b41f 100644 --- a/src/Entity/OgMembership.php +++ b/src/Entity/OgMembership.php @@ -479,9 +479,6 @@ public function save() { Og::reset(); \Drupal::service('og.access')->reset(); - // Invalidate the group membership manager. - \Drupal::service('og.membership_manager')->reset(); - return $result; } diff --git a/src/MembershipManager.php b/src/MembershipManager.php index 2fac2e265..cbd9d7e07 100644 --- a/src/MembershipManager.php +++ b/src/MembershipManager.php @@ -3,6 +3,9 @@ namespace Drupal\og; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheBackendInterface; +use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Session\AccountInterface; @@ -11,16 +14,16 @@ use Drupal\og\Entity\OgMembership; /** - * Membership manager. + * Service for managing memberships and group content. */ class MembershipManager implements MembershipManagerInterface { /** - * Static cache of the memberships and group association. + * The static cache backend. * - * @var array + * @var \Drupal\Core\Cache\CacheBackendInterface */ - protected $cache; + protected $staticCache; /** * The entity type manager. @@ -43,10 +46,14 @@ class MembershipManager implements MembershipManagerInterface { * The entity type manager. * @param \Drupal\og\OgGroupAudienceHelperInterface $group_audience_helper * The OG group audience helper. + * @param \Drupal\Core\Cache\CacheBackendInterface $cache + * The static cache backend. */ - public function __construct(EntityTypeManagerInterface $entity_type_manager, OgGroupAudienceHelperInterface $group_audience_helper) { + public function __construct(EntityTypeManagerInterface $entity_type_manager, OgGroupAudienceHelperInterface $group_audience_helper, CacheBackendInterface $cache) { + assert($cache instanceof CacheTagsInvalidatorInterface, 'The cache backend must support cache tag invalidation.'); $this->entityTypeManager = $entity_type_manager; $this->groupAudienceHelper = $group_audience_helper; + $this->staticCache = $cache; } /** @@ -85,26 +92,26 @@ public function getMemberships(AccountInterface $user, array $states = [OgMember // states. $states = $this->prepareConditionArray($states, OgMembership::ALL_STATES); - $identifier = [ + $cid = [ __METHOD__, - 'user', $user->id(), implode('|', $states), ]; - $identifier = implode(':', $identifier); + $cid = implode(':', $cid); // Use cached result if it exists. - if (!isset($this->cache[$identifier])) { + if (!$membership_ids = $this->staticCache->get($cid)->data ?? []) { $query = $this->entityTypeManager ->getStorage('og_membership') ->getQuery() ->condition('uid', $user->id()) ->condition('state', $states, 'IN'); - $this->cache[$identifier] = $query->execute(); + $membership_ids = $query->execute(); + $this->cacheMembershipIds($cid, $membership_ids); } - return $this->loadMemberships($this->cache[$identifier]); + return $this->loadMemberships($membership_ids); } /** @@ -140,17 +147,17 @@ public function getGroupMembershipIdsByRoleNames(EntityInterface $group, array $ $role_names = $this->prepareConditionArray($role_names); $states = $this->prepareConditionArray($states, OgMembership::ALL_STATES); - $identifier = [ + $cid = [ __METHOD__, $group->getEntityTypeId(), $group->id(), implode('|', $role_names), implode('|', $states), ]; - $identifier = implode(':', $identifier); + $cid = implode(':', $cid); // Only query the database if no cached result exists. - if (!isset($this->cache[$identifier])) { + if (!$membership_ids = $this->staticCache->get($cid)->data ?? []) { $entity_type_id = $group->getEntityTypeId(); $query = $this->entityTypeManager @@ -169,10 +176,11 @@ public function getGroupMembershipIdsByRoleNames(EntityInterface $group, array $ $query->condition('roles', $role_ids, 'IN'); } - $this->cache[$identifier] = $query->execute(); + $membership_ids = $query->execute(); + $this->cacheMembershipIds($cid, $membership_ids); } - return $this->cache[$identifier]; + return $membership_ids; } /** @@ -206,7 +214,7 @@ public function getGroupIds(EntityInterface $entity, $group_type_id = NULL, $gro throw new \InvalidArgumentException('\Drupal\og\MembershipManager::getGroupIds() cannot be used for user entities. Use \Drupal\og\MembershipManager::getUserGroups() instead.'); } - $identifier = [ + $cid = [ __METHOD__, $entity->getEntityTypeId(), $entity->id(), @@ -214,21 +222,23 @@ public function getGroupIds(EntityInterface $entity, $group_type_id = NULL, $gro $group_bundle, ]; - $identifier = implode(':', $identifier); + $cid = implode(':', $cid); - if (isset($this->cache[$identifier])) { + if ($group_ids = $this->staticCache->get($cid)->data ?? []) { // Return cached values. - return $this->cache[$identifier]; + return $group_ids; } $group_ids = []; + $tags = $entity->getCacheTagsToInvalidate(); $fields = $this->groupAudienceHelper->getAllGroupAudienceFields($entity->getEntityTypeId(), $entity->bundle(), $group_type_id, $group_bundle); foreach ($fields as $field) { - $target_type = $field->getFieldStorageDefinition()->getSetting('target_type'); + $target_type_id = $field->getFieldStorageDefinition()->getSetting('target_type'); + $target_type_definition = $this->entityTypeManager->getDefinition($target_type_id); // Optionally filter by group type. - if (!empty($group_type_id) && $group_type_id !== $target_type) { + if (!empty($group_type_id) && $group_type_id !== $target_type_id) { continue; } @@ -250,9 +260,9 @@ public function getGroupIds(EntityInterface $entity, $group_type_id = NULL, $gro // Query the database to get the actual list of groups. The target IDs may // contain groups that no longer exist. Entity reference doesn't clean up // orphaned target IDs. - $entity_type = $this->entityTypeManager->getDefinition($target_type); + $entity_type = $this->entityTypeManager->getDefinition($target_type_id); $query = $this->entityTypeManager - ->getStorage($target_type) + ->getStorage($target_type_id) ->getQuery() // Disable entity access check so fetching the groups related to group // content are not affected by the current user. Furthermore, when @@ -267,10 +277,14 @@ public function getGroupIds(EntityInterface $entity, $group_type_id = NULL, $gro $query->condition($entity_type->getKey('bundle'), $group_bundle); } - $group_ids = NestedArray::mergeDeep($group_ids, [$target_type => $query->execute()]); + // Add the list cache tags for the entity type, so that the cache gets + // invalidated if one of the group entities is deleted. + $tags = Cache::mergeTags($target_type_definition->getListCacheTags(), $tags); + + $group_ids = NestedArray::mergeDeep($group_ids, [$target_type_id => $query->execute()]); } - $this->cache[$identifier] = $group_ids; + $this->staticCache->set($cid, $group_ids, Cache::PERMANENT, $tags); return $group_ids; } @@ -367,13 +381,6 @@ public function isMemberBlocked(EntityInterface $group, AccountInterface $user) return $this->isMember($group, $user, [OgMembershipInterface::STATE_BLOCKED]); } - /** - * {@inheritdoc} - */ - public function reset() { - $this->cache = []; - } - /** * Prepares a conditional array for use in a cache identifier and query. * @@ -400,6 +407,22 @@ protected function prepareConditionArray(array $value, array $default = NULL) { return array_unique($value); } + /** + * Stores the given list of membership IDs in the static cache backend. + * + * @param string $cid + * The cache ID. + * @param array $membership_ids + * The list of membership IDs to store in the static cache. + */ + protected function cacheMembershipIds($cid, array $membership_ids) { + $tags = Cache::buildTags('og_membership', $membership_ids); + // Also invalidate the list cache tags so that if a new membership is + // created it will appear in this list. + $tags = Cache::mergeTags(['og_membership_list'], $tags); + $this->staticCache->set($cid, $membership_ids, Cache::PERMANENT, $tags); + } + /** * Returns the full membership entities with the given memberships IDs. * diff --git a/src/MembershipManagerInterface.php b/src/MembershipManagerInterface.php index c987128bf..835b9cadb 100644 --- a/src/MembershipManagerInterface.php +++ b/src/MembershipManagerInterface.php @@ -273,9 +273,4 @@ public function isMemberPending(EntityInterface $group, AccountInterface $user); */ public function isMemberBlocked(EntityInterface $group, AccountInterface $user); - /** - * Reset the internal cache. - */ - public function reset(); - } diff --git a/src/Og.php b/src/Og.php index a29d30682..4783982fa 100644 --- a/src/Og.php +++ b/src/Og.php @@ -342,9 +342,6 @@ public static function invalidateCache() { \Drupal::entityTypeManager()->clearCachedDefinitions(); \Drupal::service('entity_field.manager')->clearCachedFieldDefinitions(); - // Invalidate the group membership manager. - \Drupal::service('og.membership_manager')->reset(); - // Let other OG modules know we invalidate cache. \Drupal::moduleHandler()->invokeAll('og_invalidate_cache'); } diff --git a/tests/src/Kernel/Entity/CacheInvalidationOnGroupChangeTest.php b/tests/src/Kernel/Entity/CacheInvalidationOnGroupChangeTest.php index 2546c5f97..e903f2de1 100644 --- a/tests/src/Kernel/Entity/CacheInvalidationOnGroupChangeTest.php +++ b/tests/src/Kernel/Entity/CacheInvalidationOnGroupChangeTest.php @@ -86,11 +86,6 @@ public function testCacheInvalidationOnGroupChange() { $tags = Cache::buildTags('og-group-content', $group1->getCacheTagsToInvalidate()); $bin->set($cid, $this->randomString(), Cache::PERMANENT, $tags); - // Change the group content entity group. We're clearing first the static - // cache of membership manager because in a real application, usually, the - // static cache is warmed in a previous request, so that in the request - // where the audience is changed, is already empty. - $this->container->get('og.membership_manager')->reset(); $group_content ->set(OgGroupAudienceHelperInterface::DEFAULT_FIELD, $group2->id()) ->save(); diff --git a/tests/src/Kernel/Entity/GroupMembershipManagerTest.php b/tests/src/Kernel/Entity/GroupMembershipManagerTest.php index 5f44a3477..57d517eaa 100644 --- a/tests/src/Kernel/Entity/GroupMembershipManagerTest.php +++ b/tests/src/Kernel/Entity/GroupMembershipManagerTest.php @@ -88,6 +88,7 @@ protected function setUp() { $this->installEntitySchema('og_membership'); $this->installEntitySchema('user'); $this->installSchema('system', 'sequences'); + $this->installSchema('user', 'users_data'); $this->entityTypeManager = $this->container->get('entity_type.manager'); $this->membershipManager = $this->container->get('og.membership_manager'); @@ -575,6 +576,20 @@ protected function doTestGetGroupMembershipsByRoleNames($method_name, callable $ } } } + + // Test that the correct memberships are returned when one of the users is + // deleted. We are using the second node group as a test case. This group + // has one pending administrator: the user with key '2'. + $group = $this->groups['node'][1]; + $role_names = [OgRoleInterface::ADMINISTRATOR]; + $states = [OgMembershipInterface::STATE_PENDING]; + $memberships = $this->membershipManager->$method_name($group, $role_names, $states); + $this->assertCount(1, $memberships); + + // Delete the user and check that it no longer appears in the result. + $this->users[2]->delete(); + $memberships = $this->membershipManager->$method_name($group, $role_names, $states); + $this->assertCount(0, $memberships); } /** diff --git a/tests/src/Unit/CreateMembershipTest.php b/tests/src/Unit/CreateMembershipTest.php index 3a461c003..1b0a6c6f5 100644 --- a/tests/src/Unit/CreateMembershipTest.php +++ b/tests/src/Unit/CreateMembershipTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\og\Unit; +use Drupal\Core\Cache\MemoryCache\MemoryCacheInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageInterface; @@ -43,6 +44,13 @@ class CreateMembershipTest extends UnitTestCase { */ protected $groupAudienceHelper; + /** + * The mocked memory cache backend. + * + * @var \Drupal\Core\Cache\MemoryCache\MemoryCacheInterface|\Prophecy\Prophecy\ObjectProphecy + */ + protected $staticCache; + /** * The entity storage prophecy used in the test. * @@ -98,6 +106,7 @@ public function setUp() { $this->entityTypeManager = $this->prophesize(EntityTypeManagerInterface::class); $this->entityTypeRepository = $this->prophesize(EntityTypeRepositoryInterface::class); $this->groupAudienceHelper = $this->prophesize(OgGroupAudienceHelperInterface::class); + $this->staticCache = $this->prophesize(MemoryCacheInterface::class); $this->entityTypeManager->getStorage('og_membership') ->willReturn($this->entityStorage->reveal()); @@ -139,7 +148,7 @@ public function setUp() { * @covers ::createMembership */ public function testNewGroup() { - $membership_manager = new MembershipManager($this->entityTypeManager->reveal(), $this->groupAudienceHelper->reveal()); + $membership_manager = new MembershipManager($this->entityTypeManager->reveal(), $this->groupAudienceHelper->reveal(), $this->staticCache->reveal()); $membership = $membership_manager->createMembership($this->group->reveal(), $this->user->reveal()); $this->assertInstanceOf(OgMembershipInterface::class, $membership); }