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.

2 files changed, 65 insertions, 31 deletions

diff --git a/tests/multiple_database/routers.py b/tests/multiple_database/routers.py
index 6b85b93a27..e467cf563f 100644
--- a/tests/multiple_database/routers.py
+++ b/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
 
 
diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py
index 5da69b560f..6dc64be040 100644
--- a/tests/multiple_database/tests.py
+++ b/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