From 2cee1d464217ff9f0d0af1e14ecd0b9b70ad1b7e Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Tue, 17 Jun 2014 23:27:03 -0700 Subject: [PATCH] Fixed #22861: Internal migrations done first so __first__ works Thanks to Chris Beaven. --- django/db/migrations/graph.py | 4 +-- django/db/migrations/loader.py | 11 +++++++ tests/migrations/test_graph.py | 8 ++--- tests/migrations/test_loader.py | 20 +++++++++++++ .../test_migrations_first/__init__.py | 0 .../test_migrations_first/second.py | 30 +++++++++++++++++++ .../test_migrations_first/thefirst.py | 30 +++++++++++++++++++ .../test_migrations_2_first/0001_initial.py | 26 ++++++++++++++++ .../test_migrations_2_first/0002_second.py | 22 ++++++++++++++ .../test_migrations_2_first/__init__.py | 0 10 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 tests/migrations/test_migrations_first/__init__.py create mode 100644 tests/migrations/test_migrations_first/second.py create mode 100644 tests/migrations/test_migrations_first/thefirst.py create mode 100644 tests/migrations2/test_migrations_2_first/0001_initial.py create mode 100644 tests/migrations2/test_migrations_2_first/0002_second.py create mode 100644 tests/migrations2/test_migrations_2_first/__init__.py diff --git a/django/db/migrations/graph.py b/django/db/migrations/graph.py index 8e034f2bd0a..59578b110f1 100644 --- a/django/db/migrations/graph.py +++ b/django/db/migrations/graph.py @@ -74,7 +74,7 @@ class MigrationGraph(object): for node in self.nodes: if not any(key[0] == node[0] for key in self.dependencies.get(node, set())) and (not app or app == node[0]): roots.add(node) - return roots + return sorted(roots) def leaf_nodes(self, app=None): """ @@ -88,7 +88,7 @@ class MigrationGraph(object): for node in self.nodes: if not any(key[0] == node[0] for key in self.dependents.get(node, set())) and (not app or app == node[0]): leaves.add(node) - return leaves + return sorted(leaves) def dfs(self, start, get_children): """ diff --git a/django/db/migrations/loader.py b/django/db/migrations/loader.py index 86987db02c3..5465f0ad18a 100644 --- a/django/db/migrations/loader.py +++ b/django/db/migrations/loader.py @@ -223,8 +223,19 @@ class MigrationLoader(object): self.graph = MigrationGraph() for key, migration in normal.items(): self.graph.add_node(key, migration) + # Add all internal dependencies first to ensure __first__ dependencies + # find the correct root node. for key, migration in normal.items(): for parent in migration.dependencies: + if parent[0] != key[0] or parent[1] == '__first__': + # Ignore __first__ references to the same app (#22325) + continue + self.graph.add_dependency(key, parent) + for key, migration in normal.items(): + for parent in migration.dependencies: + if parent[0] == key[0]: + # Internal dependencies already added. + continue parent = self.check_key(parent, key[0]) if parent is not None: self.graph.add_dependency(key, parent) diff --git a/tests/migrations/test_graph.py b/tests/migrations/test_graph.py index e3d5a28283a..b6a7c625619 100644 --- a/tests/migrations/test_graph.py +++ b/tests/migrations/test_graph.py @@ -51,11 +51,11 @@ class GraphTests(TestCase): # Test roots and leaves self.assertEqual( graph.root_nodes(), - set([('app_a', '0001'), ('app_b', '0001')]), + [('app_a', '0001'), ('app_b', '0001')], ) self.assertEqual( graph.leaf_nodes(), - set([('app_a', '0004'), ('app_b', '0002')]), + [('app_a', '0004'), ('app_b', '0002')], ) def test_complex_graph(self): @@ -105,11 +105,11 @@ class GraphTests(TestCase): # Test roots and leaves self.assertEqual( graph.root_nodes(), - set([('app_a', '0001'), ('app_b', '0001'), ('app_c', '0001')]), + [('app_a', '0001'), ('app_b', '0001'), ('app_c', '0001')], ) self.assertEqual( graph.leaf_nodes(), - set([('app_a', '0004'), ('app_b', '0002'), ('app_c', '0002')]), + [('app_a', '0004'), ('app_b', '0002'), ('app_c', '0002')], ) def test_circular_graph(self): diff --git a/tests/migrations/test_loader.py b/tests/migrations/test_loader.py index 364faf5e005..c0f9f1a6dd8 100644 --- a/tests/migrations/test_loader.py +++ b/tests/migrations/test_loader.py @@ -142,6 +142,26 @@ class LoaderTests(TestCase): ], ) + @override_settings(MIGRATION_MODULES={ + "migrations": "migrations.test_migrations_first", + "migrations2": "migrations2.test_migrations_2_first", + }) + @modify_settings(INSTALLED_APPS={'append': 'migrations2'}) + def test_first(self): + """ + Makes sure the '__first__' migrations build correctly. + """ + migration_loader = MigrationLoader(connection) + self.assertEqual( + migration_loader.graph.forwards_plan(("migrations", "second")), + [ + ("migrations", "thefirst"), + ("migrations2", "0001_initial"), + ("migrations2", "0002_second"), + ("migrations", "second"), + ], + ) + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"}) def test_name_match(self): "Tests prefix name matching" diff --git a/tests/migrations/test_migrations_first/__init__.py b/tests/migrations/test_migrations_first/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/migrations/test_migrations_first/second.py b/tests/migrations/test_migrations_first/second.py new file mode 100644 index 00000000000..e85828856dd --- /dev/null +++ b/tests/migrations/test_migrations_first/second.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("migrations", "thefirst"), + ("migrations2", "0002_second"), + ] + + operations = [ + + migrations.DeleteModel("Tribble"), + + migrations.RemoveField("Author", "silly_field"), + + migrations.AddField("Author", "rating", models.IntegerField(default=0)), + + migrations.CreateModel( + "Book", + [ + ("id", models.AutoField(primary_key=True)), + ("author", models.ForeignKey("migrations.Author", null=True)), + ], + ) + + ] diff --git a/tests/migrations/test_migrations_first/thefirst.py b/tests/migrations/test_migrations_first/thefirst.py new file mode 100644 index 00000000000..581d5368143 --- /dev/null +++ b/tests/migrations/test_migrations_first/thefirst.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + 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)), + ("silly_field", models.BooleanField(default=False)), + ], + ), + + migrations.CreateModel( + "Tribble", + [ + ("id", models.AutoField(primary_key=True)), + ("fluffy", models.BooleanField(default=True)), + ], + ) + + ] diff --git a/tests/migrations2/test_migrations_2_first/0001_initial.py b/tests/migrations2/test_migrations_2_first/0001_initial.py new file mode 100644 index 00000000000..e31d1d501fe --- /dev/null +++ b/tests/migrations2/test_migrations_2_first/0001_initial.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("migrations", "__first__"), + ] + + operations = [ + + migrations.CreateModel( + "OtherAuthor", + [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=255)), + ("slug", models.SlugField(null=True)), + ("age", models.IntegerField(default=0)), + ("silly_field", models.BooleanField(default=False)), + ], + ), + + ] diff --git a/tests/migrations2/test_migrations_2_first/0002_second.py b/tests/migrations2/test_migrations_2_first/0002_second.py new file mode 100644 index 00000000000..a3ca7dac393 --- /dev/null +++ b/tests/migrations2/test_migrations_2_first/0002_second.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [("migrations2", "0001_initial")] + + operations = [ + + migrations.CreateModel( + "Bookstore", + [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=255)), + ("slug", models.SlugField(null=True)), + ], + ), + + ] diff --git a/tests/migrations2/test_migrations_2_first/__init__.py b/tests/migrations2/test_migrations_2_first/__init__.py new file mode 100644 index 00000000000..e69de29bb2d