diff --git a/fs_attachment/models/fs_storage.py b/fs_attachment/models/fs_storage.py index f0d745e6c7..4506da430c 100644 --- a/fs_attachment/models/fs_storage.py +++ b/fs_attachment/models/fs_storage.py @@ -478,8 +478,18 @@ def recompute_urls(self) -> None: in staging are done in a different directory and will not impact the production. """ - attachments = self.env["ir.attachment"].search( - [("fs_storage_id", "in", self.ids)] - ) + # The weird "res_field = False OR res_field != False" domain + # is required! It's because of an override of _search in ir.attachment + # which adds ('res_field', '=', False) when the domain does not + # contain 'res_field'. + # https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/ + # odoo/addons/base/ir/ir_attachment.py#L344-L347 + domain = [ + ("fs_storage_id", "in", self.ids), + "|", + ("res_field", "=", False), + ("res_field", "!=", False), + ] + attachments = self.env["ir.attachment"].search(domain) attachments._compute_fs_url() attachments._compute_fs_url_path() diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py index d026fc88f8..3379ab721d 100644 --- a/fs_attachment/models/ir_attachment.py +++ b/fs_attachment/models/ir_attachment.py @@ -321,6 +321,9 @@ def write(self, vals): ), ).write(vals) + if "name" in vals: + self._enforce_meaningful_storage_filename() + return True @api.model @@ -365,8 +368,14 @@ def _set_attachment_data(self, asbytes) -> None: # pylint: disable=missing-retu def _storage_file_read(self, fname: str) -> bytes | None: """Read the file from the filesystem storage""" fs, _storage, fname = self._fs_parse_store_fname(fname) - with fs.open(fname, "rb") as f: - return f.read() + try: + with fs.open(fname, "rb") as f: + return f.read() + except OSError: + _logger.info( + "Error reading %s on storage %s", fname, _storage, exc_info=True + ) + return b"" @api.model def _storage_file_write(self, bin_data: bytes) -> str: diff --git a/fs_attachment/models/ir_binary.py b/fs_attachment/models/ir_binary.py index b988bb7e4a..1683dd213c 100644 --- a/fs_attachment/models/ir_binary.py +++ b/fs_attachment/models/ir_binary.py @@ -43,6 +43,34 @@ def _record_to_stream(self, record, field_name): return FsStream.from_fs_attachment(fs_attachment) return super()._record_to_stream(record, field_name) + def _get_stream_from( + self, + record, + field_name="raw", + filename=None, + filename_field="name", + mimetype=None, + default_mimetype="application/octet-stream", + ): + stream = super()._get_stream_from( + record, + field_name=field_name, + filename=filename, + filename_field=filename_field, + mimetype=mimetype, + default_mimetype=default_mimetype, + ) + + if stream.type == "fs": + if mimetype: + stream.mimetype = mimetype + if filename: + stream.download_name = filename + elif record and filename_field in record: + stream.download_name = record[filename_field] or stream.download_name + + return stream + def _get_image_stream_from( self, record, diff --git a/fs_attachment/readme/newsfragments/361.bugfix b/fs_attachment/readme/newsfragments/361.bugfix new file mode 100644 index 0000000000..f4bfb546dd --- /dev/null +++ b/fs_attachment/readme/newsfragments/361.bugfix @@ -0,0 +1,7 @@ +No crash o missign file. + +Prior to this change, Odoo was crashing as soon as access to a file stored into +an external filesytem was not possible. This can lead to a complete system block. +This change prevents this kind of blockage by ignoring access error to files +stored into external system on read operations. These kind of errors are logged +into the log files for traceability. diff --git a/fs_attachment/static/description/index.html b/fs_attachment/static/description/index.html index b89ac71868..51a1d40647 100644 --- a/fs_attachment/static/description/index.html +++ b/fs_attachment/static/description/index.html @@ -8,10 +8,11 @@ /* :Author: David Goodger (goodger@python.org) -:Id: $Id: html4css1.css 8954 2022-01-20 10:10:25Z milde $ +:Id: $Id: html4css1.css 9511 2024-01-13 09:50:07Z milde $ :Copyright: This stylesheet has been placed in the public domain. Default cascading style sheet for the HTML output of Docutils. +Despite the name, some widely supported CSS2 features are used. See https://docutils.sourceforge.io/docs/howto/html-stylesheets.html for how to customize this style sheet. @@ -274,7 +275,7 @@ margin-left: 2em ; margin-right: 2em } -pre.code .ln { color: grey; } /* line numbers */ +pre.code .ln { color: gray; } /* line numbers */ pre.code, code { background-color: #eeeeee } pre.code .comment, code .comment { color: #5C6576 } pre.code .keyword, code .keyword { color: #3B0D06; font-weight: bold } @@ -300,7 +301,7 @@ span.pre { white-space: pre } -span.problematic { +span.problematic, pre.problematic { color: red } span.section-subtitle { @@ -777,7 +778,9 @@

Contributors

Maintainers

This module is maintained by the OCA.

-Odoo Community Association + +Odoo Community Association +

OCA, or the Odoo Community Association, is a nonprofit organization whose mission is to support the collaborative development of Odoo features and promote its widespread use.

diff --git a/fs_attachment/tests/test_fs_attachment.py b/fs_attachment/tests/test_fs_attachment.py index 09a72ac3ad..2a943fe75a 100644 --- a/fs_attachment/tests/test_fs_attachment.py +++ b/fs_attachment/tests/test_fs_attachment.py @@ -455,3 +455,14 @@ def test_update_png_to_svg(self): ) self.assertEqual(attachment.mimetype, "image/svg+xml") + + def test_write_name(self): + self.temp_backend.use_as_default_for_attachments = True + attachment = self.ir_attachment_model.create( + {"name": "file.bin", "datas": b"aGVsbG8gd29ybGQK"} + ) + self.assertTrue(attachment.fs_filename.startswith("file-")) + self.assertTrue(attachment.fs_filename.endswith(".bin")) + attachment.write({"name": "file2.txt"}) + self.assertTrue(attachment.fs_filename.startswith("file2-")) + self.assertTrue(attachment.fs_filename.endswith(".txt")) diff --git a/fs_attachment/tests/test_fs_storage.py b/fs_attachment/tests/test_fs_storage.py index e4b4f035c0..259bf7100f 100644 --- a/fs_attachment/tests/test_fs_storage.py +++ b/fs_attachment/tests/test_fs_storage.py @@ -323,3 +323,53 @@ def test_force_field_and_model_create_attachment(self): self.env.flush_all() self.assertFalse(attachment.fs_storage_code) + + def test_recompute_urls(self): + """ + Mark temp_backend as default and set its base_url. Create one attachment + in temp_backend that is linked to a field and one that is not. * Check + that after updating the base_url for the backend, executing + recompute_urls updates fs_url for both attachments, whether they are + linked to a field or not + """ + self.temp_backend.base_url = "https://acsone.eu/media" + self.temp_backend.use_as_default_for_attachments = True + self.ir_attachment_model.create( + { + "name": "field.txt", + "raw": "Attachment linked to a field", + "res_model": "res.partner", + "res_field": "name", + } + ) + self.ir_attachment_model.create( + { + "name": "no_field.txt", + "raw": "Attachment not linked to a field", + } + ) + self.env.flush_all() + + self.env.cr.execute( + f""" + SELECT COUNT(*) + FROM ir_attachment + WHERE fs_storage_id = {self.temp_backend.id} + AND fs_url LIKE '{self.temp_backend.base_url}%' + """ + ) + self.assertEqual(self.env.cr.dictfetchall()[0].get("count"), 2) + + self.temp_backend.base_url = "https://forgeflow.com/media" + self.temp_backend.recompute_urls() + self.env.flush_all() + + self.env.cr.execute( + f""" + SELECT COUNT(*) + FROM ir_attachment + WHERE fs_storage_id = {self.temp_backend.id} + AND fs_url LIKE '{self.temp_backend.base_url}%' + """ + ) + self.assertEqual(self.env.cr.dictfetchall()[0].get("count"), 2) diff --git a/fs_attachment/tests/test_stream.py b/fs_attachment/tests/test_stream.py index c7172877bf..9d19df60b4 100644 --- a/fs_attachment/tests/test_stream.py +++ b/fs_attachment/tests/test_stream.py @@ -108,6 +108,20 @@ def test_content_url(self): }, assert_content=self.content, ) + url = ( + f"/web/content/{self.attachment_binary.id}/" + "?filename=test2.txt&mimetype=text/csv" + ) + self.assertDownload( + url, + headers={}, + assert_status_code=200, + assert_headers={ + "Content-Type": "text/csv; charset=utf-8", + "Content-Disposition": "inline; filename=test2.txt", + }, + assert_content=self.content, + ) def test_image_url(self): self.authenticate("admin", "admin")