diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index c05a728e49..659fe63282 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals import getpass import unicodedata -from django.apps import apps +from django.apps import apps as global_apps from django.contrib.auth import get_permission_codename from django.core import exceptions from django.db import DEFAULT_DB_ALIAS, router @@ -37,11 +37,14 @@ def _get_builtin_permissions(opts): return perms -def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, **kwargs): +def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs): if not app_config.models_module: return + app_label = app_config.label try: + app_config = apps.get_app_config(app_label) + ContentType = apps.get_model('contenttypes', 'ContentType') Permission = apps.get_model('auth', 'Permission') except LookupError: return @@ -49,8 +52,6 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_ if not router.allow_migrate_model(using, Permission): return - from django.contrib.contenttypes.models import ContentType - # This will hold the permissions we're looking for as # (content_type, (codename, name)) searched_perms = list() diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index bfc25b3405..7af1a78e36 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management.py @@ -1,10 +1,10 @@ -from django.apps import apps +from django.apps import apps as global_apps from django.db import DEFAULT_DB_ALIAS, router from django.utils import six from django.utils.six.moves import input -def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, **kwargs): +def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs): """ Creates content types for models in the given app, removing any model entries that no longer have a matching model class. @@ -12,7 +12,9 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT if not app_config.models_module: return + app_label = app_config.label try: + app_config = apps.get_app_config(app_label) ContentType = apps.get_model('contenttypes', 'ContentType') except LookupError: return @@ -21,8 +23,9 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT return ContentType.objects.clear_cache() - - app_label = app_config.label + # Always clear the global content types cache. + if apps is not global_apps: + global_apps.get_model('contenttypes', 'ContentType').objects.clear_cache() app_models = { model._meta.model_name: model diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index 97869d7aca..21432f897a 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals from django.apps import apps from django.db import models -from django.db.utils import IntegrityError, OperationalError, ProgrammingError from django.utils.encoding import force_text, python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ @@ -48,24 +47,15 @@ class ContentTypeManager(models.Manager): # The ContentType entry was not found in the cache, therefore we # proceed to load or create it. try: - try: - # We start with get() and not get_or_create() in order to use - # the db_for_read (see #20401). - ct = self.get(app_label=opts.app_label, model=opts.model_name) - except self.model.DoesNotExist: - # Not found in the database; we proceed to create it. This time we - # use get_or_create to take care of any race conditions. - ct, created = self.get_or_create( - app_label=opts.app_label, - model=opts.model_name, - ) - except (OperationalError, ProgrammingError, IntegrityError): - # It's possible to migrate a single app before contenttypes, - # as it's not a required initial dependency (it's contrib!) - # Have a nice error for this. - raise RuntimeError( - "Error creating new content types. Please make sure contenttypes " - "is migrated before trying to migrate apps individually." + # Start with get() and not get_or_create() in order to use + # the db_for_read (see #20401). + ct = self.get(app_label=opts.app_label, model=opts.model_name) + except self.model.DoesNotExist: + # Not found in the database; we proceed to create it. This time + # use get_or_create to take care of any race conditions. + ct, created = self.get_or_create( + app_label=opts.app_label, + model=opts.model_name, ) self._add_to_cache(self.db, ct) return ct diff --git a/django/contrib/sites/management.py b/django/contrib/sites/management.py index 1f403075b8..34262336a2 100644 --- a/django/contrib/sites/management.py +++ b/django/contrib/sites/management.py @@ -2,13 +2,13 @@ Creates the default Site object. """ -from django.apps import apps +from django.apps import apps as global_apps from django.conf import settings from django.core.management.color import no_style from django.db import DEFAULT_DB_ALIAS, connections, router -def create_default_site(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, **kwargs): +def create_default_site(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs): try: Site = apps.get_model('sites', 'Site') except LookupError: diff --git a/tests/auth_tests/test_management.py b/tests/auth_tests/test_management.py index 34b3465b9c..8eff655c99 100644 --- a/tests/auth_tests/test_management.py +++ b/tests/auth_tests/test_management.py @@ -13,6 +13,7 @@ from django.contrib.auth.models import Group, Permission, User from django.contrib.contenttypes.models import ContentType from django.core.management import call_command from django.core.management.base import CommandError +from django.db import migrations from django.test import TestCase, mock, override_settings from django.utils import six from django.utils.encoding import force_str @@ -561,11 +562,12 @@ class MultiDBCreatesuperuserTestCase(TestCase): self.assertEqual(user.email, 'joe@somewhere.org') -class PermissionTestCase(TestCase): +class CreatePermissionsTests(TestCase): def setUp(self): self._original_permissions = Permission._meta.permissions[:] self._original_default_permissions = Permission._meta.default_permissions + self.app_config = apps.get_app_config('auth') def tearDown(self): Permission._meta.permissions = self._original_permissions @@ -573,13 +575,11 @@ class PermissionTestCase(TestCase): ContentType.objects.clear_cache() def test_default_permissions(self): - auth_app_config = apps.get_app_config('auth') - permission_content_type = ContentType.objects.get_by_natural_key('auth', 'permission') Permission._meta.permissions = [ ('my_custom_permission', 'Some permission'), ] - create_permissions(auth_app_config, verbosity=0) + create_permissions(self.app_config, verbosity=0) # add/change/delete permission by default + custom permission self.assertEqual(Permission.objects.filter( @@ -588,9 +588,23 @@ class PermissionTestCase(TestCase): Permission.objects.filter(content_type=permission_content_type).delete() Permission._meta.default_permissions = [] - create_permissions(auth_app_config, verbosity=0) + create_permissions(self.app_config, verbosity=0) # custom permission only since default permissions is empty self.assertEqual(Permission.objects.filter( content_type=permission_content_type, ).count(), 1) + + def test_unvailable_models(self): + """ + #24075 - Permissions shouldn't be created or deleted if the ContentType + or Permission models aren't available. + """ + state = migrations.state.ProjectState() + # Unvailable contenttypes.ContentType + with self.assertNumQueries(0): + create_permissions(self.app_config, verbosity=0, apps=state.apps) + # Unvailable auth.Permission + state = migrations.state.ProjectState(real_apps=['contenttypes']) + with self.assertNumQueries(0): + create_permissions(self.app_config, verbosity=0, apps=state.apps) diff --git a/tests/contenttypes_tests/test_models.py b/tests/contenttypes_tests/test_models.py index d388990c06..18795f9633 100644 --- a/tests/contenttypes_tests/test_models.py +++ b/tests/contenttypes_tests/test_models.py @@ -3,9 +3,8 @@ from __future__ import unicode_literals from django.contrib.contenttypes.models import ContentType, ContentTypeManager from django.contrib.contenttypes.views import shortcut from django.contrib.sites.shortcuts import get_current_site -from django.db.utils import IntegrityError, OperationalError, ProgrammingError from django.http import Http404, HttpRequest -from django.test import TestCase, mock, override_settings +from django.test import TestCase, override_settings from django.utils import six from .models import ( @@ -243,26 +242,3 @@ class ContentTypesTests(TestCase): # Instead, just return the ContentType object and let the app detect stale states. ct_fetched = ContentType.objects.get_for_id(ct.pk) self.assertIsNone(ct_fetched.model_class()) - - @mock.patch('django.contrib.contenttypes.models.ContentTypeManager.get_or_create') - @mock.patch('django.contrib.contenttypes.models.ContentTypeManager.get') - def test_message_if_get_for_model_fails(self, mocked_get, mocked_get_or_create): - """ - Check that `RuntimeError` with nice error message is raised if - `get_for_model` fails because of database errors. - """ - - def _test_message(mocked_method): - for ExceptionClass in (IntegrityError, OperationalError, ProgrammingError): - mocked_method.side_effect = ExceptionClass - with self.assertRaisesMessage( - RuntimeError, - "Error creating new content types. Please make sure contenttypes " - "is migrated before trying to migrate apps individually." - ): - ContentType.objects.get_for_model(ContentType) - - _test_message(mocked_get) - - mocked_get.side_effect = ContentType.DoesNotExist - _test_message(mocked_get_or_create) diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index da4b3311c4..5db01773a4 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -404,6 +404,16 @@ class UpdateContentTypesTests(TestCase): self.assertIn("Stale content types remain.", stdout.getvalue()) self.assertEqual(ContentType.objects.count(), self.before_count + 1) + def test_unavailable_content_type_model(self): + """ + #24075 - A ContentType shouldn't be created or deleted if the model + isn't available. + """ + apps = Apps() + with self.assertNumQueries(0): + management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps) + self.assertEqual(ContentType.objects.count(), self.before_count + 1) + class TestRouter(object): def db_for_read(self, model, **hints): diff --git a/tests/sites_tests/tests.py b/tests/sites_tests/tests.py index 3cbd6af2ee..fa618b4acc 100644 --- a/tests/sites_tests/tests.py +++ b/tests/sites_tests/tests.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals from django.apps import apps +from django.apps.registry import Apps from django.conf import settings from django.contrib.sites import models from django.contrib.sites.management import create_default_site @@ -293,6 +294,14 @@ class CreateDefaultSiteTests(TestCase): create_default_site(self.app_config, verbosity=0) self.assertEqual(Site.objects.get().pk, 1) + def test_unavailable_site_model(self): + """ + #24075 - A Site shouldn't be created if the model isn't available. + """ + apps = Apps() + create_default_site(self.app_config, verbosity=0, apps=apps) + self.assertFalse(Site.objects.exists()) + class MiddlewareTest(TestCase):