From f355d253f81c78b595e25e84309b648c24208d73 Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Tue, 17 Jun 2014 17:45:38 -0700 Subject: [PATCH] [1.7.x] Fixed #22853: Swapped models are now ignored for migration operations. --- django/db/migrations/operations/base.py | 12 +++ django/db/migrations/operations/fields.py | 15 ++- django/db/migrations/operations/models.py | 20 ++-- docs/ref/settings.txt | 6 ++ docs/topics/auth/customizing.txt | 3 +- tests/migrations/test_operations.py | 117 ++++++++++++++++++++-- 6 files changed, 147 insertions(+), 26 deletions(-) diff --git a/django/db/migrations/operations/base.py b/django/db/migrations/operations/base.py index d0ab0a39ad..5d4b649065 100644 --- a/django/db/migrations/operations/base.py +++ b/django/db/migrations/operations/base.py @@ -1,4 +1,5 @@ from __future__ import unicode_literals +from django.db import router class Operation(object): @@ -97,6 +98,17 @@ class Operation(object): """ return self.references_model(model_name, app_label) + def allowed_to_migrate(self, connection_alias, model): + """ + Returns if we're allowed to migrate the model. Checks the router, + if it's a proxy, and if it's swapped out. + """ + return ( + router.allow_migrate(connection_alias, model) and + not model._meta.proxy and + not model._meta.swapped + ) + def __repr__(self): return "<%s %s%s>" % ( self.__class__.__name__, diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index fbffa19961..dee9337e95 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -from django.db import router from django.db.models.fields import NOT_PROVIDED from django.utils import six from .base import Operation @@ -29,7 +28,7 @@ class AddField(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) to_model = to_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): field = to_model._meta.get_field_by_name(self.name)[0] if not self.preserve_default: field.default = self.field.default @@ -42,7 +41,7 @@ class AddField(Operation): def database_backwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, from_model): + if self.allowed_to_migrate(schema_editor.connection.alias, from_model): schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0]) def describe(self): @@ -81,13 +80,13 @@ class RemoveField(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, from_model): + if self.allowed_to_migrate(schema_editor.connection.alias, from_model): schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0]) def database_backwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) to_model = to_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): schema_editor.add_field(from_model, to_model._meta.get_field_by_name(self.name)[0]) def describe(self): @@ -118,7 +117,7 @@ class AlterField(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) to_model = to_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): from_field = from_model._meta.get_field_by_name(self.name)[0] to_field = to_model._meta.get_field_by_name(self.name)[0] # If the field is a relatedfield with an unresolved rel.to, just @@ -170,7 +169,7 @@ class RenameField(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) to_model = to_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): schema_editor.alter_field( from_model, from_model._meta.get_field_by_name(self.old_name)[0], @@ -180,7 +179,7 @@ class RenameField(Operation): def database_backwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.model_name) to_model = to_state.render().get_model(app_label, self.model_name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): schema_editor.alter_field( from_model, from_model._meta.get_field_by_name(self.new_name)[0], diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index a3c0cc74b2..b6d75ccc44 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from django.db import models, router +from django.db import models from django.db.models.options import normalize_together from django.db.migrations.state import ModelState from django.db.migrations.operations.base import Operation @@ -32,13 +32,13 @@ class CreateModel(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): apps = to_state.render() model = apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy: + if self.allowed_to_migrate(schema_editor.connection.alias, model): schema_editor.create_model(model) def database_backwards(self, app_label, schema_editor, from_state, to_state): apps = from_state.render() model = apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy: + if self.allowed_to_migrate(schema_editor.connection.alias, model): schema_editor.delete_model(model) def describe(self): @@ -85,13 +85,13 @@ class DeleteModel(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): apps = from_state.render() model = apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy: + if self.allowed_to_migrate(schema_editor.connection.alias, model): schema_editor.delete_model(model) def database_backwards(self, app_label, schema_editor, from_state, to_state): apps = to_state.render() model = apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy: + if self.allowed_to_migrate(schema_editor.connection.alias, model): schema_editor.create_model(model) def references_model(self, name, app_label=None): @@ -141,7 +141,7 @@ class RenameModel(Operation): new_apps = to_state.render() old_model = old_apps.get_model(app_label, self.old_name) new_model = new_apps.get_model(app_label, self.new_name) - if router.allow_migrate(schema_editor.connection.alias, new_model): + if self.allowed_to_migrate(schema_editor.connection.alias, new_model): # Move the main table schema_editor.alter_db_table( new_model, @@ -194,7 +194,7 @@ class AlterModelTable(Operation): new_apps = to_state.render() old_model = old_apps.get_model(app_label, self.name) new_model = new_apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, new_model): + if self.allowed_to_migrate(schema_editor.connection.alias, new_model): schema_editor.alter_db_table( new_model, old_model._meta.db_table, @@ -231,7 +231,7 @@ class AlterUniqueTogether(Operation): new_apps = to_state.render() old_model = old_apps.get_model(app_label, self.name) new_model = new_apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, new_model): + if self.allowed_to_migrate(schema_editor.connection.alias, new_model): schema_editor.alter_unique_together( new_model, getattr(old_model._meta, "unique_together", set()), @@ -268,7 +268,7 @@ class AlterIndexTogether(Operation): new_apps = to_state.render() old_model = old_apps.get_model(app_label, self.name) new_model = new_apps.get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, new_model): + if self.allowed_to_migrate(schema_editor.connection.alias, new_model): schema_editor.alter_index_together( new_model, getattr(old_model._meta, "index_together", set()), @@ -301,7 +301,7 @@ class AlterOrderWithRespectTo(Operation): def database_forwards(self, app_label, schema_editor, from_state, to_state): from_model = from_state.render().get_model(app_label, self.name) to_model = to_state.render().get_model(app_label, self.name) - if router.allow_migrate(schema_editor.connection.alias, to_model): + if self.allowed_to_migrate(schema_editor.connection.alias, to_model): # Remove a field if we need to if from_model._meta.order_with_respect_to and not to_model._meta.order_with_respect_to: schema_editor.remove_field(from_model, from_model._meta.get_field_by_name("_order")[0]) diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index b4b432149b..6ea6c2c113 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -2432,6 +2432,12 @@ Default: 'auth.User' The model to use to represent a User. See :ref:`auth-custom-user`. +.. warning:: + You cannot change the AUTH_USER_MODEL setting during the lifetime of + a project (i.e. once you have made and migrated models that depend on it) + without serious effort. It is intended to be set at the project start. + See :ref:`auth-custom-user` for more details. + .. setting:: LOGIN_REDIRECT_URL LOGIN_REDIRECT_URL diff --git a/docs/topics/auth/customizing.txt b/docs/topics/auth/customizing.txt index 3ab9baeb53..610de93d01 100644 --- a/docs/topics/auth/customizing.txt +++ b/docs/topics/auth/customizing.txt @@ -383,7 +383,8 @@ use as your User model. Changing this setting after you have tables created is not supported by :djadmin:`makemigrations` and will result in you having to manually - write a set of migrations to fix your schema. + fix your schema, port your data from the old user table, and possibly + manually reapply some migrations. Referencing the User model -------------------------- diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 0b82d56f05..48eedd193c 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -8,6 +8,7 @@ except ImportError: sqlparse = None from django import test +from django.test import override_settings from django.db import connection, migrations, models, router from django.db.migrations.migration import Migration from django.db.migrations.state import ProjectState @@ -18,11 +19,9 @@ from django.db.utils import IntegrityError, DatabaseError from .test_base import MigrationTestBase -class OperationTests(MigrationTestBase): +class OperationTestBase(MigrationTestBase): """ - Tests running the operations and making sure they do what they say they do. - Each test looks at their state changing, and then their database operation - - both forwards and backwards. + Common functions to help test operations. """ def apply_operations(self, app_label, project_state, operations): @@ -37,6 +36,16 @@ class OperationTests(MigrationTestBase): with connection.schema_editor() as editor: return migration.unapply(project_state, editor) + def make_test_state(self, app_label, operation, **kwargs): + """ + Makes a test state using set_up_test_model and returns the + original state and the state after the migration is applied. + """ + project_state = self.set_up_test_model(app_label, **kwargs) + new_state = project_state.clone() + operation.state_forwards(app_label, new_state) + return project_state, new_state + def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False, proxy_model=False): """ Creates a test model state and database table. @@ -74,6 +83,9 @@ class OperationTests(MigrationTestBase): ("pink", models.IntegerField(default=3)), ("weight", models.FloatField()), ], + options = { + "swappable": "TEST_SWAP_MODEL", + }, )] if second_model: operations.append(migrations.CreateModel( @@ -122,6 +134,15 @@ class OperationTests(MigrationTestBase): return self.apply_operations(app_label, ProjectState(), operations) + +class OperationTests(OperationTestBase): + """ + Tests running the operations and making sure they do what they say they do. + Each test looks at their state changing, and then their database operation - + both forwards and backwards. + """ + + def test_create_model(self): """ Tests the CreateModel operation. @@ -382,15 +403,13 @@ class OperationTests(MigrationTestBase): """ Tests the AddField operation. """ - project_state = self.set_up_test_model("test_adfl") # Test the state alteration operation = migrations.AddField( "Pony", "height", models.FloatField(null=True, default=5), ) - new_state = project_state.clone() - operation.state_forwards("test_adfl", new_state) + project_state, new_state = self.make_test_state("test_adfl", operation) self.assertEqual(len(new_state.models["test_adfl", "pony"].fields), 4) field = [ f for n, f in new_state.models["test_adfl", "pony"].fields @@ -1117,3 +1136,87 @@ class MultiDBOperationTests(MigrationTestBase): with connection.schema_editor() as editor: operation.database_backwards("test_crmo", editor, new_state, project_state) self.assertTableNotExists("test_crmo_pony") + + +class SwappableOperationTests(OperationTestBase): + """ + Tests that key operations ignore swappable models + (we don't want to replicate all of them here, as the functionality + is in a common base class anyway) + """ + + available_apps = [ + "migrations", + "django.contrib.auth", + ] + + @override_settings(TEST_SWAP_MODEL="migrations.SomeFakeModel") + def test_create_ignore_swapped(self): + """ + Tests that the CreateTable operation ignores swapped models. + """ + operation = migrations.CreateModel( + "Pony", + [ + ("id", models.AutoField(primary_key=True)), + ("pink", models.IntegerField(default=1)), + ], + options = { + "swappable": "TEST_SWAP_MODEL", + }, + ) + # Test the state alteration (it should still be there!) + project_state = ProjectState() + new_state = project_state.clone() + operation.state_forwards("test_crigsw", new_state) + self.assertEqual(new_state.models["test_crigsw", "pony"].name, "Pony") + self.assertEqual(len(new_state.models["test_crigsw", "pony"].fields), 2) + # Test the database alteration + self.assertTableNotExists("test_crigsw_pony") + with connection.schema_editor() as editor: + operation.database_forwards("test_crigsw", editor, project_state, new_state) + self.assertTableNotExists("test_crigsw_pony") + # And test reversal + with connection.schema_editor() as editor: + operation.database_backwards("test_crigsw", editor, new_state, project_state) + self.assertTableNotExists("test_crigsw_pony") + + @override_settings(TEST_SWAP_MODEL="migrations.SomeFakeModel") + def test_delete_ignore_swapped(self): + """ + Tests the DeleteModel operation ignores swapped models. + """ + operation = migrations.DeleteModel("Pony") + project_state, new_state = self.make_test_state("test_dligsw", operation) + # Test the database alteration + self.assertTableNotExists("test_dligsw_pony") + with connection.schema_editor() as editor: + operation.database_forwards("test_dligsw", editor, project_state, new_state) + self.assertTableNotExists("test_dligsw_pony") + # And test reversal + with connection.schema_editor() as editor: + operation.database_backwards("test_dligsw", editor, new_state, project_state) + self.assertTableNotExists("test_dligsw_pony") + + @override_settings(TEST_SWAP_MODEL="migrations.SomeFakeModel") + def test_add_field_ignore_swapped(self): + """ + Tests the AddField operation. + """ + # Test the state alteration + operation = migrations.AddField( + "Pony", + "height", + models.FloatField(null=True, default=5), + ) + project_state, new_state = self.make_test_state("test_adfligsw", operation) + # Test the database alteration + self.assertTableNotExists("test_adfligsw_pont") + self.assertColumnNotExists("test_adfligsw_pony", "height") + with connection.schema_editor() as editor: + operation.database_forwards("test_adfligsw", editor, project_state, new_state) + self.assertColumnNotExists("test_adfligsw_pony", "height") + # And test reversal + with connection.schema_editor() as editor: + operation.database_backwards("test_adfligsw", editor, new_state, project_state) + self.assertColumnNotExists("test_adfligsw_pony", "height")