Skip to content

Commit f179113

Browse files
charettescharettes
committed
Fixed #24067 -- Renamed content types upon model renaming.
Thanks to Tim for the extensive review.
1 parent 354acd0 commit f179113

File tree

5 files changed

+198
-11
lines changed

5 files changed

+198
-11
lines changed

‎django/contrib/contenttypes/apps.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
from django.apps import AppConfig
22
from django.contrib.contenttypes.checks import check_generic_foreign_keys
33
from django.core import checks
4-
from django.db.models.signals import post_migrate
4+
from django.db.models.signals import post_migrate, pre_migrate
55
from django.utils.translation import ugettext_lazy as _
66

7-
from .management import update_contenttypes
7+
from .management import (
8+
inject_rename_contenttypes_operations, update_contenttypes,
9+
)
810

911

1012
class ContentTypesConfig(AppConfig):
1113
name = 'django.contrib.contenttypes'
1214
verbose_name = _("Content Types")
1315

1416
def ready(self):
17+
pre_migrate.connect(inject_rename_contenttypes_operations, sender=self)
1518
post_migrate.connect(update_contenttypes)
1619
checks.register(check_generic_foreign_keys, checks.Tags.models)

‎django/contrib/contenttypes/management.py

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,91 @@
11
from django.apps import apps as global_apps
2-
from django.db import DEFAULT_DB_ALIAS, router
2+
from django.db import DEFAULT_DB_ALIAS, migrations, router, transaction
3+
from django.db.utils import IntegrityError
34
from django.utils import six
45
from django.utils.six.moves import input
56

67

8+
class RenameContentType(migrations.RunPython):
9+
def __init__(self, app_label, old_model, new_model):
10+
self.app_label = app_label
11+
self.old_model = old_model
12+
self.new_model = new_model
13+
super(RenameContentType, self).__init__(self.rename_forward, self.rename_backward)
14+
15+
def _rename(self, apps, schema_editor, old_model, new_model):
16+
ContentType = apps.get_model('contenttypes', 'ContentType')
17+
db = schema_editor.connection.alias
18+
if not router.allow_migrate_model(db, ContentType):
19+
return
20+
21+
try:
22+
content_type = ContentType.objects.db_manager(db).get_by_natural_key(self.app_label, old_model)
23+
except ContentType.DoesNotExist:
24+
pass
25+
else:
26+
content_type.model = new_model
27+
try:
28+
with transaction.atomic(using=db):
29+
content_type.save(update_fields={'model'})
30+
except IntegrityError:
31+
# Gracefully fallback if a stale content type causes a
32+
# conflict as update_contenttypes will take care of asking the
33+
# user what should be done next.
34+
content_type.model = old_model
35+
else:
36+
# Clear the cache as the `get_by_natual_key()` call will cache
37+
# the renamed ContentType instance by its old model name.
38+
ContentType.objects.clear_cache()
39+
40+
def rename_forward(self, apps, schema_editor):
41+
self._rename(apps, schema_editor, self.old_model, self.new_model)
42+
43+
def rename_backward(self, apps, schema_editor):
44+
self._rename(apps, schema_editor, self.new_model, self.old_model)
45+
46+
47+
def inject_rename_contenttypes_operations(plan=None, apps=global_apps, using=DEFAULT_DB_ALIAS, **kwargs):
48+
"""
49+
Insert a `RenameContentType` operation after every planned `RenameModel`
50+
operation.
51+
"""
52+
if plan is None:
53+
return
54+
55+
# Determine whether or not the ContentType model is available.
56+
try:
57+
ContentType = apps.get_model('contenttypes', 'ContentType')
58+
except LookupError:
59+
available = False
60+
else:
61+
if not router.allow_migrate_model(using, ContentType):
62+
return
63+
available = True
64+
65+
for migration, backward in plan:
66+
if ((migration.app_label, migration.name) == ('contenttypes', '0001_initial')):
67+
# There's no point in going forward if the initial contenttypes
68+
# migration is unapplied as the ContentType model will be
69+
# unavailable from this point.
70+
if backward:
71+
break
72+
else:
73+
available = True
74+
continue
75+
# The ContentType model is not available yet.
76+
if not available:
77+
continue
78+
inserts = []
79+
for index, operation in enumerate(migration.operations):
80+
if isinstance(operation, migrations.RenameModel):
81+
operation = RenameContentType(
82+
migration.app_label, operation.old_name_lower, operation.new_name_lower
83+
)
84+
inserts.append((index + 1, operation))
85+
for inserted, (index, operation) in enumerate(inserts):
86+
migration.operations.insert(inserted + index, operation)
87+
88+
789
def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs):
890
"""
991
Creates content types for models in the given app, removing any model

‎tests/contenttypes_tests/operations_migrations/0001_initial.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# -*- coding: utf-8 -*-
2+
from __future__ import unicode_literals
3+
4+
from django.db import migrations, models
5+
6+
7+
def assert_foo_contenttype_not_cached(apps, schema_editor):
8+
ContentType = apps.get_model('contenttypes', 'ContentType')
9+
try:
10+
content_type = ContentType.objects.get_by_natural_key('contenttypes_tests', 'foo')
11+
except ContentType.DoesNotExist:
12+
pass
13+
else:
14+
if not ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists():
15+
raise AssertionError('The contenttypes_tests.Foo ContentType should not be cached.')
16+
elif content_type.model != 'foo':
17+
raise AssertionError(
18+
"The cached contenttypes_tests.Foo ContentType should have "
19+
"its model set to 'foo'."
20+
)
21+
22+
23+
class Migration(migrations.Migration):
24+
25+
operations = [
26+
migrations.CreateModel(
27+
'Foo',
28+
[
29+
('id', models.AutoField(primary_key=True)),
30+
],
31+
),
32+
migrations.RenameModel('Foo', 'RenamedFoo'),
33+
migrations.RunPython(assert_foo_contenttype_not_cached, migrations.RunPython.noop)
34+
]

‎tests/contenttypes_tests/operations_migrations/__init__.py

Whitespace-only changes.

‎tests/contenttypes_tests/tests.py

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,18 @@
44
import datetime
55

66
from django.apps.registry import Apps, apps
7-
from django.contrib.contenttypes import management
7+
from django.conf import settings
8+
from django.contrib.contenttypes import management as contenttypes_management
89
from django.contrib.contenttypes.fields import (
910
GenericForeignKey, GenericRelation,
1011
)
1112
from django.contrib.contenttypes.models import ContentType
1213
from django.contrib.sites.models import Site
13-
from django.core import checks
14-
from django.db import connections, models
15-
from django.test import SimpleTestCase, TestCase, mock, override_settings
14+
from django.core import checks, management
15+
from django.db import connections, migrations, models
16+
from django.test import (
17+
SimpleTestCase, TestCase, TransactionTestCase, mock, override_settings,
18+
)
1619
from django.test.utils import captured_stdout, isolate_apps
1720
from django.utils.encoding import force_str, force_text
1821

@@ -388,9 +391,9 @@ def test_interactive_true(self):
388391
interactive mode of update_contenttypes() (the default) should delete
389392
stale contenttypes.
390393
"""
391-
management.input = lambda x: force_str("yes")
394+
contenttypes_management.input = lambda x: force_str("yes")
392395
with captured_stdout() as stdout:
393-
management.update_contenttypes(self.app_config)
396+
contenttypes_management.update_contenttypes(self.app_config)
394397
self.assertIn("Deleting stale content type", stdout.getvalue())
395398
self.assertEqual(ContentType.objects.count(), self.before_count)
396399

