Opened 18 years ago

Closed 15 years ago

Last modified 12 years ago

#2698 closed defect (fixed)

Custom default managers interfere with delete operations

Reported by: anonymous Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version: dev
Severity: normal Keywords:
Cc: ned@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In our model, we have a Story class, with a boolean 'deleted'
attribute. We defined a custom default manager to return all of the
stories that are not deleted, so that most of the code on the site will
not have to deal with filtering out the deleted stories (there's another
custom manager called with_deleted for those code paths that need to see
the deleted stories as well).

The problem comes up when deleting a user. The deletion code iterates
the user's stories to delete them, but uses the default manager, so
stories with deleted=1 are not found. So (ironically) any stories
marked with the 'deleted' field are not actually deleted when the user
is removed. This prevents the user from being deleted, because the
story table still has a foreign key reference to the user table, and
MySQL prevents the user record from being deleted.

The docs for custom managers say: "it's generally a good idea for the
first Manager to be relatively unfiltered". But it seems that any
filtering in the default manager will interfere with cascading deletes.

If the default manager does any filtering at all, then deleting an object may fail because its related objects will not all be deleted, and the database's referential integrity will prevent the deletion. When looking for related obejcts to delete, you have to always get all of them. No filtering allowed.

Change History (9)

comment:1 by anonymous, 18 years ago

Cc: ned@… added

comment:2 by Jacob, 16 years ago

Triage Stage: UnreviewedAccepted

Looks like the objects-to-delete gathererer needs to figure out how to use an unfiltered queryset.

comment:3 by Malcolm Tredinnick, 15 years ago

Owner: changed from nobody to Malcolm Tredinnick
Status: newassigned

comment:4 by Malcolm Tredinnick, 15 years ago

(In [10056]) Use plain model.Manager, or suitable proxy, for model saving.

We can't use the default manager in Model.save_base(), since we need to
retrieve existing objects which might be filtered out by that manager. We now
always use a plain Manager instance at that point (or something that can
replace it, such as a GeoManager), making all existing rows in the
database visible to the saving code.

The logic for detecting a "suitable replacement" plain base is the same as for
related fields: if the use_for_related_fields is set on the manager subclass,
we can use it. The general requirement here is that we want a base class that
returns the appropriate QuerySet subclass, but does not restrict the rows
returned.

Fixed #8990, #9527.

Refs #2698 (which is not fixed by this change, but it's the first part of a
larger change to fix that bug.)

comment:5 by Malcolm Tredinnick, 15 years ago

(In [10059]) [1.0.X] Use plain model.Manager, or suitable proxy, for model saving.

We can't use the default manager in Model.save_base(), since we need to
retrieve existing objects which might be filtered out by that manager. We now
always use a plain Manager instance at that point (or something that can
replace it, such as a GeoManager), making all existing rows in the
database visible to the saving code.

The logic for detecting a "suitable replacement" plain base is the same as for
related fields: if the use_for_related_fields is set on the manager subclass,
we can use it. The general requirement here is that we want a base class that
returns the appropriate QuerySet subclass, but does not restrict the rows
returned.

Fixed #8990, #9527.

Refs #2698 (which is not fixed by this change, but it's the first part of a
larger change to fix that bug.)

Backport of r10056 from trunk.

comment:6 by Malcolm Tredinnick, 15 years ago

milestone: 1.1

Okay, I think I just worked out how to solve this without having to rewrite all the foreign relation fields for 1.1 (it's too big a job for the time we've got). So I can put this on the 1.1 milestone now.

comment:7 by Malcolm Tredinnick, 15 years ago

Resolution: fixed
Status: assignedclosed

(In [10104]) Fixed #2698 -- Fixed deleting in the presence of custom managers.

A custom manager on a related object that filtered away objects would prevent
those objects being deleted via the relation. This is now fixed.

comment:8 by Malcolm Tredinnick, 15 years ago

(In [10106]) [1.0.X] Fixed #2698 -- Fixed deleting in the presence of custom managers.

A custom manager on a related object that filtered away objects would prevent
those objects being deleted via the relation. This is now fixed.

Backport of r10104 from trunk.

comment:9 by Jacob, 12 years ago

milestone: 1.1

Milestone 1.1 deleted

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