Fixed #26421 -- Refactored ModelSignal to use Apps.lazy_model_operation()

5 files changed, 86 insertions, 120 deletions

diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py
index a3b8f1b7f3..0c12f9b96a 100644
--- a/django/core/checks/model_checks.py
+++ b/django/core/checks/model_checks.py
@@ -31,40 +31,6 @@ def check_all_models(app_configs=None, **kwargs):
     return errors
 
 
-@register(Tags.models, Tags.signals)
-def check_model_signals(app_configs=None, **kwargs):
-    """
-    Ensure lazily referenced model signals senders are installed.
-    """
-    # Avoid circular import
-    from django.db import models
-
-    errors = []
-    for name in dir(models.signals):
-        obj = getattr(models.signals, name)
-        if isinstance(obj, models.signals.ModelSignal):
-            for reference, receivers in obj.unresolved_references.items():
-                for receiver, _, _ in receivers:
-                    # The receiver is either a function or an instance of class
-                    # defining a `__call__` method.
-                    if isinstance(receiver, types.FunctionType):
-                        description = "The '%s' function" % receiver.__name__
-                    else:
-                        description = "An instance of the '%s' class" % receiver.__class__.__name__
-                    errors.append(
-                        Error(
-                            "%s was connected to the '%s' signal "
-                            "with a lazy reference to the '%s' sender, "
-                            "which has not been installed." % (
-                                description, name, '.'.join(reference)
-                            ),
-                            obj=receiver.__module__,
-                            id='signals.E001'
-                        )
-                    )
-    return errors
-
-
 def _check_lazy_references(apps, ignore=None):
     """
     Ensure all lazy (i.e. string) model references have been resolved.
@@ -82,6 +48,12 @@ def _check_lazy_references(apps, ignore=None):
     if not pending_models:
         return []
 
+    from django.db.models import signals
+    model_signals = {
+        signal: name for name, signal in vars(signals).items()
+        if isinstance(signal, signals.ModelSignal)
+    }
+
     def extract_operation(obj):
         """
         Take a callable found in Apps._pending_operations and identify the
@@ -127,6 +99,29 @@ def _check_lazy_references(apps, ignore=None):
         }
         return Error(error_msg % params, obj=keywords['field'], id='fields.E307')
 
