From b30d32ff24096e4191d661e2aaa734c92795237f Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Sun, 22 Jun 2014 11:23:45 -0700 Subject: [PATCH] Fixed #22875: Optimizer did not take through= into account. --- django/db/migrations/optimizer.py | 25 ++++++++++++++++---- tests/migrations/test_optimizer.py | 38 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/django/db/migrations/optimizer.py b/django/db/migrations/optimizer.py index 52a822ebf72..6c5dd8cbf7f 100644 --- a/django/db/migrations/optimizer.py +++ b/django/db/migrations/optimizer.py @@ -160,6 +160,19 @@ class MigrationOptimizer(object): return om(operation, other, in_between or []) return None + def model_to_key(self, model): + """ + Takes either a model class or a "appname.ModelName" string + and returns (appname, modelname) + """ + if isinstance(model, six.string_types): + return model.split(".", 1) + else: + return ( + model._meta.app_label, + model._meta.object_name, + ) + def reduce_model_create_delete(self, operation, other, in_between): """ Folds a CreateModel and a DeleteModel into nothing. @@ -206,13 +219,15 @@ class MigrationOptimizer(object): # Don't allow optimisations of FKs through models they reference if hasattr(other.field, "rel") and other.field.rel: for between in in_between: - if isinstance(other.field.rel.to, six.string_types): - object_name, app_label = other.field.rel.to.split(".", 1) - else: - object_name = other.field.rel.to._meta.object_name - app_label = other.field.rel.to._meta.app_label + # Check that it doesn't point to the model + app_label, object_name = self.model_to_key(other.field.rel.to) if between.references_model(object_name, app_label): return None + # Check that it's not through the model + if getattr(other.field.rel, "through", None): + app_label, object_name = self.model_to_key(other.field.rel.through) + if between.references_model(object_name, app_label): + return None # OK, that's fine return [ migrations.CreateModel( diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py index 346d30ae289..d36b93355ee 100644 --- a/tests/migrations/test_optimizer.py +++ b/tests/migrations/test_optimizer.py @@ -201,6 +201,44 @@ class OptimizerTests(TestCase): ], ) + def test_create_model_add_field_not_through_fk(self): + """ + AddField should NOT optimize into CreateModel if it's an FK to a model + that's between them. + """ + self.assertOptimizesTo( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Link", [("url", models.TextField())]), + migrations.AddField("Foo", "link", models.ForeignKey("migrations.Link")), + ], + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Link", [("url", models.TextField())]), + migrations.AddField("Foo", "link", models.ForeignKey("migrations.Link")), + ], + ) + + def test_create_model_add_field_not_through_m2m_through(self): + """ + AddField should NOT optimize into CreateModel if it's an M2M using a + through that's created between them. + """ + # Note: The middle model is not actually a valid through model, + # but that doesn't matter, as we never render it. + self.assertOptimizesTo( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("LinkThrough", []), + migrations.AddField("Foo", "link", models.ManyToManyField("migrations.Link", through="migrations.LinkThrough")), + ], + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("LinkThrough", []), + migrations.AddField("Foo", "link", models.ManyToManyField("migrations.Link", through="migrations.LinkThrough")), + ], + ) + def test_create_model_alter_field(self): """ AlterField should optimize into CreateModel.