Browse Source

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.
Marti Raudsepp 8 years ago
parent
commit
fda55c71a8

+ 1 - 0
AUTHORS

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

+ 0 - 6
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:

+ 4 - 0
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

+ 11 - 3
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."""

+ 0 - 3
docs/ref/django-admin.txt

@@ -876,9 +876,6 @@ If the :doc:`staticfiles</ref/contrib/staticfiles>` contrib app is enabled
 (default in new projects) the :djadmin:`runserver` command will be overridden
 with its own :ref:`runserver<staticfiles-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.
 

+ 4 - 8
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):

+ 10 - 12
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