Fixed #23452 -- Prevented infinite migrations for empty unique/index_together.
Thanks fwkroon for the report.
This commit is contained in:
parent
ef8ef2a42d
commit
6d5958c7a3
|
@ -871,23 +871,28 @@ class MigrationAutodetector(object):
|
||||||
old_model_name = self.renamed_models.get((app_label, model_name), model_name)
|
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]
|
old_model_state = self.from_state.models[app_label, old_model_name]
|
||||||
new_model_state = self.to_state.models[app_label, 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
|
# 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 = old_model_state.options.get(option_name) or set()
|
||||||
old_value = None
|
if old_value:
|
||||||
else:
|
|
||||||
old_value = set([
|
old_value = set([
|
||||||
tuple(
|
tuple(
|
||||||
self.renamed_fields.get((app_label, model_name, n), n)
|
self.renamed_fields.get((app_label, model_name, n), n)
|
||||||
for n in unique
|
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(
|
self.add_operation(
|
||||||
app_label,
|
app_label,
|
||||||
operation(
|
operation(
|
||||||
name=model_name,
|
name=model_name,
|
||||||
**{option_name: new_model_state.options.get(option_name)}
|
**{option_name: new_value}
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
@ -233,9 +233,7 @@ class AlterUniqueTogether(Operation):
|
||||||
def __init__(self, name, unique_together):
|
def __init__(self, name, unique_together):
|
||||||
self.name = name
|
self.name = name
|
||||||
unique_together = normalize_together(unique_together)
|
unique_together = normalize_together(unique_together)
|
||||||
# need None rather than an empty set to prevent infinite migrations
|
self.unique_together = set(tuple(cons) for cons in unique_together)
|
||||||
# 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):
|
def state_forwards(self, app_label, state):
|
||||||
model_state = state.models[app_label, self.name.lower()]
|
model_state = state.models[app_label, self.name.lower()]
|
||||||
|
@ -273,9 +271,7 @@ class AlterIndexTogether(Operation):
|
||||||
def __init__(self, name, index_together):
|
def __init__(self, name, index_together):
|
||||||
self.name = name
|
self.name = name
|
||||||
index_together = normalize_together(index_together)
|
index_together = normalize_together(index_together)
|
||||||
# need None rather than an empty set to prevent infinite migrations
|
self.index_together = set(tuple(cons) for cons in index_together)
|
||||||
# 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):
|
def state_forwards(self, app_label, state):
|
||||||
model_state = state.models[app_label, self.name.lower()]
|
model_state = state.models[app_label, self.name.lower()]
|
||||||
|
|
|
@ -31,3 +31,6 @@ Bugfixes
|
||||||
``'auth.User'`` model (:ticket:`11775`). As a side effect, the setting now
|
``'auth.User'`` model (:ticket:`11775`). As a side effect, the setting now
|
||||||
adds a ``get_absolute_url()`` method to any model that appears in
|
adds a ``get_absolute_url()`` method to any model that appears in
|
||||||
``ABSOLUTE_URL_OVERRIDES`` but doesn't define ``get_absolute_url()``.
|
``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`).
|
||||||
|
|
|
@ -529,6 +529,50 @@ class AutodetectorTests(TestCase):
|
||||||
# Right number of migrations?
|
# Right number of migrations?
|
||||||
self.assertEqual(len(changes), 0)
|
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):
|
def test_unique_together_ordering(self):
|
||||||
"Tests that unique_together also triggers on ordering changes"
|
"Tests that unique_together also triggers on ordering changes"
|
||||||
# Make state
|
# Make state
|
||||||
|
@ -582,7 +626,7 @@ class AutodetectorTests(TestCase):
|
||||||
# Right actions?
|
# Right actions?
|
||||||
action = migration.operations[0]
|
action = migration.operations[0]
|
||||||
self.assertEqual(action.__class__.__name__, "AlterIndexTogether")
|
self.assertEqual(action.__class__.__name__, "AlterIndexTogether")
|
||||||
self.assertEqual(action.index_together, None)
|
self.assertEqual(action.index_together, set())
|
||||||
|
|
||||||
def test_remove_unique_together(self):
|
def test_remove_unique_together(self):
|
||||||
author_unique_together = ModelState("testapp", "Author", [
|
author_unique_together = ModelState("testapp", "Author", [
|
||||||
|
@ -601,7 +645,7 @@ class AutodetectorTests(TestCase):
|
||||||
# Right actions?
|
# Right actions?
|
||||||
action = migration.operations[0]
|
action = migration.operations[0]
|
||||||
self.assertEqual(action.__class__.__name__, "AlterUniqueTogether")
|
self.assertEqual(action.__class__.__name__, "AlterUniqueTogether")
|
||||||
self.assertEqual(action.unique_together, None)
|
self.assertEqual(action.unique_together, set())
|
||||||
|
|
||||||
def test_proxy(self):
|
def test_proxy(self):
|
||||||
"Tests that the autodetector correctly deals with proxy models"
|
"Tests that the autodetector correctly deals with proxy models"
|
||||||
|
|
Loading…
Reference in New Issue