diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py index a3b8f1b7f32..0c12f9b96a6 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 b69a2812a64..56bf1ba1750 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 e68c1a5a574..03bdba97dbe 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -248,7 +248,8 @@ Signals ~~~~~~~ * **signals.E001**: ```` was connected to the ```` signal with - a lazy reference to the ```` sender, which has not been installed. + a lazy reference to the sender ``.``, but app ```` + isn't installed or doesn't provide model ````. Backwards Compatibility ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/model_validation/tests.py b/tests/model_validation/tests.py index b98fd2ddac9..0e137af5f22 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, " @@ -88,6 +62,22 @@ class ModelValidationTest(SimpleTestCase): obj=dummy_function, 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 " @@ -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 b02b2159584..fd18b2191a5 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: