Inconsistent Behavior with Abstract Models and Signals

[Ken Weaver]

Recently we came across an issue with abstract models and signals, specifically many to many signals. It seems that for most signals, models that have inherited from an abstract class (or perhaps any class) will not also inherit the signals of that class. This is not unexpected. However, for m2m_changed signals, the signal fires for every many to many change in the entire application. I have created a minimum (ish) working example ​here, and I will paste the relevant code below as well.

I think ideally django would stop you from binding signals to abstract models if they can't ever work, or spat out a warning when it was attempted. If that isn't possible, never firing the signal would be preferable to always firing the signal; the latter is highly likely to break parts of your application you weren't testing.

Admittedly, I don't think the documentation ever specifies what the behavior should be related to abstract/inherited models and signals. If none of the changes here are possible, I think a warning in the documentation on the model page, the signals page, or both would be helpful, and I would be more than happy to work on that.

class Related(models.Model):
    pass

class Abstract(models.Model):
    m2m_field = models.ManyToManyField(to=Related)
    int_field = models.IntegerField(default=0)
    class Meta:
        abstract = True

class OtherRelated(models.Model):
    pass

class Other(models.Model):
    m2m_field = models.ManyToManyField(to=OtherRelated)

@receiver(m2m_changed, sender=Abstract.m2m_field.through)
def abstract_m2m_signal(sender, **kwargs):
    wrapped_abstract_m2m(sender, **kwargs)

    @patch('app.models.wrapped_abstract_m2m', side_effect=mock_signal)
    def test_abstract_m2m_signal_not_called(self, signal_mock):
        other = Other.objects.create()
        related = OtherRelated.objects.create()
        other.m2m_field.add(related)
        self.assertEqual(signal_mock.call_count, 0) # this fails

[Mariusz Felisiak]

Thanks for the report. You can create custom signals and send them from anywhere, so I don't think any validation of a sender is feasible here. When connecting the receivers,
we don't know if the code sending the signals is reachable. For example, folks can send built-in signals from abstract models of their own, and this will work fine. pre_save, post_save, pre_delete, and post_delete signals connected to an abstract models will never be send (without custom senders) because save() and delete() methods are not called on abstract models. I'm afraid that any additional note in docs will confuse folks even more, however I'd be happy to review your concrete proposal.

[Ken Weaver]

Thanks for the response. The inability to validate sender makes sense. However, it does still strike me as strange that sender is essentially ignored in the example I gave; is that truly intended behavior?
Replying to Mariusz Felisiak:

Thanks for the report. You can create custom signals and send them from anywhere, so I don't think any validation of a sender is feasible here. When connecting the receivers,
we don't know if the code sending the signals is reachable. For example, folks can send built-in signals from abstract models of their own, and this will work fine. pre_save, post_save, pre_delete, and post_delete signals connected to an abstract models will never be send (without custom senders) because save() and delete() methods are not called on abstract models. I'm afraid that any additional note in docs will confuse folks even more, however I'd be happy to review your concrete proposal.

[Mariusz Felisiak]

However, it does still strike me as strange that sender is essentially ignored in the example I gave; is that truly intended behavior?

sender is not ignored, see my comment:

pre_save, post_save, pre_delete, and post_delete signals connected to an abstract models will never be send (without custom senders) because save() and delete() methods are not called on abstract models.

So these signals are not send for abstract models by Django itself. However, users can send them manually, for example by overwriting save() in a child model:

    def save(
        self, force_insert=False, force_update=False, using=None, update_fields=None
    ):
        pre_save.send(sender=Parent, ...)
        super().save(...)

[Ken Weaver]

Yes I understand, but as the code above demonstrates, when adding a receiver for a m2m_changed signal and Abstract.m2m_field.through field as the sender, the code operates the same as if the sender was None. This, not the fact that pre_save, post_save etc. signals are not automatically sent for abstract models, is the peculiar behavior that led to this ticket.

[Mariusz Felisiak]

but as the code above demonstrates, when adding a receiver for a m2m_changed signal and Abstract.m2m_field.through field as the sender, the code operates the same as if the sender was None.

Abstract is not used in models from the ticket description🤔, it's an abstract model that is not inherited from any concrete model. I'm puzzled.

[Ken Weaver]

Yes it seemed very strange to me too, I thought the signal should be received either when an inherited model's ManyToManyField changed (this was my mistake) or not at all. But instead it is received for every m2m_changed, even for completely unrelated models.

I now understand why; I just checked and Abstract.m2m_field.through itself is actually None. This makes sense (somewhat) since of course you can't have a concrete many to many table for an abstract model. I would think one could change ManyToManyField to itself be some kind of abstract model or a dummy class when attached to an abstract model, but you will know more about the inners of Django to know if something like that is feasible or justified.
Replying to Mariusz Felisiak:

Abstract is not used in models from the ticket description🤔, it's an abstract model that is not inherited from any concrete model. I'm puzzled.

[Mariusz Felisiak]

Replying to Ken Weaver:

I now understand why; I just checked and Abstract.m2m_field.through itself is actually None. This makes sense (somewhat) since of course you can't have a concrete many to many table for an abstract model.

Ahh, yes. You can set sender to None to receive events from any sender.

I would think one could change ManyToManyField to itself be some kind of abstract model or a dummy class when attached to an abstract model, but you will know more about the inners of Django to know if something like that is feasible or justified.

I don't think it is feasible.