From fda55c71a8846364b9dc4f4efac4d7034ef6dd6f Mon Sep 17 00:00:00 2001 From: Marti Raudsepp Date: Sat, 5 Nov 2016 10:16:23 +0200 Subject: [PATCH] Fixed #27858 -- Prevented read-only management commands from creating the django_migrations table. MigrationRecorder now assumes that if the django_migrations table doesn't exist, then no migrations are applied. Reverted documentation change from refs #23808. --- AUTHORS | 1 + django/core/management/base.py | 6 ------ django/db/migrations/executor.py | 4 ++++ django/db/migrations/recorder.py | 14 +++++++++++--- docs/ref/django-admin.txt | 3 --- tests/admin_scripts/tests.py | 12 ++++-------- tests/migrations/test_commands.py | 22 ++++++++++------------ 7 files changed, 30 insertions(+), 32 deletions(-) diff --git a/AUTHORS b/AUTHORS index fb912fd2e0..30a0ba7c88 100644 --- a/AUTHORS +++ b/AUTHORS @@ -506,6 +506,7 @@ answer newbie questions, and generally made Django that much better: Markus Amalthea Magnuson Markus Holtermann Marten Kenbeek + Marti Raudsepp martin.glueck@gmail.com Martin Green Martin Kosír diff --git a/django/core/management/base.py b/django/core/management/base.py index bfc6811e28..41b6b0fa91 100644 --- a/django/core/management/base.py +++ b/django/core/management/base.py @@ -12,7 +12,6 @@ from django.core import checks from django.core.exceptions import ImproperlyConfigured from django.core.management.color import color_style, no_style from django.db import DEFAULT_DB_ALIAS, connections -from django.db.migrations.exceptions import MigrationSchemaMissing class CommandError(Exception): @@ -429,11 +428,6 @@ class BaseCommand: except ImproperlyConfigured: # No databases are configured (or the dummy one) return - except MigrationSchemaMissing: - self.stdout.write(self.style.NOTICE( - "\nNot checking migrations as it is not possible to access/create the django_migrations table." - )) - return plan = executor.migration_plan(executor.loader.graph.leaf_nodes()) if plan: diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py index ab412fe6d1..ea7bc70db3 100644 --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -86,6 +86,10 @@ class MigrationExecutor: Django first needs to create all project states before a migration is (un)applied and in a second step run all the database operations. """ + # The django_migrations table must be present to record applied + # migrations. + self.recorder.ensure_schema() + if plan is None: plan = self.migration_plan(targets) # Create the forwards plan Django would follow on an empty database diff --git a/django/db/migrations/recorder.py b/django/db/migrations/recorder.py index dcc27d525c..2a3ff8bb99 100644 --- a/django/db/migrations/recorder.py +++ b/django/db/migrations/recorder.py @@ -39,11 +39,15 @@ class MigrationRecorder: def migration_qs(self): return self.Migration.objects.using(self.connection.alias) + def has_table(self): + """Return True if the django_migrations table exists.""" + return self.Migration._meta.db_table in self.connection.introspection.table_names(self.connection.cursor()) + def ensure_schema(self): """Ensure the table exists and has the correct schema.""" # If the table's there, that's fine - we've never changed its schema # in the codebase. - if self.Migration._meta.db_table in self.connection.introspection.table_names(self.connection.cursor()): + if self.has_table(): return # Make the table try: @@ -54,8 +58,12 @@ class MigrationRecorder: def applied_migrations(self): """Return a set of (app, name) of applied migrations.""" - self.ensure_schema() - return {tuple(x) for x in self.migration_qs.values_list("app", "name")} + if self.has_table(): + return {tuple(x) for x in self.migration_qs.values_list('app', 'name')} + else: + # If the django_migrations table doesn't eixst, then no migrations + # are applied. + return set() def record_applied(self, app, name): """Record that a migration was applied.""" diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index 29522ebe92..20e1bb02de 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -876,9 +876,6 @@ If the :doc:`staticfiles` contrib app is enabled (default in new projects) the :djadmin:`runserver` command will be overridden with its own :ref:`runserver` command. -If :djadmin:`migrate` was not previously executed, the table that stores the -history of migrations is created at first run of ``runserver``. - Logging of each request and response of the server is sent to the :ref:`django-server-logger` logger. diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py index 9ef2da83a0..e2d2a3bca1 100644 --- a/tests/admin_scripts/tests.py +++ b/tests/admin_scripts/tests.py @@ -22,7 +22,6 @@ from django.core.management import ( BaseCommand, CommandError, call_command, color, ) from django.db import ConnectionHandler -from django.db.migrations.exceptions import MigrationSchemaMissing from django.db.migrations.recorder import MigrationRecorder from django.test import ( LiveServerTestCase, SimpleTestCase, TestCase, override_settings, @@ -1339,15 +1338,12 @@ class ManageRunserver(AdminScriptTestCase): def test_readonly_database(self): """ - Ensure runserver.check_migrations doesn't choke when a database is read-only - (with possibly no django_migrations table). + runserver.check_migrations() doesn't choke when a database is read-only. """ - with mock.patch.object( - MigrationRecorder, 'ensure_schema', - side_effect=MigrationSchemaMissing()): + with mock.patch.object(MigrationRecorder, 'has_table', return_value=False): self.cmd.check_migrations() - # Check a warning is emitted - self.assertIn("Not checking migrations", self.output.getvalue()) + # You have # ... + self.assertIn('unapplied migration(s)', self.output.getvalue()) class ManageRunserverMigrationWarning(TestCase): diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index 3b022e666c..777081e3c7 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -11,9 +11,7 @@ from django.db import ( ConnectionHandler, DatabaseError, connection, connections, models, ) from django.db.backends.base.schema import BaseDatabaseSchemaEditor -from django.db.migrations.exceptions import ( - InconsistentMigrationHistory, MigrationSchemaMissing, -) +from django.db.migrations.exceptions import InconsistentMigrationHistory from django.db.migrations.recorder import MigrationRecorder from django.test import override_settings @@ -697,35 +695,35 @@ class MakeMigrationsTests(MigrationTestBase): The history consistency checks in makemigrations respect settings.DATABASE_ROUTERS. """ - def patched_ensure_schema(migration_recorder): + def patched_has_table(migration_recorder): if migration_recorder.connection is connections['other']: - raise MigrationSchemaMissing('Patched') + raise Exception('Other connection') else: return mock.DEFAULT self.assertTableNotExists('migrations_unicodemodel') apps.register_model('migrations', UnicodeModel) with mock.patch.object( - MigrationRecorder, 'ensure_schema', - autospec=True, side_effect=patched_ensure_schema) as ensure_schema: + MigrationRecorder, 'has_table', + autospec=True, side_effect=patched_has_table) as has_table: with self.temporary_migration_module() as migration_dir: call_command("makemigrations", "migrations", verbosity=0) initial_file = os.path.join(migration_dir, "0001_initial.py") self.assertTrue(os.path.exists(initial_file)) - self.assertEqual(ensure_schema.call_count, 1) # 'default' is checked + self.assertEqual(has_table.call_count, 1) # 'default' is checked # Router says not to migrate 'other' so consistency shouldn't # be checked. with self.settings(DATABASE_ROUTERS=['migrations.routers.TestRouter']): call_command('makemigrations', 'migrations', verbosity=0) - self.assertEqual(ensure_schema.call_count, 2) # 'default' again + self.assertEqual(has_table.call_count, 2) # 'default' again # With a router that doesn't prohibit migrating 'other', # consistency is checked. with self.settings(DATABASE_ROUTERS=['migrations.routers.EmptyRouter']): - with self.assertRaisesMessage(MigrationSchemaMissing, 'Patched'): + with self.assertRaisesMessage(Exception, 'Other connection'): call_command('makemigrations', 'migrations', verbosity=0) - self.assertEqual(ensure_schema.call_count, 4) # 'default' and 'other' + self.assertEqual(has_table.call_count, 4) # 'default' and 'other' # With a router that doesn't allow migrating on any database, # no consistency checks are made. @@ -741,7 +739,7 @@ class MakeMigrationsTests(MigrationTestBase): self.assertIn(connection_alias, ['default', 'other']) # Raises an error if invalid app_name/model_name occurs. apps.get_app_config(app_name).get_model(call_kwargs['model_name']) - self.assertEqual(ensure_schema.call_count, 4) + self.assertEqual(has_table.call_count, 4) def test_failing_migration(self): # If a migration fails to serialize, it shouldn't generate an empty file. #21280