[patch] Add support for a faux ON DELETE RESTRICT functionality

[Jeff Forcier]

I wanted a way to specify that records should not be deleted if they have other records associated with them, similar to SQL's 'ON DELETE RESTRICT'. Currently the only documented behavior for Django is closer to an in-code 'ON DELETE CASCADE'.

To make things a little more complex, I also wanted to quantify what's checked for in the 'RESTRICT'--specifically, sub-items of a record, such as an Address for a Contact, should NOT be part of the RESTRICT, but other, more complex relationships should be checked.

This patch adds a 'restrict_delete' keyword to the META classes of Model classes, which is a boolean that defaults to False (e.g. the normal CASCADE behavior). If this is set to True for a Model class, the following happens on a call to the .delete() method of that class's instances:

• A set consisting of all related fields is constructed, mirroring the calls used to find out what to delete.
• Any fields marked as 'follow' (this includes the implicit 'follow' set on things edited_inline) are taken out of this set - they are the 'sub-items' mentioned above.
• Calls to get_count for the remaining related fields are made, and if any are non-zero, an AssertionError is raised and the deletion does not occur.

This is primarily to scratch a personal itch for a project, but I assume others may want the functionality, and I certainly think that the core devs would want to review delete()'s current behavior at some point as it's quite inflexible. I've seen one unanswered question on django-users asking for something like this, for example :)

[Jacob]

Ooh, cool patch; I like this a lot! Can you update the patch to raise a more specific error? AssertionError doesn't really describe what's happening. I'd suggest creating a new exception in django.core.exceptions.

Also, how does this interact deleting an object in the admin?

Finally, can you please add some unit tests to the patch?

[Jeff Forcier]

Yea, this was a real quick hack job, hence the poor exception subclass choice. I actually was thinking the best solution would be a new one in core.exceptions, but since that file has so few I wasn't sure if this would be important enough to sit among them :) I'll put something in....maybe 'DeletionRestrictedError' or 'IntegrityError' (like from psycopg).

My unit-test fu is extremely weak (nee nonexistent), I'm afraid, but if I find the time in the near future I'll try to improve it and get some put in.

AFAIK if I were to use the admin with this patch installed, it would bomb out upon seeing the raised exception--my view code looks for the exception and act accordingly. However, since it's otherwise identical to the generic view, and if the generic view is used in the admin, I may extend the patch to stick such a try-catch block into said generic view.

[Jeff Forcier]

Updated with new exception subclass (now modifies exceptions.py as well)

[Jeff Forcier]

In retrospect, I really should have paid attention to that little note about naming the patch the same in order to replace it. :) Consider the first attachment deprecated.

I'll look at the generic view / admin deletion stuff tomorrow sometime, I have to roll some code out the door tonight now that I have this particular issue working at all.

Also, if anyone has a better exception name / text than what I have now (IntegrityError, "The requested object could not be deleted because it is referenced by other objects") please speak up :)

[Jacob]

Can you please update this patch to work against magic-removal? Delete functionaility changed somewhat, and this will not longer apply cleanly. The behavior will probably change again when follow goes away, so this may want to be defered until then.

[Jeff Forcier]

Yea, I haven't even so much as glanced at the magic-removal branch since I've been busy using some trunk post-0.91 for work, and have not had the time nor inclination to investigate the new branch.

I plan to take a look once the branch is merged (mostly for personal use) and will try to update the patch at that time.

[Marc Fargas <telenieko@…>]

As far as I understood the patch does a DELETE on all related objects and then checks that it has 0 related objects, if this is that way:

• Hope you have a transactional database and use transactions properly

Otherwise you would end up with part of the related objects deleted!

again, assuming I understood correctly.

PS: to the triage: as the current patch is for pre-magic, maybe it would be better to say "it has no patch, but requires a better one" :)

[Marc Fargas <telenieko@…>]

As far as I understood the patch does a DELETE on all related objects and then checks that it has 0 related objects, if this is that way:

• Hope you have a transactional database and use transactions properly

Otherwise you would end up with part of the related objects deleted!

again, assuming I understood correctly.

PS: to the triage: as the current patch is for pre-magic, maybe it would be better to say "it has no patch, but requires a better one" :)

[Jeff Forcier <jeff@…>]

Marc,

If you read my initial description of the issue, you'll see that what the patch does is gather a list of all related objects, minus the 'followed' ones (usually objects which are "owned by" the one in question, i.e. sub-objects). If it finds non-followed related objects, it raises an error; if it does not, it goes ahead with the deletion as normal.

So it does not perform any deletions until it has determined the state of the object's relationships, and then it is a clean binary decision: delete nothing (and raise an excetion) or delete everything (which was the pre-patch behavior at the time).

Regarding this ticket's current state, I will be upgrading my existing project to use 0.95+ in the very near future (at long last!) and will deal with updating or retracting the patch at that time.

[Marc Fargas <telenieko@…>]

Thanks for clarifying Jeff, I see it clear now!

[Jeff Forcier <reg@…>]

Patch updated (redone, really) for 0.95.1

[Jeff Forcier <reg@…>]

Re: recently attached patch: since things appeared to be re-jiggered a fair amount in the move from 0.91 to 0.95 (in the deletion mechanisms, that is) I had to re-examine my old patch and redo it. The end result should be exactly the same, but it gets there in a more efficient manner now - it just does an check during the iteration in _collect_sub_objects() to see if the current related object is set to follow or not, and triggers the 'RESTRICT'-like behavior if so.

No mucking with sets or calling get_related_many_to_many_objects() prematurely seems to be necessary--or rather, I realized that it was okay to delete items if they have M2M relationships, since those don't appear to trigger the CASCADE-like behavior evidenced with more direct relationships like FKs. Don't remember if that was different back in 0.91.

I just noticed I neglected to add the extra Exception subclass, so I will overwrite the new diff with that included. Since I have no test-fu I will also try and take a whack at that in the near future - learning is good.

[Jeff Forcier <reg@…>]

Updated patch against 0.95.1

[Jeff Forcier <reg@…>]

Got a 'need TRAC_ADMIN' error when trying to overwrite the patch, so now this ticket has even more attachment garbage--hooray! :( Apologies for comment spam.

[Jeff Forcier <jeff@…>]

Note to self, while examining latest trunk and trying to follow latest patch, looks like some line numbers have changed - make sure to double check when updating ticket for unittest.

Note also that contrib/admin/views/main.py::_get_deleted_objects() and friends appear to be the part of the admin dealing with deleted objects, including the similar case of displaying an error for items user lacks perms to delete. This is probably the area to investigate if/when the patch is extended to play well with admin.

[Jeff Forcier <jeff@…>]

Should try to attack the semi-dupe #28 when examining the admin-level implications of this change, namely incorporating options for reparenting objects instead of simply not allowing parent deletion. Primary difference is that the existing patches here deal with non-followed objects, and I assume a solution to #28 would deal only with followed objects. A comprehensive solution should be fairly simple-ish and elegant to boot.

[James Bennett]

Closing in favor of #2288, which has more recent activity.