Skip to content

Commit

Permalink
Don't count users when CMS_RAW_ID_USERS=True
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Pankrat committed Aug 16, 2021
1 parent 38aa11c commit 056d402
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 32 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -6,6 +6,8 @@ Changelog
unreleased
==========

* Improved performance when using CMS_RAW_ID_USERS=True on a Postgres DB with many users

3.9.0 (2021-06-30)
==================

Expand Down
58 changes: 26 additions & 32 deletions cms/admin/permissionadmin.py
Expand Up @@ -29,6 +29,29 @@ class TabularInline(admin.TabularInline):
pass


def users_exceed_threshold():
"""
Check if the number of users exceed the configured threshold. Only bother
counting the users when using an integer threshold, otherwise return the
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:
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
Expand All @@ -52,19 +75,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):
"""
Expand Down Expand Up @@ -133,13 +144,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

Expand All @@ -158,19 +164,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'):
Expand Down
52 changes: 52 additions & 0 deletions cms/tests/test_admin.py
Expand Up @@ -18,6 +18,7 @@
from cms import api
from cms.api import create_page, create_title, add_plugin, publish_page
from cms.admin.pageadmin import PageAdmin
from cms.admin.permissionadmin import GlobalPagePermissionAdmin, PagePermissionInlineAdmin
from cms.constants import TEMPLATE_INHERITANCE_MAGIC
from cms.models import StaticPlaceholder
from cms.models.pagemodel import Page, PageType
Expand Down Expand Up @@ -1385,3 +1386,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'])

0 comments on commit 056d402

Please sign in to comment.