Opened 16 years ago

Closed 13 years ago

Last modified 12 years ago

#6108 closed Uncategorized (wontfix)

send all_objects_to_be_deleted in the pre_delete signal

Reported by: Gábor Farkas <gabor@…> Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Carl Meyer, plandry@…, cgieringer Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

when django deletes an object, it also deletes all related objects.

it would be great to send the list of "to be deleted objects" in the pre_delete signal, because then a developer could implement in his application things like only-delete-object-when-there-are-no-related-objects (by raising an Exception in the listener-function), etc.

it can be implemented by an one-line change in db/models/query.py (patch attached).

Attachments (1)

pre_delete.patch (1.8 KB ) - added by Gábor Farkas <gabor@…> 16 years ago.

Download all attachments as: .zip

Change History (14)

by Gábor Farkas <gabor@…>, 16 years ago

Attachment: pre_delete.patch added

comment:1 by Simon G <dev@…>, 16 years ago

Has patch: set
Triage Stage: UnreviewedReady for checkin

comment:2 by Jacob, 16 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinDesign decision needed

This unfortunately needs to be more complicated to handle all the use cases for this feature. We need the following features:

  • Listeners should be able to modify the list of to-be-deleted objects. I think that'll work with your patch, but a test is needed to make sure that happens.
  • Because each doomed object gets a signal sent listener code might end up running multiple times. So we might need to figure out which objects have already been signaled. We could avoid a duplicate list (and the duplicate memory that would require) with some other lightweight structure -- perhaps the list of objects-to-delete could be a list of (object, has_been_notified)?
  • Another option to avoid duplicated callbacks would be an initial bulk-pre-delete signal (that is, use the same signal but with instance=None and instance_list=list_of_doomed_objects.
  • We also need to verify that we can in fact implement ON DELETE CASCADE-like behavior with this. If not, it's something of a worthless addition.

comment:3 by Gábor Farkas <gabor@…>, 16 years ago

could you please elaborate on the"ON DELETE CASCADE-like behavior" part?

what kind of behaviors do you imagine here?

as i see, the current django-delete behavior is basically on-delete-cascade...
so did you mean maybe to implement "ON DELETE SET NULL" and other such behaviors?
or "filtered cascading"?

from what i see here, most of the situations where you want to delete less objects,
than what django wants do delete, are implementable.

in short:

i will write the testcases, if you tell me what kind of situations i need to test :)

also, from the 2 possibilities to avoid duplicate-callbacks, i think the initial bulk-pre-delete signal one is the best.

comment:4 by mh, 16 years ago

honestly, _please_ implement this (by merging this patch or calling the default Manager delete method, I won't be picky about this)

with the current code base (unless there is some magic hidden in db/models/query.py ...) i found no possibility to prevent a object from being deleted - apart from creating an ON DELETE - Trigger in the DB that silently prevents the deletion of a record

this is, however, not possible in all environments (if all databases were full-featured, this whole hack would not need to exist ...), and leads to unnecessary separation/fragmentation of logic. so please fix this issue one way or another

comment:5 by Gábor Farkas, 16 years ago

the following (ugly) code should work:

overload the delete() method of the object like this:

    def delete(self):
        from django.utils.datastructures import SortedDict
        s = SortedDict()
        self._collect_sub_objects(s)
        #here s will contain all the objects that are planned to be deleted
        if s.items() == [(type(u), { u.pk : u })]:
            super(YourModel, self).delete()
        else:
            raise Exception("related objects found")

the code is untested, but i'm doing something very similar and it works.
if you want to make it 100% safe, you have to run it in a serializable-transaction unfortunately :(
otherwise it could happen, that the call to _collect_sub_objects returns
that all is fine, but while the code proceeds to delete-the-object,
some other process already inserted new "dependent" objects,
and those will get deleted.

yes, it contains a call to a private undocumented method.
no, afaik no other way exists.

comment:6 by Gábor Farkas, 15 years ago

if anyone is planning to use above-mentioned code-snippet, please note, that for django-1.0 it has to be changed slightly:

def delete(self):
    from django.db.models.query import CollectedObjects
    s = CollectedObjects()
    self._collect_sub_objects(s)
    #here s will contain all the objects that are planned to be deleted
    if s.items() == [(type(u), { u.pk : u })]:
        super(YourModel, self).delete()
    else:
        raise Exception("related objects found")

yes, this is a good example of why not to use private undocumented methods (because they might change between versions...)

comment:7 by Carl Meyer, 15 years ago

Cc: Carl Meyer added

comment:8 by Thomas Güttler, 15 years ago

Cc: hv@… added

comment:9 by Thomas Güttler, 15 years ago

Related: #7539 Add ON DELETE and ON UPDATE support to Django

comment:10 by anonymous, 14 years ago

Cc: plandry@… added

comment:11 by cgieringer, 14 years ago

Cc: cgieringer added

Related: #6870. My two cents is that the signal's arguments are fine as-is, it just needs to be called at an earlier point in time. Signals are simple and elegant; they tell you just enough information to find the rest of the information you might want. Adding the list of to-be-deleted objects is unnecessary because a signal listener could create this list without the signal including it.

comment:12 by Carl Meyer, 13 years ago

Resolution: wontfix
Status: newclosed

Sending a list of related objects to be deleted with the pre_delete signal is fraught with complications, given that the pre_delete signal is sent once for every individual object to be deleted. Either objects get sent as part of the related-list potentially many times over (in which case how do you reconcile conflicts if the listener in one case removes the object and in another case doesn't?), or you have to make some arbitrary decisions about when a given object is included or not.

In any case, now that #7539 is fixed, all of the justifying use-cases here have workable alternatives.

comment:13 by Thomas Güttler, 12 years ago

Cc: hv@… removed
Easy pickings: unset
Severity: Normal
Type: Uncategorized
UI/UX: unset
Note: See TracTickets for help on using tickets.
Back to Top