diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index a8647243d9..c8b7db7539 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -871,23 +871,28 @@ class MigrationAutodetector(object): 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] + # We run the old version through the field renames to account for those - if old_model_state.options.get(option_name) is None: - old_value = None - else: + old_value = old_model_state.options.get(option_name) or set() + if old_value: old_value = set([ tuple( self.renamed_fields.get((app_label, model_name, n), n) for n in unique ) - for unique in old_model_state.options[option_name] + for unique in old_value ]) - if old_value != new_model_state.options.get(option_name): + + new_value = new_model_state.options.get(option_name) or set() + if new_value: + new_value = set(new_value) + + if old_value != new_value: self.add_operation( app_label, operation( name=model_name, - **{option_name: new_model_state.options.get(option_name)} + **{option_name: new_value} ) ) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 5b1a23eab7..eade3ee878 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -233,9 +233,7 @@ class AlterUniqueTogether(Operation): def __init__(self, name, unique_together): self.name = name unique_together = normalize_together(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 + self.unique_together = set(tuple(cons) for cons in unique_together) def state_forwards(self, app_label, state): model_state = state.models[app_label, self.name.lower()] @@ -273,9 +271,7 @@ class AlterIndexTogether(Operation): def __init__(self, name, index_together): self.name = name index_together = normalize_together(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 + self.index_together = set(tuple(cons) for cons in index_together) def state_forwards(self, app_label, state): model_state = state.models[app_label, self.name.lower()] diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt index 46a19f1f6d..b98ec1b847 100644 --- a/docs/releases/1.7.1.txt +++ b/docs/releases/1.7.1.txt @@ -31,3 +31,6 @@ Bugfixes ``'auth.User'`` model (:ticket:`11775`). As a side effect, the setting now adds a ``get_absolute_url()`` method to any model that appears in ``ABSOLUTE_URL_OVERRIDES`` but doesn't define ``get_absolute_url()``. + +* Empty ``index_together`` or ``unique_together`` model options no longer + results in infinite migrations (:ticket:`23452`). diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 526c018907..0ba05d3495 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -502,6 +502,50 @@ class AutodetectorTests(TestCase): # Right number of migrations? self.assertEqual(len(changes), 0) + def test_empty_foo_together(self): + "#23452 - Empty unique/index_togther shouldn't generate a migration." + # Explicitly testing for not specified, since this is the case after + # a CreateModel operation w/o any definition on the original model + model_state_not_secified = ModelState("a", "model", + [("id", models.AutoField(primary_key=True))] + ) + # Explicitly testing for None, since this was the issue in #23452 after + # a AlterFooTogether operation with e.g. () as value + model_state_none = ModelState("a", "model", + [("id", models.AutoField(primary_key=True))], + {"unique_together": None, "index_together": None} + ) + # Explicitly testing for the empty set, since we now always have sets. + # During removal (('col1', 'col2'),) --> () this becomes set([]) + model_state_empty = ModelState("a", "model", + [("id", models.AutoField(primary_key=True))], + {"unique_together": set(), "index_together": set()} + ) + + def test(from_state, to_state, msg): + before = self.make_project_state([from_state]) + after = self.make_project_state([to_state]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + if len(changes) > 0: + ops = ', '.join(o.__class__.__name__ for o in changes['a'][0].operations) + self.fail('Created operation(s) %s from %s' % (ops, msg)) + + tests = ( + (model_state_not_secified, model_state_not_secified, '"not specified" to "not specified"'), + (model_state_not_secified, model_state_none, '"not specified" to "None"'), + (model_state_not_secified, model_state_empty, '"not specified" to "empty"'), + (model_state_none, model_state_not_secified, '"None" to "not specified"'), + (model_state_none, model_state_none, '"None" to "None"'), + (model_state_none, model_state_empty, '"None" to "empty"'), + (model_state_empty, model_state_not_secified, '"empty" to "not specified"'), + (model_state_empty, model_state_none, '"empty" to "None"'), + (model_state_empty, model_state_empty, '"empty" to "empty"'), + ) + + for t in tests: + test(*t) + def test_unique_together_ordering(self): "Tests that unique_together also triggers on ordering changes" # Make state @@ -555,7 +599,7 @@ class AutodetectorTests(TestCase): # Right actions? action = migration.operations[0] self.assertEqual(action.__class__.__name__, "AlterIndexTogether") - self.assertEqual(action.index_together, None) + self.assertEqual(action.index_together, set()) def test_remove_unique_together(self): author_unique_together = ModelState("testapp", "Author", [ @@ -574,7 +618,7 @@ class AutodetectorTests(TestCase): # Right actions? action = migration.operations[0] self.assertEqual(action.__class__.__name__, "AlterUniqueTogether") - self.assertEqual(action.unique_together, None) + self.assertEqual(action.unique_together, set()) def test_proxy(self): "Tests that the autodetector correctly deals with proxy models"