From bbbed99f6260a8b3e65cb990e49721b1ce4a441b Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Sun, 11 Jan 2015 20:13:31 +0100 Subject: [PATCH] Fixed #24123 -- Used all available migrations to generate the initial migration state Thanks Collin Anderson for the input when creating the patch and Tim Graham for the review. --- django/db/migrations/executor.py | 32 +++++-- .../lookuperror_a/__init__.py | 0 .../lookuperror_a/migrations/0001_initial.py | 19 ++++ .../lookuperror_a/migrations/0002_a2.py | 20 +++++ .../lookuperror_a/migrations/0003_a3.py | 24 +++++ .../lookuperror_a/migrations/0004_a4.py | 20 +++++ .../lookuperror_a/migrations/__init__.py | 0 .../lookuperror_a/models.py | 18 ++++ .../lookuperror_b/__init__.py | 0 .../lookuperror_b/migrations/0001_initial.py | 19 ++++ .../lookuperror_b/migrations/0002_b2.py | 22 +++++ .../lookuperror_b/migrations/0003_b3.py | 20 +++++ .../lookuperror_b/migrations/__init__.py | 0 .../lookuperror_b/models.py | 13 +++ .../lookuperror_c/__init__.py | 0 .../lookuperror_c/migrations/0001_initial.py | 19 ++++ .../lookuperror_c/migrations/0002_c2.py | 22 +++++ .../lookuperror_c/migrations/0003_c3.py | 20 +++++ .../lookuperror_c/migrations/__init__.py | 0 .../lookuperror_c/models.py | 13 +++ tests/migrations/test_executor.py | 90 +++++++++++++++++++ 21 files changed, 364 insertions(+), 7 deletions(-) create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/migrations/0001_initial.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/migrations/0002_a2.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/migrations/0003_a3.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/migrations/0004_a4.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/migrations/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_a/models.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/migrations/0001_initial.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/migrations/0002_b2.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/migrations/0003_b3.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/migrations/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_b/models.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/migrations/0001_initial.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/migrations/0002_c2.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/migrations/0003_c3.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/migrations/__init__.py create mode 100644 tests/migrations/migrations_test_apps/lookuperror_c/models.py diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py index a7989c4107..c9ef387d58 100644 --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -4,6 +4,7 @@ from django.db import migrations from django.apps.registry import apps as global_apps from .loader import MigrationLoader from .recorder import MigrationRecorder +from .state import ProjectState class MigrationExecutor(object): @@ -18,12 +19,15 @@ class MigrationExecutor(object): self.recorder = MigrationRecorder(self.connection) self.progress_callback = progress_callback - def migration_plan(self, targets): + def migration_plan(self, targets, clean_start=False): """ Given a set of targets, returns a list of (Migration instance, backwards?). """ plan = [] - applied = set(self.loader.applied_migrations) + if clean_start: + applied = set() + else: + applied = set(self.loader.applied_migrations) for target in targets: # If the target is (app_label, None), that means unmigrate everything if target[1] is None: @@ -60,17 +64,31 @@ class MigrationExecutor(object): def migrate(self, targets, plan=None, fake=False): """ Migrates the database up to the given targets. + + Django first needs to create all project states before a migration is + (un)applied and in a second step run all the database operations. """ if plan is None: plan = self.migration_plan(targets) - state = None + migrations_to_run = {m[0] for m in plan} + # Create the forwards plan Django would follow on an empty database + full_plan = self.migration_plan(self.loader.graph.leaf_nodes(), clean_start=True) + # Holds all states right before and right after a migration is applied + # if the migration is being run. + states = {} + state = ProjectState(real_apps=list(self.loader.unmigrated_apps)) + state.apps # Render all real_apps -- performance critical + # Phase 1 -- Store all required states + for migration, _ in full_plan: + if migration in migrations_to_run: + states[migration] = state.clone() + state = migration.mutate_state(state) # state is cloned inside + # Phase 2 -- Run the migrations for migration, backwards in plan: - if state is None: - state = self.loader.project_state((migration.app_label, migration.name), at_end=False) if not backwards: - state = self.apply_migration(state, migration, fake=fake) + self.apply_migration(states[migration], migration, fake=fake) else: - state = self.unapply_migration(state, migration, fake=fake) + self.unapply_migration(states[migration], migration, fake=fake) def collect_sql(self, plan): """ diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_a/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0001_initial.py b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0001_initial.py new file mode 100644 index 0000000000..c20e811696 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0001_initial.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='A1', + fields=[ + ('id', models.AutoField(serialize=False, verbose_name='ID', auto_created=True, primary_key=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0002_a2.py b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0002_a2.py new file mode 100644 index 0000000000..0c75368e04 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0002_a2.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_a', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='A2', + fields=[ + ('id', models.AutoField(verbose_name='ID', primary_key=True, serialize=False, auto_created=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0003_a3.py b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0003_a3.py new file mode 100644 index 0000000000..655ea03216 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0003_a3.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_c', '0002_c2'), + ('lookuperror_b', '0002_b2'), + ('lookuperror_a', '0002_a2'), + ] + + operations = [ + migrations.CreateModel( + name='A3', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, primary_key=True, verbose_name='ID')), + ('b2', models.ForeignKey(to='lookuperror_b.B2')), + ('c2', models.ForeignKey(to='lookuperror_c.C2')), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0004_a4.py b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0004_a4.py new file mode 100644 index 0000000000..c45faf587f --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/0004_a4.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_a', '0003_a3'), + ] + + operations = [ + migrations.CreateModel( + name='A4', + fields=[ + ('id', models.AutoField(auto_created=True, serialize=False, verbose_name='ID', primary_key=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/migrations/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_a/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_a/models.py b/tests/migrations/migrations_test_apps/lookuperror_a/models.py new file mode 100644 index 0000000000..65dfb6bd85 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_a/models.py @@ -0,0 +1,18 @@ +from django.db import models + + +class A1(models.Model): + pass + + +class A2(models.Model): + pass + + +class A3(models.Model): + b2 = models.ForeignKey('lookuperror_b.B2') + c2 = models.ForeignKey('lookuperror_c.C2') + + +class A4(models.Model): + pass diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_b/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0001_initial.py b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0001_initial.py new file mode 100644 index 0000000000..67dafcae28 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0001_initial.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='B1', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, primary_key=True, verbose_name='ID')), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0002_b2.py b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0002_b2.py new file mode 100644 index 0000000000..7972ef8d84 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0002_b2.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_a', '0002_a2'), + ('lookuperror_b', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='B2', + fields=[ + ('id', models.AutoField(primary_key=True, verbose_name='ID', auto_created=True, serialize=False)), + ('a1', models.ForeignKey(to='lookuperror_a.A1')), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0003_b3.py b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0003_b3.py new file mode 100644 index 0000000000..f5147c4f7f --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/0003_b3.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_b', '0002_b2'), + ] + + operations = [ + migrations.CreateModel( + name='B3', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, primary_key=True, auto_created=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/migrations/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_b/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_b/models.py b/tests/migrations/migrations_test_apps/lookuperror_b/models.py new file mode 100644 index 0000000000..f2c21aa1d4 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_b/models.py @@ -0,0 +1,13 @@ +from django.db import models + + +class B1(models.Model): + pass + + +class B2(models.Model): + a1 = models.ForeignKey('lookuperror_a.A1') + + +class B3(models.Model): + pass diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_c/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0001_initial.py b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0001_initial.py new file mode 100644 index 0000000000..c2e46bee70 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0001_initial.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='C1', + fields=[ + ('id', models.AutoField(serialize=False, verbose_name='ID', auto_created=True, primary_key=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0002_c2.py b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0002_c2.py new file mode 100644 index 0000000000..1f7f440d83 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0002_c2.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_a', '0002_a2'), + ('lookuperror_c', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='C2', + fields=[ + ('id', models.AutoField(auto_created=True, verbose_name='ID', primary_key=True, serialize=False)), + ('a1', models.ForeignKey(to='lookuperror_a.A1')), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0003_c3.py b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0003_c3.py new file mode 100644 index 0000000000..d4d48fc32f --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/0003_c3.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lookuperror_c', '0002_c2'), + ] + + operations = [ + migrations.CreateModel( + name='C3', + fields=[ + ('id', models.AutoField(auto_created=True, serialize=False, verbose_name='ID', primary_key=True)), + ], + ), + ] diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/migrations/__init__.py b/tests/migrations/migrations_test_apps/lookuperror_c/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/migrations_test_apps/lookuperror_c/models.py b/tests/migrations/migrations_test_apps/lookuperror_c/models.py new file mode 100644 index 0000000000..d52a1e7543 --- /dev/null +++ b/tests/migrations/migrations_test_apps/lookuperror_c/models.py @@ -0,0 +1,13 @@ +from django.db import models + + +class C1(models.Model): + pass + + +class C2(models.Model): + a1 = models.ForeignKey('lookuperror_a.A1') + + +class C3(models.Model): + pass diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py index 78ee131d18..9f8b2282c0 100644 --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -233,6 +233,96 @@ class ExecutorTests(MigrationTestBase): self.assertTableNotExists("migrations_author") self.assertTableNotExists("migrations_tribble") + @override_settings( + INSTALLED_APPS=[ + "migrations.migrations_test_apps.lookuperror_a", + "migrations.migrations_test_apps.lookuperror_b", + "migrations.migrations_test_apps.lookuperror_c" + ] + ) + def test_unrelated_model_lookups_forwards(self): + """ + #24123 - Tests that all models of apps already applied which are + unrelated to the first app being applied are part of the initial model + state. + """ + try: + executor = MigrationExecutor(connection) + self.assertTableNotExists("lookuperror_a_a1") + self.assertTableNotExists("lookuperror_b_b1") + self.assertTableNotExists("lookuperror_c_c1") + executor.migrate([("lookuperror_b", "0003_b3")]) + self.assertTableExists("lookuperror_b_b3") + # Rebuild the graph to reflect the new DB state + executor.loader.build_graph() + + # Migrate forwards -- This led to a lookup LookupErrors because + # lookuperror_b.B2 is already applied + executor.migrate([ + ("lookuperror_a", "0004_a4"), + ("lookuperror_c", "0003_c3"), + ]) + self.assertTableExists("lookuperror_a_a4") + self.assertTableExists("lookuperror_c_c3") + + # Rebuild the graph to reflect the new DB state + executor.loader.build_graph() + finally: + # Cleanup + executor.migrate([ + ("lookuperror_a", None), + ("lookuperror_b", None), + ("lookuperror_c", None), + ]) + self.assertTableNotExists("lookuperror_a_a1") + self.assertTableNotExists("lookuperror_b_b1") + self.assertTableNotExists("lookuperror_c_c1") + + @override_settings( + INSTALLED_APPS=[ + "migrations.migrations_test_apps.lookuperror_a", + "migrations.migrations_test_apps.lookuperror_b", + "migrations.migrations_test_apps.lookuperror_c" + ] + ) + def test_unrelated_model_lookups_backwards(self): + """ + #24123 - Tests that all models of apps being unapplied which are + unrelated to the first app being unapplied are part of the initial + model state. + """ + try: + executor = MigrationExecutor(connection) + self.assertTableNotExists("lookuperror_a_a1") + self.assertTableNotExists("lookuperror_b_b1") + self.assertTableNotExists("lookuperror_c_c1") + executor.migrate([ + ("lookuperror_a", "0004_a4"), + ("lookuperror_b", "0003_b3"), + ("lookuperror_c", "0003_c3"), + ]) + self.assertTableExists("lookuperror_b_b3") + self.assertTableExists("lookuperror_a_a4") + self.assertTableExists("lookuperror_c_c3") + # Rebuild the graph to reflect the new DB state + executor.loader.build_graph() + + # Migrate backwards -- This led to a lookup LookupErrors because + # lookuperror_b.B2 is not in the initial state (unrelated to app c) + executor.migrate([("lookuperror_a", None)]) + + # Rebuild the graph to reflect the new DB state + executor.loader.build_graph() + finally: + # Cleanup + executor.migrate([ + ("lookuperror_b", None), + ("lookuperror_c", None) + ]) + self.assertTableNotExists("lookuperror_a_a1") + self.assertTableNotExists("lookuperror_b_b1") + self.assertTableNotExists("lookuperror_c_c1") + class FakeLoader(object): def __init__(self, graph, applied):