Skip to content

Commit fda55c7

Browse files
intgrtimgraham
authored andcommitted
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.
1 parent 4f1eb64 commit fda55c7

File tree

7 files changed

+30
-32
lines changed

7 files changed

+30
-32
lines changed

AUTHORS

+1
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ answer newbie questions, and generally made Django that much better:
506506
Markus Amalthea Magnuson <markus.magnuson@gmail.com>
507507
Markus Holtermann <https://markusholtermann.eu>
508508
Marten Kenbeek <marten.knbk+django@gmail.com>
509+
Marti Raudsepp <marti@juffo.org>
509510
martin.glueck@gmail.com
510511
Martin Green
511512
Martin Kosír <martin@martinkosir.net>

django/core/management/base.py

-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from django.core.exceptions import ImproperlyConfigured
1313
from django.core.management.color import color_style, no_style
1414
from django.db import DEFAULT_DB_ALIAS, connections
15-
from django.db.migrations.exceptions import MigrationSchemaMissing
1615

1716

1817
class CommandError(Exception):
@@ -429,11 +428,6 @@ def check_migrations(self):
429428
except ImproperlyConfigured:
430429
# No databases are configured (or the dummy one)
431430
return
432-
except MigrationSchemaMissing:
433-
self.stdout.write(self.style.NOTICE(
434-
"\nNot checking migrations as it is not possible to access/create the django_migrations table."
435-
))
436-
return
437431

438432
plan = executor.migration_plan(executor.loader.graph.leaf_nodes())
439433
if plan:

django/db/migrations/executor.py

+4
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ def migrate(self, targets, plan=None, state=None, fake=False, fake_initial=False
8686
Django first needs to create all project states before a migration is
8787
(un)applied and in a second step run all the database operations.
8888
"""
89+
# The django_migrations table must be present to record applied
90+
# migrations.
91+
self.recorder.ensure_schema()
92+
8993
if plan is None:
9094
plan = self.migration_plan(targets)
9195
# Create the forwards plan Django would follow on an empty database

django/db/migrations/recorder.py

+11-3
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,15 @@ def __init__(self, connection):
3939
def migration_qs(self):
4040
return self.Migration.objects.using(self.connection.alias)
4141

42+
def has_table(self):
43+
"""Return True if the django_migrations table exists."""
44+
return self.Migration._meta.db_table in self.connection.introspection.table_names(self.connection.cursor())
45+
4246
def ensure_schema(self):
4347
"""Ensure the table exists and has the correct schema."""
4448
# If the table's there, that's fine - we've never changed its schema
4549
# in the codebase.
46-
if self.Migration._meta.db_table in self.connection.introspection.table_names(self.connection.cursor()):
50+
if self.has_table():
4751
return
4852
# Make the table
4953
try:
@@ -54,8 +58,12 @@ def ensure_schema(self):
5458

5559
def applied_migrations(self):
5660
"""Return a set of (app, name) of applied migrations."""
57-
self.ensure_schema()
58-
return {tuple(x) for x in self.migration_qs.values_list("app", "name")}
61+
if self.has_table():
62+
return {tuple(x) for x in self.migration_qs.values_list('app', 'name')}
63+
else:
64+
# If the django_migrations table doesn't eixst, then no migrations
65+
# are applied.
66+
return set()
5967

6068
def record_applied(self, app, name):
6169
"""Record that a migration was applied."""

docs/ref/django-admin.txt

-3
Original file line numberDiff line numberDiff line change
@@ -876,9 +876,6 @@ If the :doc:`staticfiles</ref/contrib/staticfiles>` contrib app is enabled
876876
(default in new projects) the :djadmin:`runserver` command will be overridden
877877
with its own :ref:`runserver<staticfiles-runserver>` command.
878878

879-
If :djadmin:`migrate` was not previously executed, the table that stores the
880-
history of migrations is created at first run of ``runserver``.
881-
882879
Logging of each request and response of the server is sent to the
883880
:ref:`django-server-logger` logger.
884881

tests/admin_scripts/tests.py

+4-8
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
BaseCommand, CommandError, call_command, color,
2323
)
2424
from django.db import ConnectionHandler
25-
from django.db.migrations.exceptions import MigrationSchemaMissing
2625
from django.db.migrations.recorder import MigrationRecorder
2726
from django.test import (
2827
LiveServerTestCase, SimpleTestCase, TestCase, override_settings,
@@ -1339,15 +1338,12 @@ def test_no_database(self):
13391338

13401339
def test_readonly_database(self):
13411340
"""
1342-
Ensure runserver.check_migrations doesn't choke when a database is read-only
1343-
(with possibly no django_migrations table).
1341+
runserver.check_migrations() doesn't choke when a database is read-only.
13441342
"""
1345-
with mock.patch.object(
1346-
MigrationRecorder, 'ensure_schema',
1347-
side_effect=MigrationSchemaMissing()):
1343+
with mock.patch.object(MigrationRecorder, 'has_table', return_value=False):
13481344
self.cmd.check_migrations()
1349-
# Check a warning is emitted
1350-
self.assertIn("Not checking migrations", self.output.getvalue())
1345+
# You have # ...
1346+
self.assertIn('unapplied migration(s)', self.output.getvalue())
13511347

13521348

13531349
class ManageRunserverMigrationWarning(TestCase):

tests/migrations/test_commands.py

+10-12
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@
1111
ConnectionHandler, DatabaseError, connection, connections, models,
1212
)
1313
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
14-
from django.db.migrations.exceptions import (
15-
InconsistentMigrationHistory, MigrationSchemaMissing,
16-
)
14+
from django.db.migrations.exceptions import InconsistentMigrationHistory
1715
from django.db.migrations.recorder import MigrationRecorder
1816
from django.test import override_settings
1917

@@ -697,35 +695,35 @@ def test_makemigrations_consistency_checks_respect_routers(self):
697695
The history consistency checks in makemigrations respect
698696
settings.DATABASE_ROUTERS.
699697
"""
700-
def patched_ensure_schema(migration_recorder):
698+
def patched_has_table(migration_recorder):
701699
if migration_recorder.connection is connections['other']:
702-
raise MigrationSchemaMissing('Patched')
700+
raise Exception('Other connection')
703701
else:
704702
return mock.DEFAULT
705703

706704
self.assertTableNotExists('migrations_unicodemodel')
707705
apps.register_model('migrations', UnicodeModel)
708706
with mock.patch.object(
709-
MigrationRecorder, 'ensure_schema',
710-
autospec=True, side_effect=patched_ensure_schema) as ensure_schema:
707+
MigrationRecorder, 'has_table',
708+
autospec=True, side_effect=patched_has_table) as has_table:
711709
with self.temporary_migration_module() as migration_dir:
712710
call_command("makemigrations", "migrations", verbosity=0)
713711
initial_file = os.path.join(migration_dir, "0001_initial.py")
714712
self.assertTrue(os.path.exists(initial_file))
715-
self.assertEqual(ensure_schema.call_count, 1) # 'default' is checked
713+
self.assertEqual(has_table.call_count, 1) # 'default' is checked
716714

