Add delete_orphans keyword in favor of delete signals (#210)

Author: codingjoe

README.md

@@ -67,6 +67,7 @@ class MyModel(models.Model):
         'large': (600, 400),
         'thumbnail': (100, 100, True),
         'medium': (300, 200),
+        delete_orphans=True,
     })
 ```
 
@@ -108,26 +109,30 @@ As storage isn't expensive, you shouldn't restrict upload dimensions.
 If you seek prevent users form overflowing your memory you should restrict the HTTP upload body size.
 
 ### Deleting images
+
 Django [dropped support](https://docs.djangoproject.com/en/dev/releases/1.3/#deleting-a-model-doesn-t-delete-associated-files)
 for automated deletions in version 1.3.
-Implementing file deletion [should be done](http://stackoverflow.com/questions/5372934/how-do-i-get-django-admin-to-delete-files-when-i-remove-an-object-from-the-data)
-inside your own applications using the `post_delete` or `pre_delete` signal.
-Clearing the field if blank is true, does not delete the file. This can also be achieved using `pre_save` and `post_save` signals.
-This packages contains two signal callback methods that handle file deletion for all SdtImageFields of a model.
 
-```python
-from django.db.models.signals import pre_delete, pre_save
-from stdimage.utils import pre_delete_delete_callback, pre_save_delete_callback
+Since version 5, this package supports a `delete_orphans` argument. It will delete
+orphaned files, should a file be delete or replaced via Django form or and object with
+a `StdImageField` be deleted. It will not be deleted if the field value is changed or
+reassigned programatically. In those rare cases, you will need to handle proper deletion
+yourself.
 
-from . import models
+```python
+from django.db import models
+from stdimage.models import StdImageField
 
 
-pre_delete.connect(pre_delete_delete_callback, sender=models.MyModel)
-pre_save.connect(pre_save_delete_callback, sender=models.MyModel)
+class MyModel(models.Model):
+    image = StdImageField(
+        upload_to='path/to/files',
+        variations={'thumbnail': (100, 75)},
+        delete_orphans=True,
+        blank=True,
+    )
 ```
 
-**Warning:** You should not use the signal callbacks in production. They may result in data loss.
-
 ### Async image processing
 Tools like celery allow to execute time-consuming tasks outside of the request. If you don't want
 to wait for your variations to be rendered in request, StdImage provides your the option to pass a

stdimage/models.py

@@ -153,18 +153,15 @@ class StdImageField(ImageField):
     Django ImageField that is able to create different size variations.
 
     Extra features are:
-        - Django-Storages compatible (S3)
-        - Python 2, 3 and PyPy support
-        - Django 1.5 and later support
-        - Resize images to different sizes
-        - Access thumbnails on model level, no template tags required
-        - Preserves original image
-        - Asynchronous rendering (Celery & Co)
-        - Multi threading and processing for optimum performance
-        - Restrict accepted image dimensions
-        - Rename files to a standardized name (using a callable upload_to)
-
-    :param variations: size variations of the image
+
+    -   Django-Storages compatible (S3)
+    -   Access thumbnails on model level, no template tags required
+    -   Preserves original image
+    -   Asynchronous rendering (Celery & Co)
+    -   Multi threading and processing for optimum performance
+    -   Restrict accepted image dimensions
+    -   Rename files to a standardized name (using a callable upload_to)
+
     """
 
     descriptor_class = StdImageFileDescriptor
@@ -177,19 +174,34 @@ class StdImageField(ImageField):
     }
 
     def __init__(self, verbose_name=None, name=None, variations=None,
-                 render_variations=True, force_min_size=False,
-                 *args, **kwargs):
+                 render_variations=True, force_min_size=False, delete_orphans=False,
+                 **kwargs):
         """
         Standardized ImageField for Django.
 
