33

I'm having a problem because I'm deleting a Widget by using some_widget_instance.delete(). I also have a model called WidgetFile with an override delete() method so that I can delete files off my hard drive when a WidgetFile is deleted. The problem I'm having is that if I delete a Widget, and it has WidgetFiles related to it like this:

class WidgetFile(models.Model):

    widget = models.ForeignKey(Widget)

Well, when I delete that Widget, it's WidgetFiles are deleted but the delete() method doesn't trigger and do my extra hard drive stuff. Any help is much appreciated.

orokusaki
  • 55,146
  • 59
  • 179
  • 257
  • This problem arose because when a widget is deleted it doesn't trigger the delete() method on each of it's dependents (classes that have a foreign key reference to it). It simply deletes the related objects from the DB. This makes it more efficient but obviously leads to problems like this. – orokusaki Dec 17 '09 at 19:03

9 Answers9

81

I'm doing the same thing and noticed a nugget in the Django docs that you should think about.

Overriding predefined model methods

Overriding Delete Note that the delete() method for an object is not necessarily called when deleting objects in bulk using a QuerySet. To ensure customized delete logic gets executed, you can use pre_delete and/or post_delete signals.

This means your snippet will not always do what you want. Using Signals is a better option for dealing with deletions.

I went with the following:

import shutil
from django.db.models.signals import pre_delete 
from django.dispatch import receiver

@receiver(pre_delete)
def delete_repo(sender, instance, **kwargs):
    if sender == Set:
        shutil.rmtree(instance.repo)
ElementalVoid
  • 931
  • 1
  • 6
  • 5
  • 3
    What's a `Set`? the model you're monitoring on deleting? – John Wang Nov 07 '19 at 07:31
  • "Signals can make your code harder to maintain. Consider implementing a helper method on a custom manager, to both update your models and perform additional logic, or else overriding model methods before using model signals.", check this out: https://docs.djangoproject.com/en/4.1/ref/signals/ – gianlop3z Nov 16 '22 at 14:44
47

I figured it out. I just put this on that Widget model:

def delete(self):
    files = WidgetFile.objects.filter(widget=self)
    if files:
        for file in files:
            file.delete()
    super(Widget, self).delete()

This triggered the necessary delete() method on each of the related objects, thus triggering my custom file deleting code. It's more database expensive yes, but when you're trying to delete files on a hard drive anyway, it's not such a big expense to hit the db a few extra times.

orokusaki
  • 55,146
  • 59
  • 179
  • 257
  • It's unclear as to why you think Celery or cron would not encounter a similar situation where the file being deleted could be already opened for a read/write operation from another process. In either case, you would have to code to handle the special case. – Joseph Paetz Dec 11 '13 at 19:34
  • 4
    I removed that bit... 4.5 years ago I thought it might be a good idea, but I'm not quite sure why. – orokusaki Mar 31 '14 at 01:23
4

Using clear() prior to deleting, removes all objects from the related object set.

see django-following-relationships-backward

example:

group.link_set.clear() 
group.delete()
panchicore
  • 11,451
  • 12
  • 74
  • 100
1

This seems only be sense-full if one Widget is connected to one WidgetFile exactly. In that case you should use a OneToOneField

from On-to-one examples:

# Delete the restaurant; the waiter should also be removed
>>> r = Restaurant.objects.get(pk=1)
>>> r.delete()
vikingosegundo
  • 52,040
  • 14
  • 137
  • 178
  • true indeed, but Django does a database level mass delete on all waiters without triggering each of their delete methods, which is less expensive, but also less conventional. – orokusaki Oct 08 '09 at 17:07
1

Just to throw in a possible way around this problem: pre-delete signal. (Not in any way implying there's no actual solution.)

che
  • 12,097
  • 7
  • 42
  • 71
1

It should look like described on the django site:

class Blog(models.Model):
    name = models.CharField(max_length=100)
    tagline = models.TextField()
    def save(self, *args, **kwargs):
        do_something()
        super(Blog, self).save(*args, **kwargs) # Call the "real" save() method.
        do_something_else()

http://docs.djangoproject.com/en/dev/topics/db/models/#overriding-predefined-model-methods

you forgot to pass some arguments

Dominik Bartkowski
  • 19
  • 1
0

Is some_widget_instance and instance of Widget or of WidgetFile ? Because if it is an instance of Widget it won't get your custom delete() function, which is in the WidgetFile class.

thornomad
  • 6,707
  • 10
  • 53
  • 78
0

I refactored this answer from the top answer above

import shutil
from django.db.models.signals import pre_delete 
from django.dispatch import receiver


"""WidgetFile is the model for the widget files.
pre_delete means delete run this function before the object is deleted from database"""
@receiver(pre_delete, sender=WidgetFile)
def delete_preo(sender, instance, **kwargs):
    shutil.rmtree(instance.repo)

Override model methods Django signals

NetrobeWebby
  • 76
  • 5
  • 1
    Your answer could be improved with additional supporting information. Please [edit] to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Jul 26 '22 at 10:51
-2

From Django 1.9, if You would just define on_delete=models.CASCADE for field, it will remove all related objects on delete.

CLTanuki
  • 22
  • 2
  • 4
  • 2
    This isn't right. The question is not about cascading deletes. It's about ensuring related `delete` methods are called. – orokusaki Apr 08 '16 at 22:13