From c9b3d9e15b79019fb1d44681d02fd091369dad3b Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 3 Nov 2023 12:45:06 +0000 Subject: [PATCH] Check for ContentType and Permission models before migration (#11184) This prevents `create_extra_permissions` from querying or creating ContentType or Permission objects if the models cannot be found. When using multiple databases which do not have the auth_permission table `create_extra_permissions` will cause migrate to fail. This is similar to https://code.djangoproject.com/ticket/24075. Thanks to @crccheck for additional bug reporting. --- CHANGELOG.txt | 1 + CONTRIBUTORS.md | 1 + docs/releases/5.2.1.md | 1 + wagtail/snippets/models.py | 20 ++++++++++++++--- wagtail/snippets/tests/test_management.py | 26 +++++++++++++++++++++++ 5 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 wagtail/snippets/tests/test_management.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 136dfd8c16..f8046a3eed 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -51,6 +51,7 @@ Changelog * Fix: Fix crash when using the locale switcher on the snippets create view (Sage Abdullah) * Fix: Fix performance regression on reports from calling `decorate_paginated_queryset` before pagination / filtering (Alex Tomkins) * Fix: Make searching on specific fields work correctly on Elasticsearch when boost is in use (Matt Westcott) + * Fix: Prevent snippet permission post-migrate hook from failing on multiple database configurations (Joe Tsoi) * Docs: Fix code example for `{% picture ... as ... %}` template tag (Rezyapkin) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 82edffbc6d..e701f107d9 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -767,6 +767,7 @@ * Andrey Nehaychik * Meli Imelda * Kehinde Bobade +* Joe Tsoi ## Translators diff --git a/docs/releases/5.2.1.md b/docs/releases/5.2.1.md index 922864b281..37ab9ee60a 100644 --- a/docs/releases/5.2.1.md +++ b/docs/releases/5.2.1.md @@ -24,6 +24,7 @@ depth: 1 * Fix crash when using the locale switcher on the snippets create view (Sage Abdullah) * Fix performance regression on reports from calling `decorate_paginated_queryset` before pagination / filtering (Alex Tomkins) * Make searching on specific fields work correctly on Elasticsearch when boost is in use (Matt Westcott) + * Prevent snippet permission post-migrate hook from failing on multiple database configurations (Joe Tsoi) ### Documentation diff --git a/wagtail/snippets/models.py b/wagtail/snippets/models.py index b5c4a8e8f8..d223bf2ea8 100644 --- a/wagtail/snippets/models.py +++ b/wagtail/snippets/models.py @@ -1,10 +1,11 @@ from functools import lru_cache +from django.apps import apps as global_apps from django.contrib.admin.utils import quote from django.contrib.auth import get_permission_codename from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType -from django.db import DEFAULT_DB_ALIAS, models +from django.db import DEFAULT_DB_ALIAS, models, router from django.urls import reverse from django.utils.module_loading import import_string @@ -120,8 +121,21 @@ def register_deferred_snippets(): _register_snippet_immediately(registerable, viewset) -def create_extra_permissions(*args, using=DEFAULT_DB_ALIAS, **kwargs): - model_cts = ContentType.objects.get_for_models( +def create_extra_permissions( + app_config, *args, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs +): + app_label = app_config.label + try: + app_config = apps.get_app_config(app_label) + apps.get_model("contenttypes", "ContentType") + apps.get_model("auth", "Permission") + except LookupError: + return + + if not router.allow_migrate_model(using, Permission): + return + + model_cts = ContentType.objects.db_manager(using).get_for_models( *get_snippet_models(), for_concrete_models=False ) diff --git a/wagtail/snippets/tests/test_management.py b/wagtail/snippets/tests/test_management.py new file mode 100644 index 0000000000..70ac3e5cdb --- /dev/null +++ b/wagtail/snippets/tests/test_management.py @@ -0,0 +1,26 @@ +from django.apps import apps +from django.contrib.contenttypes.models import ContentType +from django.db import migrations +from django.test import TestCase + +from wagtail.snippets.models import create_extra_permissions + + +class TestCreatePermissions(TestCase): + def setUp(self): + self.app_config = apps.get_app_config("auth") + + def tearDown(self): + ContentType.objects.clear_cache() + + def test_unavailable_models(self): + state = migrations.state.ProjectState() + + # Unavailable contenttypes.ContentType + with self.assertNumQueries(0): + create_extra_permissions(self.app_config, verbosity=0, apps=state.apps) + + # Unavailable auth.Permission + state = migrations.state.ProjectState(real_apps={"contenttypes"}) + with self.assertNumQueries(0): + create_extra_permissions(self.app_config, verbosity=0, apps=state.apps)