summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarkus Holtermann <info@markusholtermann.eu>2014-09-09 23:28:45 +0200
committerTim Graham <timograham@gmail.com>2014-09-10 07:57:31 -0400
commit67872bfff12de7dcf22c966970fe21f65fe593c8 (patch)
tree36f9e57c165ff6331552e5ab23ff261b9651b4d8
parentc2fe985733ecf5ec113a92f155cc6b769932d544 (diff)
[1.7.x] Fixed #23452 -- Prevented infinite migrations for empty unique/index_together.
Thanks fwkroon for the report. Backport of 6d5958c7a3 from master
-rw-r--r--django/db/migrations/autodetector.py17
-rw-r--r--django/db/migrations/operations/models.py8
-rw-r--r--docs/releases/1.7.1.txt3
-rw-r--r--tests/migrations/test_autodetector.py48
4 files changed, 62 insertions, 14 deletions
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"