@@ -400,7 +403,7 @@ def test_interactive_false(self):
400403
content types.
401404
"""
402405
with captured_stdout() as stdout:
403-
management.update_contenttypes(self.app_config, interactive=False)
406+
contenttypes_management.update_contenttypes(self.app_config, interactive=False)
404407
self.assertIn("Stale content types remain.", stdout.getvalue())
405408
self.assertEqual(ContentType.objects.count(), self.before_count + 1)
406409

@@ -411,7 +414,7 @@ def test_unavailable_content_type_model(self):
411414
"""
412415
apps = Apps()
413416
with self.assertNumQueries(0):
414-
management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps)
417+
contenttypes_management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps)
415418
self.assertEqual(ContentType.objects.count(), self.before_count + 1)
416419

417420

@@ -445,3 +448,68 @@ def test_multidb(self):
445448
with self.assertNumQueries(0, using='default'), \
446449
self.assertNumQueries(1, using='other'):
447450
ContentType.objects.get_for_model(Author)
451+
452+
453+
@override_settings(
454+
MIGRATION_MODULES=dict(settings.MIGRATION_MODULES, contenttypes_tests='contenttypes_tests.operations_migrations'),
455+
)
456+
class ContentTypeOperationsTests(TransactionTestCase):
457+
available_apps = ['django.contrib.contenttypes', 'contenttypes_tests']
458+
459+
def setUp(self):
460+
app_config = apps.get_app_config('contenttypes_tests')
461+
models.signals.post_migrate.connect(self.assertOperationsInjected, sender=app_config)
462+
463+
def tearDown(self):
464+
app_config = apps.get_app_config('contenttypes_tests')
465+
models.signals.post_migrate.disconnect(self.assertOperationsInjected, sender=app_config)
466+
467+
def assertOperationsInjected(self, plan, **kwargs):
468+
for migration, _backward in plan:
469+
operations = iter(migration.operations)
470+
for operation in operations:
471+
if isinstance(operation, migrations.RenameModel):
472+
next_operation = next(operations)
473+
self.assertIsInstance(next_operation, contenttypes_management.RenameContentType)
474+
self.assertEqual(next_operation.app_label, migration.app_label)
475+
self.assertEqual(next_operation.old_model, operation.old_name_lower)
476+
self.assertEqual(next_operation.new_model, operation.new_name_lower)
477+
478+
def test_existing_content_type_rename(self):
479+
ContentType.objects.create(app_label='contenttypes_tests', model='foo')
480+
management.call_command(
481+
'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0,
482+
)
483+
self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
484+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())
485+
management.call_command(
486+
'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0,
487+
)
488+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
489+
self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())
490+
491+
def test_missing_content_type_rename_ignore(self):
492+
management.call_command(
493+
'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0,
494+
)
495+
self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
496+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())
497+
management.call_command(
498+
'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0,
499+
)
500+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
501+
self.assertFalse(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())
502+
503+
def test_content_type_rename_conflict(self):
504+
ContentType.objects.create(app_label='contenttypes_tests', model='foo')
505+
ContentType.objects.create(app_label='contenttypes_tests', model='renamedfoo')
506+
management.call_command(
507+
'migrate', 'contenttypes_tests', database='default', interactive=False, verbosity=0,
508+
)
509+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
510+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())
511+
management.call_command(
512+
'migrate', 'contenttypes_tests', 'zero', database='default', interactive=False, verbosity=0,
513+
)
514+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='foo').exists())
515+
self.assertTrue(ContentType.objects.filter(app_label='contenttypes_tests', model='renamedfoo').exists())

0 commit comments

Comments
 (0)