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.

5 files changed, 79 insertions, 41 deletions

diff --git a/docs/howto/writing-migrations.txt b/docs/howto/writing-migrations.txt
index 59a636626a..37a8b4ed5a 100644
--- a/docs/howto/writing-migrations.txt
+++ b/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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 0bb3b39c20..a597baf439 100644
--- a/docs/internals/deprecation.txt
+++ b/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
diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index 0200e18d48..d82995bafc 100644
--- a/docs/releases/1.8.txt
+++ b/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
diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt
index ced33bbe09..94a97d5b0e 100644
--- a/docs/topics/cache.txt
+++ b/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
 
diff --git a/docs/topics/db/multi-db.txt b/docs/topics/db/multi-db.txt
index 76df8610e3..e84906bb49 100644
--- a/docs/topics/db/multi-db.txt
+++ b/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.
             """