From ff1f1040b6e59ef67aafbd455dda5fa2df8b84d2 Mon Sep 17 00:00:00 2001 From: imperosol Date: Wed, 21 May 2025 11:14:02 +0200 Subject: [PATCH] simplify `User.cached_groups` --- core/apps.py | 1 - core/models.py | 15 +++------------ core/signals.py | 15 --------------- core/tests/test_core.py | 22 +++------------------- 4 files changed, 6 insertions(+), 47 deletions(-) delete mode 100644 core/signals.py diff --git a/core/apps.py b/core/apps.py index 5f1b9fc3..be517dc8 100644 --- a/core/apps.py +++ b/core/apps.py @@ -32,7 +32,6 @@ class SithConfig(AppConfig): verbose_name = "Core app of the Sith" def ready(self): - import core.signals # noqa F401 from forum.models import Forum cache.clear() diff --git a/core/models.py b/core/models.py index 5c873c6c..6a77738f 100644 --- a/core/models.py +++ b/core/models.py @@ -396,19 +396,10 @@ class User(AbstractUser): return self.is_root return group in self.cached_groups - @property + @cached_property def cached_groups(self) -> list[Group]: - """Get the list of groups this user is in. - - The result is cached for the default duration (should be 5 minutes) - - Returns: A list of all the groups this user is in. - """ - groups = cache.get(f"user_{self.id}_groups") - if groups is None: - groups = list(self.groups.all()) - cache.set(f"user_{self.id}_groups", groups) - return groups + """Get the list of groups this user is in.""" + return list(self.groups.all()) @cached_property def is_root(self) -> bool: diff --git a/core/signals.py b/core/signals.py deleted file mode 100644 index 067915c5..00000000 --- a/core/signals.py +++ /dev/null @@ -1,15 +0,0 @@ -from django.core.cache import cache -from django.db.models.signals import m2m_changed -from django.dispatch import receiver - -from core.models import User - - -@receiver(m2m_changed, sender=User.groups.through, dispatch_uid="user_groups_changed") -def user_groups_changed(sender, instance: User, **kwargs): - """Clear the cached groups of the user.""" - # As a m2m relationship doesn't live within the model - # but rather on an intermediary table, there is no - # model method to override, meaning we must use - # a signal to invalidate the cache when a user is removed from a group - cache.delete(f"user_{instance.pk}_groups") diff --git a/core/tests/test_core.py b/core/tests/test_core.py index cdc9ebb1..72cde11c 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -411,10 +411,11 @@ class TestUserIsInGroup(TestCase): """Test that the number of db queries is stable and that less queries are made when making a new call. """ - # make sure Skia is in at least one group group_in = baker.make(Group) self.public_user.groups.add(group_in) + # clear the cached property `User.cached_groups` + self.public_user.__dict__.pop("cached_groups", None) cache.clear() # Test when the user is in the group with self.assertNumQueries(2): @@ -423,6 +424,7 @@ class TestUserIsInGroup(TestCase): self.public_user.is_in_group(pk=group_in.id) group_not_in = baker.make(Group) + self.public_user.__dict__.pop("cached_groups", None) cache.clear() # Test when the user is not in the group with self.assertNumQueries(2): @@ -447,24 +449,6 @@ class TestUserIsInGroup(TestCase): ) assert cached_membership == "not_member" - def test_cache_properly_cleared_group(self): - """Test that when a user is removed from a group, - the is_in_group_method return False when calling it again. - """ - # testing with pk - self.public_user.groups.add(self.com_admin.pk) - assert self.public_user.is_in_group(pk=self.com_admin.pk) is True - - self.public_user.groups.remove(self.com_admin.pk) - assert self.public_user.is_in_group(pk=self.com_admin.pk) is False - - # testing with name - self.public_user.groups.add(self.sas_admin.pk) - assert self.public_user.is_in_group(name="SAS admin") is True - - self.public_user.groups.remove(self.sas_admin.pk) - assert self.public_user.is_in_group(name="SAS admin") is False - def test_not_existing_group(self): """Test that searching for a not existing group returns False.