Browse Source

Implement allow_migrate for migration operations

Andrew Godwin 11 years ago
parent
commit
fddc5957c5

+ 27 - 19
django/db/migrations/operations/fields.py

@@ -1,3 +1,4 @@
+from django.db import router
 from .base import Operation
 
 
@@ -17,11 +18,13 @@ 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)
-        schema_editor.add_field(from_model, to_model._meta.get_field_by_name(self.name)[0])
+        if router.allow_migrate(schema_editor.connection.alias, to_model):
+            schema_editor.add_field(from_model, to_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)
-        schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0])
+        if router.allow_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):
         return "Add field %s to %s" % (self.name, self.model_name)
@@ -45,12 +48,14 @@ 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)
-        schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0])
+        if router.allow_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)
-        schema_editor.add_field(from_model, to_model._meta.get_field_by_name(self.name)[0])
+        if router.allow_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):
         return "Remove field %s from %s" % (self.name, self.model_name)
@@ -74,11 +79,12 @@ 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)
-        schema_editor.alter_field(
-            from_model,
-            from_model._meta.get_field_by_name(self.name)[0],
-            to_model._meta.get_field_by_name(self.name)[0],
-        )
+        if router.allow_migrate(schema_editor.connection.alias, to_model):
+            schema_editor.alter_field(
+                from_model,
+                from_model._meta.get_field_by_name(self.name)[0],
+                to_model._meta.get_field_by_name(self.name)[0],
+            )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         self.database_forwards(app_label, schema_editor, from_state, to_state)
@@ -105,20 +111,22 @@ 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)
-        schema_editor.alter_field(
-            from_model,
-            from_model._meta.get_field_by_name(self.old_name)[0],
-            to_model._meta.get_field_by_name(self.new_name)[0],
-        )
+        if router.allow_migrate(schema_editor.connection.alias, to_model):
+            schema_editor.alter_field(
+                from_model,
+                from_model._meta.get_field_by_name(self.old_name)[0],
+                to_model._meta.get_field_by_name(self.new_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)
-        schema_editor.alter_field(
-            from_model,
-            from_model._meta.get_field_by_name(self.new_name)[0],
-            to_model._meta.get_field_by_name(self.old_name)[0],
-        )
+        if router.allow_migrate(schema_editor.connection.alias, to_model):
+            schema_editor.alter_field(
+                from_model,
+                from_model._meta.get_field_by_name(self.new_name)[0],
+                to_model._meta.get_field_by_name(self.old_name)[0],
+            )
 
     def describe(self):
         return "Rename field %s on %s to %s" % (self.old_name, self.model_name, self.new_name)

+ 39 - 22
django/db/migrations/operations/models.py

@@ -1,5 +1,5 @@
 from .base import Operation
-from django.db import models
+from django.db import models, router
 from django.db.migrations.state import ModelState
 
 
@@ -17,13 +17,17 @@ class CreateModel(Operation):
     def state_forwards(self, app_label, state):
         state.models[app_label, self.name.lower()] = ModelState(app_label, self.name, self.fields, self.options, self.bases)
 
-    def database_forwards(self, app, schema_editor, from_state, to_state):
+    def database_forwards(self, app_label, schema_editor, from_state, to_state):
         app_cache = to_state.render()
-        schema_editor.create_model(app_cache.get_model(app, self.name))
+        model = app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, model):
+            schema_editor.create_model(model)
 
-    def database_backwards(self, app, schema_editor, from_state, to_state):
+    def database_backwards(self, app_label, schema_editor, from_state, to_state):
         app_cache = from_state.render()
-        schema_editor.delete_model(app_cache.get_model(app, self.name))
+        model = app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, model):
+            schema_editor.delete_model(model)
 
     def describe(self):
         return "Create model %s" % (self.name, )
@@ -42,11 +46,15 @@ class DeleteModel(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         app_cache = from_state.render()
-        schema_editor.delete_model(app_cache.get_model(app_label, self.name))
+        model = app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, model):
+            schema_editor.delete_model(model)
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         app_cache = to_state.render()
-        schema_editor.create_model(app_cache.get_model(app_label, self.name))
+        model = app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, model):
+            schema_editor.create_model(model)
 
     def describe(self):
         return "Delete model %s" % (self.name, )
@@ -67,11 +75,14 @@ class AlterModelTable(Operation):
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         old_app_cache = from_state.render()
         new_app_cache = to_state.render()
-        schema_editor.alter_db_table(
-            new_app_cache.get_model(app_label, self.name),
-            old_app_cache.get_model(app_label, self.name)._meta.db_table,
-            new_app_cache.get_model(app_label, self.name)._meta.db_table,
-        )
+        old_model = old_app_cache.get_model(app_label, self.name)
+        new_model = new_app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, new_model):
+            schema_editor.alter_db_table(
+                new_model,
+                old_model._meta.db_table,
+                new_model._meta.db_table,
+            )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         return self.database_forwards(app_label, schema_editor, from_state, to_state)
@@ -97,11 +108,14 @@ class AlterUniqueTogether(Operation):
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         old_app_cache = from_state.render()
         new_app_cache = to_state.render()
-        schema_editor.alter_unique_together(
-            new_app_cache.get_model(app_label, self.name),
-            getattr(old_app_cache.get_model(app_label, self.name)._meta, "unique_together", set()),
-            getattr(new_app_cache.get_model(app_label, self.name)._meta, "unique_together", set()),
-        )
+        old_model = old_app_cache.get_model(app_label, self.name)
+        new_model = new_app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, new_model):
+            schema_editor.alter_unique_together(
+                new_model,
+                getattr(old_model._meta, "unique_together", set()),
+                getattr(new_model._meta, "unique_together", set()),
+            )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         return self.database_forwards(app_label, schema_editor, from_state, to_state)
@@ -127,11 +141,14 @@ class AlterIndexTogether(Operation):
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         old_app_cache = from_state.render()
         new_app_cache = to_state.render()
