diff --git a/README.md b/README.md index 32a9dbe2d2..fd9b4e16b3 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Available addons addon | version | maintainers | summary --- | --- | --- | --- [fs_attachment](fs_attachment/) | 17.0.1.6.2 | lmignon | Store attachments on external object store -[fs_attachment_s3](fs_attachment_s3/) | 17.0.1.2.1 | lmignon | Store attachments into S3 complient filesystem +[fs_attachment_s3](fs_attachment_s3/) | 17.0.1.3.0 | lmignon | Store attachments into S3 complient filesystem [fs_base_multi_image](fs_base_multi_image/) | 17.0.1.0.1 | lmignon | Mulitple Images from External File System [fs_base_multi_media](fs_base_multi_media/) | 17.0.1.0.0 | lmignon | Give the possibility to store media data in external filesystem from odoo [fs_file](fs_file/) | 17.0.1.0.0 | lmignon | Field to store files into filesystem storages diff --git a/fs_attachment_s3/README.rst b/fs_attachment_s3/README.rst index 788dc97b94..fad832a894 100644 --- a/fs_attachment_s3/README.rst +++ b/fs_attachment_s3/README.rst @@ -11,7 +11,7 @@ Fs Attachment S3 !! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - !! source digest: sha256:c01d32f225802fc30d7d79a6130c073016c0d98c69ba20dd6d6c0d1213e9f92d + !! source digest: sha256:4d28167cd71224963df5f53ad5a7672299c585cf6eda6903e5bb499c3aee1718 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png @@ -166,6 +166,7 @@ Contributors - Laurent Mignon laurent.mignon@acsone.eu (https://www.acsone.eu) - Stéphane Bidoul stephane.bidoul@acsone.eu (https://www.acsone.eu) +- Antoni Marroig amarroig@apsl.net (https://apsl.tech) Other credits ------------- diff --git a/fs_attachment_s3/__manifest__.py b/fs_attachment_s3/__manifest__.py index fb52781cdc..472b3ad27b 100644 --- a/fs_attachment_s3/__manifest__.py +++ b/fs_attachment_s3/__manifest__.py @@ -4,7 +4,7 @@ { "name": "Fs Attachment S3", "summary": """Store attachments into S3 complient filesystem""", - "version": "17.0.1.2.1", + "version": "17.0.1.3.0", "license": "AGPL-3", "author": "ACSONE SA/NV,Odoo Community Association (OCA)", "website": "https://github.com/OCA/storage", diff --git a/fs_attachment_s3/i18n/fs_attachment_s3.pot b/fs_attachment_s3/i18n/fs_attachment_s3.pot index bd3d203e80..71aba2e6bf 100644 --- a/fs_attachment_s3/i18n/fs_attachment_s3.pot +++ b/fs_attachment_s3/i18n/fs_attachment_s3.pot @@ -23,6 +23,11 @@ msgstr "" msgid "FS Storage" msgstr "" +#. module: fs_attachment_s3 +#: model:ir.model,name:fs_attachment_s3.model_fs_file_gc +msgid "Filesystem storage file garbage collector" +msgstr "" + #. module: fs_attachment_s3 #: model:ir.model.fields,help:fs_attachment_s3.field_fs_storage__s3_uses_signed_url_for_x_sendfile msgid "" diff --git a/fs_attachment_s3/i18n/it.po b/fs_attachment_s3/i18n/it.po index d03ad91248..54f029f2ca 100644 --- a/fs_attachment_s3/i18n/it.po +++ b/fs_attachment_s3/i18n/it.po @@ -26,11 +26,16 @@ msgstr "Allegato" msgid "FS Storage" msgstr "Deposito FS" +#. module: fs_attachment_s3 +#: model:ir.model,name:fs_attachment_s3.model_fs_file_gc +msgid "Filesystem storage file garbage collector" +msgstr "" + #. module: fs_attachment_s3 #: model:ir.model.fields,help:fs_attachment_s3.field_fs_storage__s3_uses_signed_url_for_x_sendfile msgid "" -"If checked, the storage will use signed URLs for attachments when using " -"X-Accel-Redirect. This is useful for S3 storage where the file path is not " +"If checked, the storage will use signed URLs for attachments when using X-" +"Accel-Redirect. This is useful for S3 storage where the file path is not " "directly accessible without authentication." msgstr "" "Se selezionata, l'archiviazione utilizzerà URL firmati per gli allegati " diff --git a/fs_attachment_s3/models/__init__.py b/fs_attachment_s3/models/__init__.py index 45a28cbdca..2ed0909478 100644 --- a/fs_attachment_s3/models/__init__.py +++ b/fs_attachment_s3/models/__init__.py @@ -1,2 +1,3 @@ from . import fs_storage from . import ir_attachment +from . import fs_file_gc diff --git a/fs_attachment_s3/models/fs_file_gc.py b/fs_attachment_s3/models/fs_file_gc.py new file mode 100644 index 0000000000..ad722ea3f0 --- /dev/null +++ b/fs_attachment_s3/models/fs_file_gc.py @@ -0,0 +1,126 @@ +# Copyright 2026 APSL-Nagarro Antoni Marroig +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). + +import gc +import logging + +from odoo import models + +_logger = logging.getLogger(__name__) + + +class FsFileGc(models.Model): + _inherit = "fs.file.gc" + + # S3 bulk delete limit is 1000 objects per request + _GC_BATCH_SIZE = 1000 + + def _gc_files_unsafe(self) -> None: + """ + Overrides to optimize S3 storage using bulk deletes and + delegates other storage types to the super() method. + """ + # 1. HEAVY LIFTING: Clean up S3 storages first using mass deletion + self._gc_s3_bulk_delete() + + # 2. DELEGATION: Call super() for any remaining storage types + # Since S3 records have already been removed from fs_file_gc, + # super() will only process remaining types (Filestore, SFTP, etc.) + # without hitting timeouts. + return super()._gc_files_unsafe() + + def _gc_s3_bulk_delete(self): + """ + Specific logic for S3 using Boto3 Batch. + Filters storages in memory since autovacuum_gc is not a stored field. + """ + # Search for all storages and filter by the computed field in Python + all_storages = self.env["fs.storage"].search([]) + # We filter those with autovacuum active and verify it's an S3-based path + storages_to_gc = all_storages.filtered( + lambda s: s.autovacuum_gc and s.is_s3_storage + ) + + for storage in storages_to_gc: + code = storage.code + + try: + # directory_path usually contains the bucket name in S3 configurations + bucket_name = storage.get_directory_path().strip("/") + root_fs = storage._get_root_filesystem() + + s3_client = root_fs.s3 + + if not s3_client or not bucket_name: + _logger.warning( + "GC: Could not retrieve S3 client or Bucket name for %s", code + ) + continue + + _logger.info( + "GC: Starting optimized S3 cleanup for %s in bucket %s", + code, + bucket_name, + ) + + while True: + # Select the next batch of orphaned files + self._cr.execute( + """ + SELECT store_fname + FROM fs_file_gc + WHERE fs_storage_code = %s + AND NOT EXISTS ( + SELECT 1 FROM ir_attachment + WHERE store_fname = fs_file_gc.store_fname + ) + LIMIT %s + """, + (code, self._GC_BATCH_SIZE), + ) + + rows = self._cr.fetchall() + if not rows: + break + + fnames = [row[0] for row in rows] + # Prepare the keys by removing the protocol prefix (e.g., s3://) + objects_to_delete = [{"Key": f.partition("://")[2]} for f in fnames] + + try: + # Perform mass deletion (1 HTTP request per 1000 files) + _logger.info( + "GC: Sending bulk delete request to S3 (%s files)", + len(objects_to_delete), + ) + self.env["fs.storage"]._s3_call_delete_objects( + s3_client, + Bucket=bucket_name, + Delete={"Objects": objects_to_delete}, + ) + # Mass delete from database + self._cr.execute( + "DELETE FROM fs_file_gc WHERE store_fname = ANY(%s)", + (fnames,), + ) + + # Commit per batch: releases locks and ensures progress is saved + self._cr.commit() # pylint: disable=invalid-commit + except Exception as e: + _logger.error( + "GC Error: Failed S3 delete_objects for %s: %s", code, e + ) + # We break the loop for this storage + # to prevent DB deletion if S3 fails + break + + # Explicit memory cleanup + gc.collect() + + except Exception as e: + _logger.error( + "GC Error: Could not initialize S3 storage %s: %s", code, e + ) + continue + + _logger.info("GC: Optimized S3 cleanup process finished.") diff --git a/fs_attachment_s3/models/fs_storage.py b/fs_attachment_s3/models/fs_storage.py index 1b737c722d..25170100fe 100644 --- a/fs_attachment_s3/models/fs_storage.py +++ b/fs_attachment_s3/models/fs_storage.py @@ -1,6 +1,8 @@ # Copyright 2025 ACSONE SA/NV # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). +import threading + import fsspec.asyn from odoo import api, fields, models @@ -52,3 +54,24 @@ def _s3_call_generate_presigned_url(self, s3_client, *args, **kwargs): timeout=None, **kwargs, ) + + @api.model + def _s3_call_delete_objects(self, s3_client, *args, **kwargs): + """Delete multiple objects in S3 using the delete_objects API.""" + # s3fs uses aiobotocore as s3 client, which is asynchronous. + # We need to run the async function in a synchronous context. + if self._in_test_mode(): + return s3_client.delete_objects(*args, **kwargs) + return fsspec.asyn.sync( + fsspec.asyn.get_loop(), + s3_client.delete_objects, + *args, + timeout=None, + **kwargs, + ) + + def _in_test_mode(self): + """Check if the current environment is in test mode.""" + return self.env.registry.in_test_mode() or getattr( + threading.current_thread(), "testing", False + ) diff --git a/fs_attachment_s3/readme/CONTRIBUTORS.md b/fs_attachment_s3/readme/CONTRIBUTORS.md index 06d49341ab..b738845804 100644 --- a/fs_attachment_s3/readme/CONTRIBUTORS.md +++ b/fs_attachment_s3/readme/CONTRIBUTORS.md @@ -1,2 +1,3 @@ - Laurent Mignon (https://www.acsone.eu) - Stéphane Bidoul (https://www.acsone.eu) +- Antoni Marroig (https://apsl.tech) diff --git a/fs_attachment_s3/static/description/index.html b/fs_attachment_s3/static/description/index.html index 233c7217f1..cf5e577569 100644 --- a/fs_attachment_s3/static/description/index.html +++ b/fs_attachment_s3/static/description/index.html @@ -372,7 +372,7 @@

Fs Attachment S3

!! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -!! source digest: sha256:c01d32f225802fc30d7d79a6130c073016c0d98c69ba20dd6d6c0d1213e9f92d +!! source digest: sha256:4d28167cd71224963df5f53ad5a7672299c585cf6eda6903e5bb499c3aee1718 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->

Beta License: AGPL-3 OCA/storage Translate me on Weblate Try me on Runboat

This module extends the functionality of @@ -521,6 +521,7 @@

Contributors

diff --git a/fs_attachment_s3/tests/__init__.py b/fs_attachment_s3/tests/__init__.py index a788fc4772..7102627b74 100644 --- a/fs_attachment_s3/tests/__init__.py +++ b/fs_attachment_s3/tests/__init__.py @@ -1 +1,2 @@ from . import test_fs_attachment_s3 +from . import test_fs_file_gc diff --git a/fs_attachment_s3/tests/test_fs_file_gc.py b/fs_attachment_s3/tests/test_fs_file_gc.py new file mode 100644 index 0000000000..27b0132f58 --- /dev/null +++ b/fs_attachment_s3/tests/test_fs_file_gc.py @@ -0,0 +1,77 @@ +# Copyright 2026 APSL-Nagarro Antoni Marroig +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from .common import TestFSAttachmentS3Common + + +class TestFsFileGcS3(TestFSAttachmentS3Common): + def setUp(self): + super().setUp() + self.gc_file_model = self.env["fs.file.gc"] + + def _mark_for_gc(self, *store_fnames): + for store_fname in store_fnames: + self.gc_file_model._mark_for_gc(store_fname) + + def test_gc_s3_bulk_delete_removes_orphaned_files(self): + orphan_1 = "s3tst://dir/sub/orphan_1.txt" + orphan_2 = "s3tst://dir/sub/orphan_2.txt" + referenced = self.fake_attachment_s3.store_fname + self._mark_for_gc(orphan_1, orphan_2, referenced) + + s3_client = MagicMock() + root_fs = SimpleNamespace(s3=s3_client) + + storage_class = type(self.s3_backend) + with ( + patch.object( + storage_class, + "is_s3_storage", + new=property(lambda storage: storage.code == self.s3_backend.code), + ), + patch.object(storage_class, "_get_root_filesystem", return_value=root_fs), + patch.object(type(self.env.cr), "commit", return_value=None), + ): + self.gc_file_model._gc_s3_bulk_delete() + + s3_client.delete_objects.assert_called_once() + _, kwargs = s3_client.delete_objects.call_args + self.assertEqual(kwargs["Bucket"], "test-bucket") + self.assertCountEqual( + kwargs["Delete"]["Objects"], + [ + {"Key": "dir/sub/orphan_1.txt"}, + {"Key": "dir/sub/orphan_2.txt"}, + ], + ) + remaining_files = self.gc_file_model.search( + [("store_fname", "in", [orphan_1, orphan_2, referenced])] + ).mapped("store_fname") + self.assertNotIn(orphan_1, remaining_files) + self.assertNotIn(orphan_2, remaining_files) + self.assertIn(referenced, remaining_files) + + def test_gc_s3_bulk_delete_keeps_rows_when_s3_delete_fails(self): + orphan = "s3tst://dir/sub/orphan.txt" + self._mark_for_gc(orphan) + + s3_client = MagicMock() + s3_client.delete_objects.side_effect = Exception("S3 is unavailable") + root_fs = SimpleNamespace(s3=s3_client) + + storage_class = type(self.s3_backend) + with ( + patch.object( + storage_class, + "is_s3_storage", + new=property(lambda storage: storage.code == self.s3_backend.code), + ), + patch.object(storage_class, "_get_root_filesystem", return_value=root_fs), + patch.object(type(self.env.cr), "commit", return_value=None), + ): + self.gc_file_model._gc_s3_bulk_delete() + + self.assertTrue(self.gc_file_model.search_count([("store_fname", "=", orphan)]))