From f287bec5833d75750fa6368bc2802741b7924533 Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Thu, 12 Feb 2015 12:48:28 +0100 Subject: [PATCH] Fixed #24184 -- Prevented automatic soft-apply of migrations Previously Django only checked for the table name in CreateModel operations in initial migrations and faked the migration automatically. This led to various errors and unexpected behavior. The newly introduced --fake-initial flag to the migrate command must be passed to get the same behavior again. With this change Django will bail out in with a "duplicate relation / table" error instead. Thanks Carl Meyer and Tim Graham for the documentation update, report and review. --- django/core/management/commands/migrate.py | 8 ++- django/db/migrations/executor.py | 17 +++--- docs/ref/django-admin.txt | 13 +++++ docs/releases/1.8.txt | 5 ++ docs/topics/migrations.txt | 39 +++++++++----- tests/migrations/test_commands.py | 61 +++++++++++++++++++++- tests/migrations/test_executor.py | 10 +++- 7 files changed, 130 insertions(+), 23 deletions(-) diff --git a/django/core/management/commands/migrate.py b/django/core/management/commands/migrate.py index d1aff756f0..5dc7dc00f7 100644 --- a/django/core/management/commands/migrate.py +++ b/django/core/management/commands/migrate.py @@ -40,6 +40,10 @@ class Command(BaseCommand): 'Defaults to the "default" database.') parser.add_argument('--fake', action='store_true', dest='fake', default=False, help='Mark migrations as run without actually running them') + parser.add_argument('--fake-initial', action='store_true', dest='fake_initial', default=False, + help='Detect if tables already exist and fake-apply initial migrations if so. Make sure ' + 'that the current database schema matches your initial migration before using this ' + 'flag. Django will only check for an existing table name.') parser.add_argument('--list', '-l', action='store_true', dest='list', default=False, help='Show a list of all known migrations and which are applied') @@ -186,7 +190,9 @@ class Command(BaseCommand): "apply them." )) else: - executor.migrate(targets, plan, fake=options.get("fake", False)) + fake = options.get("fake") + fake_initial = options.get("fake_initial") + executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial) # Send the post_migrate signal, so individual apps can do whatever they need # to do at this point. diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py index 8a55ed0d69..d1a8dd64dc 100644 --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -62,7 +62,7 @@ class MigrationExecutor(object): applied.add(migration) return plan - def migrate(self, targets, plan=None, fake=False): + def migrate(self, targets, plan=None, fake=False, fake_initial=False): """ Migrates the database up to the given targets. @@ -91,7 +91,7 @@ class MigrationExecutor(object): # Phase 2 -- Run the migrations for migration, backwards in plan: if not backwards: - self.apply_migration(states[migration], migration, fake=fake) + self.apply_migration(states[migration], migration, fake=fake, fake_initial=fake_initial) else: self.unapply_migration(states[migration], migration, fake=fake) @@ -113,18 +113,19 @@ class MigrationExecutor(object): statements.extend(schema_editor.collected_sql) return statements - def apply_migration(self, state, migration, fake=False): + def apply_migration(self, state, migration, fake=False, fake_initial=False): """ Runs a migration forwards. """ if self.progress_callback: self.progress_callback("apply_start", migration, fake) if not fake: - # Test to see if this is an already-applied initial migration - applied, state = self.detect_soft_applied(state, migration) - if applied: - fake = True - else: + if fake_initial: + # Test to see if this is an already-applied initial migration + applied, state = self.detect_soft_applied(state, migration) + if applied: + fake = True + if not fake: # Alright, do it normally with self.connection.schema_editor() as schema_editor: state = migration.apply(state, schema_editor) diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index ce3a78f915..fedec7aefa 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -721,6 +721,19 @@ be warned that using ``--fake`` runs the risk of putting the migration state table into a state where manual recovery will be needed to make migrations run correctly. +.. versionadded:: 1.8 + +.. django-admin-option:: --fake-initial + +The ``--fake-initial`` option can be used to allow Django to skip an app's +initial migration if all database tables with the names of all models created +by all :class:`~django.db.migrations.operations.CreateModel` operations in that +migration already exist. This option is intended for use when first running +migrations against a database that preexisted the use of migrations. This +option does not, however, check for matching database schema beyond matching +table names and so is only safe to use if you are confident that your existing +schema matches what is recorded in your initial migration. + .. deprecated:: 1.8 The ``--list`` option has been moved to the :djadmin:`showmigrations` diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 4f77f063a3..4c7076a51e 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1135,6 +1135,11 @@ Miscellaneous has been removed by a migration and replaced by a property. That means it's not possible to query or filter a ``ContentType`` by this field any longer. +* :djadmin:`migrate` now accepts the :djadminopt:`--fake-initial` option to + allow faking initial migrations. In 1.7 initial migrations were always + automatically faked if all tables created in an initial migration already + existed. + .. _deprecated-features-1.8: Features deprecated in 1.8 diff --git a/docs/topics/migrations.txt b/docs/topics/migrations.txt index eaeb6da7c5..c1c14512c8 100644 --- a/docs/topics/migrations.txt +++ b/docs/topics/migrations.txt @@ -140,6 +140,13 @@ developers (or your production servers) check out the code, they'll get both the changes to your models and the accompanying migration at the same time. +.. versionadded:: 1.8 + +If you want to give the migration(s) a meaningful name instead of a generated +one, you can use the :djadminopt:`--name` option:: + + $ python manage.py makemigrations --name changed_my_model your_app_label + Version control ~~~~~~~~~~~~~~~ @@ -282,10 +289,12 @@ need to convert it to use migrations; this is a simple process:: $ python manage.py makemigrations your_app_label -This will make a new initial migration for your app. Now, when you run -:djadmin:`migrate`, Django will detect that you have an initial migration -*and* that the tables it wants to create already exist, and will mark the -migration as already applied. +This will make a new initial migration for your app. Now, run ``python +manage.py migrate --fake-initial``, and Django will detect that you have an +initial migration *and* that the tables it wants to create already exist, and +will mark the migration as already applied. (Without the +:djadminopt:`--fake-initial` flag, the :djadmin:`migrate` command would error +out because the tables it wants to create already exist.) Note that this only works given two things: @@ -297,12 +306,11 @@ Note that this only works given two things: that your database doesn't match your models, you'll just get errors when migrations try to modify those tables. -.. versionadded:: 1.8 +.. versionchanged: 1.8 -If you want to give the migration(s) a meaningful name instead of a generated one, -you can use the :djadminopt:`--name` option:: - - $ python manage.py makemigrations --name changed_my_model your_app_label + The ``--fake-initial`` flag to :djadmin:`migrate` was added. Previously, + Django would always automatically fake-apply initial migrations if it + detected that the tables exist. .. _historical-models: @@ -706,9 +714,10 @@ If you already have pre-existing migrations created with ``__init__.py`` - make sure you remove the ``.pyc`` files too. * Run ``python manage.py makemigrations``. Django should see the empty migration directories and make new initial migrations in the new format. -* Run ``python manage.py migrate``. Django will see that the tables for the - initial migrations already exist and mark them as applied without running - them. +* Run ``python manage.py migrate --fake-initial``. Django will see that the + tables for the initial migrations already exist and mark them as applied + without running them. (Django won't check that the table schema match your + models, just that the right table names exist). That's it! The only complication is if you have a circular dependency loop of foreign keys; in this case, ``makemigrations`` might make more than one @@ -716,6 +725,12 @@ initial migration, and you'll need to mark them all as applied using:: python manage.py migrate --fake yourappnamehere +.. versionchanged:: 1.8 + + The :djadminopt:`--fake-initial` flag was added to :djadmin:`migrate`; + previously, initial migrations were always automatically fake-applied if + existing tables were detected. + Libraries/Third-party Apps ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index 95defa61f8..f20a083fb6 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -8,7 +8,7 @@ import shutil from django.apps import apps from django.core.management import CommandError, call_command -from django.db import connection, models +from django.db import DatabaseError, connection, models from django.db.migrations import questioner from django.test import ignore_warnings, mock, override_settings from django.utils import six @@ -52,6 +52,65 @@ class MigrateTests(MigrationTestBase): self.assertTableNotExists("migrations_tribble") self.assertTableNotExists("migrations_book") + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"}) + def test_migrate_fake_initial(self): + """ + #24184 - Tests that --fake-initial only works if all tables created in + the initial migration of an app exists + """ + # Make sure no tables are created + self.assertTableNotExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + # Run the migrations to 0001 only + call_command("migrate", "migrations", "0001", verbosity=0) + # Make sure the right tables exist + self.assertTableExists("migrations_author") + self.assertTableExists("migrations_tribble") + # Fake a roll-back + call_command("migrate", "migrations", "zero", fake=True, verbosity=0) + # Make sure the tables still exist + self.assertTableExists("migrations_author") + self.assertTableExists("migrations_tribble") + # Try to run initial migration + with self.assertRaises(DatabaseError): + call_command("migrate", "migrations", "0001", verbosity=0) + # Run initial migration with an explicit --fake-initial + out = six.StringIO() + with mock.patch('django.core.management.color.supports_color', lambda *args: False): + call_command("migrate", "migrations", "0001", fake_initial=True, stdout=out, verbosity=1) + self.assertIn( + "migrations.0001_initial... faked", + out.getvalue().lower() + ) + # Run migrations all the way + call_command("migrate", verbosity=0) + # Make sure the right tables exist + self.assertTableExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + self.assertTableExists("migrations_book") + # Fake a roll-back + call_command("migrate", "migrations", "zero", fake=True, verbosity=0) + # Make sure the tables still exist + self.assertTableExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + self.assertTableExists("migrations_book") + # Try to run initial migration + with self.assertRaises(DatabaseError): + call_command("migrate", "migrations", verbosity=0) + # Run initial migration with an explicit --fake-initial + with self.assertRaises(DatabaseError): + # Fails because "migrations_tribble" does not exist but needs to in + # order to make --fake-initial work. + call_command("migrate", "migrations", fake_initial=True, verbosity=0) + # Fake a apply + call_command("migrate", "migrations", fake=True, verbosity=0) + # Unmigrate everything + call_command("migrate", "migrations", "zero", verbosity=0) + # Make sure it's all gone + self.assertTableNotExists("migrations_author") + self.assertTableNotExists("migrations_tribble") + self.assertTableNotExists("migrations_book") + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_conflict"}) def test_migrate_conflict_exit(self): """ diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py index e6482fb830..b88307ceea 100644 --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -2,6 +2,7 @@ from django.apps.registry import apps as global_apps from django.db import connection from django.db.migrations.executor import MigrationExecutor from django.db.migrations.graph import MigrationGraph +from django.db.utils import DatabaseError from django.test import TestCase, modify_settings, override_settings from .test_base import MigrationTestBase @@ -186,7 +187,14 @@ class ExecutorTests(MigrationTestBase): (executor.loader.graph.nodes["migrations", "0001_initial"], False), ], ) - executor.migrate([("migrations", "0001_initial")]) + # Applying the migration should raise a database level error + # because we haven't given the --fake-initial option + with self.assertRaises(DatabaseError): + executor.migrate([("migrations", "0001_initial")]) + # Reset the faked state + state = {"faked": None} + # Allow faking of initial CreateModel operations + executor.migrate([("migrations", "0001_initial")], fake_initial=True) self.assertEqual(state["faked"], True) # And migrate back to clean up the database executor.loader.build_graph()