diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index a8417d4f66..216ff0958a 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -121,15 +121,11 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): Removes a field from a model. Usually involves deleting a column, but for M2Ms may involve deleting a table. """ - # M2M fields are a special case - if isinstance(field, ManyToManyField): - # For implicit M2M tables, delete the auto-created table - if field.rel.through._meta.auto_created: - self.delete_model(field.rel.through) - # For explicit "through" M2M fields, do nothing + # Special-case implicit M2M tables + if isinstance(field, ManyToManyField) and field.rel.through._meta.auto_created: + return self.delete_model(field.rel.through) # For everything else, remake. - else: - self._remake_table(model, delete_fields=[field]) + self._remake_table(model, delete_fields=[field]) def alter_field(self, model, old_field, new_field, strict=False): """ diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 134dcfed95..adf3686d59 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -223,6 +223,16 @@ class MigrationAutodetector(object): unique_together=unique_together ) ) + # Removing models + removed_models = set(old_model_keys) - set(new_model_keys) + for app_label, model_name in removed_models: + model_state = self.from_state.models[app_label, model_name] + self.add_to_migration( + app_label, + operations.DeleteModel( + model_state.name, + ) + ) # Changes within models kept_models = set(old_model_keys).intersection(new_model_keys) old_fields = set() @@ -338,16 +348,6 @@ class MigrationAutodetector(object): ) for app_label, operation in unique_together_operations: self.add_to_migration(app_label, operation) - # Removing models - removed_models = set(old_model_keys) - set(new_model_keys) - for app_label, model_name in removed_models: - model_state = self.from_state.models[app_label, model_name] - self.add_to_migration( - app_label, - operations.DeleteModel( - model_state.name, - ) - ) # Alright, now add internal dependencies for app_label, migrations in self.migrations.items(): for m1, m2 in zip(migrations, migrations[1:]): diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py index 4ebaa431ba..0114dba60c 100644 --- a/django/db/migrations/migration.py +++ b/django/db/migrations/migration.py @@ -127,7 +127,6 @@ class Migration(object): to_run.reverse() for operation, to_state, from_state in to_run: operation.database_backwards(self.app_label, schema_editor, from_state, to_state) - return project_state def swappable_dependency(value): diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index 86693f4398..d146bfd8bc 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -51,16 +51,6 @@ class ProjectState(object): if len(new_unrendered_models) == len(unrendered_models): raise InvalidBasesError("Cannot resolve bases for %r" % new_unrendered_models) unrendered_models = new_unrendered_models - # make sure apps has no dangling references - if self.apps._pending_lookups: - # Raise an error with a best-effort helpful message - # (only for the first issue). Error message should look like: - # "ValueError: Lookup failed for model referenced by - # field migrations.Book.author: migrations.Author" - dangling_lookup = list(self.apps._pending_lookups.items())[0] - raise ValueError("Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}".format( - field=dangling_lookup[1][0][1], - model=dangling_lookup[0])) return self.apps @classmethod diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 2b8e940c4e..a56e31568a 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -33,15 +33,11 @@ class AutodetectorTests(TestCase): other_stable = ModelState("otherapp", "Stable", [("id", models.AutoField(primary_key=True))]) third_thing = ModelState("thirdapp", "Thing", [("id", models.AutoField(primary_key=True))]) book = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))]) - book_with_no_author = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("title", models.CharField(max_length=200))]) book_with_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))]) book_with_field_and_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("writer", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))]) - book_with_multiple_authors = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("authors", models.ManyToManyField("testapp.Author")), ("title", models.CharField(max_length=200))]) - book_with_multiple_authors_through_attribution = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("authors", models.ManyToManyField("testapp.Author", through="otherapp.Attribution")), ("title", models.CharField(max_length=200))]) book_unique = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("author", "title")]}) book_unique_2 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "author")]}) book_unique_3 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("newfield", models.IntegerField()), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "newfield")]}) - attribution = ModelState("otherapp", "Attribution", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("book", models.ForeignKey("otherapp.Book"))]) edition = ModelState("thirdapp", "Edition", [("id", models.AutoField(primary_key=True)), ("book", models.ForeignKey("otherapp.Book"))]) custom_user = ModelState("thirdapp", "CustomUser", [("id", models.AutoField(primary_key=True)), ("username", models.CharField(max_length=255))]) knight = ModelState("eggs", "Knight", [("id", models.AutoField(primary_key=True))]) @@ -556,65 +552,18 @@ class AutodetectorTests(TestCase): """ # Make state before = self.make_project_state([self.author_with_publisher_string]) - after = self.make_project_state([self.author_with_publisher, self.publisher]) + after = self.make_project_state([self.author_with_publisher]) autodetector = MigrationAutodetector(before, after) changes = autodetector._detect_changes() # Right number of migrations? self.assertEqual(len(changes['testapp']), 1) # Right number of actions? migration = changes['testapp'][0] - self.assertEqual(len(migration.operations), 3) + self.assertEqual(len(migration.operations), 2) # Right actions? action = migration.operations[0] - self.assertEqual(action.__class__.__name__, "CreateModel") - self.assertEqual(action.name, "Publisher") - action = migration.operations[1] self.assertEqual(action.__class__.__name__, "AddField") self.assertEqual(action.name, "publisher") - action = migration.operations[2] + action = migration.operations[1] self.assertEqual(action.__class__.__name__, "RemoveField") self.assertEqual(action.name, "publisher_name") - - def test_foreign_key_removed_before_target_model(self): - """ - Removing an FK and the model it targets in the same change must remove - the FK field before the model to maintain consistency. - """ - before = self.make_project_state([self.author_with_publisher, self.publisher]) - after = self.make_project_state([self.author_name]) # removes both the model and FK - autodetector = MigrationAutodetector(before, after) - changes = autodetector._detect_changes() - # Right number of migrations? - self.assertEqual(len(changes['testapp']), 1) - # Right number of actions? - migration = changes['testapp'][0] - self.assertEqual(len(migration.operations), 2) - # Right actions in right order? - action = migration.operations[0] - self.assertEqual(action.__class__.__name__, "RemoveField") - self.assertEqual(action.name, "publisher") - action = migration.operations[1] - self.assertEqual(action.__class__.__name__, "DeleteModel") - self.assertEqual(action.name, "Publisher") - - def test_many_to_many_removed_before_through_model(self): - """ - Removing a ManyToManyField and the "through" model in the same change must remove - the field before the model to maintain consistency. - """ - before = self.make_project_state([self.book_with_multiple_authors_through_attribution, self.author_name, self.attribution]) - after = self.make_project_state([self.book_with_no_author, self.author_name]) # removes both the through model and ManyToMany - autodetector = MigrationAutodetector(before, after) - changes = autodetector._detect_changes() - # Right number of migrations? - self.assertEqual(len(changes['otherapp']), 1) - # Right number of actions? - migration = changes['otherapp'][0] - self.assertEqual(len(migration.operations), 2) - # Right actions in right order? - action = migration.operations[0] - self.assertEqual(action.__class__.__name__, "RemoveField") - self.assertEqual(action.name, "authors") - action = migration.operations[1] - self.assertEqual(action.__class__.__name__, "DeleteModel") - self.assertEqual(action.name, "Attribution") diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index ec0d046df8..6472fea42c 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1,12 +1,9 @@ import unittest - -from django.db import connection, migrations, models, router -from django.db.migrations.migration import Migration -from django.db.migrations.state import ProjectState +from django.db import connection, models, migrations, router from django.db.models.fields import NOT_PROVIDED from django.db.transaction import atomic from django.db.utils import IntegrityError - +from django.db.migrations.state import ProjectState from .test_base import MigrationTestBase @@ -18,16 +15,15 @@ class OperationTests(MigrationTestBase): """ def apply_operations(self, app_label, project_state, operations): - migration = Migration('name', app_label) - migration.operations = operations - with connection.schema_editor() as editor: - return migration.apply(project_state, editor) + new_state = project_state.clone() + for operation in operations: + operation.state_forwards(app_label, new_state) - def unapply_operations(self, app_label, project_state, operations): - migration = Migration('name', app_label) - migration.operations = operations + # Set up the database with connection.schema_editor() as editor: - return migration.unapply(project_state, editor) + for operation in operations: + operation.database_forwards(app_label, editor, project_state, new_state) + return new_state def set_up_test_model(self, app_label, second_model=False, related_model=False, mti_model=False): """ @@ -400,38 +396,6 @@ class OperationTests(MigrationTestBase): Pony = new_apps.get_model("test_alflmm", "Pony") self.assertTrue(Pony._meta.get_field('stables').blank) - def test_remove_field_m2m(self): - project_state = self.set_up_test_model("test_rmflmm", second_model=True) - - project_state = self.apply_operations("test_rmflmm", project_state, operations=[ - migrations.AddField("Pony", "stables", models.ManyToManyField("Stable", related_name="ponies")) - ]) - self.assertTableExists("test_rmflmm_pony_stables") - - operations = [migrations.RemoveField("Pony", "stables")] - self.apply_operations("test_rmflmm", project_state, operations=operations) - self.assertTableNotExists("test_rmflmm_pony_stables") - - # And test reversal - self.unapply_operations("test_rmflmm", project_state, operations=operations) - self.assertTableExists("test_rmflmm_pony_stables") - - def test_remove_field_m2m_with_through(self): - project_state = self.set_up_test_model("test_rmflmmwt", second_model=True) - - self.assertTableNotExists("test_rmflmmwt_ponystables") - project_state = self.apply_operations("test_rmflmmwt", project_state, operations=[ - migrations.CreateModel("PonyStables", fields=[ - ("pony", models.ForeignKey('test_rmflmmwt.Pony')), - ("stable", models.ForeignKey('test_rmflmmwt.Stable')), - ]), - migrations.AddField("Pony", "stables", models.ManyToManyField("Stable", related_name="ponies", through='test_rmflmmwt.PonyStables')) - ]) - self.assertTableExists("test_rmflmmwt_ponystables") - - operations = [migrations.RemoveField("Pony", "stables")] - self.apply_operations("test_rmflmmwt", project_state, operations=operations) - def test_remove_field(self): """ Tests the RemoveField operation. diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 045ac6be10..10f5e7d9ab 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -291,33 +291,3 @@ class StateTests(TestCase): )) self.assertNotEqual(project_state, other_state) self.assertEqual(project_state == other_state, False) - - def test_dangling_references_throw_error(self): - class Author(models.Model): - name = models.TextField() - - class Book(models.Model): - author = models.ForeignKey(Author) - - class Magazine(models.Model): - authors = models.ManyToManyField(Author) - - # Make a valid ProjectState and render it - project_state = ProjectState() - project_state.add_model_state(ModelState.from_model(Author)) - project_state.add_model_state(ModelState.from_model(Book)) - project_state.add_model_state(ModelState.from_model(Magazine)) - rendered_state = project_state.render() - self.assertEqual(len(rendered_state.get_models()), 3) - - # now make an invalid one with a ForeignKey - project_state = ProjectState() - project_state.add_model_state(ModelState.from_model(Book)) - with self.assertRaises(ValueError): - rendered_state = project_state.render() - - # and another with ManyToManyField - project_state = ProjectState() - project_state.add_model_state(ModelState.from_model(Magazine)) - with self.assertRaises(ValueError): - rendered_state = project_state.render()