Fixed #24075 -- Used post-migration models in contrib apps receivers.

Thanks Markus and Tim for the review.

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()

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()
-
+    # Always clear the global content types cache.
-    app_label = app_config.label
+    if apps is not global_apps:
+        global_apps.get_model('contenttypes', 'ContentType').objects.clear_cache()
 
     app_models = {
         model._meta.model_name: model

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:
+            # Start with get() and not get_or_create() in order to use
-                # We start with get() and not get_or_create() in order to use
+            # the db_for_read (see #20401).
-                # the db_for_read (see #20401).
+            ct = self.get(app_label=opts.app_label, model=opts.model_name)
-                ct = self.get(app_label=opts.app_label, model=opts.model_name)
+        except self.model.DoesNotExist:
-            except self.model.DoesNotExist:
+            # Not found in the database; we proceed to create it. This time
-                # Not found in the database; we proceed to create it.  This time we
+            # use get_or_create to take care of any race conditions.
-                # use get_or_create to take care of any race conditions.
+            ct, created = self.get_or_create(
-                ct, created = self.get_or_create(
+                app_label=opts.app_label,
-                    app_label=opts.app_label,
+                model=opts.model_name,
-                    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."
             )
         self._add_to_cache(self.db, ct)
         return ct

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:

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)

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)

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):

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):