Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18083 closed Bug (fixed)

Multi-table inheritance subclasses proxy deletion is broken

Reported by: charettes Owned by: charettes
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: model proxy multi-table inheritance signals
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given the following models

#test_app/models.py
from django.db import models

class A(models.Model):
    pass

class B(A):
    pass

class C(B):
    class Meta:
        proxy = True

Running the following tests fails

#test_app/tests.py
from django.test import TestCase

from .models import *


class SimpleTest(TestCase):

    def test_model_subclass_proxy_deletion(self):
        c = C.objects.create()
        self.assertEqual(1, C.objects.count())
        self.assertEqual(1, B.objects.count())
        self.assertEqual(1, A.objects.count())

        c.delete()
        self.assertEqual(0, C.objects.count())
        self.assertEqual(0, B.objects.count())
        self.assertEqual(0, A.objects.count())

    def test_model_subclass_deletion(self):
        b = B.objects.create()
        self.assertEqual(1, C.objects.count())
        self.assertEqual(1, B.objects.count())
        self.assertEqual(1, A.objects.count())

        b.delete()
        self.assertEqual(0, C.objects.count())
        self.assertEqual(0, B.objects.count())
        self.assertEqual(0, A.objects.count())
simon@simon-laptop:~/Bureau/test_deletion$ ./manage.py test test_app 
Creating test database for alias 'default'...
.F
======================================================================
FAIL: test_model_subclass_proxy_deletion (test_app.tests.SimpleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/simon/Bureau/test_deletion/test_app/tests.py", line 27, in test_model_subclass_proxy_deletion
    self.assertEqual(0, A.objects.count())
AssertionError: 0 != 1

----------------------------------------------------------------------
Ran 2 tests in 0.014s

FAILED (failures=1)
Destroying test database for alias 'default'...

Deleting a multi-table inherited model proxy do not delete all related parent instances while doing it on the concrete model works perfectly.

Attachments (2)

ticket-18083-failling-testcase.diff (2.8 KB) - added by charettes 3 years ago.
Failing testcase incorporated into django's test suite
ticket-18083-fix-thanks-to-carljm.diff (3.7 KB) - added by charettes 3 years ago.

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by charettes

Failing testcase incorporated into django's test suite

comment:1 Changed 3 years ago by charettes

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to charettes
  • Patch needs improvement unset

Changed 3 years ago by charettes

comment:2 Changed 3 years ago by charettes

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted

Attached a patch that fixes the issue but change the way pre_delete and post_delete signals are sent.

Before patch only the ConcreteModelSubclassProxy's signals we're sent which was definitely buggy.

When the patch is applied ConcreteModelSubclassProxy's and then ConcreteModel's signals are sent which is still buggy but better IMHO.

In this particular case I beleive signals for the three classes should be triggered in this order: ConcreteModelSubclassProxy then ConcreteModelSubclass and finally ConcreteModel. But that’s another story, as suggested by @carljm I'll open a new ticket that depends on this one and attempt to fix the signal issue there.

I'm also exceptionally marking as Accepted my own bug because I had agreement it's a valid bug on IRC.

comment:3 Changed 3 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

In [17887]:

Fixed #18083 -- Fixed cascade deletion with proxy model of concrete subclass. Thanks Simon Charette for report and patch.

comment:4 Changed 3 years ago by charettes

The signal issue ticket is #18094.

comment:5 Changed 3 years ago by jdunck

  • Keywords signals added
Note: See TracTickets for help on using tickets.
Back to Top