Opened 3 years ago

Closed 3 years ago

#25505 closed Bug (fixed)

Incomplete relation_tree for inherited proxied Models.

Reported by: Jelko Arnds Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords: models, meta, deletion, relation, inheritance
Cc: Simon Charette Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Options._populate_directed_relation_graph produces an incomplete Options._relation_tree for Models that have set Meta.Proxy = True.

Consider this setup:

BaseModel(models.Model) -> ProxyModel(BaseModel) [with Proxy = True] -> ChildModel(ProxyModel)

In this case the OneToOne relation from ProxyModel to ChildModel is not included in the proxymodel's _meta._relation_tree.

This is caused because Options._populate_directed_relation_graph collects the relations by calling model._meta._get_fields(reverse=False, include_parents=False). Therefore the parent, in this case the ProxyModel is not included.

This results in some unwanted situations. If you call delete() on a ProxyModel, the BaseModel gets deleted, but the ChildModel instance stays in the database and becomes orphaned.

Have a look at the attached models.py to replicate the setup. Compare the behavior of delete() for instances of ChipShop and IndianRestaurant, to see the problem.

Attachments (2)

models.py (386 bytes) - added by Jelko Arnds 3 years ago.
25505-reproduced-issue.diff (1.9 KB) - added by Simon Charette 3 years ago.
Integrated test case

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by Jelko Arnds

Attachment: models.py added

comment:1 Changed 3 years ago by Simon Charette

Hi jelko,

Could have a look at the patch proposed for #18012 and see if that fixes the reported issue. I haven't tried it myself but from the description you provided it looks like the same scenario.

comment:2 Changed 3 years ago by Simon Charette

Cc: Simon Charette added
Triage Stage: UnreviewedAccepted

It looks like this is another can of worms.

The patch proposed for #18012 doesn't fix the reported issue which I managed to reproduce with the attached test case.

Changed 3 years ago by Simon Charette

Attachment: 25505-reproduced-issue.diff added

Integrated test case

comment:3 Changed 3 years ago by Simon Charette

Note that the test case uses exists() instead of get() because the latter won't return any results and hide the issue because of deferred constraint checks in the transaction wrapping the test case.

comment:4 Changed 3 years ago by Simon Charette

This looks related to #23076.

comment:5 Changed 3 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: newclosed

In 211486f3:

Fixed #23076, #25505 -- Fixed deletion of intermediate proxy models.

Thanks to James Murty for his work on an alternate patch.

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