Changing "Photo" image leaves extra files on server

[gjstein]

Changing the image property of the ImageModel class from the admin interface leaves a number of files on the server. When the save function is called, it relies on a call to self.clear_cache() to delete the images from the server. However, value of self.image is changes before save is called, which means that it is looking for the images in the wrong place. Rather, it should be using the value of self._old_image.name for calculations.

There is no-doubt a more elegant way to factor the code, but I solved the problem by modifying a portion of the save function in the ImageModel class:

    def save(self, *args, **kwargs):

        image_has_changed = False
        if self._get_pk_val() and (self._old_image != self.image):
            image_has_changed = True

            # ==== BEGIN CHANGES ====

            # Delete base image
            self._old_image.storage.delete(self._old_image.name)

            # Get components of filenames
            cache_path = os.path.join(os.path.dirname(self._old_image.name), "cache")
            old_filename = os.path.basename(force_text(self._old_image.name))
            base, ext = os.path.splitext(old_filename)
            cache = PhotoSizeCache()

            # Loop through file sizes
            for photosize in cache.sizes.values():
                # Get filename & delete files
                size_name = photosize.name
                filename = os.path.join(cache_path,''.join([base, '_', size_name, ext]))
                if self._old_image.storage.exists(filename):
                    self._old_image.storage.delete(filename)

            # ==== END CHANGES ====

        if self.date_taken is None or image_has_changed:
            # Attempt to get the date the photo was taken from the EXIF data.
            try:
                exif_date = self.EXIF(self.image.file).get('EXIF DateTimeOriginal', None)
                if exif_date is not None:
                    d, t = str.split(exif_date.values)
                    year, month, day = d.split(':')
                    hour, minute, second = t.split(':')
                    self.date_taken = datetime(int(year), int(month), int(day),
                                               int(hour), int(minute), int(second))
            except:
                logger.error('Failed to read EXIF DateTimeOriginal', exc_info=True)
        super(ImageModel, self).save(*args, **kwargs)
        self.pre_cache()

[richardbarran]

Thanks for spotting the bug and taking the time to write a fix! I took your code and modified it slightly, basically tricking clear_cache to operate on the old image:

 new_image = self.image
 self.image = self._old_image
 self.clear_cache()
 self.image = new_image  # Back to the new image.
 self._old_image.storage.delete(self._old_image.name)  # Delete (old) base image.

Could you try this out please? It works for me, I'd just like a second opinion that all is ok :-)

[gjstein]

LGTM. I just tried it out on my server and it's a drop-in replacement for mine. Thanks for the quick fix!

[richardbarran]

I've just committed some code that should fix this - thanks for your help!