From 7e06065d8b4b725330440e785accba243748da81 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sat, 7 Aug 2010 06:27:52 +0000 Subject: [PATCH] Fixed #13552 -- Added a 'using' parameter to database signals. Thanks to gmandx for the suggestion, and Andrew Godwin for the patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@13538 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- AUTHORS | 1 + django/db/models/base.py | 4 +- django/db/models/fields/related.py | 18 +-- django/db/models/query.py | 5 +- django/db/models/signals.py | 10 +- docs/ref/signals.txt | 29 ++++- .../multiple_database/tests.py | 112 ++++++++++++++++++ 7 files changed, 157 insertions(+), 22 deletions(-) diff --git a/AUTHORS b/AUTHORS index 31433adbda..7324a699a5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -190,6 +190,7 @@ answer newbie questions, and generally made Django that much better: martin.glueck@gmail.com Artyom Gnilov Ben Godfrey + Andrew Godwin GomoX Guilherme Mesquita Gondim Mario Gonzalez diff --git a/django/db/models/base.py b/django/db/models/base.py index 6304e009d3..07fc501ea0 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -456,7 +456,7 @@ class Model(object): meta = cls._meta if origin and not meta.auto_created: - signals.pre_save.send(sender=origin, instance=self, raw=raw) + signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using) # If we are in a raw save, save the object exactly as presented. # That means that we don't try to be smart about saving attributes @@ -540,7 +540,7 @@ class Model(object): # Signal that the save is complete if origin and not meta.auto_created: signals.post_save.send(sender=origin, instance=self, - created=(not record_exists), raw=raw) + created=(not record_exists), raw=raw, using=using) save_base.alters_data = True diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 232d5d5e74..1634d7d974 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -566,7 +566,7 @@ def create_many_related_manager(superclass, rel=False): # duplicate data row for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action='pre_add', instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=new_ids) + model=self.model, pk_set=new_ids, using=db) # Add the ones that aren't there already for obj_id in new_ids: self.through._default_manager.using(db).create(**{ @@ -578,7 +578,7 @@ def create_many_related_manager(superclass, rel=False): # duplicate data row for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action='post_add', instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=new_ids) + model=self.model, pk_set=new_ids, using=db) def _remove_items(self, source_field_name, target_field_name, *objs): # source_col_name: the PK colname in join_table for the source object @@ -594,14 +594,16 @@ def create_many_related_manager(superclass, rel=False): old_ids.add(obj.pk) else: old_ids.add(obj) + # Work out what DB we're operating on + db = router.db_for_write(self.through.__class__, instance=self.instance) + # Send a signal to the other end if need be. if self.reverse or source_field_name == self.source_field_name: # Don't send the signal when we are deleting the # duplicate data row for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action="pre_remove", instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=old_ids) + model=self.model, pk_set=old_ids, using=db) # Remove the specified objects from the join table - db = router.db_for_write(self.through.__class__, instance=self.instance) self.through._default_manager.using(db).filter(**{ source_field_name: self._pk_val, '%s__in' % target_field_name: old_ids @@ -611,17 +613,17 @@ def create_many_related_manager(superclass, rel=False): # duplicate data row for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action="post_remove", instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=old_ids) + model=self.model, pk_set=old_ids, using=db) def _clear_items(self, source_field_name): + db = router.db_for_write(self.through.__class__, instance=self.instance) # source_col_name: the PK colname in join_table for the source object if self.reverse or source_field_name == self.source_field_name: # Don't send the signal when we are clearing the # duplicate data rows for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action="pre_clear", instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=None) - db = router.db_for_write(self.through.__class__, instance=self.instance) + model=self.model, pk_set=None, using=db) self.through._default_manager.using(db).filter(**{ source_field_name: self._pk_val }).delete() @@ -630,7 +632,7 @@ def create_many_related_manager(superclass, rel=False): # duplicate data rows for symmetrical reverse entries. signals.m2m_changed.send(sender=rel.through, action="post_clear", instance=self.instance, reverse=self.reverse, - model=self.model, pk_set=None) + model=self.model, pk_set=None, using=db) return ManyRelatedManager diff --git a/django/db/models/query.py b/django/db/models/query.py index d9fbd9b8cb..9ecfb745fd 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1311,7 +1311,8 @@ def delete_objects(seen_objs, using): # Pre-notify all instances to be deleted. for pk_val, instance in items: if not cls._meta.auto_created: - signals.pre_delete.send(sender=cls, instance=instance) + signals.pre_delete.send(sender=cls, instance=instance, + using=using) pk_list = [pk for pk,instance in items] @@ -1343,7 +1344,7 @@ def delete_objects(seen_objs, using): setattr(instance, field.attname, None) if not cls._meta.auto_created: - signals.post_delete.send(sender=cls, instance=instance) + signals.post_delete.send(sender=cls, instance=instance, using=using) setattr(instance, cls._meta.pk.attname, None) if forced_managed: diff --git a/django/db/models/signals.py b/django/db/models/signals.py index cd0350bc01..48872e7e7f 100644 --- a/django/db/models/signals.py +++ b/django/db/models/signals.py @@ -5,12 +5,12 @@ class_prepared = Signal(providing_args=["class"]) pre_init = Signal(providing_args=["instance", "args", "kwargs"]) post_init = Signal(providing_args=["instance"]) -pre_save = Signal(providing_args=["instance", "raw"]) -post_save = Signal(providing_args=["instance", "raw", "created"]) +pre_save = Signal(providing_args=["instance", "raw", "using"]) +post_save = Signal(providing_args=["instance", "raw", "created", "using"]) -pre_delete = Signal(providing_args=["instance"]) -post_delete = Signal(providing_args=["instance"]) +pre_delete = Signal(providing_args=["instance", "using"]) +post_delete = Signal(providing_args=["instance", "using"]) post_syncdb = Signal(providing_args=["class", "app", "created_models", "verbosity", "interactive"]) -m2m_changed = Signal(providing_args=["action", "instance", "reverse", "model", "pk_set"]) +m2m_changed = Signal(providing_args=["action", "instance", "reverse", "model", "pk_set", "using"]) diff --git a/docs/ref/signals.txt b/docs/ref/signals.txt index 12372dd0f1..a4dc0f705d 100644 --- a/docs/ref/signals.txt +++ b/docs/ref/signals.txt @@ -33,9 +33,9 @@ module system. If you override these methods on your model, you must call the parent class' methods for this signals to be sent. - Note also that Django stores signal handlers as weak references by default, - so if your handler is a local function, it may be garbage collected. To - prevent this, pass ``weak=False`` when you call the signal's :meth:`~django.dispatch.Signal.connect`. + Note also that Django stores signal handlers as weak references by default, + so if your handler is a local function, it may be garbage collected. To + prevent this, pass ``weak=False`` when you call the signal's :meth:`~django.dispatch.Signal.connect`. pre_init -------- @@ -113,6 +113,9 @@ Arguments sent with this signal: ``instance`` The actual instance being saved. + ``using`` + The database alias being used. + post_save --------- @@ -133,6 +136,9 @@ Arguments sent with this signal: ``created`` A boolean; ``True`` if a new record was created. + ``using`` + The database alias being used. + pre_delete ---------- @@ -150,6 +156,9 @@ Arguments sent with this signal: ``instance`` The actual instance being deleted. + ``using`` + The database alias being used. + post_delete ----------- @@ -170,6 +179,9 @@ Arguments sent with this signal: Note that the object will no longer be in the database, so be very careful what you do with this instance. + ``using`` + The database alias being used. + m2m_changed ----------- @@ -215,8 +227,8 @@ Arguments sent with this signal: Sent *after* the relation is cleared ``reverse`` - Indicates which side of the relation is updated (i.e., if it is the - forward or reverse relation that is being modified). + Indicates which side of the relation is updated (i.e., if it is the + forward or reverse relation that is being modified). ``model`` The class of the objects that are added to, removed from or cleared @@ -228,6 +240,9 @@ Arguments sent with this signal: For the ``"clear"`` action, this is ``None``. + ``using`` + The database alias being used. + For example, if a ``Pizza`` can have multiple ``Topping`` objects, modeled like this: @@ -266,6 +281,8 @@ the arguments sent to a :data:`m2m_changed` handler would be: ``Pizza``) ``pk_set`` ``[t.id]`` (since only ``Topping t`` was added to the relation) + + ``using`` ``"default"`` (since the default router sends writes here) ============== ============================================================ And if we would then do something like this: @@ -293,6 +310,8 @@ the arguments sent to a :data:`m2m_changed` handler would be: ``pk_set`` ``[p.id]`` (since only ``Pizza p`` was removed from the relation) + + ``using`` ``"default"`` (since the default router sends writes here) ============== ============================================================ class_prepared diff --git a/tests/regressiontests/multiple_database/tests.py b/tests/regressiontests/multiple_database/tests.py index 6675fdcc6c..b55e708944 100644 --- a/tests/regressiontests/multiple_database/tests.py +++ b/tests/regressiontests/multiple_database/tests.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.core import management from django.db import connections, router, DEFAULT_DB_ALIAS +from django.db.models import signals from django.db.utils import ConnectionRouter from django.test import TestCase @@ -1619,3 +1620,114 @@ class PickleQuerySetTestCase(TestCase): Book.objects.using(db).create(title='Dive into Python', published=datetime.date(2009, 5, 4)) qs = Book.objects.all() self.assertEqual(qs.db, pickle.loads(pickle.dumps(qs)).db) + + +class DatabaseReceiver(object): + """ + Used in the tests for the database argument in signals (#13552) + """ + def __call__(self, signal, sender, **kwargs): + self._database = kwargs['using'] + +class WriteToOtherRouter(object): + """ + A router that sends all writes to the other database. + """ + def db_for_write(self, model, **hints): + return "other" + +class SignalTests(TestCase): + multi_db = True + + def setUp(self): + self.old_routers = router.routers + + def tearDown(self): + router.routser = self.old_routers + + def _write_to_other(self): + "Sends all writes to 'other'." + router.routers = [WriteToOtherRouter()] + + def _write_to_default(self): + "Sends all writes to the default DB" + router.routers = self.old_routers + + def test_database_arg_save_and_delete(self): + """ + Tests that the pre/post_save signal contains the correct database. + (#13552) + """ + # Make some signal receivers + pre_save_receiver = DatabaseReceiver() + post_save_receiver = DatabaseReceiver() + pre_delete_receiver = DatabaseReceiver() + post_delete_receiver = DatabaseReceiver() + # Make model and connect receivers + signals.pre_save.connect(sender=Person, receiver=pre_save_receiver) + signals.post_save.connect(sender=Person, receiver=post_save_receiver) + signals.pre_delete.connect(sender=Person, receiver=pre_delete_receiver) + signals.post_delete.connect(sender=Person, receiver=post_delete_receiver) + p = Person.objects.create(name='Darth Vader') + # Save and test receivers got calls + p.save() + self.assertEqual(pre_save_receiver._database, DEFAULT_DB_ALIAS) + self.assertEqual(post_save_receiver._database, DEFAULT_DB_ALIAS) + # Delete, and test + p.delete() + self.assertEqual(pre_delete_receiver._database, DEFAULT_DB_ALIAS) + self.assertEqual(post_delete_receiver._database, DEFAULT_DB_ALIAS) + # Save again to a different database + p.save(using="other") + self.assertEqual(pre_save_receiver._database, "other") + self.assertEqual(post_save_receiver._database, "other") + # Delete, and test + p.delete(using="other") + self.assertEqual(pre_delete_receiver._database, "other") + self.assertEqual(post_delete_receiver._database, "other") + + def test_database_arg_m2m(self): + """ + Test that the m2m_changed signal has a correct database arg (#13552) + """ + # Make a receiver + receiver = DatabaseReceiver() + # Connect it, and make the models + signals.m2m_changed.connect(receiver=receiver) + + b = Book.objects.create(title="Pro Django", + published=datetime.date(2008, 12, 16)) + + p = Person.objects.create(name="Marty Alchin") + + # Test addition + b.authors.add(p) + self.assertEqual(receiver._database, DEFAULT_DB_ALIAS) + self._write_to_other() + b.authors.add(p) + self._write_to_default() + self.assertEqual(receiver._database, "other") + + # Test removal + b.authors.remove(p) + self.assertEqual(receiver._database, DEFAULT_DB_ALIAS) + self._write_to_other() + b.authors.remove(p) + self._write_to_default() + self.assertEqual(receiver._database, "other") + + # Test addition in reverse + p.book_set.add(b) + self.assertEqual(receiver._database, DEFAULT_DB_ALIAS) + self._write_to_other() + p.book_set.add(b) + self._write_to_default() + self.assertEqual(receiver._database, "other") + + # Test clearing + b.authors.clear() + self.assertEqual(receiver._database, DEFAULT_DB_ALIAS) + self._write_to_other() + b.authors.clear() + self._write_to_default() + self.assertEqual(receiver._database, "other")