From f179113e6cbc8ba0a8d4e87e1d4410fb61d63e75 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Thu, 17 Dec 2015 20:12:05 -0500 Subject: [PATCH] Fixed #24067 -- Renamed content types upon model renaming. Thanks to Tim for the extensive review. --- django/contrib/contenttypes/apps.py | 7 +- django/contrib/contenttypes/management.py | 84 ++++++++++++++++++- .../operations_migrations/0001_initial.py | 34 ++++++++ .../operations_migrations/__init__.py | 0 tests/contenttypes_tests/tests.py | 84 +++++++++++++++++-- 5 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 tests/contenttypes_tests/operations_migrations/0001_initial.py create mode 100644 tests/contenttypes_tests/operations_migrations/__init__.py diff --git a/django/contrib/contenttypes/apps.py b/django/contrib/contenttypes/apps.py index f6c3d430fd..e5708adc99 100644 --- a/django/contrib/contenttypes/apps.py +++ b/django/contrib/contenttypes/apps.py @@ -1,10 +1,12 @@ from django.apps import AppConfig from django.contrib.contenttypes.checks import check_generic_foreign_keys from django.core import checks -from django.db.models.signals import post_migrate +from django.db.models.signals import post_migrate, pre_migrate from django.utils.translation import ugettext_lazy as _ -from .management import update_contenttypes +from .management import ( + inject_rename_contenttypes_operations, update_contenttypes, +) class ContentTypesConfig(AppConfig): @@ -12,5 +14,6 @@ class ContentTypesConfig(AppConfig): verbose_name = _("Content Types") def ready(self): + pre_migrate.connect(inject_rename_contenttypes_operations, sender=self) post_migrate.connect(update_contenttypes) checks.register(check_generic_foreign_keys, checks.Tags.models) diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index 7af1a78e36..0bdf8ed9fa 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management.py @@ -1,9 +1,91 @@ from django.apps import apps as global_apps -from django.db import DEFAULT_DB_ALIAS, router +from django.db import DEFAULT_DB_ALIAS, migrations, router, transaction +from django.db.utils import IntegrityError from django.utils import six from django.utils.six.moves import input +class RenameContentType(migrations.RunPython): + def __init__(self, app_label, old_model, new_model): + self.app_label = app_label + self.old_model = old_model + self.new_model = new_model + super(RenameContentType, self).__init__(self.rename_forward, self.rename_backward) + + def _rename(self, apps, schema_editor, old_model, new_model): + ContentType = apps.get_model('contenttypes', 'ContentType') + db = schema_editor.connection.alias + if not router.allow_migrate_model(db, ContentType): + return + + try: + content_type = ContentType.objects.db_manager(db).get_by_natural_key(self.app_label, old_model) + except ContentType.DoesNotExist: + pass + else: + content_type.model = new_model + try: + with transaction.atomic(using=db): + content_type.save(update_fields={'model'}) + except IntegrityError: + # Gracefully fallback if a stale content type causes a + # conflict as update_contenttypes will take care of asking the + # user what should be done next. + content_type.model = old_model + else: + # Clear the cache as the `get_by_natual_key()` call will cache + # the renamed ContentType instance by its old model name. + ContentType.objects.clear_cache() + + def rename_forward(self, apps, schema_editor): + self._rename(apps, schema_editor, self.old_model, self.new_model) + + def rename_backward(self, apps, schema_editor): + self._rename(apps, schema_editor, self.new_model, self.old_model) + + +def inject_rename_contenttypes_operations(plan=None, apps=global_apps, using=DEFAULT_DB_ALIAS, **kwargs): + """ + Insert a `RenameContentType` operation after every planned `RenameModel` + operation. + """ + if plan is None: + return + + # Determine whether or not the ContentType model is available. + try: + ContentType = apps.get_model('contenttypes', 'ContentType') + except LookupError: + available = False + else: + if not router.allow_migrate_model(using, ContentType): + return + available = True + + for migration, backward in plan: + if ((migration.app_label, migration.name) == ('contenttypes', '0001_initial')): + # There's no point in going forward if the initial contenttypes + # migration is unapplied as the ContentType model will be + # unavailable from this point. + if backward: + break + else: + available = True + continue + # The ContentType model is not available yet. + if not available: + continue + inserts = [] + for index, operation in enumerate(migration.operations): + if isinstance(operation, migrations.RenameModel): + operation = RenameContentType( + migration.app_label, operation.old_name_lower, operation.new_name_lower + ) + inserts.append((index + 1, operation)) + for inserted, (index, operation) in enumerate(inserts): + migration.operations.insert(inserted + index, operation) + + 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 diff --git a/tests/contenttypes_tests/operations_migrations/0001_initial.py b/tests/contenttypes_tests/operations_migrations/0001_initial.py new file mode 100644 index 0000000000..0df7a8ef9c --- /dev/null +++ b/tests/contenttypes_tests/operations_migrations/0001_initial.py @@ -0,0 +1,34 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +def assert_foo_contenttype_not_cached(apps, schema_editor): + ContentType = apps.get_model('contenttypes', 'ContentType') + try: + content_type = ContentType.objects.get_by_natural_key('contenttypes_tests', 'foo') + except ContentType.DoesNotExist: + pass + else: + if not ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists(): + raise AssertionError('The contenttypes_tests.Foo ContentType should not be cached.') + elif content_type.model != 'foo': + raise AssertionError( + "The cached contenttypes_tests.Foo ContentType should have " + "its model set to 'foo'." + ) + + +class Migration(migrations.Migration): + + operations = [ + migrations.CreateModel( + 'Foo', + [ + ('id', models.AutoField(primary_key=True)), + ], + ), + migrations.RenameModel('Foo', 'RenamedFoo'), + migrations.RunPython(assert_foo_contenttype_not_cached, migrations.RunPython.noop) + ] diff --git a/tests/contenttypes_tests/operations_migrations/__init__.py b/tests/contenttypes_tests/operations_migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index 5db01773a4..7f65fb6f9b 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -4,15 +4,18 @@ from __future__ import unicode_literals import datetime from django.apps.registry import Apps, apps -from django.contrib.contenttypes import management +from django.conf import settings +from django.contrib.contenttypes import management as contenttypes_management from django.contrib.contenttypes.fields import ( GenericForeignKey, GenericRelation, ) from django.contrib.contenttypes.models import ContentType from django.contrib.sites.models import Site -from django.core import checks -from django.db import connections, models -from django.test import SimpleTestCase, TestCase, mock, override_settings +from django.core import checks, management +from django.db import connections, migrations, models +from django.test import ( + SimpleTestCase, TestCase, TransactionTestCase, mock, override_settings, +) from django.test.utils import captured_stdout, isolate_apps from django.utils.encoding import force_str, force_text @@ -388,9 +391,9 @@ class UpdateContentTypesTests(TestCase): interactive mode of update_contenttypes() (the default) should delete stale contenttypes. """ - management.input = lambda x: force_str("yes") + contenttypes_management.input = lambda x: force_str("yes") with captured_stdout() as stdout: - management.update_contenttypes(self.app_config) + contenttypes_management.update_contenttypes(self.app_config) self.assertIn("Deleting stale content type", stdout.getvalue()) self.assertEqual(ContentType.objects.count(), self.before_count) @@ -400,7 +403,7 @@ class UpdateContentTypesTests(TestCase): content types. """ with captured_stdout() as stdout: - management.update_contenttypes(self.app_config, interactive=False) + contenttypes_management.update_contenttypes(self.app_config, interactive=False) self.assertIn("Stale content types remain.", stdout.getvalue()) self.assertEqual(ContentType.objects.count(), self.before_count + 1) @@ -411,7 +414,7 @@ class UpdateContentTypesTests(TestCase): """ apps = Apps() with self.assertNumQueries(0): - management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps) + contenttypes_management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps) self.assertEqual(ContentType.objects.count(), self.before_count + 1) @@ -445,3 +448,68 @@ class ContentTypesMultidbTestCase(TestCase): with self.assertNumQueries(0, using='default'), \ self.assertNumQueries(1, using='other'): ContentType.objects.get_for_model(Author) + + +@override_settings( + MIGRATION_MODULES=dict(settings.MIGRATION_MODULES, contenttypes_tests='contenttypes_tests.operations_migrations'), +) +class ContentTypeOperationsTests(TransactionTestCase): + available_apps = ['django.contrib.contenttypes', 'contenttypes_tests'] + + def setUp(self): + app_config = apps.get_app_config('contenttypes_tests') + models.signals.post_migrate.connect(self.assertOperationsInjected, sender=app_config) + + def tearDown(self): + app_config = apps.get_app_config('contenttypes_tests') + models.signals.post_migrate.disconnect(self.assertOperationsInjected, sender=app_config) + + def assertOperationsInjected(self, plan, **kwargs): + for migration, _backward in plan: + operations = iter(migration.operations) + for operation in operations: + if isinstance(operation, migrations.RenameModel): + next_operation = next(operations) + self.assertIsInstance(next_operation, contenttypes_management.RenameContentType) + self.assertEqual(next_operation.app_label, migration.app_label) + self.assertEqual(next_operation.old_model, operation.old_name_lower) + self.assertEqual(next_operation.new_model, operation.new_name_lower) + + def test_existing_content_type_rename(self): + ContentType.objects.create(app_label='contenttypes_tests', model='foo') + management.call_command( + 'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0, + ) + self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists()) + management.call_command( + 'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0, + ) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists()) + + def test_missing_content_type_rename_ignore(self): + management.call_command( + 'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0, + ) + self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists()) + management.call_command( + 'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0, + ) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists()) + + def test_content_type_rename_conflict(self): + ContentType.objects.create(app_label='contenttypes_tests', model='foo') + ContentType.objects.create(app_label='contenttypes_tests', model='renamedfoo') + management.call_command( + 'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0, + ) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists()) + management.call_command( + 'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0, + ) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists()) + self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())