diff --git a/fs_attachment/README.rst b/fs_attachment/README.rst
index df4133537f..b58346c6d5 100644
--- a/fs_attachment/README.rst
+++ b/fs_attachment/README.rst
@@ -7,7 +7,7 @@ Base Attachment Object Store
!! This file is generated by oca-gen-addon-readme !!
!! changes will be overwritten. !!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
- !! source digest: sha256:05b68b54a5be6b982d8c997577edfced5e945460759b3869f559cdf1ee5a9a49
+ !! source digest: sha256:a1e2408096e0d95e31cbee39d95122596fd4e0b807750de9a8d28386205f270c
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png
@@ -400,6 +400,7 @@ Stephane Mangin
Laurent Mignon
Marie Lejeune
Wolfgang Pichler
+Nans Lefebvre
Maintainers
~~~~~~~~~~~
diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py
index 8774ed77aa..51dd959af5 100644
--- a/fs_attachment/models/ir_attachment.py
+++ b/fs_attachment/models/ir_attachment.py
@@ -463,7 +463,7 @@ def _enforce_meaningful_storage_filename(self) -> None:
# we need to update the store_fname with the new filename by
# calling the write method of the field since the write method
# of ir_attachment prevent normal write on store_fname
- attachment._force_write_store_fname(f"{storage}://{new_filename}")
+ attachment._force_write_store_fname(f"{storage}://{new_filename_with_path}")
self._fs_mark_for_gc(attachment.store_fname)
def _force_write_store_fname(self, store_fname):
diff --git a/fs_attachment/readme/CONTRIBUTORS.rst b/fs_attachment/readme/CONTRIBUTORS.rst
index 886c390695..d8537f5671 100644
--- a/fs_attachment/readme/CONTRIBUTORS.rst
+++ b/fs_attachment/readme/CONTRIBUTORS.rst
@@ -11,3 +11,4 @@ Stephane Mangin
Laurent Mignon
Marie Lejeune
Wolfgang Pichler
+Nans Lefebvre
diff --git a/fs_attachment/readme/newsfragments/312.bugfix b/fs_attachment/readme/newsfragments/312.bugfix
new file mode 100644
index 0000000000..36fd06d0d5
--- /dev/null
+++ b/fs_attachment/readme/newsfragments/312.bugfix
@@ -0,0 +1 @@
+Fix the error retrieving attachment files when the storage is set to optimize directory paths.
diff --git a/fs_attachment/static/description/index.html b/fs_attachment/static/description/index.html
index d3571bb3d1..9a09c966e9 100644
--- a/fs_attachment/static/description/index.html
+++ b/fs_attachment/static/description/index.html
@@ -367,7 +367,7 @@ Base Attachment Object Store
!! This file is generated by oca-gen-addon-readme !!
!! changes will be overwritten. !!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-!! source digest: sha256:05b68b54a5be6b982d8c997577edfced5e945460759b3869f559cdf1ee5a9a49
+!! source digest: sha256:a1e2408096e0d95e31cbee39d95122596fd4e0b807750de9a8d28386205f270c
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->

In some cases, you need to store attachment in another system that the Odoo’s
@@ -740,7 +740,8 @@
Stephane Mangin <stephane.mangin@camptocamp.com>
Laurent Mignon <laurent.mignon@acsone.eu>
Marie Lejeune <marie.lejeune@acsone.eu>
-Wolfgang Pichler <wpichler@callino.at>
+Wolfgang Pichler <
diff --git a/fs_attachment/tests/common.py b/fs_attachment/tests/common.py
index 95ea76d006..076717a90b 100644
--- a/fs_attachment/tests/common.py
+++ b/fs_attachment/tests/common.py
@@ -21,6 +21,15 @@ def setUpClass(cls):
"directory_path": temp_dir,
}
)
+ cls.backend_optimized = cls.env["fs.storage"].create(
+ {
+ "name": "Temp Optimized FS Storage",
+ "protocol": "file",
+ "code": "tmp_opt",
+ "directory_path": temp_dir,
+ "optimizes_directory_path": True,
+ }
+ )
cls.temp_dir = temp_dir
cls.gc_file_model = cls.env["fs.file.gc"]
cls.ir_attachment_model = cls.env["ir.attachment"]
@@ -40,12 +49,24 @@ def setUp(self):
"directory_path": self.temp_dir,
}
)
+ self.backend_optimized.write(
+ {
+ "protocol": "file",
+ "code": "tmp_opt",
+ "directory_path": self.temp_dir,
+ "optimizes_directory_path": True,
+ }
+ )
def tearDown(self) -> None:
super().tearDown()
# empty the temp dir
for f in os.listdir(self.temp_dir):
- os.remove(os.path.join(self.temp_dir, f))
+ full_path = os.path.join(self.temp_dir, f)
+ if os.path.isfile(full_path):
+ os.remove(full_path)
+ else: # using optimizes_directory_path, we'll have a directory
+ shutil.rmtree(full_path)
class MyException(Exception):
diff --git a/fs_attachment/tests/test_fs_attachment.py b/fs_attachment/tests/test_fs_attachment.py
index 7ba0728b9e..e1f1fde313 100644
--- a/fs_attachment/tests/test_fs_attachment.py
+++ b/fs_attachment/tests/test_fs_attachment.py
@@ -30,6 +30,51 @@ def test_create_attachment_explicit_location(self):
f.write(b"new")
self.assertEqual(attachment.raw, b"new")
+ def test_create_attachment_with_meaningful_name(self):
+ """In this test we use a backend with 'optimizes_directory_path',
+ which rewrites the filename to be a meaningful name.
+ We ensure that the rewritten path is consistently used,
+ meaning we can read the file after.
+ """
+ content = b"This is a test attachment"
+ attachment = (
+ self.env["ir.attachment"]
+ .with_context(
+ storage_location=self.backend_optimized.code,
+ force_storage_key="test.txt",
+ )
+ .create({"name": "test.txt", "raw": content})
+ )
+ # the expected store_fname is made of the storage code,
+ # a random middle part, and the filename
+ # example: tmp_opt://te/st/test-198-0.txt
+ # The storage root is NOT part of the store_fname
+ self.assertFalse("tmp/" in attachment.store_fname)
+
+ # remove protocol and file name to keep the middle part
+ sub_path = os.path.dirname(attachment.store_fname.split("://")[1])
+ # the subpath is consistently 'te/st' because the file storage key is forced
+ # if it's arbitrary we might get a random name (3fbc5er....txt), in which case
+ # the middle part would also be 'random', in our example 3f/bc
+ self.assertEqual(sub_path, "te/st")
+
+ # we can read the file, so storage finds it correctly
+ with attachment.open("rb") as f:
+ self.assertEqual(f.read(), content)
+
+ new_content = b"new content"
+ with attachment.open("wb") as f:
+ f.write(new_content)
+
+ # the store fname should have changed, as its version number has increased
+ # e.g. tmp_opt://te/st/test-1766-0.txt to tmp_opt://te/st/test-1766-1.txt
+ # but the protocol and sub path should be the same
+ new_sub_path = os.path.dirname(attachment.store_fname.split("://")[1])
+ self.assertEqual(sub_path, new_sub_path)
+
+ with attachment.open("rb") as f:
+ self.assertEqual(f.read(), new_content)
+
def test_open_attachment_in_db(self):
self.env["ir.config_parameter"].sudo().set_param("ir_attachment.location", "db")
content = b"This is a test attachment in db"