From ff1f1040b6e59ef67aafbd455dda5fa2df8b84d2 Mon Sep 17 00:00:00 2001
From: imperosol <thgirod@hotmail.com>
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.