From 040bd7c9387cfbf70a543c507b3b9eeb8f2725dc Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 3 Oct 2016 19:23:18 -0400 Subject: [PATCH] Fixed #27279 -- Fixed a migration performance regression related to RenameModel operations. Thanks Trac alias mtomiyoshi for the report, Marten Kenbeek for the initial patch and Tim for the review. --- django/db/migrations/operations/models.py | 7 +++++++ docs/releases/1.10.3.txt | 3 +++ tests/migrations/test_operations.py | 22 +++++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 027ebfe95a..a7024b5dde 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -278,6 +278,11 @@ class RenameModel(ModelOperation): ) def state_forwards(self, app_label, state): + # In cases where state doesn't have rendered apps, prevent subsequent + # reload_model() calls from rendering models for performance + # reasons. This method should be refactored to avoid relying on + # state.apps (#27310). + reset_apps = 'apps' not in state.__dict__ apps = state.apps model = apps.get_model(app_label, self.old_name) model._meta.apps = apps @@ -286,6 +291,8 @@ class RenameModel(ModelOperation): f for f in model._meta.get_fields(include_hidden=True) if f.auto_created and not f.concrete and (not f.hidden or f.many_to_many) ) + if reset_apps: + del state.__dict__['apps'] # Rename the model state.models[app_label, self.new_name_lower] = state.models[app_label, self.old_name_lower] state.models[app_label, self.new_name_lower].name = self.new_name diff --git a/docs/releases/1.10.3.txt b/docs/releases/1.10.3.txt index 2560b1db00..3e40ab6854 100644 --- a/docs/releases/1.10.3.txt +++ b/docs/releases/1.10.3.txt @@ -11,3 +11,6 @@ Bugfixes * Allowed ``User.is_authenticated`` and ``User.is_anonymous`` properties to be tested for ``set`` membership (:ticket:`27309`). + +* Fixed a performance regression when running ``migrate`` in projects + with ``RenameModel`` operations (:ticket:`27279`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 553ed58ea4..cab0caf75a 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -5,7 +5,7 @@ import unittest from django.db import connection, migrations, models, transaction from django.db.migrations.migration import Migration from django.db.migrations.operations import CreateModel -from django.db.migrations.state import ProjectState +from django.db.migrations.state import ModelState, ProjectState from django.db.models.fields import NOT_PROVIDED from django.db.transaction import atomic from django.db.utils import IntegrityError @@ -590,6 +590,26 @@ class OperationTests(OperationTestBase): self.assertEqual(definition[1], []) self.assertEqual(definition[2], {'old_name': "Pony", 'new_name': "Horse"}) + def test_rename_model_state_forwards(self): + """ + RenameModel operations shouldn't trigger the caching of rendered apps + on state without prior apps. + """ + state = ProjectState() + state.add_model(ModelState('migrations', 'Foo', [])) + operation = migrations.RenameModel('Foo', 'Bar') + operation.state_forwards('migrations', state) + self.assertNotIn('apps', state.__dict__) + self.assertNotIn(('migrations', 'foo'), state.models) + self.assertIn(('migrations', 'bar'), state.models) + # Now with apps cached. + apps = state.apps + operation = migrations.RenameModel('Bar', 'Foo') + operation.state_forwards('migrations', state) + self.assertIs(state.apps, apps) + self.assertNotIn(('migrations', 'bar'), state.models) + self.assertIn(('migrations', 'foo'), state.models) + def test_rename_model_with_self_referential_fk(self): """ Tests the RenameModel operation on model with self referential FK.