Skip to content

Commit cd04d34

Browse files
committed
Merge PR #599 into 17.0
Signed-off-by lmignon
2 parents 513e440 + 954b69d commit cd04d34

8 files changed

Lines changed: 238 additions & 17 deletions

File tree

fs_attachment_s3/README.rst

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
.. image:: https://odoo-community.org/readme-banner-image
2-
:target: https://odoo-community.org/get-involved?utm_source=readme
3-
:alt: Odoo Community Association
4-
51
================
62
Fs Attachment S3
73
================
@@ -17,7 +13,7 @@ Fs Attachment S3
1713
.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png
1814
:target: https://odoo-community.org/page/development-status
1915
:alt: Beta
20-
.. |badge2| image:: https://img.shields.io/badge/license-AGPL--3-blue.png
16+
.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png
2117
:target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
2218
:alt: License: AGPL-3
2319
.. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github
@@ -166,6 +162,7 @@ Contributors
166162

167163
- Laurent Mignon laurent.mignon@acsone.eu (https://www.acsone.eu)
168164
- Stéphane Bidoul stephane.bidoul@acsone.eu (https://www.acsone.eu)
165+
- Antoni Marroig amarroig@apsl.net (https://apsl.tech)
169166

170167
Other credits
171168
-------------
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
from . import fs_storage
22
from . import ir_attachment
3+
from . import fs_file_gc
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
# Copyright 2026 APSL-Nagarro Antoni Marroig
2+
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
3+
4+
import gc
5+
import logging
6+
7+
from odoo import models
8+
9+
_logger = logging.getLogger(__name__)
10+
11+
12+
class FsFileGc(models.Model):
13+
_inherit = "fs.file.gc"
14+
15+
# S3 bulk delete limit is 1000 objects per request
16+
_GC_BATCH_SIZE = 1000
17+
18+
def _gc_files_unsafe(self) -> None:
19+
"""
20+
Overrides to optimize S3 storage using bulk deletes and
21+
delegates other storage types to the super() method.
22+
"""
23+
# 1. HEAVY LIFTING: Clean up S3 storages first using mass deletion
24+
self._gc_s3_bulk_delete()
25+
26+
# 2. DELEGATION: Call super() for any remaining storage types
27+
# Since S3 records have already been removed from fs_file_gc,
28+
# super() will only process remaining types (Filestore, SFTP, etc.)
29+
# without hitting timeouts.
30+
return super()._gc_files_unsafe()
31+
32+
def _gc_s3_bulk_delete(self):
33+
"""
34+
Specific logic for S3 using Boto3 Batch.
35+
Filters storages in memory since autovacuum_gc is not a stored field.
36+
"""
37+
# Search for all storages and filter by the computed field in Python
38+
all_storages = self.env["fs.storage"].search([])
39+
# We filter those with autovacuum active and verify it's an S3-based path
40+
storages_to_gc = all_storages.filtered(
41+
lambda s: s.autovacuum_gc and s.is_s3_storage
42+
)
43+
44+
for storage in storages_to_gc:
45+
code = storage.code
46+
47+
try:
48+
# directory_path usually contains the bucket name in S3 configurations
49+
bucket_name = storage.get_directory_path().strip("/")
50+
root_fs = storage._get_root_filesystem()
51+
52+
s3_client = root_fs.s3
53+
54+
if not s3_client or not bucket_name:
55+
_logger.warning(
56+
"GC: Could not retrieve S3 client or Bucket name for %s", code
57+
)
58+
continue
59+
60+
_logger.info(
61+
"GC: Starting optimized S3 cleanup for %s in bucket %s",
62+
code,
63+
bucket_name,
64+
)
65+
66+
while True:
67+
# Select the next batch of orphaned files
68+
self._cr.execute(
69+
"""
70+
SELECT store_fname
71+
FROM fs_file_gc
72+
WHERE fs_storage_code = %s
73+
AND NOT EXISTS (
74+
SELECT 1 FROM ir_attachment
75+
WHERE store_fname = fs_file_gc.store_fname
76+
)
77+
LIMIT %s
78+
""",
79+
(code, self._GC_BATCH_SIZE),
80+
)
81+
82+
rows = self._cr.fetchall()
83+
if not rows:
84+
break
85+
86+
fnames = [row[0] for row in rows]
87+
# Prepare the keys by removing the protocol prefix (e.g., s3://)
88+
objects_to_delete = [{"Key": f.partition("://")[2]} for f in fnames]
89+
90+
try:
91+
# Perform mass deletion (1 HTTP request per 1000 files)
92+
_logger.info(
93+
"GC: Sending bulk delete request to S3 (%s files)",
94+
len(objects_to_delete),
95+
)
96+
self.env["fs.storage"]._s3_call_delete_objects(
97+
s3_client,
98+
Bucket=bucket_name,
99+
Delete={"Objects": objects_to_delete},
100+
)
101+
# Mass delete from database
102+
self._cr.execute(
103+
"DELETE FROM fs_file_gc WHERE store_fname = ANY(%s)",
104+
(fnames,),
105+
)
106+
107+
# Commit per batch: releases locks and ensures progress is saved
108+
self._cr.commit() # pylint: disable=invalid-commit
109+
except Exception as e:
110+
_logger.error(
111+
"GC Error: Failed S3 delete_objects for %s: %s", code, e
112+
)
113+
# We break the loop for this storage
114+
# to prevent DB deletion if S3 fails
115+
break
116+
117+
# Explicit memory cleanup
118+
gc.collect()
119+
120+
except Exception as e:
121+
_logger.error(
122+
"GC Error: Could not initialize S3 storage %s: %s", code, e
123+
)
124+
continue
125+
126+
_logger.info("GC: Optimized S3 cleanup process finished.")

fs_attachment_s3/models/fs_storage.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# Copyright 2025 ACSONE SA/NV
22
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
33

4+
import threading
5+
46
import fsspec.asyn
57

68
from odoo import api, fields, models
@@ -52,3 +54,24 @@ def _s3_call_generate_presigned_url(self, s3_client, *args, **kwargs):
5254
timeout=None,
5355
**kwargs,
5456
)
57+
58+
@api.model
59+
def _s3_call_delete_objects(self, s3_client, *args, **kwargs):
60+
"""Delete multiple objects in S3 using the delete_objects API."""
61+
# s3fs uses aiobotocore as s3 client, which is asynchronous.
62+
# We need to run the async function in a synchronous context.
63+
if self._in_test_mode():
64+
return s3_client.delete_objects(*args, **kwargs)
65+
return fsspec.asyn.sync(
66+
fsspec.asyn.get_loop(),
67+
s3_client.delete_objects,
68+
*args,
69+
timeout=None,
70+
**kwargs,
71+
)
72+
73+
def _in_test_mode(self):
74+
"""Check if the current environment is in test mode."""
75+
return self.env.registry.in_test_mode() or getattr(
76+
threading.current_thread(), "testing", False
77+
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
- Laurent Mignon <laurent.mignon@acsone.eu> (https://www.acsone.eu)
22
- Stéphane Bidoul <stephane.bidoul@acsone.eu> (https://www.acsone.eu)
3+
- Antoni Marroig <amarroig@apsl.net> (https://apsl.tech)

fs_attachment_s3/static/description/index.html

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<head>
44
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
55
<meta name="generator" content="Docutils: https://docutils.sourceforge.io/" />
6-
<title>README.rst</title>
6+
<title>Fs Attachment S3</title>
77
<style type="text/css">
88

99
/*
@@ -360,21 +360,16 @@
360360
</style>
361361
</head>
362362
<body>
363-
<div class="document">
363+
<div class="document" id="fs-attachment-s3">
364+
<h1 class="title">Fs Attachment S3</h1>
364365

365-
366-
<a class="reference external image-reference" href="https://odoo-community.org/get-involved?utm_source=readme">
367-
<img alt="Odoo Community Association" src="https://odoo-community.org/readme-banner-image" />
368-
</a>
369-
<div class="section" id="fs-attachment-s3">
370-
<h1>Fs Attachment S3</h1>
371366
<!-- !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
372367
!! This file is generated by oca-gen-addon-readme !!
373368
!! changes will be overwritten. !!
374369
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
375370
!! source digest: sha256:c01d32f225802fc30d7d79a6130c073016c0d98c69ba20dd6d6c0d1213e9f92d
376371
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
377-
<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/license-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/storage/tree/17.0/fs_attachment_s3"><img alt="OCA/storage" src="https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/storage-17-0/storage-17-0-fs_attachment_s3"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/storage&amp;target_branch=17.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
372+
<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/storage/tree/17.0/fs_attachment_s3"><img alt="OCA/storage" src="https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/storage-17-0/storage-17-0-fs_attachment_s3"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/storage&amp;target_branch=17.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
378373
<p>This module extends the functionality of
379374
<a class="reference external" href="https://github.com/OCA/storage/tree/16.0/fs_attachment">fs_attachment</a>
380375
to better support Amazon S3 storage. It includes features such as:</p>
@@ -411,7 +406,7 @@ <h1>Fs Attachment S3</h1>
411406
</ul>
412407
</div>
413408
<div class="section" id="configuration">
414-
<h2><a class="toc-backref" href="#toc-entry-1">Configuration</a></h2>
409+
<h1><a class="toc-backref" href="#toc-entry-1">Configuration</a></h1>
415410
<p>On the Odoo instance, go to <em>Settings</em> &gt; <em>Technical</em> &gt; <em>Storage</em> &gt; <em>File
416411
Storage</em>.</p>
417412
<p>When you create a new storage for s3 or modify an existing one, when you
@@ -476,7 +471,7 @@ <h2><a class="toc-backref" href="#toc-entry-1">Configuration</a></h2>
476471
directory name as bucket name, and the file path.</p>
477472
</div>
478473
<div class="section" id="changelog">
479-
<h2><a class="toc-backref" href="#toc-entry-2">Changelog</a></h2>
474+
<h1><a class="toc-backref" href="#toc-entry-2">Changelog</a></h1>
480475
<div class="section" id="section-1">
481476
<h3><a class="toc-backref" href="#toc-entry-3">17.0.1.2.1 (2026-05-27)</a></h3>
482477
<div class="section" id="bugfixes">
@@ -521,6 +516,7 @@ <h3><a class="toc-backref" href="#toc-entry-10">Contributors</a></h3>
521516
<ul class="simple">
522517
<li>Laurent Mignon <a class="reference external" href="mailto:laurent.mignon&#64;acsone.eu">laurent.mignon&#64;acsone.eu</a> (<a class="reference external" href="https://www.acsone.eu">https://www.acsone.eu</a>)</li>
523518
<li>Stéphane Bidoul <a class="reference external" href="mailto:stephane.bidoul&#64;acsone.eu">stephane.bidoul&#64;acsone.eu</a> (<a class="reference external" href="https://www.acsone.eu">https://www.acsone.eu</a>)</li>
519+
<li>Antoni Marroig <a class="reference external" href="mailto:amarroig&#64;apsl.net">amarroig&#64;apsl.net</a> (<a class="reference external" href="https://apsl.tech">https://apsl.tech</a>)</li>
524520
</ul>
525521
</div>
526522
<div class="section" id="other-credits">
@@ -547,6 +543,5 @@ <h3><a class="toc-backref" href="#toc-entry-12">Maintainers</a></h3>
547543
</div>
548544
</div>
549545
</div>
550-
</div>
551546
</body>
552547
</html>

fs_attachment_s3/tests/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
from . import test_fs_attachment_s3
2+
from . import test_fs_file_gc
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Copyright 2026 APSL-Nagarro Antoni Marroig
2+
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
3+
4+
from types import SimpleNamespace
5+
from unittest.mock import MagicMock, patch
6+
7+
from .common import TestFSAttachmentS3Common
8+
9+
10+
class TestFsFileGcS3(TestFSAttachmentS3Common):
11+
def setUp(self):
12+
super().setUp()
13+
self.gc_file_model = self.env["fs.file.gc"]
14+
15+
def _mark_for_gc(self, *store_fnames):
16+
for store_fname in store_fnames:
17+
self.gc_file_model._mark_for_gc(store_fname)
18+
19+
def test_gc_s3_bulk_delete_removes_orphaned_files(self):
20+
orphan_1 = "s3tst://dir/sub/orphan_1.txt"
21+
orphan_2 = "s3tst://dir/sub/orphan_2.txt"
22+
referenced = self.fake_attachment_s3.store_fname
23+
self._mark_for_gc(orphan_1, orphan_2, referenced)
24+
25+
s3_client = MagicMock()
26+
root_fs = SimpleNamespace(s3=s3_client)
27+
28+
storage_class = type(self.s3_backend)
29+
with (
30+
patch.object(
31+
storage_class,
32+
"is_s3_storage",
33+
new=property(lambda storage: storage.code == self.s3_backend.code),
34+
),
35+
patch.object(storage_class, "_get_root_filesystem", return_value=root_fs),
36+
patch.object(type(self.env.cr), "commit", return_value=None),
37+
):
38+
self.gc_file_model._gc_s3_bulk_delete()
39+
40+
s3_client.delete_objects.assert_called_once()
41+
_, kwargs = s3_client.delete_objects.call_args
42+
self.assertEqual(kwargs["Bucket"], "test-bucket")
43+
self.assertCountEqual(
44+
kwargs["Delete"]["Objects"],
45+
[
46+
{"Key": "dir/sub/orphan_1.txt"},
47+
{"Key": "dir/sub/orphan_2.txt"},
48+
],
49+
)
50+
remaining_files = self.gc_file_model.search(
51+
[("store_fname", "in", [orphan_1, orphan_2, referenced])]
52+
).mapped("store_fname")
53+
self.assertNotIn(orphan_1, remaining_files)
54+
self.assertNotIn(orphan_2, remaining_files)
55+
self.assertIn(referenced, remaining_files)
56+
57+
def test_gc_s3_bulk_delete_keeps_rows_when_s3_delete_fails(self):
58+
orphan = "s3tst://dir/sub/orphan.txt"
59+
self._mark_for_gc(orphan)
60+
61+
s3_client = MagicMock()
62+
s3_client.delete_objects.side_effect = Exception("S3 is unavailable")
63+
root_fs = SimpleNamespace(s3=s3_client)
64+
65+
storage_class = type(self.s3_backend)
66+
with (
67+
patch.object(
68+
storage_class,
69+
"is_s3_storage",
70+
new=property(lambda storage: storage.code == self.s3_backend.code),
71+
),
72+
patch.object(storage_class, "_get_root_filesystem", return_value=root_fs),
73+
patch.object(type(self.env.cr), "commit", return_value=None),
74+
):
75+
self.gc_file_model._gc_s3_bulk_delete()
76+
77+
self.assertTrue(self.gc_file_model.search_count([("store_fname", "=", orphan)]))

0 commit comments

Comments
 (0)