From 870d900cdcd29564bcc7f47cc67d51af2617f614 Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Sun, 8 Feb 2015 00:31:12 +0500 Subject: [PATCH] [1.8.x] Refs #24299 -- Made contenttypes migrations signal handler more robust. Backport of d392c1e150a46aca72e37263c44c034d92a79f79 from master --- django/contrib/contenttypes/models.py | 25 +++++++++++---------- django/contrib/contenttypes/tests/tests.py | 26 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index 5205ec80f9..2aadf80027 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -4,7 +4,7 @@ import warnings from django.apps import apps from django.db import models -from django.db.utils import OperationalError, ProgrammingError +from django.db.utils import IntegrityError, OperationalError, ProgrammingError from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text, python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ @@ -59,10 +59,18 @@ class ContentTypeManager(models.Manager): # The ContentType entry was not found in the cache, therefore we # proceed to load or create it. 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 (OperationalError, ProgrammingError): + 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. @@ -70,13 +78,6 @@ class ContentTypeManager(models.Manager): "Error creating new content types. Please make sure contenttypes " "is migrated before trying to migrate apps individually." ) - 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, - ) self._add_to_cache(self.db, ct) return ct diff --git a/django/contrib/contenttypes/tests/tests.py b/django/contrib/contenttypes/tests/tests.py index 8e4dae3b37..eb436b6a64 100644 --- a/django/contrib/contenttypes/tests/tests.py +++ b/django/contrib/contenttypes/tests/tests.py @@ -5,8 +5,9 @@ import warnings from django.contrib.contenttypes.models import ContentType 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, override_settings +from django.test import TestCase, mock, override_settings from django.utils import six from .models import ( @@ -264,3 +265,26 @@ class ContentTypesTests(TestCase): "ContentType.name field doesn't exist any longer. Please remove it from your code." ) self.assertTrue(ContentType.objects.filter(model='OldModel').exists()) + + @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)