-        Usage: StdImageField(upload_to='PATH',
-         variations={'thumbnail': {"width", "height", "crop", "resample"}})
-        :param variations: size variations of the image
-        :rtype variations: StdImageField
-        :param render_variations: boolean or callable that returns a boolean.
-         The callable gets passed the app_name, model, field_name and pk.
-         Default: True
-        :rtype render_variations: bool, callable
+        Usage::
+
+            StdImageField(
+                upload_to='PATH',
+                variations={
+                    'thumbnail': {"width", "height", "crop", "resample"},
+                },
+                delete_orphans=True,
+            )
+
+        Args:
+            variations (dict):
+                Different size variations of the image.
+            render_variations (bool, callable):
+                Boolean or callable that returns a boolean. If True, the built-in
+                image render will be used. The callable gets passed the ``app_name``,
+                ``model``, ``field_name`` and ``pk``. Default: ``True``
+            delete_orphans (bool):
+                If ``True``, files orphaned files will be removed in case a new file
+                is assigned or the field is cleared. This will only remove work for
+                Django forms. If you unassign or reassign a field in code, you will
+                need to remove the orphaned files yourself.
+
         """
         if not variations:
             variations = {}
@@ -207,6 +219,7 @@ def __init__(self, verbose_name=None, name=None, variations=None,
         self.force_min_size = force_min_size
         self.render_variations = render_variations
         self.variations = {}
+        self.delete_orphans = delete_orphans
 
         for nm, prm in list(variations.items()):
             self.add_variation(nm, prm)
@@ -219,7 +232,7 @@ def __init__(self, verbose_name=None, name=None, variations=None,
                     key=lambda x: x["height"])["height"]
             )
 
-        super().__init__(verbose_name, name, *args, **kwargs)
+        super().__init__(verbose_name=verbose_name, name=name, **kwargs)
 
     def add_variation(self, name, params):
         variation = self.def_variation.copy()
@@ -253,12 +266,24 @@ def set_variations(self, instance=None, **kwargs):
                                                      variation_name)
                     setattr(field, name, variation_field)
 
+    def post_delete_callback(self, sender, instance, **kwargs):
+        getattr(instance, self.name).delete(False)
+
     def contribute_to_class(self, cls, name):
         """Generate all operations on specified signals."""
         super().contribute_to_class(cls, name)
         signals.post_init.connect(self.set_variations, sender=cls)
+        if self.delete_orphans:
+            signals.post_delete.connect(self.post_delete_callback, sender=cls)
 
     def validate(self, value, model_instance):
         super().validate(value, model_instance)
         if self.force_min_size:
             MinSizeValidator(self.min_size[0], self.min_size[1])(value)
+
+    def save_form_data(self, instance, data):
+        if self.delete_orphans and self.blank and (data is False or data is not None):
+            file = getattr(instance, self.name)
+            if file and file._committed and file != data:
+                file.delete(save=False)
+        super().save_form_data(instance, data)

stdimage/utils.py

@@ -1,23 +1,6 @@
 from django.core.files.storage import default_storage
 
-from .models import StdImageField, StdImageFieldFile
-
-
-def pre_delete_delete_callback(sender, instance, **kwargs):
-    for field in instance._meta.fields:
-        if isinstance(field, StdImageField):
-            getattr(instance, field.name).delete(False)
-
-
-def pre_save_delete_callback(sender, instance, **kwargs):
-    if instance.pk:
-        obj = sender.objects.get(pk=instance.pk)
-        for field in instance._meta.fields:
-            if isinstance(field, StdImageField):
-                obj_field = getattr(obj, field.name)
-                instance_field = getattr(instance, field.name)
-                if obj_field and obj_field != instance_field:
-                    obj_field.delete(False)
+from .models import StdImageFieldFile
 
 
 def render_variations(file_name, variations, replace=False,

tests/forms.py

@@ -0,0 +1,10 @@
+from django import forms
+
+from . import models
+
+
+class ThumbnailModelForm(forms.ModelForm):
+
+    class Meta:
+        model = models.ThumbnailModel
+        fields = '__all__'

tests/models.py

@@ -3,13 +3,11 @@
 from django.core.files.base import ContentFile
 from django.core.files.storage import FileSystemStorage
 from django.db import models
-from django.db.models.signals import post_delete, pre_save
 from PIL import Image
 
 from stdimage import StdImageField
 from stdimage.models import StdImageFieldFile
-from stdimage.utils import (pre_delete_delete_callback, pre_save_delete_callback,
-                            render_variations,)
+from stdimage.utils import render_variations
 from stdimage.validators import MaxSizeValidator, MinSizeValidator
 
 upload_to = 'img/'
@@ -24,7 +22,11 @@ class AdminDeleteModel(models.Model):
     """can be deleted through admin"""
     image = StdImageField(
         upload_to=upload_to,
-        blank=True
+        variations={
+            'thumbnail': (100, 75),
+        },
+        blank=True,
+        delete_orphans=True,
     )
 
 
@@ -52,7 +54,8 @@ class ThumbnailModel(models.Model):
     image = StdImageField(
         upload_to=upload_to,
         blank=True,
-        variations={'thumbnail': (100, 75)}
+        variations={'thumbnail': (100, 75)},
+        delete_orphans=True,
     )
 
 
@@ -162,7 +165,3 @@ class CustomRenderVariationsModel(models.Model):
         variations={'thumbnail': (150, 150)},
         render_variations=custom_render_variations,
     )
-
-
-post_delete.connect(pre_delete_delete_callback, sender=SimpleModel)
-pre_save.connect(pre_save_delete_callback, sender=AdminDeleteModel)

tests/test_forms.py

@@ -0,0 +1,48 @@
+import os
+
+from tests.test_models import TestStdImage
+
+from . import forms, models
+
+
+class TestStdImageField(TestStdImage):
+
+    def test_save_form_data__new(self, db):
+        instance = models.ThumbnailModel.objects.create(image=self.fixtures['100.gif'])
+        org_path = instance.image.path
+        assert os.path.exists(org_path)
+        form = forms.ThumbnailModelForm(
+            files=dict(image=self.fixtures['600x400.jpg']),
+            instance=instance,
+        )
+        assert form.is_valid()
+        obj = form.save()
+        assert obj.image.name == 'img/600x400.jpg'
+        assert os.path.exists(instance.image.path)
+        assert not os.path.exists(org_path)
+
+    def test_save_form_data__false(self, db):
+        instance = models.ThumbnailModel.objects.create(image=self.fixtures['100.gif'])
+        org_path = instance.image.path
+        assert os.path.exists(org_path)
+        form = forms.ThumbnailModelForm(
+            data={'image-clear': '1'},
+            instance=instance,
+        )
+        assert form.is_valid()
+        obj = form.save()
+        assert obj.image._file is None
+        assert not os.path.exists(org_path)
+
+    def test_save_form_data__none(self, db):
+        instance = models.ThumbnailModel.objects.create(image=self.fixtures['100.gif'])
+        org_path = instance.image.path
+        assert os.path.exists(org_path)
+        form = forms.ThumbnailModelForm(
+            data={'image': None},
+            instance=instance,
+        )
+        assert form.is_valid()
+        obj = form.save()
+        assert obj.image
+        assert os.path.exists(org_path)

tests/test_models.py

@@ -170,20 +170,30 @@ class TestUtils(TestStdImage):
     """Tests Utils"""
 
     def test_deletion_singnal_receiver(self, db):
-        obj = SimpleModel.objects.create(
+        obj = AdminDeleteModel.objects.create(
             image=self.fixtures['100.gif']
         )
+        path = obj.image.path
         obj.delete()
-        assert not os.path.exists(os.path.join(IMG_DIR, 'image.gif'))
+        assert not os.path.exists(path)
+
+    def test_deletion_singnal_receiver_many(self, db):
+        obj = AdminDeleteModel.objects.create(
+            image=self.fixtures['100.gif']
+        )
+        path = obj.image.path
+        AdminDeleteModel.objects.all().delete()
+        assert not os.path.exists(path)
 
     def test_pre_save_delete_callback_clear(self, admin_client):
-        AdminDeleteModel.objects.create(
+        obj = AdminDeleteModel.objects.create(
             image=self.fixtures['100.gif']
         )
+        path = obj.image.path
         admin_client.post('/admin/tests/admindeletemodel/1/change/', {
             'image-clear': 'checked',
         })
-        assert not os.path.exists(os.path.join(IMG_DIR, 'image.gif'))
+        assert not os.path.exists(path)
 
     def test_pre_save_delete_callback_new(self, admin_client):
         AdminDeleteModel.objects.create(
@@ -195,11 +205,8 @@ def test_pre_save_delete_callback_new(self, admin_client):
         assert not os.path.exists(os.path.join(IMG_DIR, 'image.gif'))
 
     def test_render_variations_callback(self, db):
-        UtilVariationsModel.objects.create(image=self.fixtures['100.gif'])
-        file_path = os.path.join(
-            IMG_DIR,
-            '100.thumbnail.gif'
-        )
+        obj = UtilVariationsModel.objects.create(image=self.fixtures['100.gif'])
+        file_path = obj.image.thumbnail.path
         assert os.path.exists(file_path)