Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#18083 closed Bug (fixed)

Multi-table inheritance subclasses proxy deletion is broken

Reported by: Simon Charette Owned by: Simon Charette
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 Simon Charette 5 years ago.
Failing testcase incorporated into django's test suite
ticket-18083-fix-thanks-to-carljm.diff (3.7 KB) - added by Simon Charette 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Simon Charette

Failing testcase incorporated into django's test suite

comment:1 Changed 5 years ago by Simon Charette

Owner: changed from nobody to Simon Charette

Changed 5 years ago by Simon Charette

comment:2 Changed 5 years ago by Simon Charette

Has patch: set
Triage Stage: UnreviewedAccepted

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 5 years ago by Carl Meyer

Resolution: fixed
Status: newclosed

In [17887]:

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

comment:4 Changed 5 years ago by Simon Charette

The signal issue ticket is #18094.

comment:5 Changed 4 years ago by Jeremy Dunck

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