From 3cf4ed05a9bc11aa91f4cf10ecbf1c073dcd6b16 Mon Sep 17 00:00:00 2001 From: Holger Brunn Date: Sat, 27 Apr 2024 10:22:05 +0200 Subject: [PATCH 1/7] [FIX] fs_attachment: respect controller parameters for /web/content/ --- fs_attachment/models/ir_binary.py | 28 ++++++++++++++++++++++++++++ fs_attachment/tests/test_stream.py | 11 +++++++++++ 2 files changed, 39 insertions(+) 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/tests/test_stream.py b/fs_attachment/tests/test_stream.py index c7172877bf..f06acec6b1 100644 --- a/fs_attachment/tests/test_stream.py +++ b/fs_attachment/tests/test_stream.py @@ -108,6 +108,17 @@ 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") From 6256db8a230886b8c971121987d8083b47455cf4 Mon Sep 17 00:00:00 2001 From: Holger Brunn Date: Sat, 27 Apr 2024 14:32:29 +0200 Subject: [PATCH 2/7] [FIX] fs_attachment: Rename file in storage when name is written --- fs_attachment/models/ir_attachment.py | 3 +++ fs_attachment/tests/test_fs_attachment.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py index d026fc88f8..44452d1a9d 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 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")) From 3e7700138a34fe32250a911a1895f376da52cbb4 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Mon, 22 Apr 2024 10:08:50 +0200 Subject: [PATCH 3/7] [IMP] fs_attachment: No crash on missing file When trying to get access to files stored into an external filesystem, don't crash if the files cannot be accessed. This is necessary to avoid blocking Odoo's operation if an external system is not accessible. --- fs_attachment/models/ir_attachment.py | 10 ++++++++-- fs_attachment/readme/newsfragments/361.bugfix | 7 +++++++ fs_attachment/static/description/index.html | 11 +++++++---- 3 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 fs_attachment/readme/newsfragments/361.bugfix diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py index 44452d1a9d..0e2836c748 100644 --- a/fs_attachment/models/ir_attachment.py +++ b/fs_attachment/models/ir_attachment.py @@ -368,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 IOError: + _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/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.

From f09cdb0d448d67b6884309d73e4e51d81357dcaa Mon Sep 17 00:00:00 2001 From: Pablo Esteban Date: Wed, 17 Jul 2024 09:40:48 +0200 Subject: [PATCH 4/7] [FIX] fs_attachment: prevent recompute_urls from skipping records with res_field != False --- fs_attachment/models/fs_storage.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) 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() From 9cc4da91278c5c2152d7a01825253d6a4572022f Mon Sep 17 00:00:00 2001 From: Pablo Esteban Date: Wed, 24 Jul 2024 12:44:10 +0200 Subject: [PATCH 5/7] [IMP] fs_attachment: add test for recompute_urls --- fs_attachment/tests/test_fs_storage.py | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/fs_attachment/tests/test_fs_storage.py b/fs_attachment/tests/test_fs_storage.py index e4b4f035c0..8e04b2c2ba 100644 --- a/fs_attachment/tests/test_fs_storage.py +++ b/fs_attachment/tests/test_fs_storage.py @@ -323,3 +323,52 @@ 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) From 949d68d6492d32ee96d9187636b48c6beec7fb41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Mon, 7 Oct 2024 15:31:43 +0200 Subject: [PATCH 6/7] [IMP] fs_attachment OSError is an alias for the deprecated IOError --- fs_attachment/models/ir_attachment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py index 0e2836c748..3379ab721d 100644 --- a/fs_attachment/models/ir_attachment.py +++ b/fs_attachment/models/ir_attachment.py @@ -371,7 +371,7 @@ def _storage_file_read(self, fname: str) -> bytes | None: try: with fs.open(fname, "rb") as f: return f.read() - except IOError: + except OSError: _logger.info( "Error reading %s on storage %s", fname, _storage, exc_info=True ) From 6fd50643ea0107fca7c506105845f140728aa40e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Mon, 7 Oct 2024 15:33:08 +0200 Subject: [PATCH 7/7] [IMP] fs_attachment: wrap long lines --- fs_attachment/tests/test_fs_storage.py | 9 +++++---- fs_attachment/tests/test_stream.py | 5 ++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/fs_attachment/tests/test_fs_storage.py b/fs_attachment/tests/test_fs_storage.py index 8e04b2c2ba..259bf7100f 100644 --- a/fs_attachment/tests/test_fs_storage.py +++ b/fs_attachment/tests/test_fs_storage.py @@ -326,10 +326,11 @@ def test_force_field_and_model_create_attachment(self): 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 + 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 diff --git a/fs_attachment/tests/test_stream.py b/fs_attachment/tests/test_stream.py index f06acec6b1..9d19df60b4 100644 --- a/fs_attachment/tests/test_stream.py +++ b/fs_attachment/tests/test_stream.py @@ -108,7 +108,10 @@ def test_content_url(self): }, assert_content=self.content, ) - url = f"/web/content/{self.attachment_binary.id}/?filename=test2.txt&mimetype=text/csv" + url = ( + f"/web/content/{self.attachment_binary.id}/" + "?filename=test2.txt&mimetype=text/csv" + ) self.assertDownload( url, headers={},