un1t/django-cleanup

Does this package support djangorestframework (drf)?

johnthagen opened this issue ยท 12 comments

I am using DRF to provide a REST API on top of Django. I recently noticed that Django does not automatically delete files on disk when the FileField is deleted, so I was hoping django-cleanup could address this.

setup.py:

INSTALLED_APPS = [
    'grappelli',
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'rest_framework',
    'myapp.apps.MyAppConfig',
    'debug_toolbar',
    'guardian',
    'django_cleanup.apps.CleanupConfig',
]

models.py:

class Project(models.Model):
    pass

class MyModel(models.Model):
    project = models.ForeignKey(Project, related_name='my_models', on_delete=models.CASCADE)
    image = models.ImageField(upload_to='my_images')

But when I do a RESTful DELETE of an instance of Project, the files still don't delete?

The DRF ModelSerializer for Project deletes all of the Project and MyModel instances correctly (the database is empty), but the files remain.

Is there any configuration needed in MyAppConfig to associate the model? Is there a way to debug why django-cleanup doesn't seem to be getting called?

Wanted to check that this wasn't some kind of known limitation.

Also @vinnyrose, I just noticed that our avatars are kindred spirits โค๏ธ ๐Ÿคฃ ๐ŸŽฎ

In case it matters, I'm also using PostgreSQL.

@vinnyrose I'm working on trying to reproduce the issue on a minimal example.

I created a minimal DRF application here: https://github.com/johnthagen/drf-cleanup-test

So far django-cleanup works as expected, so it is compatible with the base DRF at least. I'm guessing that somehow a third party package I'm using (you can see them registered in the INSTALLED_APPS at the top of this thread) is breaking django-cleanup.

I'll try to iteratively add in the third party packages until I can get it to break.

I presume that there is nothing about PostgreSQL that could break django-cleanup? The example project I created uses SQLite for simplicity.

I hope the example project helps as a triage starting point for you as well.

Going back to my actual project, I was able to get an image in a top level Project Model to properly delete, but a lower level MyModel with an image did not. I'll continue to try to determine what the difference is. So django-cleanup is at least registering some of my Model's.

Is there a way to debug which Model's django-cleanup automatically is discovering/hooking?

Is there any configuration needed in MyAppConfig to associate the model?

no

Is there a way to debug why django-cleanup doesn't seem to be getting called?

