remove cache calls to fetch user membership

club/models.py

@@ -26,7 +26,6 @@ from __future__ import annotations
 from typing import Iterable, Self
 
 from django.conf import settings
-from django.core.cache import cache
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.core.validators import RegexValidator, validate_email
 from django.db import models, transaction
@@ -187,9 +186,6 @@ class Club(models.Model):
         self.page.save(force_lock=True)
 
     def delete(self, *args, **kwargs) -> tuple[int, dict[str, int]]:
-        # Invalidate the cache of this club and of its memberships
-        for membership in self.members.ongoing().select_related("user"):
-            cache.delete(f"membership_{self.id}_{membership.user.id}")
         self.board_group.delete()
         self.members_group.delete()
         return super().delete(*args, **kwargs)
@@ -210,24 +206,15 @@ class Club(models.Model):
         """Method to see if that object can be edited by the given user."""
         return self.has_rights_in_club(user)
 
-    def get_membership_for(self, user: User) -> Membership | None:
+    @cached_property
-        """Return the current membership the given user.
+    def current_members(self) -> list[Membership]:
+        return self.members.ongoing().select_related("user").order_by("-role")
 
-        Note:
+    def get_membership_for(self, user: User) -> Membership | None:
-            The result is cached.
+        """Return the current membership of the given user."""
-        """
         if user.is_anonymous:
             return None
-        membership = cache.get(f"membership_{self.id}_{user.id}")
+        return next((m for m in self.current_members if m.user_id == user.id), None)
-        if membership == "not_member":
-            return None
-        if membership is None:
-            membership = self.members.filter(user=user, end_date=None).first()
-            if membership is None:
-                cache.set(f"membership_{self.id}_{user.id}", "not_member")
-            else:
-                cache.set(f"membership_{self.id}_{user.id}", membership)
-        return membership
 
     def has_rights_in_club(self, user: User) -> bool:
         return user.is_in_group(pk=self.board_group_id)
@@ -295,28 +282,20 @@ class MembershipQuerySet(models.QuerySet):
         and add them in the club groups they should be in.
 
         Be aware that this adds three db queries :