-        schema_editor.alter_index_together(
-            new_app_cache.get_model(app_label, self.name),
-            getattr(old_app_cache.get_model(app_label, self.name)._meta, "index_together", set()),
-            getattr(new_app_cache.get_model(app_label, self.name)._meta, "index_together", set()),
-        )
+        old_model = old_app_cache.get_model(app_label, self.name)
+        new_model = new_app_cache.get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, new_model):
+            schema_editor.alter_index_together(
+                new_model,
+                getattr(old_model._meta, "index_together", set()),
+                getattr(new_model._meta, "index_together", set()),
+            )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         return self.database_forwards(app_label, schema_editor, from_state, to_state)

+ 12 - 0
docs/releases/1.7.txt

@@ -110,6 +110,18 @@ Backwards incompatible changes in 1.7
     deprecation timeline for a given feature, its removal may appear as a
     backwards incompatible change.
 
+allow_syncdb/allow_migrate
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While Django will still look at ``allow_syncdb`` methods even though they
+should be renamed to ``allow_migrate``, there is a subtle difference in which
+models get passed to these methods.
+
+For apps with migrations, ``allow_migrate`` will now get passed
+:ref:`historical models <historical-models>`, which are special versioned models
+without custom attributes, methods or managers. Make sure your ``allow_migrate``
+methods are only referring to fields or other items in ``model._meta``.
+
 Miscellaneous
 ~~~~~~~~~~~~~
 

+ 8 - 2
docs/topics/db/multi-db.txt

@@ -163,8 +163,14 @@ A database Router is a class that provides up to four methods:
     the router has no opinion. This method can be used to determine
     the availability of a model on a given database.
 
-    Note that if this returns ``True`` for an app with migrations but
-    ``False`` for an app those migrations depend on, Django will error.
+    Note that migrations will just silently not perform any operations
+    on a model for which this returns ``False``. This may result in broken
+    ForeignKeys, extra tables or missing tables if you change it once you
+    have applied some migrations.
+
+    The value passed for ``model`` may be a
+    :ref:`historical model <historical-models>`, and thus not have any
+    custom attributes, methods or managers. You should only rely on ``_meta``.
 
 A router doesn't have to provide *all* these methods -- it may omit one
 or more of them. If one of the methods is omitted, Django will skip

+ 23 - 0
docs/topics/migrations.txt

@@ -272,3 +272,26 @@ Note that this only works given two things:
 * You have not manually edited your database - Django won't be able to detect
   that your database doesn't match your models, you'll just get errors when
   migrations try and modify those tables.
+
+
+.. historical-models:
+
+Historical models
+-----------------
+
+When you run migrations, Django is working from historical versions of
+your models stored in the migration files. If you write Python code
+using the ``django.db.migrations.RunPython`` operation, or if you have
+``allow_migrate`` methods on your database routers, you will be exposed
+to these versions of your models.
+
+Because it's impossible to serialize arbitrary Python code, these historical
+models will not have any custom methods or managers that you have defined.
+They will, however, have the same fields, relationships and ``Meta`` options
+(also versioned, so they may be different from your current ones).
+
+In addition, the base classes of the model are just stored as pointers,
+so you must always keep base classes around for as long as there is a migration
+that contains a reference to them. On the plus side, methods and managers
+from these base classes inherit normally, so if you absolutely need access
+to these you can opt to move them into a superclass.

+ 47 - 1
tests/migrations/test_operations.py

@@ -1,4 +1,4 @@
-from django.db import connection, models, migrations
+from django.db import connection, models, migrations, router
 from django.db.transaction import atomic
 from django.db.utils import IntegrityError
 from django.db.migrations.state import ProjectState
@@ -271,3 +271,49 @@ class OperationTests(MigrationTestBase):
         with connection.schema_editor() as editor:
             operation.database_backwards("test_alinto", editor, new_state, project_state)
         self.assertIndexNotExists("test_alinto_pony", ["pink", "weight"])
+
+
+class MigrateNothingRouter(object):
+    """
+    A router that sends all writes to the other database.
+    """
+    def allow_migrate(self, db, model):
+        return False
+
+
+class MultiDBOperationTests(MigrationTestBase):
+    multi_db = True
+
+    def setUp(self):
+        # Make the 'other' database appear to be a slave of the 'default'
+        self.old_routers = router.routers
+        router.routers = [MigrateNothingRouter()]
+
+    def tearDown(self):
+        # Restore the 'other' database as an independent database
+        router.routers = self.old_routers
+
+    def test_create_model(self):
+        """
+        Tests that CreateModel honours multi-db settings.
+        """
+        operation = migrations.CreateModel(
+            "Pony",
+            [
+                ("id", models.AutoField(primary_key=True)),
+                ("pink", models.IntegerField(default=1)),
+            ],
+        )
+        # Test the state alteration
+        project_state = ProjectState()
+        new_state = project_state.clone()
+        operation.state_forwards("test_crmo", new_state)
+        # Test the database alteration
+        self.assertTableNotExists("test_crmo_pony")
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_crmo", editor, project_state, new_state)
+        self.assertTableNotExists("test_crmo_pony")
+        # And test reversal
+        with connection.schema_editor() as editor:
+            operation.database_backwards("test_crmo", editor, new_state, project_state)
+        self.assertTableNotExists("test_crmo_pony")