From 51cfad681c46fc1124fcddaffa2e1c27cbcf3dbc Mon Sep 17 00:00:00 2001 From: Antoni Marroig Campomar Date: Tue, 28 Apr 2026 14:33:12 +0200 Subject: [PATCH 1/5] [IMP] fs_attachment_s3: Optimize clean up storages using mass deletion --- fs_attachment_s3/README.rst | 7 +- fs_attachment_s3/models/__init__.py | 1 + fs_attachment_s3/models/fs_file_gc.py | 127 ++++++++++++++++++ fs_attachment_s3/models/fs_storage.py | 23 ++++ fs_attachment_s3/readme/CONTRIBUTORS.md | 1 + .../static/description/index.html | 35 +++-- fs_attachment_s3/tests/__init__.py | 1 + fs_attachment_s3/tests/test_fs_file_gc.py | 77 +++++++++++ 8 files changed, 247 insertions(+), 25 deletions(-) create mode 100644 fs_attachment_s3/models/fs_file_gc.py create mode 100644 fs_attachment_s3/tests/test_fs_file_gc.py diff --git a/fs_attachment_s3/README.rst b/fs_attachment_s3/README.rst index 2375322c67..8c0489e1f5 100644 --- a/fs_attachment_s3/README.rst +++ b/fs_attachment_s3/README.rst @@ -1,7 +1,3 @@ -.. image:: https://odoo-community.org/readme-banner-image - :target: https://odoo-community.org/get-involved?utm_source=readme - :alt: Odoo Community Association - ================ Fs Attachment S3 ================ @@ -17,7 +13,7 @@ Fs Attachment S3 .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png :target: https://odoo-community.org/page/development-status :alt: Beta -.. |badge2| image:: https://img.shields.io/badge/license-AGPL--3-blue.png +.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html :alt: License: AGPL-3 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github @@ -155,6 +151,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/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..ac022d00e7 --- /dev/null +++ b/fs_attachment_s3/models/fs_file_gc.py @@ -0,0 +1,127 @@ +# 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 d0150ba30a..9f7537d825 100644 --- a/fs_attachment_s3/static/description/index.html +++ b/fs_attachment_s3/static/description/index.html @@ -3,7 +3,7 @@ -README.rst +Fs Attachment S3 -
+
+

Fs Attachment S3

- - -Odoo Community Association - -
-

Fs Attachment S3

-

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

+

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

This module extends the functionality of fs_attachment to better support Amazon S3 storage. It includes features such as:

@@ -407,7 +402,7 @@

Fs Attachment S3

-

Configuration

+

Configuration

On the Odoo instance, go to Settings > Technical > Storage > File Storage.

When you create a new storage for s3 or modify an existing one, when you @@ -472,11 +467,11 @@

Configuration

directory name as bucket name, and the file path.

-

Changelog

+

Changelog

-

17.0.1.2.0 (2025-10-22)

+

17.0.1.2.0 (2025-10-22)

-

Features

+

Features

  • Adapt to handle {db_name} in directory_path. (#db_name)
    • @@ -485,7 +480,7 @@

    Features

-

Bug Tracker

+

Bug Tracker

Bugs are tracked on GitHub Issues. In case of trouble, please check there if your issue has already been reported. If you spotted it first, help us to smash it by providing a detailed and welcomed @@ -493,22 +488,23 @@

Bug Tracker

Do not contact contributors directly about support or help with technical issues.

-

Credits

+

Credits

-

Authors

+

Authors

  • ACSONE SA/NV
-

Other credits

+

Other credits

The development of this module has been financially supported by:

-

Maintainers

+

Maintainers

This module is maintained by the OCA.

Odoo Community Association @@ -531,6 +527,5 @@

Maintainers

-
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)])) From 0ccc2384468d2b8a2cdf237cfd8b3e842718f5f3 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Tue, 23 Jun 2026 13:50:31 +0200 Subject: [PATCH 2/5] [FIX] fs_attachment_s3: Fix GC bulk delete to S3 s3fs uses aiobotocore as s3 client, which is asynchronous, but Odoo does not support async calls, therefore the call to delete objects from S3 was not working. The fsspec library provides a way to run async calls in a synchronous way. The fsspec.asyn.sync function is now used to run the async call to delete objects from S3, in a synchronous way, in order to make it work in Odoo. --- fs_attachment_s3/models/fs_file_gc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/fs_attachment_s3/models/fs_file_gc.py b/fs_attachment_s3/models/fs_file_gc.py index ac022d00e7..ad722ea3f0 100644 --- a/fs_attachment_s3/models/fs_file_gc.py +++ b/fs_attachment_s3/models/fs_file_gc.py @@ -98,7 +98,6 @@ def _gc_s3_bulk_delete(self): 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)", From 07170252a57f616a62c2c836d6e10d5ccfc6786d Mon Sep 17 00:00:00 2001 From: oca-ci Date: Wed, 15 Jul 2026 08:13:34 +0000 Subject: [PATCH 3/5] [UPD] Update fs_attachment_s3.pot --- fs_attachment_s3/i18n/fs_attachment_s3.pot | 5 +++++ 1 file changed, 5 insertions(+) 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 "" From d559e1b48db46c0ad18d30eba7c41fa6ebdf501b Mon Sep 17 00:00:00 2001 From: OCA-git-bot Date: Wed, 15 Jul 2026 08:16:58 +0000 Subject: [PATCH 4/5] [BOT] post-merge updates --- README.md | 2 +- fs_attachment_s3/README.rst | 8 ++++++-- fs_attachment_s3/__manifest__.py | 2 +- .../static/description/index.html | 20 ++++++++++++------- 4 files changed, 21 insertions(+), 11 deletions(-) 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 ff3707f1b3..fad832a894 100644 --- a/fs_attachment_s3/README.rst +++ b/fs_attachment_s3/README.rst @@ -1,3 +1,7 @@ +.. image:: https://odoo-community.org/readme-banner-image + :target: https://odoo-community.org/get-involved?utm_source=readme + :alt: Odoo Community Association + ================ Fs Attachment S3 ================ @@ -7,13 +11,13 @@ 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 :target: https://odoo-community.org/page/development-status :alt: Beta -.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png +.. |badge2| image:: https://img.shields.io/badge/license-AGPL--3-blue.png :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html :alt: License: AGPL-3 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github 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/static/description/index.html b/fs_attachment_s3/static/description/index.html index 68eb5b0513..cf5e577569 100644 --- a/fs_attachment_s3/static/description/index.html +++ b/fs_attachment_s3/static/description/index.html @@ -3,7 +3,7 @@ -Fs Attachment S3 +README.rst -
-

Fs Attachment S3

+
+ + +Odoo Community Association + +
+

Fs Attachment S3

-

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

+

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

This module extends the functionality of fs_attachment to better support Amazon S3 storage. It includes features such as:

@@ -406,7 +411,7 @@

Fs Attachment S3

-

Configuration

+

Configuration

On the Odoo instance, go to Settings > Technical > Storage > File Storage.

When you create a new storage for s3 or modify an existing one, when you @@ -471,7 +476,7 @@

Configuration

directory name as bucket name, and the file path.

+
From 674fac264bca1c74159d4aaac415187670f7d3ae Mon Sep 17 00:00:00 2001 From: Weblate Date: Wed, 15 Jul 2026 08:17:07 +0000 Subject: [PATCH 5/5] Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: storage-17.0/storage-17.0-fs_attachment_s3 Translate-URL: https://translation.odoo-community.org/projects/storage-17-0/storage-17-0-fs_attachment_s3/ --- fs_attachment_s3/i18n/it.po | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 "