Merge pull request #2736 from SmileyChris/migration-run_before

Fixed #22725 - Migration.run_before does nothing

django/db/migrations/loader.py

@@ -134,6 +134,25 @@ class MigrationLoader(object):
         else:
             return self.disk_migrations[results[0]]
 
+    def check_key(self, key, current_app):
+        if key[1] != "__first__" or key in self.graph:
+            return key
+        # Special-case __first__, which means "the first migration" for
+        # migrated apps, and is ignored for unmigrated apps. It allows
+        # makemigrations to declare dependencies on apps before they even have
+        # migrations.
+        if key[0] == current_app:
+            # Ignore __first__ references to the same app (#22325)
+            return
+        if key[0] in self.unmigrated_apps:
+            # This app isn't migrated, but something depends on it.
+            # The models will get auto-added into the state, though
+            # so we're fine.
+            return
+        if key[0] in self.migrated_apps:
+            return list(self.graph.root_nodes(key[0]))[0]
+        raise ValueError("Dependency on unknown app %s" % key[0])
+
     def build_graph(self):
         """
         Builds a migration dependency graph using both the disk and database.
@@ -196,25 +215,13 @@ class MigrationLoader(object):
             self.graph.add_node(key, migration)
         for key, migration in normal.items():
             for parent in migration.dependencies:
-                # Special-case __first__, which means "the first migration" for
+                parent = self.check_key(parent, key[0])
-                # migrated apps, and is ignored for unmigrated apps. It allows
-                # makemigrations to declare dependencies on apps before they
-                # even have migrations.
-                if parent[1] == "__first__" and parent not in self.graph:
-                    if parent[0] == key[0]:
-                        # Ignore __first__ references to the same app (#22325)
-                        continue
-                    elif parent[0] in self.unmigrated_apps:
-                        # This app isn't migrated, but something depends on it.
-                        # The models will get auto-added into the state, though
-                        # so we're fine.
-                        continue
-                    elif parent[0] in self.migrated_apps:
-                        parent = list(self.graph.root_nodes(parent[0]))[0]
-                    else:
-                        raise ValueError("Dependency on unknown app %s" % parent[0])
                 if parent is not None:
                     self.graph.add_dependency(key, parent)
+            for child in migration.run_before:
+                child = self.check_key(child, key[0])
+                if child is not None:
+                    self.graph.add_dependency(child, key)
 
     def detect_conflicts(self):
         """

tests/migrations/test_loader.py

@@ -102,6 +102,22 @@ class LoaderTests(TestCase):
             ["id", "user"]
         )
 
+    @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_run_before"})
+    def test_run_before(self):
+        """
+        Makes sure the loader uses Migration.run_before.
+        """
+        # Load and test the plan
+        migration_loader = MigrationLoader(connection)
+        self.assertEqual(
+            migration_loader.graph.forwards_plan(("migrations", "0002_second")),
+            [
+                ("migrations", "0001_initial"),
+                ("migrations", "0003_third"),
+                ("migrations", "0002_second"),
+            ],
+        )
+
     @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"})
     def test_name_match(self):
         "Tests prefix name matching"

tests/migrations/test_migrations_run_before/0001_initial.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    operations = [
+
+        migrations.CreateModel(
+            "Salamander",
+            [
+                ("id", models.AutoField(primary_key=True)),
+                ("size", models.IntegerField(default=0)),
+                ("silly_field", models.BooleanField(default=False)),
+            ],
+        ),
+
+    ]

tests/migrations/test_migrations_run_before/0002_second.py

@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("migrations", "0001_initial"),
+    ]
+
+    operations = [
+
+        migrations.CreateModel(
+            "Book",
+            [
+                ("id", models.AutoField(primary_key=True)),
+                ("author", models.ForeignKey("migrations.Author", null=True)),
+            ],
+        )
+
+    ]

tests/migrations/test_migrations_run_before/0003_third.py

@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    """
+    This is a wee bit crazy, but it's just to show that run_before works.
+    """
+
+    dependencies = [
+        ("migrations", "0001_initial"),
+    ]
+
+    run_before = [
+        ("migrations", "0002_second"),
+    ]
+
+    operations = [
+
+        migrations.CreateModel(
+            "Author",
+            [
+                ("id", models.AutoField(primary_key=True)),
+                ("name", models.CharField(max_length=255)),
+                ("slug", models.SlugField(null=True)),
+                ("age", models.IntegerField(default=0)),
+            ],
+        )
+
+    ]