From e0cd07ec2f394e6db3d17de19809a8f377cd1578 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 25 Jun 2014 08:53:09 -0400 Subject: [PATCH] Fixed #22903 -- Fixed migration generation if index_together or unique_together is removed from a model. --- django/db/migrations/autodetector.py | 25 ++++++--------- django/db/migrations/operations/models.py | 28 ++++++++++------- tests/migrations/test_autodetector.py | 38 +++++++++++++++++++++++ 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index cffd1a8b2f..05c4725d54 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -761,33 +761,26 @@ class MigrationAutodetector(object): ) ) - def generate_altered_unique_together(self): + def _generate_altered_foo_together(self, operation): + option_name = operation.option_name for app_label, model_name in sorted(self.kept_model_keys): old_model_name = self.renamed_models.get((app_label, model_name), model_name) old_model_state = self.from_state.models[app_label, old_model_name] new_model_state = self.to_state.models[app_label, model_name] - if old_model_state.options.get("unique_together", None) != new_model_state.options.get("unique_together", None): + if old_model_state.options.get(option_name) != new_model_state.options.get(option_name): self.add_operation( app_label, - operations.AlterUniqueTogether( + operation( name=model_name, - unique_together=new_model_state.options['unique_together'], + **{option_name: new_model_state.options.get(option_name)} ) ) + def generate_altered_unique_together(self): + self._generate_altered_foo_together(operations.AlterUniqueTogether) + def generate_altered_index_together(self): - for app_label, model_name in sorted(self.kept_model_keys): - old_model_name = self.renamed_models.get((app_label, model_name), model_name) - old_model_state = self.from_state.models[app_label, old_model_name] - new_model_state = self.to_state.models[app_label, model_name] - if old_model_state.options.get("index_together", None) != new_model_state.options.get("index_together", None): - self.add_operation( - app_label, - operations.AlterIndexTogether( - name=model_name, - index_together=new_model_state.options['index_together'], - ) - ) + self._generate_altered_foo_together(operations.AlterIndexTogether) def generate_altered_options(self): """ diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index b6d75ccc44..7d5b0b258f 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -213,18 +213,21 @@ class AlterModelTable(Operation): class AlterUniqueTogether(Operation): """ - Changes the value of index_together to the target one. + Changes the value of unique_together to the target one. Input value of unique_together must be a set of tuples. """ + option_name = "unique_together" def __init__(self, name, unique_together): self.name = name unique_together = normalize_together(unique_together) - self.unique_together = set(tuple(cons) for cons in unique_together) + # need None rather than an empty set to prevent infinite migrations + # after removing unique_together from a model + self.unique_together = set(tuple(cons) for cons in unique_together) or None def state_forwards(self, app_label, state): model_state = state.models[app_label, self.name.lower()] - model_state.options["unique_together"] = self.unique_together + model_state.options[self.option_name] = self.unique_together def database_forwards(self, app_label, schema_editor, from_state, to_state): old_apps = from_state.render() @@ -234,8 +237,8 @@ class AlterUniqueTogether(Operation): if self.allowed_to_migrate(schema_editor.connection.alias, new_model): schema_editor.alter_unique_together( new_model, - getattr(old_model._meta, "unique_together", set()), - getattr(new_model._meta, "unique_together", set()), + getattr(old_model._meta, self.option_name, set()), + getattr(new_model._meta, self.option_name, set()), ) def database_backwards(self, app_label, schema_editor, from_state, to_state): @@ -245,7 +248,7 @@ class AlterUniqueTogether(Operation): return name.lower() == self.name.lower() def describe(self): - return "Alter unique_together for %s (%s constraints)" % (self.name, len(self.unique_together)) + return "Alter %s for %s (%s constraints)" % (self.option_name, self.name, len(self.unique_together)) class AlterIndexTogether(Operation): @@ -253,15 +256,18 @@ class AlterIndexTogether(Operation): Changes the value of index_together to the target one. Input value of index_together must be a set of tuples. """ + option_name = "index_together" def __init__(self, name, index_together): self.name = name index_together = normalize_together(index_together) - self.index_together = set(tuple(cons) for cons in index_together) + # need None rather than an empty set to prevent infinite migrations + # after removing unique_together from a model + self.index_together = set(tuple(cons) for cons in index_together) or None def state_forwards(self, app_label, state): model_state = state.models[app_label, self.name.lower()] - model_state.options["index_together"] = self.index_together + model_state.options[self.option_name] = self.index_together def database_forwards(self, app_label, schema_editor, from_state, to_state): old_apps = from_state.render() @@ -271,8 +277,8 @@ class AlterIndexTogether(Operation): if self.allowed_to_migrate(schema_editor.connection.alias, new_model): schema_editor.alter_index_together( new_model, - getattr(old_model._meta, "index_together", set()), - getattr(new_model._meta, "index_together", set()), + getattr(old_model._meta, self.option_name, set()), + getattr(new_model._meta, self.option_name, set()), ) def database_backwards(self, app_label, schema_editor, from_state, to_state): @@ -282,7 +288,7 @@ class AlterIndexTogether(Operation): return name.lower() == self.name.lower() def describe(self): - return "Alter index_together for %s (%s constraints)" % (self.name, len(self.index_together)) + return "Alter %s for %s (%s constraints)" % (self.self.option_name, self.name, len(self.index_together)) class AlterOrderWithRespectTo(Operation): diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 33083c38b5..cedfa755b4 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -567,6 +567,44 @@ class AutodetectorTests(TestCase): self.assertEqual(action2.__class__.__name__, "AlterUniqueTogether") self.assertEqual(action2.unique_together, set([("title", "newfield")])) + def test_remove_index_together(self): + author_index_together = ModelState("testapp", "Author", [ + ("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)) + ], {"index_together": [("id", "name")]}) + + before = self.make_project_state([author_index_together]) + after = self.make_project_state([self.author_name]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertEqual(len(changes['testapp']), 1) + migration = changes['testapp'][0] + # Right number of actions? + self.assertEqual(len(migration.operations), 1) + # Right actions? + action = migration.operations[0] + self.assertEqual(action.__class__.__name__, "AlterIndexTogether") + self.assertEqual(action.index_together, None) + + def test_remove_unique_together(self): + author_unique_together = ModelState("testapp", "Author", [ + ("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)) + ], {"unique_together": [("id", "name")]}) + + before = self.make_project_state([author_unique_together]) + after = self.make_project_state([self.author_name]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertEqual(len(changes['testapp']), 1) + migration = changes['testapp'][0] + # Right number of actions? + self.assertEqual(len(migration.operations), 1) + # Right actions? + action = migration.operations[0] + self.assertEqual(action.__class__.__name__, "AlterUniqueTogether") + self.assertEqual(action.unique_together, None) + def test_proxy(self): "Tests that the autodetector correctly deals with proxy models" # First, we test adding a proxy model