Commit 824e02bf authored by Andrew Godwin's avatar Andrew Godwin
Browse files

[1.7.x] Fixed #23315: Operational dependency fail with mixed create/add

parent 9dcb2cf4
Loading
Loading
Loading
Loading
+8 −4
Original line number Diff line number Diff line
@@ -286,7 +286,7 @@ class MigrationAutodetector(object):
                if not chop_mode:
                    chop_mode = True
                else:
                    raise ValueError("Cannot resolve operation dependencies")
                    raise ValueError("Cannot resolve operation dependencies: %r" % self.generated_operations)
            num_ops = new_num_ops

        # OK, add in internal dependencies among the migrations
@@ -361,9 +361,12 @@ class MigrationAutodetector(object):
        else:
            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)
        operation._auto_deps = dependencies or []
        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):
@@ -429,7 +432,7 @@ class MigrationAutodetector(object):
        that might be deferred (e.g. unique_together, index_together)
        """
        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]
            # Gather related fields
            related_fields = {}
@@ -481,6 +484,7 @@ class MigrationAutodetector(object):
                    bases=model_state.bases,
                ),
                dependencies=dependencies,
                beginning=True,
            )
            # Generate operations for each related field
            for name, field in sorted(related_fields.items()):
+65 −72
Original line number Diff line number Diff line
@@ -129,12 +129,13 @@ class AutodetectorTests(TestCase):
        operation = migration.operations[operation_index]
        for attr, value in attrs.items():
            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,
                    migration.name,
                    operation_index + 1,
                    operation_index,
                    attr,
                    value,
                    getattr(operation, attr, None),
                    self.repr_changes(changes),
                ))

@@ -401,21 +402,14 @@ class AutodetectorTests(TestCase):
        after = self.make_project_state([self.author_with_publisher, self.publisher])
        autodetector = MigrationAutodetector(before, after)
        changes = autodetector._detect_changes()
        # Right number of migrations?
        self.assertEqual(len(changes['testapp']), 1)
        # Right number of actions?
        migration = changes['testapp'][0]
        self.assertEqual(len(migration.operations), 3)
        # Right actions?
        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 number/type of migrations?
        self.assertNumberMigrations(changes, 'testapp', 1)
        self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel", "AddField"])
        self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
        self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
        self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
        # Right dependencies?
        self.assertEqual(migration.dependencies, [])
        self.assertEqual(changes['testapp'][0].dependencies, [])

    def test_circular_fk_dependency(self):
        """
@@ -428,30 +422,18 @@ class AutodetectorTests(TestCase):
        autodetector = MigrationAutodetector(before, after)
        changes = autodetector._detect_changes()
        # Right number of migrations?
        self.assertEqual(len(changes['testapp']), 1)
        self.assertEqual(len(changes['otherapp']), 2)
        # Right number of actions?
        migration1 = changes['testapp'][0]
        self.assertEqual(len(migration1.operations), 2)
        migration2 = changes['otherapp'][0]
        self.assertEqual(len(migration2.operations), 1)
        migration3 = changes['otherapp'][1]
        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")
        self.assertNumberMigrations(changes, 'testapp', 1)
        self.assertNumberMigrations(changes, 'otherapp', 2)
        # Right types?
        self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel"])
        self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"])
        self.assertOperationTypes(changes, 'otherapp', 1, ["AddField"])
        self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
        self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
        # Right dependencies?
        self.assertEqual(migration1.dependencies, [("otherapp", "auto_1")])
        self.assertEqual(migration2.dependencies, [])
        self.assertEqual(set(migration3.dependencies), set([("testapp", "auto_1"), ("otherapp", "auto_1")]))
        self.assertEqual(changes['testapp'][0].dependencies, [("otherapp", "auto_1")])
        self.assertEqual(changes['otherapp'][0].dependencies, [])
        self.assertEqual(set(changes['otherapp'][1].dependencies), set([("otherapp", "auto_1"), ("testapp", "auto_1")]))

    def test_same_app_circular_fk_dependency(self):
        """
@@ -463,24 +445,14 @@ class AutodetectorTests(TestCase):
        after = self.make_project_state([self.author_with_publisher, self.publisher_with_author])
        autodetector = MigrationAutodetector(before, after)
        changes = autodetector._detect_changes()
        # Right number of migrations?
        self.assertEqual(len(changes['testapp']), 1)
        # Right number of actions?
        migration1 = changes['testapp'][0]
        self.assertEqual(len(migration1.operations), 4)
        # Right actions?
        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 number/type of migrations?
        self.assertNumberMigrations(changes, 'testapp', 1)
        self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel", "AddField"])
        self.assertOperationAttributes(changes, "testapp", 0, 0, name="Author")
        self.assertOperationAttributes(changes, "testapp", 0, 1, name="Publisher")
        self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
        # Right dependencies?
        self.assertEqual(migration1.dependencies, [])
        self.assertEqual(changes['testapp'][0].dependencies, [])

    def test_same_app_circular_fk_dependency_and_unique_together(self):
        """
@@ -493,23 +465,13 @@ class AutodetectorTests(TestCase):
        after = self.make_project_state([self.knight, self.rabbit])
        autodetector = MigrationAutodetector(before, after)
        changes = autodetector._detect_changes()
        # Right number of migrations?
        self.assertEqual(len(changes['eggs']), 1)
        # Right number of actions?
        migration1 = changes['eggs'][0]
        self.assertEqual(len(migration1.operations), 3)
        # 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 number/type of migrations?
        self.assertNumberMigrations(changes, 'eggs', 1)
        self.assertOperationTypes(changes, 'eggs', 0, ["CreateModel", "CreateModel", "AlterUniqueTogether"])
        self.assertFalse("unique_together" in changes['eggs'][0].operations[0].options)
        self.assertFalse("unique_together" in changes['eggs'][0].operations[1].options)
        # Right dependencies?
        self.assertEqual(migration1.dependencies, [])
        self.assertEqual(changes['eggs'][0].dependencies, [])

    def test_unique_together(self):
        "Tests unique_together detection"
@@ -1098,3 +1060,34 @@ class AutodetectorTests(TestCase):
        self.assertOperationTypes(changes, 'testapp', 0, ["AddField"])
        self.assertOperationAttributes(changes, 'testapp', 0, 0, name="book")
        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"])