summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2014-04-18 01:27:30 -0400
committerSimon Charette <charette.s@gmail.com>2014-04-18 01:27:30 -0400
commit0d397e5a5b98c475b664ff12f11729671fe1ad42 (patch)
tree89ab0f29bfd487524896ab76b9ed3759307ed18d
parent214d1e1b0f1574ecab856f0096e409109efa9523 (diff)
Revert "Fixed #22397 -- Issues removing M2M field with explicit through model."
This reverts commit 00e3b9a2a992ee0b7288eeeb03e7cbd52ebc6dce. It's causing a regression when tested with the proxy_model_inheritance tests.
-rw-r--r--django/db/backends/sqlite3/schema.py12
-rw-r--r--django/db/migrations/autodetector.py20
-rw-r--r--django/db/migrations/migration.py1
-rw-r--r--django/db/migrations/state.py10
-rw-r--r--tests/migrations/test_autodetector.py57
-rw-r--r--tests/migrations/test_operations.py54
-rw-r--r--tests/migrations/test_state.py30
7 files changed, 26 insertions, 158 deletions
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]
- 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")
+ self.assertEqual(action.name, "publisher_name")
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()