+    def signal_connect_error(model_key, func, args, keywords):
+        error_msg = (
+            "%(receiver)s was connected to the '%(signal)s' signal with a "
+            "lazy reference to the sender '%(model)s', but %(model_error)s."
+        )
+        receiver = args[0]
+        # The receiver is either a function or an instance of class
+        # defining a `__call__` method.
+        if isinstance(receiver, types.FunctionType):
+            description = "The function '%s'" % receiver.__name__
+        elif isinstance(receiver, types.MethodType):
+            description = "Bound method '%s.%s'" % (receiver.__self__.__class__.__name__, receiver.__name__)
+        else:
+            description = "An instance of class '%s'" % receiver.__class__.__name__
+        signal_name = model_signals.get(func.__self__, 'unknown')
+        params = {
+            'model': '.'.join(model_key),
+            'receiver': description,
+            'signal': signal_name,
+            'model_error': app_model_error(model_key),
+        }
+        return Error(error_msg % params, obj=receiver.__module__, id='signals.E001')
+
     def default_error(model_key, func, args, keywords):
         error_msg = "%(op)s contains a lazy reference to %(model)s, but %(model_error)s."
         params = {
@@ -142,6 +137,7 @@ def _check_lazy_references(apps, ignore=None):
     known_lazy = {
         ('django.db.models.fields.related', 'resolve_related_class'): field_error,
         ('django.db.models.fields.related', 'set_managed'): None,
+        ('django.dispatch.dispatcher', 'connect'): signal_connect_error,
     }
 
     def build_error(model_key, func, args, keywords):
diff --git a/django/db/models/signals.py b/django/db/models/signals.py
index b69a2812a6..56bf1ba175 100644
--- a/django/db/models/signals.py
+++ b/django/db/models/signals.py
@@ -1,6 +1,7 @@
-from django.apps import apps
+from functools import partial
+
+from django.db.models.utils import make_model_tuple
 from django.dispatch import Signal
-from django.utils import six
 
 
 class_prepared = Signal(providing_args=["class"])
@@ -11,44 +12,15 @@ class ModelSignal(Signal):
     Signal subclass that allows the sender to be lazily specified as a string
     of the `app_label.ModelName` form.
     """
+    def connect(self, receiver, sender=None, weak=True, dispatch_uid=None, apps=None):
+        # Takes a single optional argument named "sender"
+        connect = partial(super(ModelSignal, self).connect, receiver, weak=weak, dispatch_uid=dispatch_uid)
+        models = [make_model_tuple(sender)] if sender else []
+        if not apps:
+            from django.db.models.base import Options
+            apps = sender._meta.apps if hasattr(sender, '_meta') else Options.default_apps
+        apps.lazy_model_operation(connect, *models)
 
-    def __init__(self, *args, **kwargs):
-        super(ModelSignal, self).__init__(*args, **kwargs)
-        self.unresolved_references = {}
-        class_prepared.connect(self._resolve_references)
-
-    def _resolve_references(self, sender, **kwargs):
-        opts = sender._meta
-        reference = (opts.app_label, opts.object_name)
-        try:
-            receivers = self.unresolved_references.pop(reference)
-        except KeyError:
-            pass
-        else:
-            for receiver, weak, dispatch_uid in receivers:
-                super(ModelSignal, self).connect(
-                    receiver, sender=sender, weak=weak, dispatch_uid=dispatch_uid
-                )
-
-    def connect(self, receiver, sender=None, weak=True, dispatch_uid=None):
-        if isinstance(sender, six.string_types):
-            try:
-                app_label, model_name = sender.split('.')
-            except ValueError:
-                raise ValueError(
-                    "Specified sender must either be a model or a "
-                    "model name of the 'app_label.ModelName' form."
-                )
-            try:
-                sender = apps.get_registered_model(app_label, model_name)
-            except LookupError:
-                ref = (app_label, model_name)
-                refs = self.unresolved_references.setdefault(ref, [])
-                refs.append((receiver, weak, dispatch_uid))
-                return
-        super(ModelSignal, self).connect(
-            receiver, sender=sender, weak=weak, dispatch_uid=dispatch_uid
-        )
 
 pre_init = ModelSignal(providing_args=["instance", "args", "kwargs"], use_caching=True)
 post_init = ModelSignal(providing_args=["instance"], use_caching=True)
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index e68c1a5a57..03bdba97db 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -248,7 +248,8 @@ Signals
 ~~~~~~~
 
 * **signals.E001**: ``<handler>`` was connected to the ``<signal>`` signal with
-  a lazy reference to the ``<model>`` sender, which has not been installed.
+  a lazy reference to the sender ``<app label>.<model>``, but app ``<app label>``
+  isn't installed or doesn't provide model ``<model>``.
 
 Backwards Compatibility
 ~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/model_validation/tests.py b/tests/model_validation/tests.py
index b98fd2ddac..0e137af5f2 100644
--- a/tests/model_validation/tests.py
+++ b/tests/model_validation/tests.py
@@ -1,5 +1,5 @@
 from django.core import management
-from django.core.checks import Error, run_checks
+from django.core.checks import Error
 from django.core.checks.model_checks import _check_lazy_references
 from django.db import models
 from django.db.models.signals import post_init
@@ -8,19 +8,6 @@ from django.test.utils import isolate_apps, override_settings
 from django.utils import six
 
 
-class OnPostInit(object):
-    def __call__(self, **kwargs):
-        pass
-
-
-def on_post_init(**kwargs):
-    pass
-
-
-def dummy_function(model):
-    pass
-
-
 @override_settings(
     INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'],
     SILENCED_SYSTEM_CHECKS=['fields.W342'],  # ForeignKey(unique=True)
@@ -35,32 +22,6 @@ class ModelValidationTest(SimpleTestCase):
         #       See: https://code.djangoproject.com/ticket/21375
         management.call_command("check", stdout=six.StringIO())
 
-    def test_model_signal(self):
-        unresolved_references = post_init.unresolved_references.copy()
-        post_init.connect(on_post_init, sender='missing-app.Model')
-        post_init.connect(OnPostInit(), sender='missing-app.Model')
-
-        errors = run_checks()
-        expected = [
-            Error(
-                "The 'on_post_init' function was connected to the 'post_init' "
-                "signal with a lazy reference to the 'missing-app.Model' "
-                "sender, which has not been installed.",
-                obj='model_validation.tests',
-                id='signals.E001',
-            ),
-            Error(
-                "An instance of the 'OnPostInit' class was connected to "
-                "the 'post_init' signal with a lazy reference to the "
-                "'missing-app.Model' sender, which has not been installed.",
-                obj='model_validation.tests',
-                id='signals.E001',
-            )
-        ]
-        self.assertEqual(errors, expected)
-
-        post_init.unresolved_references = unresolved_references
-
     @isolate_apps('django.contrib.auth', kwarg_name='apps')
     def test_lazy_reference_checks(self, apps):
 
@@ -70,11 +31,24 @@ class ModelValidationTest(SimpleTestCase):
             class Meta:
                 app_label = "model_validation"
 
+        class DummyClass(object):
+            def __call__(self, **kwargs):
+                pass
+
+            def dummy_method(self):
+                pass
+
+        def dummy_function(*args, **kwargs):
+            pass
+
         apps.lazy_model_operation(dummy_function, ('auth', 'imaginarymodel'))
         apps.lazy_model_operation(dummy_function, ('fanciful_app', 'imaginarymodel'))
 
-        errors = _check_lazy_references(apps)
+        post_init.connect(dummy_function, sender='missing-app.Model', apps=apps)
+        post_init.connect(DummyClass(), sender='missing-app.Model', apps=apps)
+        post_init.connect(DummyClass().dummy_method, sender='missing-app.Model', apps=apps)
 
+        errors = _check_lazy_references(apps)
         expected = [
             Error(
                 "%r contains a lazy reference to auth.imaginarymodel, "
@@ -89,6 +63,22 @@ class ModelValidationTest(SimpleTestCase):
                 id='models.E022',
             ),
             Error(
+                "An instance of class 'DummyClass' was connected to "
+                "the 'post_init' signal with a lazy reference to the sender "
+                "'missing-app.model', but app 'missing-app' isn't installed.",
+                hint=None,
+                obj='model_validation.tests',
+                id='signals.E001',
+            ),
+            Error(
+                "Bound method 'DummyClass.dummy_method' was connected to the "
+                "'post_init' signal with a lazy reference to the sender "
+                "'missing-app.model', but app 'missing-app' isn't installed.",
+                hint=None,
+                obj='model_validation.tests',
+                id='signals.E001',
+            ),
+            Error(
                 "The field model_validation.DummyModel.author was declared "
                 "with a lazy reference to 'model_validation.author', but app "
                 "'model_validation' isn't installed.",
@@ -96,6 +86,13 @@ class ModelValidationTest(SimpleTestCase):
                 obj=DummyModel.author.field,
                 id='fields.E307',
             ),
+            Error(
+                "The function 'dummy_function' was connected to the 'post_init' "
+                "signal with a lazy reference to the sender "
+                "'missing-app.model', but app 'missing-app' isn't installed.",
+                hint=None,
+                obj='model_validation.tests',
+                id='signals.E001',
+            ),
         ]
-
         self.assertEqual(errors, expected)
diff --git a/tests/signals/tests.py b/tests/signals/tests.py
index b02b215958..fd18b2191a 100644
--- a/tests/signals/tests.py
+++ b/tests/signals/tests.py
@@ -267,7 +267,7 @@ class LazyModelRefTest(BaseSignalTest):
         self.received.append(kwargs)
 
     def test_invalid_sender_model_name(self):
-        msg = "Specified sender must either be a model or a model name of the 'app_label.ModelName' form."
+        msg = "Invalid model reference 'invalid'. String model references must be of the form 'app_label.ModelName'."
         with self.assertRaisesMessage(ValueError, msg):
             signals.post_init.connect(self.receiver, sender='invalid')
 
@@ -285,10 +285,10 @@ class LazyModelRefTest(BaseSignalTest):
         finally:
             signals.post_init.disconnect(self.receiver, sender=Book)
 
-    @isolate_apps('signals')
-    def test_not_loaded_model(self):
+    @isolate_apps('signals', kwarg_name='apps')
+    def test_not_loaded_model(self, apps):
         signals.post_init.connect(
-            self.receiver, sender='signals.Created', weak=False
+            self.receiver, sender='signals.Created', weak=False, apps=apps
         )
 
         try: