Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #13513 -- Ensured that queries collecting deleted objects are i…

…ssued on the right database, especially when dealing with m2m intermediate tables. Thanks to gavoja for the report.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@13232 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit f6b0f742d178d95fbd17a40869c6de453e8cfcf0 1 parent 8d9bef9
Russell Keith-Magee authored May 11, 2010
7  django/db/models/base.py
@@ -588,20 +588,22 @@ def _collect_sub_objects(self, seen_objs, parent=None, nullable=False):
588 588
 
589 589
         for related in self._meta.get_all_related_many_to_many_objects():
590 590
             if related.field.rel.through:
  591
+                db = router.db_for_write(related.field.rel.through.__class__, instance=self)
591 592
                 opts = related.field.rel.through._meta
592 593
                 reverse_field_name = related.field.m2m_reverse_field_name()
593 594
                 nullable = opts.get_field(reverse_field_name).null
594 595
                 filters = {reverse_field_name: self}
595  
-                for sub_obj in related.field.rel.through._base_manager.filter(**filters):
  596
+                for sub_obj in related.field.rel.through._base_manager.using(db).filter(**filters):
596 597
                     sub_obj._collect_sub_objects(seen_objs, self, nullable)
597 598
 
598 599
         for f in self._meta.many_to_many:
599 600
             if f.rel.through:
  601
+                db = router.db_for_write(f.rel.through.__class__, instance=self)
600 602
                 opts = f.rel.through._meta
601 603
                 field_name = f.m2m_field_name()
602 604
                 nullable = opts.get_field(field_name).null
603 605
                 filters = {field_name: self}
604  
-                for sub_obj in f.rel.through._base_manager.filter(**filters):
  606
+                for sub_obj in f.rel.through._base_manager.using(db).filter(**filters):
605 607
                     sub_obj._collect_sub_objects(seen_objs, self, nullable)
606 608
             else:
607 609
                 # m2m-ish but with no through table? GenericRelation: cascade delete
@@ -627,7 +629,6 @@ def _collect_sub_objects(self, seen_objs, parent=None, nullable=False):
627 629
 
628 630
     def delete(self, using=None):
629 631
         using = using or router.db_for_write(self.__class__, instance=self)
630  
-        connection = connections[using]
631 632
         assert self._get_pk_val() is not None, "%s object can't be deleted because its %s attribute is set to None." % (self._meta.object_name, self._meta.pk.attname)
632 633
 
633 634
         # Find all the objects than need to be deleted.
10  tests/regressiontests/multiple_database/models.py
@@ -44,6 +44,16 @@ def __unicode__(self):
44 44
     class Meta:
45 45
         ordering = ('title',)
46 46
 
  47
+class Pet(models.Model):
  48
+    name = models.CharField(max_length=100)
  49
+    owner = models.ForeignKey(Person)
  50
+
  51
+    def __unicode__(self):
  52
+        return self.name
  53
+
  54
+    class Meta:
  55
+        ordering = ('name',)
  56
+
47 57
 class UserProfile(models.Model):
48 58
     user = models.OneToOneField(User, null=True)
49 59
     flavor = models.CharField(max_length=100)
107  tests/regressiontests/multiple_database/tests.py
@@ -10,7 +10,7 @@
10 10
 from django.db.utils import ConnectionRouter
11 11
 from django.test import TestCase
12 12
 
13  
-from models import Book, Person, Review, UserProfile
  13
+from models import Book, Person, Pet, Review, UserProfile
14 14
 
15 15
 try:
16 16
     # we only have these models if the user is using multi-db, it's safe the
@@ -321,6 +321,66 @@ def test_m2m_cross_database_protection(self):
321 321
         except ValueError:
322 322
             pass
323 323
 
  324
+    def test_m2m_deletion(self):
  325
+        "Cascaded deletions of m2m relations issue queries on the right database"
  326
+        # Create a book and author on the other database
  327
+        dive = Book.objects.using('other').create(title="Dive into Python",
  328
+                                                  published=datetime.date(2009, 5, 4))
  329
+
  330
+        mark = Person.objects.using('other').create(name="Mark Pilgrim")
  331
+        dive.authors = [mark]
  332
+
  333
+        # Check the initial state
  334
+        self.assertEquals(Person.objects.using('default').count(), 0)
  335
+        self.assertEquals(Book.objects.using('default').count(), 0)
  336
+        self.assertEquals(Book.authors.through.objects.using('default').count(), 0)
  337
+
  338
+        self.assertEquals(Person.objects.using('other').count(), 1)
  339
+        self.assertEquals(Book.objects.using('other').count(), 1)
  340
+        self.assertEquals(Book.authors.through.objects.using('other').count(), 1)
  341
+
  342
+        # Delete the object on the other database
  343
+        dive.delete(using='other')
  344
+
  345
+        self.assertEquals(Person.objects.using('default').count(), 0)
  346
+        self.assertEquals(Book.objects.using('default').count(), 0)
  347
+        self.assertEquals(Book.authors.through.objects.using('default').count(), 0)
  348
+
  349
+        # The person still exists ...
  350
+        self.assertEquals(Person.objects.using('other').count(), 1)
  351
+        # ... but the book has been deleted
  352
+        self.assertEquals(Book.objects.using('other').count(), 0)
  353
+        # ... and the relationship object has also been deleted.
  354
+        self.assertEquals(Book.authors.through.objects.using('other').count(), 0)
  355
+
  356
+        # Now try deletion in the reverse direction. Set up the relation again
  357
+        dive = Book.objects.using('other').create(title="Dive into Python",
  358
+                                                  published=datetime.date(2009, 5, 4))
  359
+        dive.authors = [mark]
  360
+
  361
+        # Check the initial state
  362
+        self.assertEquals(Person.objects.using('default').count(), 0)
  363
+        self.assertEquals(Book.objects.using('default').count(), 0)
  364
+        self.assertEquals(Book.authors.through.objects.using('default').count(), 0)
  365
+
  366
+        self.assertEquals(Person.objects.using('other').count(), 1)
  367
+        self.assertEquals(Book.objects.using('other').count(), 1)
  368
+        self.assertEquals(Book.authors.through.objects.using('other').count(), 1)
  369
+
  370
+        # Delete the object on the other database
  371
+        mark.delete(using='other')
  372
+
  373
+        self.assertEquals(Person.objects.using('default').count(), 0)
  374
+        self.assertEquals(Book.objects.using('default').count(), 0)
  375
+        self.assertEquals(Book.authors.through.objects.using('default').count(), 0)
  376
+
  377
+        # The person has been deleted ...
  378
+        self.assertEquals(Person.objects.using('other').count(), 0)
  379
+        # ... but the book still exists
  380
+        self.assertEquals(Book.objects.using('other').count(), 1)
  381
+        # ... and the relationship object has been deleted.
  382
+        self.assertEquals(Book.authors.through.objects.using('other').count(), 0)
  383
+
324 384
     def test_foreign_key_separation(self):
325 385
         "FK fields are constrained to a single database"
326 386
         # Create a book and author on the default database
@@ -498,6 +558,28 @@ def test_foreign_key_cross_database_protection(self):
498 558
         self.assertEquals(list(Book.objects.using('other').values_list('title',flat=True)),
499 559
                           [u'Dive into HTML5', u'Dive into Python', u'Dive into Water'])
500 560
 
  561
+    def test_foreign_key_deletion(self):
  562
+        "Cascaded deletions of Foreign Key relations issue queries on the right database"
  563
+        mark = Person.objects.using('other').create(name="Mark Pilgrim")
  564
+        fido = Pet.objects.using('other').create(name="Fido", owner=mark)
  565
+
  566
+        # Check the initial state
  567
+        self.assertEquals(Person.objects.using('default').count(), 0)
  568
+        self.assertEquals(Pet.objects.using('default').count(), 0)
  569
+
  570
+        self.assertEquals(Person.objects.using('other').count(), 1)
  571
+        self.assertEquals(Pet.objects.using('other').count(), 1)
  572
+
  573
+        # Delete the person object, which will cascade onto the pet
  574
+        mark.delete(using='other')
  575
+
  576
+        self.assertEquals(Person.objects.using('default').count(), 0)
  577
+        self.assertEquals(Pet.objects.using('default').count(), 0)
  578
+
  579
+        # Both the pet and the person have been deleted from the right database
  580
+        self.assertEquals(Person.objects.using('other').count(), 0)
  581
+        self.assertEquals(Pet.objects.using('other').count(), 0)
  582
+
501 583
     def test_o2o_separation(self):
502 584
         "OneToOne fields are constrained to a single database"
503 585
         # Create a user and profile on the default database
@@ -729,6 +811,29 @@ def test_generic_key_cross_database_protection(self):
729 811
         self.assertEquals(list(Review.objects.using('other').filter(object_id=dive.pk).values_list('source',flat=True)),
730 812
                           [u'Python Daily', u'Python Weekly'])
731 813
 
  814
+    def test_generic_key_deletion(self):
  815
+        "Cascaded deletions of Generic Key relations issue queries on the right database"
  816
+        dive = Book.objects.using('other').create(title="Dive into Python",
  817
+                                                  published=datetime.date(2009, 5, 4))
  818
+        review = Review.objects.using('other').create(source="Python Weekly", content_object=dive)
  819
+
  820
+        # Check the initial state
  821
+        self.assertEquals(Book.objects.using('default').count(), 0)
  822
+        self.assertEquals(Review.objects.using('default').count(), 0)
  823
+
  824
+        self.assertEquals(Book.objects.using('other').count(), 1)
  825
+        self.assertEquals(Review.objects.using('other').count(), 1)
  826
+
  827
+        # Delete the Book object, which will cascade onto the pet
  828
+        dive.delete(using='other')
  829
+
  830
+        self.assertEquals(Book.objects.using('default').count(), 0)
  831
+        self.assertEquals(Review.objects.using('default').count(), 0)
  832
+
  833
+        # Both the pet and the person have been deleted from the right database
  834
+        self.assertEquals(Book.objects.using('other').count(), 0)
  835
+        self.assertEquals(Review.objects.using('other').count(), 0)
  836
+
732 837
     def test_ordering(self):
733 838
         "get_next_by_XXX commands stick to a single database"
734 839
         pro = Book.objects.create(title="Pro Django",

0 notes on commit f6b0f74

Please sign in to comment.
Something went wrong with that request. Please try again.