Stopped automatically cleaning up processed images, always

Author: matthiask

CHANGELOG.rst

@@ -6,6 +6,11 @@ Change log
 Next version
 ~~~~~~~~~~~~
 
+- **Important:** Changed the image field to no longer automatically clean up
+  processed images when deleting the model instance or when changing the image
+  field itself. This behavior made it hard to reuse image field values when
+  copying model data. Instead, processed images are only deleted when the image
+  file itself is deleted (e.g. using ``object.image.delete()``).
 - Rewrote ``process_imagefields`` to use the multiprocessing module, which
   hopefully improves compatibility on macOS.
 - Introduced a ``--no-parallel`` argument to ``process_imagefields``.

imagefield/fields.py

@@ -292,6 +292,15 @@ def save(self, name, content, save=True):  # noqa: FBT002
 
     save.alters_data = True
 
+    def delete(self, save=True):  # noqa: FBT002
+        filename = self.name
+
+        super().delete(save=save)
+
+        self.field._clear_generated_files_for(self, filename)
+
+    delete.alters_data = True
+
 
 def verified(img):
     # Anything which exercises the machinery so that we may
@@ -394,27 +403,13 @@ def save_form_data(self, instance, data):
             if not data and self.ppoi_field:
                 setattr(instance, self.ppoi_field, "0.5x0.5")
 
-    def _cache_previous(self, instance, **kwargs):
-        # TODO We still should find a way to cache the previous value.
-        # See testapp.test_imagefield.Test.test_deferred_imagefields
-        if self.name in instance.get_deferred_fields():
-            return
-        f = getattr(instance, self.name)
-        setattr(instance, "_previous_%s" % self.name, (f.name, f._ppoi()))
-
     def _generate_files(self, instance, **kwargs):
         # Set by the process_imagefields management command
         if getattr(instance, "_skip_generate_files", False):
             return
 
         f = getattr(instance, self.name)
 
-        previous = getattr(instance, "_previous_%s" % self.name, None)
-        # TODO This will not detect replaced/overwritten files.
-        if previous and previous[0] and previous != (f.name, f._ppoi()):
-            logger.info("Clearing generated files for %s", repr(previous))
-            self._clear_generated_files_for(f, previous[0])
-
         if f.name:
             for spec in f.field.formats:
                 f.process(spec)
@@ -484,16 +479,13 @@ def check(self, **kwargs):
 def _register_signal_handlers(sender, **kwargs):
     for field in IMAGEFIELDS:
         if issubclass(sender, field.model):
-            signals.post_init.connect(field._cache_previous, sender=sender)
-
             autogenerate = settings.IMAGEFIELD_AUTOGENERATE
             if (
                 autogenerate is True
                 or autogenerate
                 and field.field_label in autogenerate
             ):
                 signals.post_save.connect(field._generate_files, sender=sender)
-                signals.post_delete.connect(field._clear_generated_files, sender=sender)
 
 
 signals.class_prepared.connect(_register_signal_handlers)

tests/testapp/test_imagefield.py

@@ -55,27 +55,12 @@ def test_model(self):
 
     def test_proxy_model(self):
         """Proxy models should also automatically process images"""
-        m = Model.objects.create(image="python-logo.png")
-        self.assertEqual(
-            contents("__processed__"),
-            ["python-logo-24f8702383e7.png", "python-logo-e6a99ea713c8.png"],
-        )
-        m.delete()
-        self.assertEqual(
-            contents("__processed__"),
-            [],
-        )
 
-        m = ProxyModel.objects.create(image="python-logo.png")
+        ProxyModel.objects.create(image="python-logo.png")
         self.assertEqual(
             contents("__processed__"),
             ["python-logo-24f8702383e7.png", "python-logo-e6a99ea713c8.png"],
         )
-        m.delete()
-        self.assertEqual(
-            contents("__processed__"),
-            [],
-        )
 
     def test_no_ppoi_admin(self):
         client = self.login()
@@ -102,7 +87,7 @@ def test_model_with_optional(self):
         )
 
     def test_upload(self):
-        """Adding and updating images does not leave old thumbs around"""
+        """Adding and updating images creates thumbs"""
         client = self.login()
         self.assertEqual(contents("__processed__"), [])
 
@@ -136,7 +121,12 @@ def test_upload(self):
         self.assertRedirects(response, "/admin/testapp/model/")
         self.assertEqual(
             contents("__processed__"),
-            ["python-logo-096bade32f42.png", "python-logo-2f5189af7eb3.png"],
+            [
+                "python-logo-096bade32f42.png",
+                "python-logo-24f8702383e7.png",
+                "python-logo-2f5189af7eb3.png",
+                "python-logo-e6a99ea713c8.png",
+            ],
         )
 
     def test_autorotate(self):
@@ -306,8 +296,6 @@ def spec(fieldfile, context):
                 "python-logo-e6a99ea713c8.jpg",
             ],
         )
-        m.delete()
-        self.assertEqual(contents("__processed__"), [])
 
     def test_adhoc_lowlevel(self):
         """Low-level processing pipelines; no saving of generated images"""
@@ -502,3 +490,17 @@ def test_invalid_ppoi(self):
 
         m.ppoi = None
         self.assertEqual(m.image._ppoi(), [0.5, 0.5])
+
+    def test_deletion_cleanup(self):
+        """Deleting the image file also deletes processed images"""
+        with openimage("python-logo.png") as f:
+            m = Model()
+            m.image.save("stuff.png", ContentFile(f.read()), save=True)
+
+        self.assertEqual(
+            contents("__processed__"),
+            ["stuff-24f8702383e7.png", "stuff-e6a99ea713c8.png"],
+        )
+
+        m.image.delete()
+        self.assertEqual(contents("__processed__"), [])