diff --git a/src/Entity/GroupRelationship.php b/src/Entity/GroupRelationship.php index 18eed5215bce905d166de9794756d3febe7b146b..3d745ca6686db955a16ecb8fd5e6aec58d1979a6 100644 --- a/src/Entity/GroupRelationship.php +++ b/src/Entity/GroupRelationship.php @@ -2,6 +2,7 @@ namespace Drupal\group\Entity; +use Drupal\Core\Cache\Cache; use Drupal\Core\Entity\Attribute\ContentEntityType; use Drupal\Core\Entity\ContentEntityBase; use Drupal\Core\Entity\EntityChangedTrait; @@ -24,6 +25,7 @@ use Drupal\group\Entity\Views\GroupRelationshipViewsData; use Drupal\group\Form\GroupJoinForm; use Drupal\group\Form\GroupLeaveForm; use Drupal\user\EntityOwnerTrait; +use Drupal\user\UserInterface; /** * Defines the relationship entity. @@ -270,11 +272,19 @@ class GroupRelationship extends ContentEntityBase implements GroupRelationshipIn if ($update === FALSE) { // We want to make sure that the entity we just added to the group behaves - // as a grouped entity. This means we may need to update access records, - // flush some caches containing the entity or perform other operations we - // cannot possibly know about. Lucky for us, all of that behavior usually - // happens when saving an entity so let's re-save the added entity. - $this->getEntity()->save(); + // as a grouped entity. Any code that runs "live" should get the new + // information, but we want to invalidate cache entries that listed the + // entity as a cacheable dependency. + // + // One drawback to the above is that not everything gets cached. Some + // things get calculated and stored as entities (or other DB entries) of + // their own without any cacheable metadata attached. One example being + // the Pathauto module generating path aliases upon entity save. + // + // It is up to these modules to listen to hook_group_relationship_insert() + // and call ::getEntity() on the relationship to also run their code on + // the entity that got added to a group. + Cache::invalidateTags($this->getEntity()->getCacheTags()); } // If a membership gets updated, but the member's roles haven't changed, we @@ -305,13 +315,14 @@ class GroupRelationship extends ContentEntityBase implements GroupRelationshipIn foreach ($entities as $group_relationship) { assert($group_relationship instanceof GroupRelationshipInterface); if ($entity = $group_relationship->getEntity()) { - // For the same reasons we re-save entities that are added to a group, - // we need to re-save entities that were removed from one. See - // ::postSave(). We only save the entity if it still exists to avoid - // trying to save an entity that just got deleted and triggered the - // deletion of its relationship entities. - // @todo Revisit when https://www.drupal.org/node/2754399 lands. - $entity->save(); + // For the same reason we invalidate the cache tags of entities that are + // added to a group, we need to invalidate the cache tags of those that + // were removed from one. See ::postSave(). + // + // We can only invalidate the cache tags of entities that still exist + // and not the ones that were just deleted and triggered the deletion of + // their group relationship entities. + Cache::invalidateTags($entity->getCacheTags()); // If a membership gets deleted, we need to reset the internal group // roles cache for the member in that group, but only if the user still @@ -319,7 +330,8 @@ class GroupRelationship extends ContentEntityBase implements GroupRelationshipIn if ($group_relationship->getPluginId() == 'group_membership') { $role_storage = \Drupal::entityTypeManager()->getStorage('group_role'); assert($role_storage instanceof GroupRoleStorageInterface); - $role_storage->resetUserGroupRoleCache($group_relationship->getEntity(), $group_relationship->getGroup()); + assert($entity instanceof UserInterface); + $role_storage->resetUserGroupRoleCache($entity, $group_relationship->getGroup()); } } } diff --git a/tests/modules/group_test/src/Hook/EntityHooks.php b/tests/modules/group_test/src/Hook/EntityHooks.php deleted file mode 100644 index 5eee1f06bf5929924f7243a5f2566ade25c4efa9..0000000000000000000000000000000000000000 --- a/tests/modules/group_test/src/Hook/EntityHooks.php +++ /dev/null @@ -1,25 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\group_test\Hook; - -use Drupal\Core\Hook\Attribute\Hook; -use Drupal\user\UserInterface; - -/** - * Entity hook implementations for Group test. - */ -final class EntityHooks { - - /** - * Implements hook_ENTITY_TYPE_update(). - */ - #[Hook('user_update')] - public function testUserUpdate(UserInterface $user): void { - if ($user->getChangedTime() == 123456789) { - $user->setChangedTime(530496000)->save(); - } - } - -} diff --git a/tests/src/Kernel/GroupRelationshipTest.php b/tests/src/Kernel/GroupRelationshipTest.php index db2109a261090d77998bc75d7cd6b93a9a1d1180..07e5e594db4466dceb42a23383b80cc422717adf 100644 --- a/tests/src/Kernel/GroupRelationshipTest.php +++ b/tests/src/Kernel/GroupRelationshipTest.php @@ -21,7 +21,7 @@ class GroupRelationshipTest extends GroupKernelTestBase { /** * {@inheritdoc} */ - protected static $modules = ['group_test', 'group_test_plugin', 'node']; + protected static $modules = ['group_test_plugin', 'node']; /** * Tests that entity url templates are functional. @@ -70,23 +70,27 @@ class GroupRelationshipTest extends GroupKernelTestBase { } /** - * Tests that after adding an entity to a group, it gets saved again. + * Tests that adding an entity to a group invalidates its cache tags. * * @covers ::postSave - * - * @see group_test_user_update() */ - public function testSubjectResaved() { - $changed = 123456789; - $account = $this->createUser([], NULL, FALSE, ['changed' => $changed]); + public function testSubjectCacheTagsInvalidated() { + $cid = 'test_group_relationship'; + $cache = \Drupal::cache(); + $account = $this->createUser(); + + $cache->set($cid, 'hello', tags: $account->getCacheTags()); + $this->assertNotFalse($cache->get($cid), 'Cache entry based on account cache tags exists.'); $group = $this->createGroup(['type' => $this->createGroupType()->id()]); - $group->addRelationship($account, 'group_membership'); + $group_relationship = $group->addRelationship($account, 'group_membership'); + $this->assertFalse($cache->get($cid), 'Cache entry based on account cache tags is flushed when account is added to a group.'); + + $cache->set($cid, 'hello', tags: $account->getCacheTags()); + $this->assertNotFalse($cache->get($cid), 'Cache entry based on account cache tags exists.'); - // All users whose changed time was set to 123456789 get their changed time - // set to 530496000 in group_test_user_update() when the account is updated. - $account_unchanged = $this->entityTypeManager->getStorage('user')->loadUnchanged($account->id()); - $this->assertEquals(530496000, $account_unchanged->getChangedTime(), 'Account was saved as part of being added to a group.'); + $group_relationship->delete(); + $this->assertFalse($cache->get($cid), 'Cache entry based on account cache tags is flushed when account is removed from a group.'); } /**