Browse Source

Fixed #24351, #24346 -- Changed the signature of allow_migrate().

The new signature enables better support for routing RunPython and
RunSQL operations, especially w.r.t. reusable and third-party apps.

This commit also takes advantage of the deprecation cycle for the old
signature to remove the backward incompatibility introduced in #22583;
RunPython and RunSQL won't call allow_migrate() when when the router
has the old signature.

Thanks Aymeric Augustin and Tim Graham for helping shape up the patch.

Refs 22583.
Loic Bistuer 10 years ago
parent
commit
bed504d70b

+ 1 - 1
django/contrib/auth/management/__init__.py

@@ -66,7 +66,7 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_
     except LookupError:
         return
 
-    if not router.allow_migrate(using, Permission):
+    if not router.allow_migrate_model(using, Permission):
         return
 
     from django.contrib.contenttypes.models import ContentType

+ 1 - 1
django/contrib/contenttypes/management.py

@@ -17,7 +17,7 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT
     except LookupError:
         return
 
-    if not router.allow_migrate(using, ContentType):
+    if not router.allow_migrate_model(using, ContentType):
         return
 
     ContentType.objects.clear_cache()

+ 1 - 0
django/contrib/contenttypes/migrations/0002_remove_content_type_name.py

@@ -33,6 +33,7 @@ class Migration(migrations.Migration):
         migrations.RunPython(
             migrations.RunPython.noop,
             add_legacy_name,
+            hints={'model_name': 'contenttype'},
         ),
         migrations.RemoveField(
             model_name='contenttype',

+ 1 - 1
django/contrib/sites/management.py

@@ -14,7 +14,7 @@ def create_default_site(app_config, verbosity=2, interactive=True, using=DEFAULT
     except LookupError:
         return
 
-    if not router.allow_migrate(using, Site):
+    if not router.allow_migrate_model(using, Site):
         return
 
     if not Site.objects.using(using).exists():

+ 1 - 0
django/core/cache/backends/db.py

@@ -30,6 +30,7 @@ class Options(object):
         self.abstract = False
         self.managed = True
         self.proxy = False
+        self.swapped = False
 
 
 class BaseDatabaseCache(BaseCache):

+ 1 - 1
django/core/management/commands/createcachetable.py

@@ -38,7 +38,7 @@ class Command(BaseCommand):
 
     def create_table(self, database, tablename):
         cache = BaseDatabaseCache(tablename, {})
-        if not router.allow_migrate(database, cache.cache_model_class):
+        if not router.allow_migrate_model(database, cache.cache_model_class):
             return
         connection = connections[database]
 

+ 1 - 1
django/core/management/commands/dumpdata.py

@@ -132,7 +132,7 @@ class Command(BaseCommand):
             for model in serializers.sort_dependencies(app_list.items()):
                 if model in excluded_models:
                     continue
-                if not model._meta.proxy and router.allow_migrate(using, model):
+                if not model._meta.proxy and router.allow_migrate_model(using, model):
                     if use_base_manager:
                         objects = model._base_manager
                     else:

+ 1 - 1
django/core/management/commands/loaddata.py

@@ -139,7 +139,7 @@ class Command(BaseCommand):
 
                 for obj in objects:
                     objects_in_fixture += 1
-                    if router.allow_migrate(self.using, obj.object.__class__):
+                    if router.allow_migrate_model(self.using, obj.object.__class__):
                         loaded_objects_in_fixture += 1
                         self.models.add(obj.object.__class__)
                         try:

+ 1 - 1
django/db/backends/base/creation.py

@@ -107,7 +107,7 @@ class BaseDatabaseCreation(object):
         def get_objects():
             for model in serializers.sort_dependencies(app_list):
                 if (not model._meta.proxy and model._meta.managed and
-                        router.allow_migrate(self.connection.alias, model)):
+                        router.allow_migrate_model(self.connection.alias, model)):
                     queryset = model._default_manager.using(self.connection.alias).order_by(model._meta.pk.name)
                     for obj in queryset.iterator():
                         yield obj

+ 6 - 4
django/db/migrations/operations/base.py

@@ -99,15 +99,17 @@ class Operation(object):
         """
         return self.references_model(model_name, app_label)
 
-    def allowed_to_migrate(self, connection_alias, model, hints=None):
+    def allow_migrate_model(self, connection_alias, model):
         """
         Returns if we're allowed to migrate the model.
+
+        This is a thin wrapper around router.allow_migrate_model() that
+        preemptively rejects any proxy, swapped out, or unmanaged model.
         """
-        # Always skip if proxy, swapped out, or unmanaged.
-        if model and (model._meta.proxy or model._meta.swapped or not model._meta.managed):
+        if model._meta.proxy or model._meta.swapped or not model._meta.managed:
             return False
 
-        return router.allow_migrate(connection_alias, model, **(hints or {}))
+        return router.allow_migrate_model(connection_alias, model)
 
     def __repr__(self):
         return "<%s %s%s>" % (

+ 7 - 7
django/db/migrations/operations/fields.py

@@ -52,7 +52,7 @@ class AddField(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.model_name)
             field = to_model._meta.get_field(self.name)
             if not self.preserve_default:
@@ -66,7 +66,7 @@ class AddField(Operation):
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         from_model = from_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, from_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, from_model):
             schema_editor.remove_field(from_model, from_model._meta.get_field(self.name))
 
     def describe(self):
@@ -117,12 +117,12 @@ class RemoveField(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         from_model = from_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, from_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, from_model):
             schema_editor.remove_field(from_model, from_model._meta.get_field(self.name))
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.model_name)
             schema_editor.add_field(from_model, to_model._meta.get_field(self.name))
 
@@ -184,7 +184,7 @@ class AlterField(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.model_name)
             from_field = from_model._meta.get_field(self.name)
             to_field = to_model._meta.get_field(self.name)
@@ -267,7 +267,7 @@ class RenameField(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.model_name)
             schema_editor.alter_field(
                 from_model,
@@ -277,7 +277,7 @@ class RenameField(Operation):
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.model_name)
             schema_editor.alter_field(
                 from_model,

+ 9 - 9
django/db/migrations/operations/models.py

@@ -55,12 +55,12 @@ class CreateModel(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, model):
+        if self.allow_migrate_model(schema_editor.connection.alias, model):
             schema_editor.create_model(model)
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         model = from_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, model):
+        if self.allow_migrate_model(schema_editor.connection.alias, model):
             schema_editor.delete_model(model)
 
     def describe(self):
@@ -111,12 +111,12 @@ class DeleteModel(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         model = from_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, model):
+        if self.allow_migrate_model(schema_editor.connection.alias, model):
             schema_editor.delete_model(model)
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, model):
+        if self.allow_migrate_model(schema_editor.connection.alias, model):
             schema_editor.create_model(model)
 
     def references_model(self, name, app_label=None):
@@ -189,7 +189,7 @@ class RenameModel(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.new_name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, new_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, new_model):
             old_model = from_state.apps.get_model(app_label, self.old_name)
             # Move the main table
             schema_editor.alter_db_table(
@@ -287,7 +287,7 @@ class AlterModelTable(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, new_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, new_model):
             old_model = from_state.apps.get_model(app_label, self.name)
             schema_editor.alter_db_table(
                 new_model,
@@ -347,7 +347,7 @@ class AlterUniqueTogether(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, new_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, new_model):
             old_model = from_state.apps.get_model(app_label, self.name)
             schema_editor.alter_unique_together(
                 new_model,
@@ -399,7 +399,7 @@ class AlterIndexTogether(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, new_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, new_model):
             old_model = from_state.apps.get_model(app_label, self.name)
             schema_editor.alter_index_together(
                 new_model,
@@ -448,7 +448,7 @@ class AlterOrderWithRespectTo(Operation):
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.name)
-        if self.allowed_to_migrate(schema_editor.connection.alias, to_model):
+        if self.allow_migrate_model(schema_editor.connection.alias, to_model):
             from_model = from_state.apps.get_model(app_label, self.name)
             # Remove a field if we need to
             if from_model._meta.order_with_respect_to and not to_model._meta.order_with_respect_to:

+ 6 - 4
django/db/migrations/operations/special.py

@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+from django.db import router
+
 from .base import Operation
 
 
@@ -94,13 +96,13 @@ class RunSQL(Operation):
             state_operation.state_forwards(app_label, state)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
-        if self.allowed_to_migrate(schema_editor.connection.alias, None, hints=self.hints):
+        if router.allow_migrate(schema_editor.connection.alias, app_label, **self.hints):
             self._run_sql(schema_editor, self.sql)
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         if self.reverse_sql is None:
             raise NotImplementedError("You cannot reverse this operation")
-        if self.allowed_to_migrate(schema_editor.connection.alias, None, hints=self.hints):
+        if router.allow_migrate(schema_editor.connection.alias, app_label, **self.hints):
             self._run_sql(schema_editor, self.reverse_sql)
 
     def describe(self):
@@ -171,7 +173,7 @@ class RunPython(Operation):
         pass
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
-        if self.allowed_to_migrate(schema_editor.connection.alias, None, hints=self.hints):
+        if router.allow_migrate(schema_editor.connection.alias, app_label, **self.hints):
             # We now execute the Python code in a context that contains a 'models'
             # object, representing the versioned models as an app registry.
             # We could try to override the global cache, but then people will still
@@ -181,7 +183,7 @@ class RunPython(Operation):
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         if self.reverse_code is None:
             raise NotImplementedError("You cannot reverse this operation")
-        if self.allowed_to_migrate(schema_editor.connection.alias, None, hints=self.hints):
+        if router.allow_migrate(schema_editor.connection.alias, app_label, **self.hints):
             self.reverse_code(from_state.apps, schema_editor)
 
     def describe(self):

+ 1 - 1
django/db/models/base.py

@@ -1549,7 +1549,7 @@ class Model(six.with_metaclass(ModelBase)):
         # Find the minimum max allowed length among all specified db_aliases.
         for db in settings.DATABASES.keys():
             # skip databases where the model won't be created
-            if not router.allow_migrate(db, cls):
+            if not router.allow_migrate_model(db, cls):
                 continue
             connection = connections[db]
             max_name_length = connection.ops.max_name_length()

+ 29 - 6
django/db/utils.py

@@ -1,5 +1,7 @@
+import inspect
 import os
 import pkgutil
+import warnings
 from importlib import import_module
 from threading import local
 
@@ -7,6 +9,7 @@ from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.utils import six
 from django.utils._os import upath
+from django.utils.deprecation import RemovedInDjango20Warning
 from django.utils.functional import cached_property
 from django.utils.module_loading import import_string
 
@@ -276,22 +279,42 @@ class ConnectionRouter(object):
                     return allow
         return obj1._state.db == obj2._state.db
 
-    def allow_migrate(self, db, model, **hints):
+    def allow_migrate(self, db, app_label, **hints):
         for router in self.routers:
             try:
                 method = router.allow_migrate
             except AttributeError:
                 # If the router doesn't have a method, skip to the next one.
-                pass
+                continue
+
+            argspec = inspect.getargspec(router.allow_migrate)
+            if len(argspec.args) == 3 and not argspec.keywords:
+                warnings.warn(
+                    "The signature of allow_migrate has changed from "
+                    "allow_migrate(self, db, model) to "
+                    "allow_migrate(self, db, app_label, model_name=None, **hints). "
+                    "Support for the old signature will be removed in Django 2.0.",
+                    RemovedInDjango20Warning)
+                model = hints.get('model')
+                allow = None if model is None else method(db, model)
             else:
-                allow = method(db, model, **hints)
-                if allow is not None:
-                    return allow
+                allow = method(db, app_label, **hints)
+
+            if allow is not None:
+                return allow
         return True
 
+    def allow_migrate_model(self, db, model):
+        return self.allow_migrate(
+            db,
+            model._meta.app_label,
+            model_name=model._meta.model_name,
+            model=model,
+        )
+
     def get_migratable_models(self, app_config, db, include_auto_created=False):
         """
         Return app models allowed to be synchronized on provided db.
         """
         models = app_config.get_models(include_auto_created=include_auto_created)
-        return [model for model in models if self.allow_migrate(db, model)]
+        return [model for model in models if self.allow_migrate_model(db, model)]

+ 5 - 1
docs/howto/writing-migrations.txt

@@ -46,7 +46,7 @@ method of database routers as ``**hints``:
 
     class MyRouter(object):
 
-        def allow_migrate(self, db, model, **hints):
+        def allow_migrate(self, db, app_label, model_name=None, **hints):
             if 'target_db' in hints:
                 return db == hints['target_db']
             return True
@@ -68,6 +68,10 @@ Then, to leverage this in your migrations, do the following::
             migrations.RunPython(forwards, hints={'target_db': 'default'}),
         ]
 
+If your ``RunPython`` or ``RunSQL`` operation only affects one model, it's good
+practice to pass ``model_name`` as a hint to make it as transparent as possible
+to the router. This is especially important for reusable and third-party apps.
+
 Migrations that add unique fields
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

+ 4 - 0
docs/internals/deprecation.txt

@@ -183,6 +183,10 @@ details on these changes.
 * Ability to specify ``ContentType.name`` when creating a content type instance
   will be removed.
 
+* Support for the old signature of ``allow_migrate`` will be removed. It changed
+  from ``allow_migrate(self, db, model)`` to
+  ``allow_migrate(self, db, app_label, model_name=None, **hints)``.
+
 .. _deprecation-removed-in-1.9:
 
 1.9

+ 24 - 13
docs/releases/1.8.txt

@@ -480,11 +480,13 @@ Migrations
   method/attribute were added to ease in making ``RunPython`` and ``RunSQL``
   operations reversible.
 
-* The :class:`~django.db.migrations.operations.RunPython` and
-  :class:`~django.db.migrations.operations.RunSQL` operations now accept a
-  ``hints`` parameter that will be passed to :meth:`allow_migrate`. To take
-  advantage of this feature you must ensure that the ``allow_migrate()`` method
-  of all your routers accept ``**hints``.
+* The migration operations :class:`~django.db.migrations.operations.RunPython`
+  and :class:`~django.db.migrations.operations.RunSQL` now call the
+  :meth:`allow_migrate` method of database routers. The router can use the
+  newly introduced ``app_label`` and ``hints`` arguments to make a routing
+  decision. To take advantage of this feature you need to update the router to
+  the new ``allow_migrate`` signature, see the :ref:`deprecation section
+  <deprecated-signature-of-allow-migrate>` for more details.
 
 Models
 ^^^^^^
@@ -1146,14 +1148,6 @@ Miscellaneous
 * :func:`django.utils.translation.get_language()` now returns ``None`` instead
   of :setting:`LANGUAGE_CODE` when translations are temporarily deactivated.
 
-* The migration operations :class:`~django.db.migrations.operations.RunPython`
-  and :class:`~django.db.migrations.operations.RunSQL` now call the
-  :meth:`allow_migrate` method of database routers. In these cases the
-  ``model`` argument of ``allow_migrate()`` is set to ``None``, so the router
-  must properly handle this value. This is most useful when used together with
-  the newly introduced ``hints`` parameter for these operations, but it can
-  also be used to disable migrations from running on a particular database.
-
 * The ``name`` field of :class:`django.contrib.contenttypes.models.ContentType`
   has been removed by a migration and replaced by a property. That means it's
   not possible to query or filter a ``ContentType`` by this field any longer.
@@ -1651,6 +1645,23 @@ aggregate methods are deprecated and should be replaced by their function-based
 aggregate equivalents (``Collect``, ``Extent``, ``Extent3D``, ``MakeLine``, and
 ``Union``).
 
+.. _deprecated-signature-of-allow-migrate:
+
+Signature of the ``allow_migrate`` router method
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The signature of the :meth:`allow_migrate` method of database routers has
+changed from ``allow_migrate(db, model)`` to
+``allow_migrate(db, app_label, model_name=None, **hints)``.
+
+When ``model_name`` is set, the value that was previously given through the
+``model`` positional argument may now be found inside the ``hints`` dictionary
+under the key ``'model'``.
+
+After switching to the new signature the router will also be called by the
+:class:`~django.db.migrations.operations.RunPython` and
+:class:`~django.db.migrations.operations.RunSQL` operations.
+
 .. removed-features-1.8:
 
 Features removed in 1.8

+ 4 - 4
docs/topics/cache.txt

@@ -226,19 +226,19 @@ operations to ``cache_replica``, and all write operations to
 
         def db_for_read(self, model, **hints):
             "All cache read operations go to the replica"
-            if model._meta.app_label in ('django_cache',):
+            if model._meta.app_label == 'django_cache':
                 return 'cache_replica'
             return None
 
         def db_for_write(self, model, **hints):
             "All cache write operations go to primary"
-            if model._meta.app_label in ('django_cache',):
+            if model._meta.app_label == 'django_cache':
                 return 'cache_primary'
             return None
 
-        def allow_migrate(self, db, model):
+        def allow_migrate(self, db, app_label, model_name=None, **hints):
             "Only install the cache model on primary"
-            if model._meta.app_label in ('django_cache',):
+            if app_label == 'django_cache':
                 return db == 'cache_primary'
             return None
 

+ 42 - 23
docs/topics/db/multi-db.txt

@@ -128,7 +128,7 @@ A database Router is a class that provides up to four methods:
     provided in the ``hints`` dictionary. Details on valid hints are
     provided :ref:`below <topics-db-multi-db-hints>`.
 
-    Returns None if there is no suggestion.
+    Returns ``None`` if there is no suggestion.
 
 .. method:: db_for_write(model, **hints)
 
@@ -140,32 +140,53 @@ A database Router is a class that provides up to four methods:
     provided in the ``hints`` dictionary. Details on valid hints are
     provided :ref:`below <topics-db-multi-db-hints>`.
 
-    Returns None if there is no suggestion.
+    Returns ``None`` if there is no suggestion.
 
 .. method:: allow_relation(obj1, obj2, **hints)
 
-    Return True if a relation between obj1 and obj2 should be
-    allowed, False if the relation should be prevented, or None if
+    Return ``True`` if a relation between ``obj1`` and ``obj2`` should be
+    allowed, ``False`` if the relation should be prevented, or ``None`` if
     the router has no opinion. This is purely a validation operation,
     used by foreign key and many to many operations to determine if a
     relation should be allowed between two objects.
 
-.. method:: allow_migrate(db, model, **hints)
+.. method:: allow_migrate(db, app_label, model_name=None, **hints)
 
-    Determine if the ``model`` should have tables/indexes created in the
-    database with alias ``db``. Return True if the model should be
-    migrated, False if it should not be migrated, or None if
-    the router has no opinion. This method can be used to determine
-    the availability of a model on a given database.
+    Determine if the migration operation is allowed to run on the database with
+    alias ``db``. Return ``True`` if the operation should run, ``False`` if it
+    shouldn't run, or ``None`` if the router has no opinion.
 
-    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 ``app_label`` positional argument is the label of the application
+    being migrated.
 
-    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``.
+    ``model_name`` is set by most migration operations to the value of
+    ``model._meta.model_name`` (the lowercased version of the model
+    ``__name__``) of the model being migrated. Its value is ``None`` for the
+    :class:`~django.db.migrations.operations.RunPython` and
+    :class:`~django.db.migrations.operations.RunSQL` operations unless they
+    provide it using hints.
+
+    ``hints`` are used by certain operations to communicate additional
+    information to the router.
+
+    When ``model_name`` is set, ``hints`` normally contains the model class
+    under the key ``'model'``. Note that it may be a :ref:`historical model
+    <historical-models>`, and thus not have any custom attributes, methods, or
+    managers. You should only rely on ``_meta``.
+
+    This method can also be used to determine the availability of a model on a
+    given database.
+
+    Note that migrations will just silently not perform any operations on a
+    model for which this returns ``False``. This may result in broken foreign
+    keys, extra tables, or missing tables if you change it once you have
+    applied some migrations.
+
+    .. versionchanged:: 1.8
+
+        The signature of ``allow_migrate`` has changed significantly from previous
+        versions. See the :ref:`deprecation notes
+        <deprecated-signature-of-allow-migrate>` for more details.
 
 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
@@ -293,15 +314,13 @@ send queries for the ``auth`` app to ``auth_db``::
                return True
             return None
 
-        def allow_migrate(self, db, model, **hints):
+        def allow_migrate(self, db, app_label, model, **hints):
             """
             Make sure the auth app only appears in the 'auth_db'
             database.
             """
-            if db == 'auth_db':
-                return model._meta.app_label == 'auth'
-            elif model._meta.app_label == 'auth':
-                return False
+            if app_label == 'auth':
+                return db == 'auth_db'
             return None
 
 And we also want a router that sends all other apps to the
@@ -333,7 +352,7 @@ from::
                 return True
             return None
 
-        def allow_migrate(self, db, model, **hints):
+        def allow_migrate(self, db, app_label, model, **hints):
             """
             All non-auth models end up in this pool.
             """

+ 5 - 2
tests/cache/tests.py

@@ -956,14 +956,17 @@ class DBCacheRouter(object):
     def db_for_read(self, model, **hints):
         if model._meta.app_label == 'django_cache':
             return 'other'
+        return None
 
     def db_for_write(self, model, **hints):
         if model._meta.app_label == 'django_cache':
             return 'other'
+        return None
 
-    def allow_migrate(self, db, model):
-        if model._meta.app_label == 'django_cache':
+    def allow_migrate(self, db, app_label, **hints):
+        if app_label == 'django_cache':
             return db == 'other'
+        return None
 
 
 @override_settings(

+ 1 - 1
tests/gis_tests/layermap/tests.py

@@ -322,7 +322,7 @@ class OtherRouter(object):
     def allow_relation(self, obj1, obj2, **hints):
         return None
 
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, **hints):
         return True
 
 

+ 4 - 4
tests/migrations/test_multidb.py

@@ -16,7 +16,7 @@ class AgnosticRouter(object):
     """
     A router that doesn't have an opinion regarding migrating.
     """
-    def allow_migrate(self, db, model, **hints):
+    def allow_migrate(self, db, app_label, **hints):
         return None
 
 
@@ -24,7 +24,7 @@ class MigrateNothingRouter(object):
     """
     A router that doesn't allow migrating.
     """
-    def allow_migrate(self, db, model, **hints):
+    def allow_migrate(self, db, app_label, **hints):
         return False
 
 
@@ -32,7 +32,7 @@ class MigrateEverythingRouter(object):
     """
     A router that always allows migrating.
     """
-    def allow_migrate(self, db, model, **hints):
+    def allow_migrate(self, db, app_label, **hints):
         return True
 
 
@@ -40,7 +40,7 @@ class MigrateWhenFooRouter(object):
     """
     A router that allows migrating depending on a hint.
     """
-    def allow_migrate(self, db, model, **hints):
+    def allow_migrate(self, db, app_label, **hints):
         return hints.get('foo', False)
 
 

+ 12 - 10
tests/multiple_database/routers.py

@@ -4,8 +4,11 @@ from django.db import DEFAULT_DB_ALIAS
 
 
 class TestRouter(object):
-    # A test router. The behavior is vaguely primary/replica, but the
-    # databases aren't assumed to propagate changes.
+    """
+    Vaguely behave like primary/replica, but the databases aren't assumed to
+    propagate changes.
+    """
+
     def db_for_read(self, model, instance=None, **hints):
         if instance:
             return instance._state.db or 'other'
@@ -17,13 +20,14 @@ class TestRouter(object):
     def allow_relation(self, obj1, obj2, **hints):
         return obj1._state.db in ('default', 'other') and obj2._state.db in ('default', 'other')
 
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, **hints):
         return True
 
 
 class AuthRouter(object):
-    """A router to control all database operations on models in
-    the contrib.auth application"""
+    """
+    Control all database operations on models in the contrib.auth application.
+    """
 
     def db_for_read(self, model, **hints):
         "Point all read operations on auth models to 'default'"
@@ -45,12 +49,10 @@ class AuthRouter(object):
             return True
         return None
 
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, **hints):
         "Make sure the auth app only appears on the 'other' db"
-        if db == 'other':
-            return model._meta.app_label == 'auth'
-        elif model._meta.app_label == 'auth':
-            return False
+        if app_label == 'auth':
+            return db == 'other'
         return None
 
 

+ 53 - 21
tests/multiple_database/tests.py

@@ -2,6 +2,7 @@ from __future__ import unicode_literals
 
 import datetime
 import pickle
+import warnings
 from operator import attrgetter
 
 from django.contrib.auth.models import User
@@ -11,6 +12,7 @@ from django.db import DEFAULT_DB_ALIAS, connections, router, transaction
 from django.db.models import signals
 from django.db.utils import ConnectionRouter
 from django.test import TestCase, override_settings
+from django.utils.encoding import force_text
 from django.utils.six import StringIO
 
 from .models import Book, Person, Pet, Review, UserProfile
@@ -901,28 +903,58 @@ class RouterTestCase(TestCase):
     def test_migrate_selection(self):
         "Synchronization behavior is predictable"
 
-        self.assertTrue(router.allow_migrate('default', User))
-        self.assertTrue(router.allow_migrate('default', Book))
+        self.assertTrue(router.allow_migrate_model('default', User))
+        self.assertTrue(router.allow_migrate_model('default', Book))
 
-        self.assertTrue(router.allow_migrate('other', User))
-        self.assertTrue(router.allow_migrate('other', Book))
+        self.assertTrue(router.allow_migrate_model('other', User))
+        self.assertTrue(router.allow_migrate_model('other', Book))
 
         with override_settings(DATABASE_ROUTERS=[TestRouter(), AuthRouter()]):
             # Add the auth router to the chain. TestRouter is a universal
             # synchronizer, so it should have no effect.
-            self.assertTrue(router.allow_migrate('default', User))
-            self.assertTrue(router.allow_migrate('default', Book))
+            self.assertTrue(router.allow_migrate_model('default', User))
+            self.assertTrue(router.allow_migrate_model('default', Book))
 
-            self.assertTrue(router.allow_migrate('other', User))
-            self.assertTrue(router.allow_migrate('other', Book))
+            self.assertTrue(router.allow_migrate_model('other', User))
+            self.assertTrue(router.allow_migrate_model('other', Book))
 
         with override_settings(DATABASE_ROUTERS=[AuthRouter(), TestRouter()]):
             # Now check what happens if the router order is reversed.
-            self.assertFalse(router.allow_migrate('default', User))
-            self.assertTrue(router.allow_migrate('default', Book))
-
-            self.assertTrue(router.allow_migrate('other', User))
-            self.assertFalse(router.allow_migrate('other', Book))
+            self.assertFalse(router.allow_migrate_model('default', User))
+            self.assertTrue(router.allow_migrate_model('default', Book))
+
+            self.assertTrue(router.allow_migrate_model('other', User))
+            self.assertTrue(router.allow_migrate_model('other', Book))
+
+    def test_migrate_legacy_router(self):
+        class LegacyRouter(object):
+            def allow_migrate(self, db, model):
+                """
+                Deprecated allow_migrate signature should trigger
+                RemovedInDjango20Warning.
+                """
+                assert db == 'default'
+                assert model is User
+                return True
+
+        with override_settings(DATABASE_ROUTERS=[LegacyRouter()]):
+            with warnings.catch_warnings(record=True) as recorded:
+                warnings.filterwarnings('always')
+
+                msg = (
+                    "The signature of allow_migrate has changed from "
+                    "allow_migrate(self, db, model) to "
+                    "allow_migrate(self, db, app_label, model_name=None, **hints). "
+                    "Support for the old signature will be removed in Django 2.0."
+                )
+
+                self.assertTrue(router.allow_migrate_model('default', User))
+                self.assertEqual(force_text(recorded.pop().message), msg)
+
+                self.assertEqual(recorded, [])
+
+                self.assertTrue(router.allow_migrate('default', 'app_label'))
+                self.assertEqual(force_text(recorded.pop().message), msg)
 
     def test_partial_router(self):
         "A router can choose to implement a subset of methods"
@@ -939,8 +971,8 @@ class RouterTestCase(TestCase):
 
         self.assertTrue(router.allow_relation(dive, dive))
 
-        self.assertTrue(router.allow_migrate('default', User))
-        self.assertTrue(router.allow_migrate('default', Book))
+        self.assertTrue(router.allow_migrate_model('default', User))
+        self.assertTrue(router.allow_migrate_model('default', Book))
 
         with override_settings(DATABASE_ROUTERS=[WriteRouter(), AuthRouter(), TestRouter()]):
             self.assertEqual(router.db_for_read(User), 'default')
@@ -951,8 +983,8 @@ class RouterTestCase(TestCase):
 
             self.assertTrue(router.allow_relation(dive, dive))
 
-            self.assertFalse(router.allow_migrate('default', User))
-            self.assertTrue(router.allow_migrate('default', Book))
+            self.assertFalse(router.allow_migrate_model('default', User))
+            self.assertTrue(router.allow_migrate_model('default', Book))
 
     def test_database_routing(self):
         marty = Person.objects.using('default').create(name="Marty Alchin")
@@ -1492,11 +1524,11 @@ class AntiPetRouter(object):
     # A router that only expresses an opinion on migrate,
     # passing pets to the 'other' database
 
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, model_name, **hints):
         if db == 'other':
-            return model._meta.object_name == 'Pet'
+            return model_name == 'pet'
         else:
-            return model._meta.object_name != 'Pet'
+            return model_name != 'pet'
 
 
 class FixtureTestCase(TestCase):
@@ -1757,7 +1789,7 @@ class RouterModelArgumentTestCase(TestCase):
 
 
 class SyncOnlyDefaultDatabaseRouter(object):
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, **hints):
         return db == DEFAULT_DB_ALIAS
 
 

+ 1 - 1
tests/sites_tests/tests.py

@@ -137,7 +137,7 @@ class SitesFrameworkTests(TestCase):
 
 
 class JustOtherRouter(object):
-    def allow_migrate(self, db, model):
+    def allow_migrate(self, db, app_label, **hints):
         return db == 'other'