perf: Don't count users when CMS_RAW_ID_USERS=True (#7414)

* perf: Don't count users when CMS_RAW_ID_USERS=True

When using CMS_RAW_ID_USERS=True on a Postgres database with many users,
counting the users is slow and will always yield the same result.

Only count users when using an integer value as a threshold and reuse
the same logic for both PagePermissionInlineAdmin and
GlobalPagePermissionAdmin.

* Ensure that only integer settings of CMS_RAW_ID_USERS are compared to the number of users

* Add documentation for the CMS_RAW_ID_USER=True setting

* fix isort for added tests

* Fix: in python this is always True: isinstance(False, int)

Co-authored-by: Pankrat <lhaehne@gmail.com>

CHANGELOG.rst

@@ -12,6 +12,7 @@ unreleased
 * Fix bug where switching color scheme affects other settings
 * Unlocalize page and node ids when rendering the page tree in the admin (#7175)
 * Fixed permission denied error after page create (#6866)
+* Improved performance when using CMS_RAW_ID_USERS=True on a Postgres DB with many users
 
 3.11.0 (2022-08-02)
 ===================

cms/admin/permissionadmin.py

@@ -30,6 +30,29 @@ class TabularInline(admin.TabularInline):
     pass
 
 
+def users_exceed_threshold():
+    """
+    Check if the number of users exceeds the configured threshold. Only bother
+    counting the users when using an integer threshold, otherwise return the
+    truthy value of the setting to avoid a potentially expensive DB query.
+    """
+    threshold = get_cms_setting('RAW_ID_USERS')
+
+    # Don't bother counting the users when not using an integer threshold
+    if threshold is True or threshold is False or not isinstance(threshold, int):
+        return (threshold)
+
+    # Given a fresh django-cms install and a django settings with the
+    # CMS_RAW_ID_USERS = CMS_PERMISSION = True
+    # django throws an OperationalError when running
+    # ./manage migrate
+    # because auth_user doesn't exists yet
+    try:
+        return get_user_model().objects.count() > threshold
+    except OperationalError:
+        return False
+
+
 class PagePermissionInlineAdmin(TabularInline):
     model = PagePermission
     # use special form, so we can override of user and group field
@@ -53,19 +76,7 @@ def has_add_permission(self, request, obj=None):
     @classproperty
     def raw_id_fields(cls):
         # Dynamically set raw_id_fields based on settings
-        threshold = get_cms_setting('RAW_ID_USERS')
-
-        # Given a fresh django-cms install and a django settings with the
-        # CMS_RAW_ID_USERS = CMS_PERMISSION = True
-        # django throws an OperationalError when running
-        # ./manage migrate
-        # because auth_user doesn't exists yet
-        try:
-            threshold = threshold and get_user_model().objects.count() > threshold
-        except OperationalError:
-            threshold = False
-
-        return ['user'] if threshold else []
+        return ['user'] if users_exceed_threshold() else []
 
     def get_queryset(self, request):
         """
@@ -86,8 +97,8 @@ def get_queryset(self, request):
     def get_formset(self, request, obj=None, **kwargs):
         """
         Some fields may be excluded here. User can change only
-        permissions which are available for him. E.g. if user does not haves
-        can_publish flag, he can't change assign can_publish permissions.
+        permissions which are available for them. E.g. if user does not have
+        can_publish flag, they can't change assign can_publish permissions.
         """
         exclude = self.exclude or []
         if obj:
@@ -134,13 +145,8 @@ class GlobalPagePermissionAdmin(admin.ModelAdmin):
     list_filter.append('can_change_advanced_settings')
 
     def get_list_filter(self, request):
-        threshold = get_cms_setting('RAW_ID_USERS')
-        try:
-            threshold = threshold and get_user_model().objects.count() > threshold
-        except OperationalError:
-            threshold = False
         filter_copy = deepcopy(self.list_filter)
-        if threshold:
+        if users_exceed_threshold():
             filter_copy.remove('user')
         return filter_copy
 
@@ -159,19 +165,7 @@ def has_delete_permission(self, request, obj=None):
     @classproperty
     def raw_id_fields(cls):
         # Dynamically set raw_id_fields based on settings
-        threshold = get_cms_setting('RAW_ID_USERS')
-
-        # Given a fresh django-cms install and a django settings with the
-        # CMS_RAW_ID_USERS = CMS_PERMISSION = True
-        # django throws an OperationalError when running
-        # ./manage migrate
-        # because auth_user doesn't exists yet
-        try:
-            threshold = threshold and get_user_model().objects.count() > threshold
-        except OperationalError:
-            threshold = False
-
-        return ['user'] if threshold else []
+        return ['user'] if users_exceed_threshold() else []
 
 
 if get_cms_setting('PERMISSION'):

cms/tests/test_admin.py

@@ -16,6 +16,7 @@
 
 from cms import api
 from cms.admin.pageadmin import PageAdmin
+from cms.admin.permissionadmin import GlobalPagePermissionAdmin, PagePermissionInlineAdmin
 from cms.api import add_plugin, create_page, create_title, publish_page
 from cms.constants import TEMPLATE_INHERITANCE_MAGIC
 from cms.models import StaticPlaceholder
@@ -1389,3 +1390,54 @@ def test_move_node(self):
         #       ⊢ Beta
         #   ⊢ Delta
         #       ⊢ Gamma
+
+
+class AdminPerformanceTests(AdminTestsBase):
+
+    def test_raw_id_threshold_page_permission_inline_admin(self):
+        """
+        Only count users when using an integer value as threshold for
+        CMS_RAW_ID_USERS.
+        """
+        with self.settings(CMS_RAW_ID_USERS=1):
+            with self.assertNumQueries(1):
+                self.assertEqual(PagePermissionInlineAdmin.raw_id_fields, [])
+
+        # Create users to check if threshold is honored
+        self._get_guys()
+
+        with self.settings(CMS_RAW_ID_USERS=False):
+            with self.assertNumQueries(0):
+                self.assertEqual(PagePermissionInlineAdmin.raw_id_fields, [])
+
+        with self.settings(CMS_RAW_ID_USERS=True):
+            with self.assertNumQueries(0):
+                self.assertEqual(PagePermissionInlineAdmin.raw_id_fields, ['user'])
+
+        with self.settings(CMS_RAW_ID_USERS=1):
+            with self.assertNumQueries(1):
+                self.assertEqual(PagePermissionInlineAdmin.raw_id_fields, ['user'])
+
+    def test_raw_id_threshold_global_page_permission_admin(self):
+        """
+        Only count users when using an integer value as threshold for
+        CMS_RAW_ID_USERS.
+        """
+        with self.settings(CMS_RAW_ID_USERS=1):
+            with self.assertNumQueries(1):
+                self.assertEqual(GlobalPagePermissionAdmin.raw_id_fields, [])
+
+        # Create users to check if threshold is honored
+        self._get_guys()
+
+        with self.settings(CMS_RAW_ID_USERS=False):
+            with self.assertNumQueries(0):
+                self.assertEqual(GlobalPagePermissionAdmin.raw_id_fields, [])
+
+        with self.settings(CMS_RAW_ID_USERS=True):
+            with self.assertNumQueries(0):
+                self.assertEqual(GlobalPagePermissionAdmin.raw_id_fields, ['user'])
+
+        with self.settings(CMS_RAW_ID_USERS=1):
+            with self.assertNumQueries(1):
+                self.assertEqual(GlobalPagePermissionAdmin.raw_id_fields, ['user'])

docs/reference/configuration.rst

@@ -851,12 +851,17 @@ This setting only applies if :setting:`CMS_PERMISSION` is ``True``
 
 The ``view restrictions`` and ``page permissions`` inlines on the
 :class:`cms.models.Page` admin change forms can cause performance problems
-where there are many thousands of users being put into simple select boxes. If
-set to a positive integer, this setting forces the inlines on that page to use
+where there are many thousands of users being put into simple select boxes.
+
+If set to a positive integer, this setting forces the inlines on that page to use
 standard Django admin raw ID widgets rather than select boxes if the number of
 users in the system is greater than that number, dramatically improving
 performance.
 
+If set to ``True`` it forces the inlines on Page admin to use raw ID widgets
+without even counting the number of users, giving an additional performance
+improvement.
+
 .. note:: Using raw ID fields in combination with ``limit_choices_to`` causes
           errors due to excessively long URLs if you have many thousands of
           users (the PKs are all included in the URL of the popup window). For
@@ -1015,7 +1020,7 @@ CMS_LIMIT_TTL_CACHE_FUNCTION
 default
     ``None``
 
-If defined, specifies the function to be called that allows to limit the page cache ttl value 
+If defined, specifies the function to be called that allows to limit the page cache ttl value
 using a business logic. The function receives one argument, the `response`, and returns an `int`
 the max business value of the page cache ttl.