Fixed #22903 -- Fixed migration generation if index_together or unique_together is removed from a model.

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