From 41e68224ca0924f9ed3e4680d96e58108e1f0f0a Mon Sep 17 00:00:00 2001 From: Julien Constant Date: Thu, 11 May 2023 16:30:01 +0200 Subject: [PATCH 1/4] Temporary solution (re-add the warning) --- core/models.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/models.py b/core/models.py index 8a01c2d57..6e51312df 100644 --- a/core/models.py +++ b/core/models.py @@ -180,14 +180,14 @@ def get_group(*, pk: int = None, name: str = None) -> Optional[Group]: :param pk: The primary key of the group :param name: The name of the group :return: The group if it exists, else None - :raises ValueError: If no group matches the criteria + :raise ValueError: If no group matches the criteria """ if pk is None and name is None: raise ValueError("Either pk or name must be set") - if name is not None: - name = name.replace(" ", "_") # avoid errors with memcached backend + pk_or_name: Union[str, int] = pk if pk is not None else name group = cache.get(f"sith_group_{pk_or_name}") + if group == "not_found": # Using None as a cache value is a little bit tricky, # so we use a special string to represent None @@ -201,7 +201,7 @@ def get_group(*, pk: int = None, name: str = None) -> Optional[Group]: group = Group.objects.filter(name=name).first() if group is not None: cache.set(f"sith_group_{group.id}", group) - cache.set(f"sith_group_{group.name.replace(' ', '_')}", group) + cache.set(f"sith_group_{group.name}", group) else: cache.set(f"sith_group_{pk_or_name}", "not_found") return group From 46bc00d9714440cbd8b18e7cd358d8171cdaf13c Mon Sep 17 00:00:00 2001 From: Julien Constant Date: Fri, 12 May 2023 11:42:24 +0200 Subject: [PATCH 2/4] Better fix --- core/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/models.py b/core/models.py index 6e51312df..41b12ccf6 100644 --- a/core/models.py +++ b/core/models.py @@ -185,7 +185,7 @@ def get_group(*, pk: int = None, name: str = None) -> Optional[Group]: if pk is None and name is None: raise ValueError("Either pk or name must be set") - pk_or_name: Union[str, int] = pk if pk is not None else name + pk_or_name: Union[str, int] = pk if pk is not None else name.replace(" ", "_") group = cache.get(f"sith_group_{pk_or_name}") if group == "not_found": @@ -201,7 +201,7 @@ def get_group(*, pk: int = None, name: str = None) -> Optional[Group]: group = Group.objects.filter(name=name).first() if group is not None: cache.set(f"sith_group_{group.id}", group) - cache.set(f"sith_group_{group.name}", group) + cache.set(f"sith_group_{group.name.replace(' ', '_')}", group) else: cache.set(f"sith_group_{pk_or_name}", "not_found") return group From 0283643cbe09efcec6a605f1bf26a9e9eeeaf9ce Mon Sep 17 00:00:00 2001 From: Julien Constant Date: Fri, 12 May 2023 12:10:44 +0200 Subject: [PATCH 3/4] Added tests --- core/tests.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/tests.py b/core/tests.py index 4c84662b4..a2e9c5263 100644 --- a/core/tests.py +++ b/core/tests.py @@ -597,12 +597,20 @@ 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.toto.groups.add(self.com_admin.pk) self.assertTrue(self.toto.is_in_group(pk=self.com_admin.pk)) self.toto.groups.remove(self.com_admin.pk) self.assertFalse(self.toto.is_in_group(pk=self.com_admin.pk)) + # testing with name + self.toto.groups.add(self.sas_admin.pk) + self.assertTrue(self.toto.is_in_group(name="SAS admin")) + + self.toto.groups.remove(self.sas_admin.pk) + self.assertFalse(self.toto.is_in_group(name="SAS admin")) + def test_not_existing_group(self): """ Test that searching for a not existing group From ef06094eda1cb120be227b6cd3b4ef2195d283cc Mon Sep 17 00:00:00 2001 From: Julien Constant Date: Fri, 12 May 2023 13:27:30 +0200 Subject: [PATCH 4/4] Added back the comment --- core/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/models.py b/core/models.py index 41b12ccf6..fb28faa69 100644 --- a/core/models.py +++ b/core/models.py @@ -185,6 +185,7 @@ def get_group(*, pk: int = None, name: str = None) -> Optional[Group]: if pk is None and name is None: raise ValueError("Either pk or name must be set") + # replace space characters to hide warnings with memcached backend pk_or_name: Union[str, int] = pk if pk is not None else name.replace(" ", "_") group = cache.get(f"sith_group_{pk_or_name}")