Fixed #23315: Operational dependency fail with mixed create/add

This commit is contained in:
Andrew Godwin 2014-08-19 19:50:14 -07:00
parent fca677fa43
commit 9247da1032
2 changed files with 73 additions and 76 deletions

View File

@ -286,7 +286,7 @@ class MigrationAutodetector(object):
if not chop_mode: if not chop_mode:
chop_mode = True chop_mode = True
else: else:
raise ValueError("Cannot resolve operation dependencies") raise ValueError("Cannot resolve operation dependencies: %r" % self.generated_operations)
num_ops = new_num_ops num_ops = new_num_ops
# OK, add in internal dependencies among the migrations # OK, add in internal dependencies among the migrations
@ -361,10 +361,13 @@ class MigrationAutodetector(object):
else: else:
raise ValueError("Can't handle dependency %r" % (dependency, )) raise ValueError("Can't handle dependency %r" % (dependency, ))
def add_operation(self, app_label, operation, dependencies=None): def add_operation(self, app_label, operation, dependencies=None, beginning=False):
# Dependencies are (app_label, model_name, field_name, create/delete as True/False) # Dependencies are (app_label, model_name, field_name, create/delete as True/False)
operation._auto_deps = dependencies or [] operation._auto_deps = dependencies or []
self.generated_operations.setdefault(app_label, []).append(operation) if beginning:
self.generated_operations.setdefault(app_label, []).insert(0, operation)
else:
self.generated_operations.setdefault(app_label, []).append(operation)
def swappable_first_key(self, item): def swappable_first_key(self, item):
""" """
@ -429,7 +432,7 @@ class MigrationAutodetector(object):
that might be deferred (e.g. unique_together, index_together) that might be deferred (e.g. unique_together, index_together)
""" """
added_models = set(self.new_model_keys) - set(self.old_model_keys) added_models = set(self.new_model_keys) - set(self.old_model_keys)
for app_label, model_name in sorted(added_models, key=self.swappable_first_key): for app_label, model_name in sorted(added_models, key=self.swappable_first_key, reverse=True):
model_state = self.to_state.models[app_label, model_name] model_state = self.to_state.models[app_label, model_name]
# Gather related fields # Gather related fields
related_fields = {} related_fields = {}
@ -481,6 +484,7 @@ class MigrationAutodetector(object):
bases=model_state.bases, bases=model_state.bases,
), ),
dependencies=dependencies, dependencies=dependencies,
beginning=True,
) )
# Generate operations for each related field # Generate operations for each related field
for name, field in sorted(related_fields.items()): for name, field in sorted(related_fields.items()):

View File

