Also delete file attr contents

CHANGELOG.rst

@@ -1,8 +1,11 @@
 CHANGELOG
 =========
-5.5.4 (2025-01-09)
+5.5.5 (2025-01-10)
 -------------------
+- More delete file handling improvements
 
+5.5.4 (2025-01-09)
+-------------------
 - Add delete file endpoint
 
 5.5.3 (2024-10-09)

VERSION

@@ -1 +1 @@
-5.5.4
+5.5.5

guillotina/contrib/redis/dm.py

@@ -85,8 +85,8 @@ def get_key(self):
     async def update(self, **kwargs):
         self._data.update(kwargs)
 
-    async def finish(self, values=None):
-        val = await super().finish(values=values)
+    async def finish(self, values=None, clear_data=False):
+        val = await super().finish(values=values, clear_data=clear_data)
         txn = get_transaction()
         txn.add_after_commit_hook(self._delete_key)
         return val

guillotina/files/adapter.py

@@ -89,41 +89,45 @@ async def update(self, **kwargs):
     async def save(self, **kwargs):
         pass
 
-    async def finish(self, values=None):
+    async def finish(self, values=None, clear_data=False):
         # create file object with new data from finished upload
         try:
             file = self.field.get(self.real_context)
         except AttributeError:
             file = None
 
-        if file is None:
-            file = self.file_storage_manager.file_class()
+        if clear_data:
+            file = None
         else:
-            # save previous data on file.
-            # we do this instead of creating a new file object on every
-            # save just in case other implementations want to use the file
-            # object to store different data
-            file._old_uri = file.uri
-            file._old_size = file.size
-            file._old_filename = file.filename
-            file._old_md5 = file.md5
-            file._old_content_type = file.guess_content_type()
-
-            if getattr(file, "_blob", None):
-                cleanup = IFileCleanup(self.context, None)
-                if cleanup is None or cleanup.should_clean(file=file):
-                    bfile = file._blob.open("r")
-                    await bfile.async_del()
-                else:
-                    file._previous_blob = getattr(file, "_blob", None)
+            if file is None:
+                file = self.file_storage_manager.file_class()
+            else:
+                # save previous data on file.
+                # we do this instead of creating a new file object on every
+                # save just in case other implementations want to use the file
+                # object to store different data
+                file._old_uri = file.uri
+                file._old_size = file.size
+                file._old_filename = file.filename
+                file._old_md5 = file.md5
+                file._old_content_type = file.guess_content_type()
+
+                if getattr(file, "_blob", None):
+                    cleanup = IFileCleanup(self.context, None)
+                    if cleanup is None or cleanup.should_clean(file=file):
+                        bfile = file._blob.open("r")
+                        await bfile.async_del()
+                    else:
+                        file._previous_blob = getattr(file, "_blob", None)
+
+            if values is None:
+                values = self._data
+            for key, value in values.items():
+                setattr(file, key, value)
 
         await notify(FileBeforeUploadFinishedEvent(self.context, field=self.field, file=file, dm=self))
 
-        if values is None:
-            values = self._data
         self.field.set(self.real_context, file)
-        for key, value in values.items():
-            setattr(file, key, value)
 
         if self.field.__name__ in getattr(self.context, "__uploads__", {}):
             del self.context.__uploads__[self.field.__name__]
@@ -261,8 +265,11 @@ async def delete(self):
                 blob = file._blob
                 bfile = blob.open("r")
                 await bfile.async_del()
+
+                # Set the file attribute to None
                 self.field.set(self.field.context or self.context, None)
                 self.field.context.register()
+
                 return True
             except AttributeError:
                 pass

guillotina/files/manager.py

@@ -475,7 +475,11 @@ async def delete(self):
         await self.dm.load()
         await self.dm.start()
 
-        result = await self.file_storage_manager.delete()
-        await self.dm.finish()
+        result = False
+        file = self.field.get(self.field.context or self.context)
+        if file:
+            result = await self.file_storage_manager.delete()
+
+        await self.dm.finish(clear_data=True)
 
         return result

guillotina/test_package.py

@@ -445,7 +445,7 @@ async def append(self, dm, iterable, offset) -> int:
         await dm.update(_chunks=chunk_count)
         return count
 
-    async def finish(self, dm):
+    async def finish(self, dm, clear_data=False):
         await dm.update(uri=dm.get("upload_file_id"), upload_file_id=None)
 
     async def exists(self):
@@ -470,9 +470,13 @@ async def copy(self, to_storage_manager, to_dm):
 
     async def delete(self):
         file = self.field.get(self.field.context or self.context)
-        if file.uri in _tmp_files:
+        if file and file.uri in _tmp_files:
             os.remove(_tmp_files[file.uri])
             del _tmp_files[file.uri]
+
+            self.field.set(self.field.context or self.context, None)
+            self.field.context.register()
+
             return True
         return False
 

guillotina/tests/test_attachment.py

@@ -877,6 +877,11 @@ async def test_delete_upload(manager_type, redis_container, container_requester)
         )
         assert status == 201
 
+        # No file to start with
+        response, status = await requester("GET", "/db/guillotina/foobar")
+        assert status == 200
+        assert response["guillotina.behaviors.attachment.IAttachment"]["file"] is None
+
         response, status = await requester(
             "PATCH",
             "/db/guillotina/foobar/@upload/file",
@@ -885,13 +890,24 @@ async def test_delete_upload(manager_type, redis_container, container_requester)
         )
         assert status == 200
 
+        # File attribute is now not empty
+        response, status = await requester("GET", "/db/guillotina/foobar")
+        assert status == 200
+        assert response["guillotina.behaviors.attachment.IAttachment"]["file"] is not None
+
         response, status = await requester("GET", "/db/guillotina/foobar/@download/file")
         assert status == 200
 
         response, status = await requester("PATCH", "/db/guillotina/foobar/@delete/file")
         assert status == 200
         assert response is True
 
+        # Check the file attribute is now empty
+        response, status = await requester("GET", "/db/guillotina/foobar")
+        assert status == 200
+        assert response["guillotina.behaviors.attachment.IAttachment"]["file"] is None
+
+        # False if we try to delete again
         response, status = await requester("PATCH", "/db/guillotina/foobar/@delete/file")
         assert status == 200
         assert response is False