Proxy Model does not send save signal

[zbyte64]

from django.test import TestCase
from django.db import models

class Test1Model(models.Model):
    field1 = models.CharField(max_length=10)

class TestProxyModel(Test1Model):
    class Meta:
        proxy = True

class ProxySignalTest(TestCase):
    def test_proxy_signal(self):
        from django.db.models import signals
        def save_signal():
            raise Exception()
        signals.post_save.connect(save_signal, sender=TestProxyModel)
        tp = TestProxyModel(field1="test1")
        self.assertRaises(Exception, tp.save)

This test case fails as the save signal never gets called.

[medhat]

the last line should be: self.assertRaises(Exception, tp.save())

[zbyte64]

Replying to medhat:

the last line should be: self.assertRaises(Exception, tp.save())

You're not suppose to call the function in the parameter, or else the function won't catch the exception, here's a simple example:

import unittest

def raise_error():
	assert False

class AssertErrorTestCase(unittest.TestCase):
	def test_if_callable(self):
		self.assertRaises(AssertionError, raise_error)

	def test_if_inline_call(self):
		self.assertRaises(AssertionError, raise_error())

if __name__ == '__main__':
	unittest.main()

The result is "test_if_inline_call" fails while "test_if_callable" passes.

[k0001]

I've attached a patch that fixes this issue. Notice we must carry around the value for the "created" argument of the signal: I thought the cleanest (and probably useful) way to do this could be to add a return value to save_base as the patch shows.

Thoughts:

Shouldn't we send both sender=Test1Model and sender=TestProxyModel post_save signals in a situation like the one on the given example? If we take this approach, in what order should we send the signals?

[Valera_Grishin]

I have a similar problem though with simpler code (w/o proxy):

from django.db import models
from django.db.models.signals import post_save

class MyModel(models.Model):
    pass

class ChangeLinstener:
    def __init__(self):
        def listener(sender, **kwargs):
            print "MyModel changed"
        post_save.connect(listener, sender=MyModel)

The listener never called. And patch doesn't help.
Is it the same problem OR should I open another ticket?

[Alex Gaynor]

That's not a bug, your signal never gets connected unless you instantiate the ChangeLitener.

[Valera_Grishin]

Well, I though so too unless following happened to work:

from django.db import models
from django.db.models.signals import post_save

class MyModel(models.Model):
    pass

def listener(sender, **kwargs):
    print "MyModel changed"

class ChangeLinstener:
   def __init__(self):
       post_save.connect(listener, sender=MyModel)

Which looks like hackish design (the "listener" function should be inside ChangeListener).

[Alex Gaynor]

That will not work either, a signal will not be called unless it is connected which doesn't happen unless that code is excecuted. Please don't use trac for asking for these support questions, use either #django on freenode or django-users mailing list.

[Valera_Grishin]

Ah, sorry. I forgot to add last line of code in both examples above:

ChangeListener = ChangeListener()

That is, I immediately instantiate ChangeListener after it has been defined. But, first example doesn't work while second one does work fine. This seems quite misterious to me since the difference is only the location of "listener" definition (inside or outside of ChangeListener).

P.S. Alex, this is not a support question but a question whether this is the same bug or separate ticket needs to be added. Thanks.

[Alex Gaynor]

My guess is the issue is that by default a weak reference is used, and since you overwrite the class references to the inner closure ceases to exist.

[Valera_Grishin]

Unfortunatelly, even following:

change_listener = ChangeListener()

works in the same way.

Btw, using "lambda" for defining "listener" function instead of "def" doesn't fix the issue.

[Alex Gaynor]

It doens't have anything to do with the name change, it's just that the local function isn't in scope so it ceases to exist, you just need to pass weak=False when you connect the signal.

[Valera_Grishin]

It works now! Thanks a lot. Sorry for taking much attention.

[Jacob]

I very much don't like the semantics of the save_base return type: usually True/False in a method like save would indicate success or failure, but here it indicates two different types of success. That's an ugly API; I think we need to find another one.

[Ulrich Petri]

I'll give this a shot.

[Ulrich Petri]

I have to leave the sprint pretty soon, so I'm reassigning to nobdy since I didn't come up with any sensible way to imporove this patch

[Ulrich Petri]

Updated to trunk (r10836)

[Ulrich Petri]

I updated the patch for current trunk and modified the way the the recursion result is returned so that it doesn't leak into the "public" API of the method (IIRC this was one of the main concerns Jacob expressed at the EuroDjangoCon sprint)

[Karen Tracey]

Jacob's objection a few comments up is he very much does not like save base returning True/False to indicate two types of success. I do not see that the new patch changes that behavior?

[Ulrich Petri]

The change is that now this behaviour only occurs internally to the method in recursive calls. The outermost (initial) call behaves as before the patch and does not return anything.

[Karen Tracey]

I wasn't at the sprint so don't know if that is sufficient to overcome Jacob's objection, all I have to go on is the comment in the ticket, which sounds a bit broader. Personally, having an internal-recursive use of the function that expects/returns a return value while the "public" use does not makes me a bit uneasy, but this is not code I know well enough to really kibitz on.

[Ulrich Petri]

Yeah, I agree that its not very nice code, but I see no better way except completely rewriting save_base.

[Russell Keith-Magee]

(In [10954]) Fixed #10672 -- Altered save_base to ensure that proxy models send a post_save signal.

[ccahoon]

(In [11001]) Fixed #10672 -- Altered save_base to ensure that proxy models send a post_save signal.

[Jacob]

Milestone 1.1 deleted