@ -131,12 +131,13 @@ class AutodetectorTests(TestCase):
operation = migration.operations[operation_index] operation = migration.operations[operation_index]
for attr, value in attrs.items(): for attr, value in attrs.items():
if getattr(operation, attr, None) != value: if getattr(operation, attr, None) != value:
self.fail("Attribute mismatch for %s.%s op #%s, %s (expected %r):\n%s" % ( self.fail("Attribute mismatch for %s.%s op #%s, %s (expected %r, got %r):\n%s" % (
app_label, app_label,
migration.name, migration.name,
operation_index + 1, operation_index,
attr, attr,
value, value,
getattr(operation, attr, None),
self.repr_changes(changes), self.repr_changes(changes),
)) ))
@ -403,21 +404,14 @@ class AutodetectorTests(TestCase):
after = self.make_project_state([self.author_with_publisher, self.publisher]) after = self.make_project_state([self.author_with_publisher, self.publisher])
autodetector = MigrationAutodetector(before, after) autodetector = MigrationAutodetector(before, after)
changes = autodetector._detect_changes() changes = autodetector._detect_changes()
# Right number of migrations? # Right number/type of migrations?
self.assertEqual(len(changes['testapp']), 1) self.assertNumberMigrations(changes, 'testapp', 1)
# Right number of actions? self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel", "AddField"])
migration = changes['testapp'][0] self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
self.assertEqual(len(migration.operations), 3) self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
# Right actions? self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
action = migration.operations[0]
self.assertEqual(action.__class__.__name__, "CreateModel")
action = migration.operations[1]
self.assertEqual(action.__class__.__name__, "CreateModel")
# Third action might vanish one day if the optimizer improves.
action = migration.operations[2]
self.assertEqual(action.__class__.__name__, "AddField")
# Right dependencies? # Right dependencies?
self.assertEqual(migration.dependencies, []) self.assertEqual(changes['testapp'][0].dependencies, [])
def test_circular_fk_dependency(self): def test_circular_fk_dependency(self):
""" """
@ -430,30 +424,18 @@ class AutodetectorTests(TestCase):
autodetector = MigrationAutodetector(before, after) autodetector = MigrationAutodetector(before, after)
changes = autodetector._detect_changes() changes = autodetector._detect_changes()
# Right number of migrations? # Right number of migrations?
self.assertEqual(len(changes['testapp']), 1) self.assertNumberMigrations(changes, 'testapp', 1)
self.assertEqual(len(changes['otherapp']), 2) self.assertNumberMigrations(changes, 'otherapp', 2)
# Right number of actions? # Right types?
migration1 = changes['testapp'][0] self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel"])
self.assertEqual(len(migration1.operations), 2) self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"])
migration2 = changes['otherapp'][0] self.assertOperationTypes(changes, 'otherapp', 1, ["AddField"])
self.assertEqual(len(migration2.operations), 1) self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
migration3 = changes['otherapp'][1] self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
self.assertEqual(len(migration3.operations), 1)
# Right actions?
action = migration1.operations[0]
self.assertEqual(action.__class__.__name__, "CreateModel")
self.assertEqual(action.name, "Author")
self.assertEqual(len(action.fields), 3)
action = migration2.operations[0]
self.assertEqual(action.__class__.__name__, "CreateModel")
self.assertEqual(len(action.fields), 2)
action = migration3.operations[0]
self.assertEqual(action.__class__.__name__, "AddField")
self.assertEqual(action.name, "author")
# Right dependencies? # Right dependencies?
self.assertEqual(migration1.dependencies, [("otherapp", "auto_1")]) self.assertEqual(changes['testapp'][0].dependencies, [("otherapp", "auto_1")])
self.assertEqual(migration2.dependencies, []) self.assertEqual(changes['otherapp'][0].dependencies, [])
self.assertEqual(set(migration3.dependencies), set([("testapp", "auto_1"), ("otherapp", "auto_1")])) self.assertEqual(set(changes['otherapp'][1].dependencies), set([("otherapp", "auto_1"), ("testapp", "auto_1")]))
def test_same_app_circular_fk_dependency(self): def test_same_app_circular_fk_dependency(self):
""" """
@ -465,24 +447,14 @@ class AutodetectorTests(TestCase):
after = self.make_project_state([self.author_with_publisher, self.publisher_with_author]) after = self.make_project_state([self.author_with_publisher, self.publisher_with_author])
autodetector = MigrationAutodetector(before, after) autodetector = MigrationAutodetector(before, after)
changes = autodetector._detect_changes() changes = autodetector._detect_changes()
# Right number of migrations? # Right number/type of migrations?
self.assertEqual(len(changes['testapp']), 1) self.assertNumberMigrations(changes, 'testapp', 1)
# Right number of actions? self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel", "AddField"])
migration1 = changes['testapp'][0] self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
self.assertEqual(len(migration1.operations), 4) self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
# Right actions? self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
action = migration1.operations[0]
self.assertEqual(action.__class__.__name__, "CreateModel")
action = migration1.operations[1]
self.assertEqual(action.__class__.__name__, "CreateModel")
action = migration1.operations[2]
self.assertEqual(action.__class__.__name__, "AddField")
self.assertEqual(action.name, "publisher")
action = migration1.operations[3]
self.assertEqual(action.__class__.__name__, "AddField")
self.assertEqual(action.name, "author")
# Right dependencies? # Right dependencies?
self.assertEqual(migration1.dependencies, []) self.assertEqual(changes['testapp'][0].dependencies, [])
def test_same_app_circular_fk_dependency_and_unique_together(self): def test_same_app_circular_fk_dependency_and_unique_together(self):
""" """
@ -495,23 +467,13 @@ class AutodetectorTests(TestCase):
after = self.make_project_state([self.knight, self.rabbit]) after = self.make_project_state([self.knight, self.rabbit])
autodetector = MigrationAutodetector(before, after) autodetector = MigrationAutodetector(before, after)
changes = autodetector._detect_changes() changes = autodetector._detect_changes()
# Right number of migrations? # Right number/type of migrations?
self.assertEqual(len(changes['eggs']), 1) self.assertNumberMigrations(changes, 'eggs', 1)
# Right number of actions? self.assertOperationTypes(changes, 'eggs', 0, ["CreateModel", "CreateModel", "AlterUniqueTogether"])
migration1 = changes['eggs'][0] self.assertFalse("unique_together" in changes['eggs'][0].operations[0].options)
self.assertEqual(len(migration1.operations), 3) self.assertFalse("unique_together" in changes['eggs'][0].operations[1].options)
# Right actions?
action = migration1.operations[0]
self.assertEqual(action.__class__.__name__, "CreateModel")
action = migration1.operations[1]
self.assertEqual(action.__class__.__name__, "CreateModel")
self.assertEqual(action.name, "Rabbit")
self.assertFalse("unique_together" in action.options)
action = migration1.operations[2]
self.assertEqual(action.__class__.__name__, "AlterUniqueTogether")
self.assertEqual(action.name, "rabbit")
# Right dependencies? # Right dependencies?
self.assertEqual(migration1.dependencies, []) self.assertEqual(changes['eggs'][0].dependencies, [])
def test_unique_together(self): def test_unique_together(self):
"Tests unique_together detection" "Tests unique_together detection"
@ -1110,3 +1072,34 @@ class AutodetectorTests(TestCase):
self.assertOperationTypes(changes, 'testapp', 0, ["AddField"]) self.assertOperationTypes(changes, 'testapp', 0, ["AddField"])
self.assertOperationAttributes(changes, 'testapp', 0, 0, name="book") self.assertOperationAttributes(changes, 'testapp', 0, 0, name="book")
self.assertEqual(changes['testapp'][0].dependencies, [("otherapp", "__first__")]) self.assertEqual(changes['testapp'][0].dependencies, [("otherapp", "__first__")])
def test_circular_dependency_mixed_addcreate(self):
"""
Tests that the dependency resolver knows to put all CreateModel
before AddField and not become unsolvable (#23315)
"""
address = ModelState("a", "Address", [
("id", models.AutoField(primary_key=True)),
("country", models.ForeignKey("b.DeliveryCountry")),
])
person = ModelState("a", "Person", [
("id", models.AutoField(primary_key=True)),
])
apackage = ModelState("b", "APackage", [
("id", models.AutoField(primary_key=True)),
("person", models.ForeignKey("a.person")),
])
country = ModelState("b", "DeliveryCountry", [
("id", models.AutoField(primary_key=True)),
])
# Make state
before = self.make_project_state([])
after = self.make_project_state([address, person, apackage, country])
autodetector = MigrationAutodetector(before, after)
changes = autodetector._detect_changes()
# Right number of migrations?
self.assertNumberMigrations(changes, 'a', 2)
self.assertNumberMigrations(changes, 'b', 1)
self.assertOperationTypes(changes, 'a', 0, ["CreateModel", "CreateModel"])
self.assertOperationTypes(changes, 'a', 1, ["AddField"])
self.assertOperationTypes(changes, 'b', 0, ["CreateModel", "CreateModel"])