From e4ed4431d5730e4a8efc3d76d79ee5c16b5a2340 Mon Sep 17 00:00:00 2001 From: Marten Kenbeek Date: Wed, 31 May 2017 13:35:43 +0200 Subject: [PATCH] Fixed #25850 -- Ignored soft applied migrations in consistency check. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignored initial migrations that have been soft-applied and may be faked with the --fake-initial flag in the migration history consistency check. Does not ignore the initial migration if a later migration in the same app has been recorded as applied. Included soft-applied migrations in the pre-migrate project state if any of its children has been applied. Thanks to Raphaël Hertzog for the initial patch. Bug: https://code.djangoproject.com/ticket/28250 Bug-Debian: https://bugs.debian.org/863267 --- django/core/management/commands/makemigrations.py | 2 +- django/core/management/commands/migrate.py | 2 +- django/db/migrations/executor.py | 7 ++- django/db/migrations/loader.py | 28 +++++++--- tests/migrations/test_executor.py | 63 +++++++++++++++++++++-- tests/migrations/test_loader.py | 27 ++++++++++ 6 files changed, 114 insertions(+), 15 deletions(-) --- a/django/core/management/commands/makemigrations.py +++ b/django/core/management/commands/makemigrations.py @@ -106,7 +106,7 @@ class Command(BaseCommand): for app_label in consistency_check_labels for model in apps.get_app_config(app_label).get_models() )): - loader.check_consistent_history(connection) + loader.check_consistent_history(connection, fake_initial=True) # Before anything else, see if there's conflicting apps and drop out # hard if there are any and they don't want to merge --- a/django/core/management/commands/migrate.py +++ b/django/core/management/commands/migrate.py @@ -83,7 +83,7 @@ class Command(BaseCommand): executor = MigrationExecutor(connection, self.migration_progress_callback) # Raise an error if any migrations are applied before their dependencies. - executor.loader.check_consistent_history(connection) + executor.loader.check_consistent_history(connection, fake_initial=options['fake_initial']) # Before anything else, see if there's conflicting apps and drop out # hard if there are any --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -76,6 +76,11 @@ class MigrationExecutor(object): for migration, _ in full_plan: if migration in applied_migrations: migration.mutate_state(state, preserve=False) + elif any( + self.loader.graph.nodes[node] in applied_migrations + for node in self.loader.graph.node_map[migration.app_label, migration.name].descendants() + ): + _, state = self.loader.detect_soft_applied(self.connection, state, migration) return state def migrate(self, targets, plan=None, state=None, fake=False, fake_initial=False): @@ -232,7 +237,7 @@ class MigrationExecutor(object): if not fake: if fake_initial: # Test to see if this is an already-applied initial migration - applied, state = self.loader.detect_soft_applied(state, migration) + applied, state = self.loader.detect_soft_applied(self.connection, state, migration) if applied: fake = True if not fake: --- a/django/db/migrations/loader.py +++ b/django/db/migrations/loader.py @@ -268,13 +268,14 @@ class MigrationLoader(object): six.reraise(NodeNotFoundError, exc_value, sys.exc_info()[2]) raise exc - def check_consistent_history(self, connection): + def check_consistent_history(self, connection, fake_initial=False): """ Raise InconsistentMigrationHistory if any applied migrations have unapplied dependencies. """ recorder = MigrationRecorder(connection) applied = recorder.applied_migrations() + msg = "Migration {}.{} is applied before its dependency {}.{} on database '{}'." for migration in applied: # If the migration is unknown, skip it. if migration not in self.graph.nodes: @@ -286,9 +287,22 @@ class MigrationLoader(object): if parent in self.replacements: if all(m in applied for m in self.replacements[parent].replaces): continue + # Skip initial migration that is going to be fake-applied + # unless a later migration in the same app has been + # applied. + if migration[0] != parent[0]: + if self.detect_soft_applied(connection, None, self.graph.nodes[parent])[0]: + if fake_initial: + continue + else: + raise InconsistentMigrationHistory( + (msg + " The migration {}.{} may be faked using '--fake-initial'.").format( + migration[0], migration[1], parent[0], parent[1], + connection.alias, parent[0], parent[1], + ) + ) raise InconsistentMigrationHistory( - "Migration {}.{} is applied before its dependency " - "{}.{} on database '{}'.".format( + msg.format( migration[0], migration[1], parent[0], parent[1], connection.alias, ) @@ -317,7 +331,7 @@ class MigrationLoader(object): """ return self.graph.make_state(nodes=nodes, at_end=at_end, real_apps=list(self.unmigrated_apps)) - def detect_soft_applied(self, project_state, migration): + def detect_soft_applied(self, connection, project_state, migration): """ Tests whether a migration has been implicitly applied - that the tables or columns it would create exist. This is intended only for use @@ -331,7 +345,7 @@ class MigrationLoader(object): return ( model._meta.proxy or not model._meta.managed or not router.allow_migrate( - self.connection.alias, migration.app_label, + connection.alias, migration.app_label, model_name=model._meta.model_name, ) ) @@ -351,7 +365,7 @@ class MigrationLoader(object): apps = after_state.apps found_create_model_migration = False found_add_field_migration = False - existing_table_names = self.connection.introspection.table_names(self.connection.cursor()) + existing_table_names = connection.introspection.table_names(connection.cursor()) # Make sure all create model and add field operations are done for operation in migration.operations: if isinstance(operation, migrations.CreateModel): @@ -387,7 +401,7 @@ class MigrationLoader(object): column_names = [ column.name for column in - self.connection.introspection.get_table_description(self.connection.cursor(), table) + connection.introspection.get_table_description(connection.cursor(), table) ] if field.column not in column_names: return False, project_state --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -297,6 +297,59 @@ class ExecutorTests(MigrationTestBase): self.assertTableNotExists("migrations_author") self.assertTableNotExists("migrations_tribble") + @override_settings(MIGRATION_MODULES={ + "migrations": "migrations.test_migrations_first", + "migrations2": "migrations2.test_migrations_2_first", + }) + def test_create_project_state_soft_applied(self): + """ + _create_project_state(with_applied_migrations=True) should apply + soft-applied migrations to the project state. + """ + executor = MigrationExecutor(connection) + # Were the tables there before? + self.assertTableNotExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + # Run it normally + self.assertEqual( + executor.migration_plan([("migrations2", "0002_second")]), + [ + (executor.loader.graph.nodes["migrations", "thefirst"], False), + (executor.loader.graph.nodes["migrations2", "0001_initial"], False), + (executor.loader.graph.nodes["migrations2", "0002_second"], False), + ], + ) + executor.migrate([("migrations2", "0002_second")]) + # Are the tables there now? + self.assertTableExists("migrations_author") + self.assertTableExists("migrations_tribble") + # Fake-revert the initial migration in "migrations". We can't + # fake-migrate backwards as that would revert other migrations + # as well. + recorder = MigrationRecorder(connection) + recorder.record_unapplied("migrations", "thefirst") + # Check if models and fields in soft-applied migrations are + # in the project state. + executor.loader.build_graph() + project_state = executor._create_project_state(with_applied_migrations=True) + self.assertIn(("migrations", "author"), project_state.models) + self.assertIn(("migrations", "tribble"), project_state.models) + # Apply migration with --fake-initial + executor.loader.build_graph() + self.assertEqual( + executor.migration_plan([("migrations", "thefirst")]), + [ + (executor.loader.graph.nodes["migrations", "thefirst"], False), + ], + ) + executor.migrate([("migrations", "thefirst")], fake_initial=True) + # And migrate back to clean up the database + executor.loader.build_graph() + executor.migrate([("migrations", None)]) + self.assertTableNotExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + executor.loader.build_graph() + @override_settings( MIGRATION_MODULES={ "migrations": "migrations.test_migrations_custom_user", @@ -326,7 +379,7 @@ class ExecutorTests(MigrationTestBase): global_apps.get_app_config("migrations").models["author"] = migrations_apps.get_model("migrations", "author") try: migration = executor.loader.get_migration("auth", "0001_initial") - self.assertIs(executor.loader.detect_soft_applied(None, migration)[0], True) + self.assertIs(executor.loader.detect_soft_applied(connection, None, migration)[0], True) finally: connection.introspection.table_names = old_table_names del global_apps.get_app_config("migrations").models["author"] @@ -365,9 +418,9 @@ class ExecutorTests(MigrationTestBase): self.assertTableExists(table) # Table detection sees 0001 is applied but not 0002. migration = executor.loader.get_migration("migrations", "0001_initial") - self.assertIs(executor.loader.detect_soft_applied(None, migration)[0], True) + self.assertIs(executor.loader.detect_soft_applied(connection, None, migration)[0], True) migration = executor.loader.get_migration("migrations", "0002_initial") - self.assertIs(executor.loader.detect_soft_applied(None, migration)[0], False) + self.assertIs(executor.loader.detect_soft_applied(connection, None, migration)[0], False) # Create the tables for both migrations but make it look like neither # has been applied. @@ -378,7 +431,7 @@ class ExecutorTests(MigrationTestBase): executor.migrate([("migrations", None)], fake=True) # Table detection sees 0002 is applied. migration = executor.loader.get_migration("migrations", "0002_initial") - self.assertIs(executor.loader.detect_soft_applied(None, migration)[0], True) + self.assertIs(executor.loader.detect_soft_applied(connection, None, migration)[0], True) # Leave the tables for 0001 except the many-to-many table. That missing # table should cause detect_soft_applied() to return False. @@ -386,7 +439,7 @@ class ExecutorTests(MigrationTestBase): for table in tables[2:]: editor.execute(editor.sql_delete_table % {"table": table}) migration = executor.loader.get_migration("migrations", "0001_initial") - self.assertIs(executor.loader.detect_soft_applied(None, migration)[0], False) + self.assertIs(executor.loader.detect_soft_applied(connection, None, migration)[0], False) # Cleanup by removing the remaining tables. with connection.schema_editor() as editor: --- a/tests/migrations/test_loader.py +++ b/tests/migrations/test_loader.py @@ -8,7 +8,7 @@ from django.db.migrations.exceptions imp ) from django.db.migrations.loader import MigrationLoader from django.db.migrations.recorder import MigrationRecorder -from django.test import TestCase, modify_settings, override_settings +from django.test import TestCase, modify_settings, override_settings, mock from django.utils import six @@ -402,6 +402,31 @@ class LoaderTests(TestCase): loader.check_consistent_history(connection) @override_settings(MIGRATION_MODULES={ + "migrations": "migrations.test_migrations_first", + "migrations2": "migrations2.test_migrations_2_first", + }) + @modify_settings(INSTALLED_APPS={'append': 'migrations2'}) + @mock.patch.object(MigrationLoader, 'detect_soft_applied', return_value=(True, None)) + def test_check_consistent_history_fake_initial(self, mock_detect_soft_applied): + """ + MigrationLoader.check_consistent_history() should ignore soft-applied + initial migrations unless a later migration in the same app has been + applied. + """ + loader = MigrationLoader(connection=None) + recorder = MigrationRecorder(connection) + recorder.record_applied('migrations2', '0001_initial') + recorder.record_applied('migrations2', '0002_second') + loader.check_consistent_history(connection, fake_initial=True) + recorder.record_applied('migrations', 'second') + msg = ( + "Migration migrations.second is applied before its dependency " + "migrations.thefirst on database 'default'." + ) + with self.assertRaisesMessage(InconsistentMigrationHistory, msg): + loader.check_consistent_history(connection, fake_initial=True) + + @override_settings(MIGRATION_MODULES={ "app1": "migrations.test_migrations_squashed_ref_squashed.app1", "app2": "migrations.test_migrations_squashed_ref_squashed.app2", })