From d5c4ea524679a787fe11c927448e44e95646096b Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 20 Aug 2016 18:05:04 -0400 Subject: [PATCH] Fixed #27100 -- Included already applied migration changes in the pre-migrate state. Refs #24100. Thanks Tim for the review. --- django/core/management/commands/migrate.py | 10 ++-- django/db/migrations/executor.py | 53 +++++++++++----------- docs/releases/1.10.1.txt | 4 ++ tests/migrate_signals/tests.py | 5 ++ 4 files changed, 42 insertions(+), 30 deletions(-) diff --git a/django/core/management/commands/migrate.py b/django/core/management/commands/migrate.py index 4c8e008374..6846b33d2a 100644 --- a/django/core/management/commands/migrate.py +++ b/django/core/management/commands/migrate.py @@ -160,7 +160,8 @@ class Command(BaseCommand): % (targets[0][1], targets[0][0]) ) - pre_migrate_apps = executor._create_project_state().apps + pre_migrate_state = executor._create_project_state(with_applied_migrations=True) + pre_migrate_apps = pre_migrate_state.apps emit_pre_migrate_signal( self.verbosity, self.interactive, connection.alias, apps=pre_migrate_apps, plan=plan, ) @@ -198,10 +199,11 @@ class Command(BaseCommand): else: fake = options['fake'] fake_initial = options['fake_initial'] - post_migrate_project_state = executor.migrate( - targets, plan, fake=fake, fake_initial=fake_initial + post_migrate_state = executor.migrate( + targets, plan=plan, state=pre_migrate_state.clone(), fake=fake, + fake_initial=fake_initial, ) - post_migrate_apps = post_migrate_project_state.apps + post_migrate_apps = post_migrate_state.apps # Re-render models of real apps to include relationships now that # we've got a final state. This wouldn't be necessary if real apps diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py index 11a96ec81b..1a0b6f6322 100644 --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -63,10 +63,25 @@ class MigrationExecutor(object): applied.add(migration) return plan - def _create_project_state(self): - return ProjectState(real_apps=list(self.loader.unmigrated_apps)) + def _create_project_state(self, with_applied_migrations=False): + """ + Create a project state including all the applications without + migrations and applied migrations if with_applied_migrations=True. + """ + state = ProjectState(real_apps=list(self.loader.unmigrated_apps)) + if with_applied_migrations: + # Create the forwards plan Django would follow on an empty database + full_plan = self.migration_plan(self.loader.graph.leaf_nodes(), clean_start=True) + applied_migrations = { + self.loader.graph.nodes[key] for key in self.loader.applied_migrations + if key in self.loader.graph.nodes + } + for migration, _ in full_plan: + if migration in applied_migrations: + migration.mutate_state(state, preserve=False) + return state - def migrate(self, targets, plan=None, fake=False, fake_initial=False): + def migrate(self, targets, plan=None, state=None, fake=False, fake_initial=False): """ Migrates the database up to the given targets. @@ -82,15 +97,9 @@ class MigrationExecutor(object): all_backwards = all(backwards for mig, backwards in plan) if not plan: - # The resulting state should include applied migrations. - state = self._create_project_state() - applied_migrations = { - self.loader.graph.nodes[key] for key in self.loader.applied_migrations - if key in self.loader.graph.nodes - } - for migration, _ in full_plan: - if migration in applied_migrations: - migration.mutate_state(state, preserve=False) + if state is None: + # The resulting state should include applied migrations. + state = self._create_project_state(with_applied_migrations=True) elif all_forwards == all_backwards: # This should only happen if there's a mixed plan raise InvalidMigrationPlan( @@ -100,7 +109,10 @@ class MigrationExecutor(object): plan ) elif all_forwards: - state = self._migrate_all_forwards(plan, full_plan, fake=fake, fake_initial=fake_initial) + if state is None: + # The resulting state should still include applied migrations. + state = self._create_project_state(with_applied_migrations=True) + state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial) else: # No need to check for `elif all_backwards` here, as that condition # would always evaluate to true. @@ -110,19 +122,14 @@ class MigrationExecutor(object): return state - def _migrate_all_forwards(self, plan, full_plan, fake, fake_initial): + def _migrate_all_forwards(self, state, plan, full_plan, fake, fake_initial): """ Take a list of 2-tuples of the form (migration instance, False) and apply them in the order they occur in the full_plan. """ migrations_to_run = {m[0] for m in plan} - state = self._create_project_state() - applied_migrations = { - self.loader.graph.nodes[key] for key in self.loader.applied_migrations - if key in self.loader.graph.nodes - } for migration, _ in full_plan: - if not migrations_to_run and not applied_migrations: + if not migrations_to_run: # We remove every migration that we applied from these sets so # that we can bail out once the last migration has been applied # and don't always run until the very end of the migration @@ -137,12 +144,6 @@ class MigrationExecutor(object): self.progress_callback("render_success") state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial) migrations_to_run.remove(migration) - elif migration in applied_migrations: - # Only mutate the state if the migration is actually applied - # to make sure the resulting state doesn't include changes - # from unrelated migrations. - migration.mutate_state(state, preserve=False) - applied_migrations.remove(migration) return state diff --git a/docs/releases/1.10.1.txt b/docs/releases/1.10.1.txt index 46188aa2da..b5c72c5e8b 100644 --- a/docs/releases/1.10.1.txt +++ b/docs/releases/1.10.1.txt @@ -72,3 +72,7 @@ Bugfixes * Fixed the creation of ``ContentType`` and ``Permission`` objects for models of applications without migrations when calling the ``migrate`` command with no migrations to apply (:ticket:`27044`). + +* Included the already applied migration state changes in the ``Apps`` instance + provided to the ``pre_migrate`` signal receivers to allow ``ContentType`` + renaming to be performed on model rename (:ticket:`27100`). diff --git a/tests/migrate_signals/tests.py b/tests/migrate_signals/tests.py index bb3ea0ccc9..84b6608f46 100644 --- a/tests/migrate_signals/tests.py +++ b/tests/migrate_signals/tests.py @@ -114,11 +114,16 @@ class MigrateSignalTests(TransactionTestCase): ['migrate_signals.Signal'] ) # Migrating with an empty plan. + pre_migrate_receiver = Receiver(signals.pre_migrate) post_migrate_receiver = Receiver(signals.post_migrate) management.call_command( 'migrate', database=MIGRATE_DATABASE, verbosity=MIGRATE_VERBOSITY, interactive=MIGRATE_INTERACTIVE, stdout=stdout, ) + self.assertEqual( + [model._meta.label for model in pre_migrate_receiver.call_args['apps'].get_models()], + ['migrate_signals.Signal'] + ) self.assertEqual( [model._meta.label for model in post_migrate_receiver.call_args['apps'].get_models()], ['migrate_signals.Signal']