From 7fc4c1db627807874966e4f96b34cff7c9af886f Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Sun, 5 Jun 2016 20:28:15 +0500 Subject: [PATCH] Fixed #26710 -- Made CreateModel.references_model() take app_label into account. --- django/db/migrations/operations/models.py | 22 +++++---- tests/migrations/test_optimizer.py | 57 ++++++++++++++++++++--- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index d18fded97c..1a3b523820 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -104,20 +104,22 @@ class CreateModel(ModelOperation): return "Create %smodel %s" % ("proxy " if self.options.get("proxy", False) else "", self.name) def references_model(self, name, app_label=None): - strings_to_check = [self.name] + name_lower = name.lower() + if name_lower == self.name_lower: + return True + # Check we didn't inherit from the model - for base in self.bases: - if isinstance(base, six.string_types): - strings_to_check.append(base.split(".")[-1]) + models_to_check = [base for base in self.bases if base is not models.Model] # Check we have no FKs/M2Ms with it for fname, field in self.fields: if field.remote_field: - if isinstance(field.remote_field.model, six.string_types): - strings_to_check.append(field.remote_field.model.split(".")[-1]) - # Now go over all the strings and compare them - for string in strings_to_check: - if string.lower() == name.lower(): - return True + models_to_check.append(field.remote_field.model) + # Now go over all the models and check against them + for model in models_to_check: + model_app_label, model_name = self.model_to_key(model) + if model_name.lower() == name_lower: + if app_label is None or not model_app_label or model_app_label == app_label: + return True return False def model_to_key(self, model): diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py index ce60bb32af..6bf09d0178 100644 --- a/tests/migrations/test_optimizer.py +++ b/tests/migrations/test_optimizer.py @@ -13,15 +13,15 @@ class OptimizerTests(SimpleTestCase): Tests the migration autodetector. """ - def optimize(self, operations): + def optimize(self, operations, app_label): """ Handy shortcut for getting results + number of loops """ optimizer = MigrationOptimizer() - return optimizer.optimize(operations), optimizer._iterations + return optimizer.optimize(operations, app_label), optimizer._iterations - def assertOptimizesTo(self, operations, expected, exact=None, less_than=None): - result, iterations = self.optimize(operations) + def assertOptimizesTo(self, operations, expected, exact=None, less_than=None, app_label=None): + result, iterations = self.optimize(operations, app_label) result = [repr(f.deconstruct()) for f in result] expected = [repr(f.deconstruct()) for f in expected] self.assertEqual(expected, result) @@ -34,8 +34,8 @@ class OptimizerTests(SimpleTestCase): "Optimization did not take less than %s iterations (it took %s)" % (less_than, iterations) ) - def assertDoesNotOptimize(self, operations): - self.assertOptimizesTo(operations, operations) + def assertDoesNotOptimize(self, operations, **kwargs): + self.assertOptimizesTo(operations, operations, **kwargs) def test_single(self): """ @@ -212,6 +212,29 @@ class OptimizerTests(SimpleTestCase): migrations.DeleteModel("Foo"), ], ) + # The same operations should be optimized if app_label is specified and + # a FK references a model from the other app. + self.assertOptimizesTo( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo", models.CASCADE))]), + migrations.DeleteModel("Foo"), + ], + [ + migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo", models.CASCADE))]), + ], + app_label="otherapp", + ) + # But it shouldn't work if a FK references a model with the same + # app_label. + self.assertDoesNotOptimize( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo", models.CASCADE))]), + migrations.DeleteModel("Foo"), + ], + app_label="testapp", + ) # This should not work - bases should block it self.assertOptimizesTo( [ @@ -225,6 +248,28 @@ class OptimizerTests(SimpleTestCase): migrations.DeleteModel("Foo"), ], ) + # The same operations should be optimized if app_label and none of + # bases belong to that app. + self.assertOptimizesTo( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo", )), + migrations.DeleteModel("Foo"), + ], + [ + migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo", )), + ], + app_label="otherapp", + ) + # But it shouldn't work if some of bases belongs to the specified app. + self.assertDoesNotOptimize( + [ + migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]), + migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo", )), + migrations.DeleteModel("Foo"), + ], + app_label="testapp", + ) def test_create_model_add_field(self): """