-        one to retrieve the updated memberships,
+
-        one to perform group removal and one to perform
+        - one to retrieve the updated memberships
-        group attribution.
+        - one to perform group removal
+        - and one to perform group attribution.
         """
         nb_rows = super().update(**kwargs)
         if nb_rows == 0:
-            # if no row was affected, no need to refresh the cache
+            # if no row was affected, no need to edit club groups
             return 0
 
-        cache_memberships = {}
         memberships = set(self.select_related("club"))
         # delete all User-Group relations and recreate the necessary ones
-        # It's more concise to write and more reliable
         Membership._remove_club_groups(memberships)
         Membership._add_club_groups(memberships)
-        for member in memberships:
-            cache_key = f"membership_{member.club_id}_{member.user_id}"
-            if member.end_date is None:
-                cache_memberships[cache_key] = member
-            else:
-                cache_memberships[cache_key] = "not_member"
-        cache.set_many(cache_memberships)
         return nb_rows
 
     def delete(self) -> tuple[int, dict[str, int]]:
@@ -339,12 +318,6 @@ class MembershipQuerySet(models.QuerySet):
         nb_rows, rows_counts = super().delete()
         if nb_rows > 0:
             Membership._remove_club_groups(memberships)
-            cache.set_many(
-                {
-                    f"membership_{m.club_id}_{m.user_id}": "not_member"
-                    for m in memberships
-                }
-            )
         return nb_rows, rows_counts
 
 
@@ -408,9 +381,6 @@ class Membership(models.Model):
         self._remove_club_groups([self])
         if self.end_date is None:
             self._add_club_groups([self])
-            cache.set(f"membership_{self.club_id}_{self.user_id}", self)
-        else:
-            cache.set(f"membership_{self.club_id}_{self.user_id}", "not_member")
 
     def get_absolute_url(self):
         return reverse("club:club_members", kwargs={"club_id": self.club_id})
@@ -431,7 +401,6 @@ class Membership(models.Model):
     def delete(self, *args, **kwargs):
         self._remove_club_groups([self])
         super().delete(*args, **kwargs)
-        cache.delete(f"membership_{self.club_id}_{self.user_id}")
 
     @staticmethod
     def _remove_club_groups(

club/tests/test_membership.py

@@ -72,25 +72,6 @@ class TestMembershipQuerySet(TestClub):
         expected.sort(key=lambda i: i.id)
         assert members == expected
 
-    def test_update_invalidate_cache(self):
-        """Test that the `update` queryset method properly invalidate cache."""
-        mem_skia = self.simple_board_member.memberships.get(club=self.club)
-        cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia)
-        self.simple_board_member.memberships.update(end_date=localtime(now()).date())
-        assert (
-            cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}")
-            == "not_member"
-        )
-
-        mem_richard = self.richard.memberships.get(club=self.club)
-        cache.set(
-            f"membership_{mem_richard.club_id}_{mem_richard.user_id}", mem_richard
-        )
-        self.richard.memberships.update(role=5)
-        new_mem = self.richard.memberships.get(club=self.club)
-        assert new_mem != "not_member"
-        assert new_mem.role == 5
-
     def test_update_change_club_groups(self):
         """Test that `update` set the user groups accordingly."""
         user = baker.make(User)
@@ -112,24 +93,6 @@ class TestMembershipQuerySet(TestClub):
         assert not user.groups.contains(members_group)
         assert not user.groups.contains(board_group)
 
-    def test_delete_invalidate_cache(self):
-        """Test that the `delete` queryset properly invalidate cache."""
-        mem_skia = self.simple_board_member.memberships.get(club=self.club)
-        mem_comptable = self.president.memberships.get(club=self.club)
-        cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia)
-        cache.set(
-            f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}", mem_comptable
-        )
-
-        # should delete the subscriptions of simple_board_member and president
-        self.club.members.ongoing().board().delete()
-
-        for membership in (mem_skia, mem_comptable):
-            cached_mem = cache.get(
-                f"membership_{membership.club_id}_{membership.user_id}"
-            )
-            assert cached_mem == "not_member"
-
     def test_delete_remove_from_groups(self):
         """Test that `delete` removes from club groups"""
         user = baker.make(User)

core/tests/test_core.py

@@ -22,19 +22,17 @@ from bs4 import BeautifulSoup
 from django.contrib.auth.hashers import make_password
 from django.contrib.auth.models import Permission
 from django.core import mail
-from django.core.cache import cache
 from django.core.exceptions import ValidationError
 from django.core.mail import EmailMessage
 from django.test import Client, RequestFactory, TestCase
 from django.urls import reverse
-from django.utils.timezone import now
 from django.views.generic import View
 from django.views.generic.base import ContextMixin
 from model_bakery import baker
 from pytest_django.asserts import assertInHTML, assertRedirects
 
 from antispam.models import ToxicDomain
-from club.models import Club, Membership
+from club.models import Club
 from core.baker_recipes import subscriber_user
 from core.markdown import markdown
 from core.models import AnonymousUser, Group, Page, User, validate_promo
@@ -436,23 +434,6 @@ class TestUserIsInGroup(TestCase):
         with self.assertNumQueries(0):
             self.public_user.is_in_group(pk=group_not_in.id)
 
-    def test_cache_properly_cleared_membership(self):
-        """Test that when the membership of a user end,
-        the cache is properly invalidated.
-        """
-        membership = baker.make(Membership, club=self.club, user=self.public_user)
-        cache.clear()
-        self.club.get_membership_for(self.public_user)  # this should populate the cache
-        assert membership == cache.get(
-            f"membership_{self.club.id}_{self.public_user.id}"
-        )
-        membership.end_date = now() - timedelta(minutes=5)
-        membership.save()
-        cached_membership = cache.get(
-            f"membership_{self.club.id}_{self.public_user.id}"
-        )
-        assert cached_membership == "not_member"
-
     def test_not_existing_group(self):
         """Test that searching for a not existing group
         returns False.