From cb4017c649ee5e7be373d7a35e700d1c322d5b63 Mon Sep 17 00:00:00 2001 From: Tidiane Dia Date: Tue, 7 Jun 2022 16:55:05 +0100 Subject: [PATCH] Cache model permission codenames in PermissionHelper --- CHANGELOG.txt | 1 + docs/releases/4.0.md | 1 + wagtail/contrib/modeladmin/helpers/permission.py | 16 ++++++++++++++-- .../modeladmin/tests/test_simple_modeladmin.py | 14 ++++++++++++++ wagtail/images/tests/test_admin_views.py | 6 +++--- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 515c0c680c..6c05144171 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -38,6 +38,7 @@ Changelog * Add `add_to_admin_menu` option for ModelAdmin (Oliver Parker) * Implement Fuzzy matching for Elasticsearch (Nick Smith) * Rename `Page.get_latest_revision_as_page` to `Page.get_latest_revision_as_object` (Sage Abdullah) + * Cache model permission codenames in PermissionHelper (Tidiane Dia) * Fix: Typo in `ResumeWorkflowActionFormatter` message (Stefan Hammer) * Fix: Throw a meaningful error when saving an image to an unrecognised image format (Christian Franke) * Fix: Remove extra padding for headers with breadcrumbs on mobile viewport (Steven Steinwand) diff --git a/docs/releases/4.0.md b/docs/releases/4.0.md index 859cda195d..4e87b414a1 100644 --- a/docs/releases/4.0.md +++ b/docs/releases/4.0.md @@ -46,6 +46,7 @@ When using a queryset to render a list of images, you can now use the ``prefetch * Added [multi-site support](api_filtering_pages_by_site) to the API (Sævar Öfjörð Magnússon) * Add `add_to_admin_menu` option for ModelAdmin (Oliver Parker) * Implement [Fuzzy matching](fuzzy_matching) for Elasticsearch (Nick Smith) + * Cache model permission codenames in `PermissionHelper` (Tidiane Dia) ### Bug fixes diff --git a/wagtail/contrib/modeladmin/helpers/permission.py b/wagtail/contrib/modeladmin/helpers/permission.py index fafe3e77bb..959d79fb3a 100644 --- a/wagtail/contrib/modeladmin/helpers/permission.py +++ b/wagtail/contrib/modeladmin/helpers/permission.py @@ -1,6 +1,9 @@ +from functools import lru_cache + from django.contrib.auth import get_permission_codename from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType +from django.utils.functional import cached_property from wagtail.models import Page, UserPagePermissionsProxy @@ -28,6 +31,14 @@ class PermissionHelper: content_type__model=self.opts.model_name, ) + @cached_property + def all_permission_codenames(self): + return list( + self.get_all_model_permissions() + .values_list("codename", flat=True) + .distinct() + ) + def get_perm_codename(self, action): return get_permission_codename(action, self.opts) @@ -39,13 +50,14 @@ class PermissionHelper: return user.has_perm("%s.%s" % (self.opts.app_label, perm_codename)) + @lru_cache(maxsize=128) def user_has_any_permissions(self, user): """ Return a boolean to indicate whether `user` has any model-wide permissions """ - for perm in self.get_all_model_permissions().values("codename"): - if self.user_has_specific_permission(user, perm["codename"]): + for perm_codename in self.all_permission_codenames: + if self.user_has_specific_permission(user, perm_codename): return True return False diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index eceacee5bb..4cc2f3d06f 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -11,7 +11,9 @@ from django.utils.timezone import make_aware from openpyxl import load_workbook from wagtail.admin.admin_url_finder import AdminURLFinder +from wagtail.admin.models import Admin from wagtail.admin.panels import FieldPanel, TabbedInterface +from wagtail.contrib.modeladmin.helpers.permission import PermissionHelper from wagtail.contrib.modeladmin.helpers.search import DjangoORMSearchHandler from wagtail.images.models import Image from wagtail.images.tests.utils import get_test_image_file @@ -917,6 +919,18 @@ class TestEditorAccess(TestCase, WagtailTestUtils): response = self.client.post("/admin/modeladmintest/book/delete/2/") self.assertRedirects(response, "/admin/") + def test_permission_helper(self): + permission_helper = PermissionHelper(Admin) + + # Populate user permissions cache + with self.assertNumQueries(2): + self.assertTrue(self.user.has_perm("wagtailadmin.access_admin")) + + with self.assertNumQueries(1): + # Only one query - to retrieve the model's codenames - should be performed. + self.assertTrue(permission_helper.user_has_any_permissions(self.user)) + self.assertTrue(permission_helper.user_can_list(self.user)) + class TestHistoryView(TestCase, WagtailTestUtils): fixtures = ["modeladmintest_test.json"] diff --git a/wagtail/images/tests/test_admin_views.py b/wagtail/images/tests/test_admin_views.py index 69dcce33c0..7ec100b2d7 100644 --- a/wagtail/images/tests/test_admin_views.py +++ b/wagtail/images/tests/test_admin_views.py @@ -266,7 +266,7 @@ class TestImageIndexView(TestCase, WagtailTestUtils): def test_num_queries(self): # Initial number of queries. - with self.assertNumQueries(23): + with self.assertNumQueries(13): self.get() # Add 5 images. @@ -276,11 +276,11 @@ class TestImageIndexView(TestCase, WagtailTestUtils): file=get_test_image_file(size=(1, 1)), ) - with self.assertNumQueries(45): + with self.assertNumQueries(35): # The renditions needed don't exist yet. We have 20 = 5 * 4 + 2 additional queries. self.get() - with self.assertNumQueries(25): + with self.assertNumQueries(15): # No extra additional queries since renditions exist and are saved in # the prefetched objects cache. self.get()