717715
# Router says not to migrate 'other' so consistency shouldn't
718716
# be checked.
719717
with self.settings(DATABASE_ROUTERS=['migrations.routers.TestRouter']):
720718
call_command('makemigrations', 'migrations', verbosity=0)
721-
self.assertEqual(ensure_schema.call_count, 2) # 'default' again
719+
self.assertEqual(has_table.call_count, 2) # 'default' again
722720

723721
# With a router that doesn't prohibit migrating 'other',
724722
# consistency is checked.
725723
with self.settings(DATABASE_ROUTERS=['migrations.routers.EmptyRouter']):
726-
with self.assertRaisesMessage(MigrationSchemaMissing, 'Patched'):
724+
with self.assertRaisesMessage(Exception, 'Other connection'):
727725
call_command('makemigrations', 'migrations', verbosity=0)
728-
self.assertEqual(ensure_schema.call_count, 4) # 'default' and 'other'
726+
self.assertEqual(has_table.call_count, 4) # 'default' and 'other'
729727

730728
# With a router that doesn't allow migrating on any database,
731729
# no consistency checks are made.
@@ -741,7 +739,7 @@ def patched_ensure_schema(migration_recorder):
741739
self.assertIn(connection_alias, ['default', 'other'])
742740
# Raises an error if invalid app_name/model_name occurs.
743741
apps.get_app_config(app_name).get_model(call_kwargs['model_name'])
744-
self.assertEqual(ensure_schema.call_count, 4)
742+
self.assertEqual(has_table.call_count, 4)
745743

746744
def test_failing_migration(self):
747745
# If a migration fails to serialize, it shouldn't generate an empty file. #21280

0 commit comments

Comments
 (0)