This app requires that all the following signals be sent for the most robust compatibility:
post_init, pre_save, post_save, post_delete.
(https://docs.djangoproject.com/en/2.2/topics/signals/)

You can test that these are being sent for your models (I put in print statements as an example, but put whatever works for your setup, such as logging or breakpoints, also haven't tested this code):

models.py:
class Project(models.Model):
    pass

class MyModel(models.Model):
    project = models.ForeignKey(Project, related_name='my_models', on_delete=models.CASCADE)
    image = models.ImageField(upload_to='my_images')


handlers.py:
from django.db.models.signals import post_delete, post_init, post_save, pre_save

def post_init_handler(sender, instance, **kwargs):
    print("post_init", sender)

def pre_save_handler(sender, instance, raw, update_fields, using, **kwargs):
    print("pre_save", sender)

def post_save_handler(sender, instance, raw, created, update_fields, using, **kwargs):
    print("post_save", sender)

def post_delete_handler(sender, instance, using, **kwargs):
    print("post_delete", sender)

def connect():
    from .models import Project, MyModel
    post_init.connect(post_init_handler, sender=Project, dispatch_uid='project_post_init')
    pre_save.connect(pre_save_handler, sender=Project, dispatch_uid='project_pre_save')
    post_save.connect(post_save_handler, sender=Project, dispatch_uid='project_post_save')
    post_delete.connect(post_delete_handler, sender=Project, dispatch_uid='project_post_delete')
    post_init.connect(post_init_handler, sender=MyModel, dispatch_uid='mymodel_post_init')
    pre_save.connect(pre_save_handler, sender=MyModel, dispatch_uid='mymodel_pre_save')
    post_save.connect(post_save_handler, sender=MyModel, dispatch_uid='mymodel_post_save')
    post_delete.connect(post_delete_handler, sender=MyModel, dispatch_uid='mymodel_post_delete')

apps.py:
from django.apps import AppConfig

from . import handlers

class MyAppConfig(AppConfig):
    name = 'myapp'

    def ready(self):
        handlers.connect()

For this example, for a deletion, I would expect to see:

post_init <class '...MyModel'>
post_delete <class '...MyModel'>

If you are getting all signals I would suggest looking to where your logs are going, for file deletion exceptions the app logs a message of the format:

logger.exception(
   'There was an exception deleting the file `%s` on field `%s.%s.%s`',
   file_, opts.app_label, opts.model_name, field_name)

(https://docs.djangoproject.com/en/2.2/topics/logging/)

I presume that there is nothing about PostgreSQL that could break django-cleanup? The example project I created uses SQLite for simplicity.

The data layer doesn't matter to this app. The app sits on top of django abstractions.

Is there a way to debug which Model's django-cleanup automatically is discovering/hooking?

If you want to get a picture of the model and field cache you can do something like this:

from django_cleanup.cache import cleanup_fields

fields = cleanup_fields()
for model_name in fields:
        print(model_name, fields[model_name])

On the minimal working example (which deletes things properly) I see:

post_init <class 'snippets.models.Project'>
post_init <class 'snippets.models.Snippet'>
post_delete <class 'snippets.models.Snippet'>
post_delete <class 'snippets.models.Project'>
[21/Sep/2019 14:29:17] "DELETE /projects/2/ HTTP/1.1" 200 9885

In the failing example (trying to delete a MyModel instance):

post_init <class 'models.MyModel'>
post_delete <class 'models.MyModel'>
post_init <class 'models.Project'>
"DELETE /rest/api/layers/2/ HTTP/1.1" 200 11089

Interesting that I see a post_init afterwards. Perhaps because it has a foreign key pointing back to a Project.

So it does look like post_delete is getting signaled in the failing example.

Edit: @vinnyrose Whoa, it started working?

But it only works if I add this minimal code:

from django.db.models.signals import post_delete

def post_delete_handler(sender, instance, using, **kwargs):
    print("post_delete", sender)

class MyAppConfig(AppConfig):
    name = 'my_app'

    def ready(self):
        post_delete(post_delete_handler, sender=MyModel, dispatch_uid='my_model_post_delete')

So only one of my model's signals (Project) are being sent by default? Is it expected to have to do this for all models that have FileField's to ensure proper operation of django-cleanup?

Okay, I've confirmed that for whatever reason, the model that is failing to delete is not automatically found in the cleanup_fields() (but connecting a post_delete handler in the above comment does cause the file to be deleted).

I printed using:

        fields = cleanup_fields()
        for model_name in fields:
            print(model_name, fields[model_name])

And it's missing. The only obvious difference I can see between the FileField's that are found and the the ones that aren't found are that the those that aren't found are using reverse foreign keys back to Project (rather than Project having direct foreign keys down the to Model's (many-to-one vs one-to-one).

How does django-cleanup find FileField's? Could this algorithm have issues finding reverse foreign keys?

So only one models' signals are being sent by default?

If the model is properly registered by django this app will register it too.

Is it expected to have to do this for all models that have FileField's to ensure proper operation of django-cleanup?

no

How does django-cleanup find FileField's?

Using the django app registry.

If your model is not in apps.get_models() during startup then it won't be found:

from django.apps import apps
apps.get_models()

(https://docs.djangoproject.com/en/2.2/ref/applications/#how-applications-are-loaded)

If your model is not in apps.get_models() during startup then it won't be found:

Thanks! We've made some progress! I was using a models/__init__.py package structure and all of my Models were not available at the start of the app, which is why they weren't in cleanup_fields() (not sure how the Django project was still working all this time without it, must have been some lazy loading from DRF?)

Importing all of the models into models/__init__.py solved one of the Models that was not being deleted properly. There is another Model that is picked up properly in cleanup_fields() but is not having it's files deleted. ๐Ÿ˜•

Edit: That is because the Model's themselves were actually being deleted. ๐Ÿคฆโ€โ™‚

I think that the registry was the underlying problem. I think that a troubleshooting section to the README that mentioned this would be really helpful! (I can try to submit a PR when I fix this last problem in my project).

@vinnyrose I was able to get everything working with your help, thanks! Hopefully #53 can help others.

image

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.