From a67d78b4edd0f950288e42d4e4a21cfb8afcdda7 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Mon, 7 Oct 2024 14:43:32 +0200 Subject: [PATCH 01/10] oca-port: store 'fs_storage' data --- .oca/oca-port/blacklist/fs_storage.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .oca/oca-port/blacklist/fs_storage.json diff --git a/.oca/oca-port/blacklist/fs_storage.json b/.oca/oca-port/blacklist/fs_storage.json new file mode 100644 index 0000000000..c2c7f1f4bb --- /dev/null +++ b/.oca/oca-port/blacklist/fs_storage.json @@ -0,0 +1,5 @@ +{ + "pull_requests": { + "OCA/storage#252": "Don't need initial requirements.txt" + } +} From f7c6533c4a3a0b5b0461e6fd1ba09fba68f1c08d Mon Sep 17 00:00:00 2001 From: JordiMForgeFlow Date: Fri, 12 Jan 2024 10:59:15 +0100 Subject: [PATCH 02/10] [IMP] fs_storage: invalidate filesystem cache when connection fails --- fs_storage/models/fs_storage.py | 23 ++++++++++++++++++++- fs_storage/readme/newsfragments/320.feature | 1 + fs_storage/static/description/index.html | 11 ++++++---- 3 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 fs_storage/readme/newsfragments/320.feature diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index b811e59d84..cf0e646e85 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -265,12 +265,33 @@ def _compute_options_properties(self) -> None: doc = inspect.getdoc(cls.__init__) rec.options_properties = f"__init__{signature}\n{doc}" + def _get_marker_file_name(self): + return ".odoo_fs_storage_%s.marker" % self.id + + def _check_connection(self, fs): + marker_file_name = self._get_marker_file_name() + try: + marker_file = fs.ls(marker_file_name, detail=False) + if not marker_file: + fs.touch(marker_file_name) + except FileNotFoundError: + fs.touch(marker_file_name) + return True + @property def fs(self) -> fsspec.AbstractFileSystem: """Get the fsspec filesystem for this backend.""" self.ensure_one() if not self.__fs: self.__fs = self._get_filesystem() + if not tools.config["test_enable"]: + # Check whether we need to invalidate FS cache or not. + # Use a marker file to limit the scope of the LS command for performance. + try: + self._check_connection(self.__fs) + except Exception as e: + self.__fs.clear_instance_cache() + raise e return self.__fs def _get_filesystem_storage_path(self) -> str: @@ -432,7 +453,7 @@ def delete(self, relative_path) -> None: def action_test_config(self) -> None: try: - self.fs.ls("", detail=False) + self._check_connection(self.__fs) title = _("Connection Test Succeeded!") message = _("Everything seems properly set up!") msg_type = "success" diff --git a/fs_storage/readme/newsfragments/320.feature b/fs_storage/readme/newsfragments/320.feature new file mode 100644 index 0000000000..5644d66085 --- /dev/null +++ b/fs_storage/readme/newsfragments/320.feature @@ -0,0 +1 @@ +Invalidate FS filesystem object cache when the connection fails, forcing a reconnection. diff --git a/fs_storage/static/description/index.html b/fs_storage/static/description/index.html index 3214c60426..5531ca73e6 100644 --- a/fs_storage/static/description/index.html +++ b/fs_storage/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 { @@ -619,7 +620,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 a0be94f43b91569576c4913e900b3e7f6dabdee2 Mon Sep 17 00:00:00 2001 From: Tran Anh Tuan Date: Thu, 4 Jan 2024 17:35:18 +0700 Subject: [PATCH 03/10] [IMP] fs_storage: improve find_files pattern match, pattern matching should only apply to file name, not full path --- fs_storage/models/fs_storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index cf0e646e85..ea5352e380 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -428,7 +428,8 @@ def find_files(self, pattern, relative_path="", **kw) -> list[str]: return [] regex = re.compile(pattern) for file_path in self.fs.ls(relative_path, detail=False): - if regex.match(file_path): + # fs.ls returns a relative path + if regex.match(os.path.basename(file_path)): result.append(file_path) return result From ee02e1823a189303268e4c753632c0fce2687aec Mon Sep 17 00:00:00 2001 From: Nils Hamerlinck Date: Wed, 7 Feb 2024 14:17:55 +0700 Subject: [PATCH 04/10] [IMP] fs_storage: use fs.info() to check for the marker file --- fs_storage/models/fs_storage.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index ea5352e380..88b33498c6 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -271,9 +271,7 @@ def _get_marker_file_name(self): def _check_connection(self, fs): marker_file_name = self._get_marker_file_name() try: - marker_file = fs.ls(marker_file_name, detail=False) - if not marker_file: - fs.touch(marker_file_name) + fs.info(marker_file_name) except FileNotFoundError: fs.touch(marker_file_name) return True From 6adbd9351b6536dcec6c12d8ff39f2f8fdaa7b06 Mon Sep 17 00:00:00 2001 From: JordiMForgeFlow Date: Wed, 7 Feb 2024 09:19:04 +0100 Subject: [PATCH 05/10] [FIX] fs_storage: fix test connection when __fs slot is not initialized --- fs_storage/models/fs_storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index 88b33498c6..bfcf64ca57 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -452,7 +452,8 @@ def delete(self, relative_path) -> None: def action_test_config(self) -> None: try: - self._check_connection(self.__fs) + # pylint: disable=W0104 + self.fs title = _("Connection Test Succeeded!") message = _("Everything seems properly set up!") msg_type = "success" From 9986bc0df52575d852e1065fc3e88513981d21be Mon Sep 17 00:00:00 2001 From: JordiMForgeFlow Date: Thu, 8 Feb 2024 15:21:43 +0100 Subject: [PATCH 06/10] [IMP] fs_storage: clear slot when connection fails --- fs_storage/models/fs_storage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index bfcf64ca57..28dbf767fa 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -289,6 +289,7 @@ def fs(self) -> fsspec.AbstractFileSystem: self._check_connection(self.__fs) except Exception as e: self.__fs.clear_instance_cache() + self.__fs = None raise e return self.__fs @@ -452,6 +453,7 @@ def delete(self, relative_path) -> None: def action_test_config(self) -> None: try: + # Accessing the property will check the connection # pylint: disable=W0104 self.fs title = _("Connection Test Succeeded!") From dd626455dfcd80003818f8123811e8d9eabfe861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Fri, 15 Mar 2024 18:21:49 +0100 Subject: [PATCH 07/10] [FIX] fs_storage: remove default protocol Since protocol is under server_environment control, it seems its default value takes priority over a value set in an XML record. Since it does really make much sense to create a storage backend without specifying the protocol, we remove the default value. --- fs_storage/models/fs_storage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index 28dbf767fa..0745df38d9 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -83,7 +83,6 @@ def __init__(self, env, ids=(), prefetch_ids=()): protocol = fields.Selection( selection="_get_protocols", required=True, - default="odoofs", help="The protocol used to access the content of filesystem.\n" "This list is the one supported by the fsspec library (see " "https://filesystem-spec.readthedocs.io/en/latest). A filesystem protocol" From d1a3698479803de4a74f89d638cb4040a2331d81 Mon Sep 17 00:00:00 2001 From: Florian da Costa Date: Fri, 5 Apr 2024 10:39:20 +0200 Subject: [PATCH 08/10] [IMP] fs_storage : Allow to choose different method to check connection --- fs_storage/__init__.py | 1 + fs_storage/__manifest__.py | 1 + fs_storage/models/fs_storage.py | 47 ++++++++++++++++++++++- fs_storage/security/ir.model.access.csv | 1 + fs_storage/views/fs_storage_view.xml | 1 + fs_storage/wizards/__init__.py | 1 + fs_storage/wizards/fs_test_connection.py | 28 ++++++++++++++ fs_storage/wizards/fs_test_connection.xml | 31 +++++++++++++++ 8 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 fs_storage/wizards/__init__.py create mode 100644 fs_storage/wizards/fs_test_connection.py create mode 100644 fs_storage/wizards/fs_test_connection.xml diff --git a/fs_storage/__init__.py b/fs_storage/__init__.py index 1df985fe64..6f3a6c7170 100644 --- a/fs_storage/__init__.py +++ b/fs_storage/__init__.py @@ -4,3 +4,4 @@ # then add normal imports from . import models +from . import wizards diff --git a/fs_storage/__manifest__.py b/fs_storage/__manifest__.py index af55e711d7..fa297d6e0c 100644 --- a/fs_storage/__manifest__.py +++ b/fs_storage/__manifest__.py @@ -16,6 +16,7 @@ "data": [ "views/fs_storage_view.xml", "security/ir.model.access.csv", + "wizards/fs_test_connection.xml", ], "demo": ["demo/fs_storage_demo.xml"], "external_dependencies": {"python": ["fsspec>=2024.5.0"]}, diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index 0745df38d9..99391bc5da 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -146,6 +146,15 @@ def __init__(self, env, ids=(), prefetch_ids=()): compute="_compute_options_properties", store=False, ) + check_connection_method = fields.Selection( + selection="_get_check_connection_method_selection", + default="marker_file", + help="Set a method if you want the connection to remote to be checked every " + "time the storage is used, in order to remove the obsolete connection from" + " the cache.\n" + "* Create Marker file : Create a file on remote and check it exists\n" + "* List File : List all files from root directory", + ) _sql_constraints = [ ( @@ -157,6 +166,13 @@ def __init__(self, env, ids=(), prefetch_ids=()): _server_env_section_name_field = "code" + @api.model + def _get_check_connection_method_selection(self): + return [ + ("marker_file", _("Create Marker file")), + ("ls", _("List File")), + ] + @property def _server_env_fields(self): return {"protocol": {}, "options": {}, "directory_path": {}} @@ -267,14 +283,29 @@ def _compute_options_properties(self) -> None: def _get_marker_file_name(self): return ".odoo_fs_storage_%s.marker" % self.id - def _check_connection(self, fs): + def _marker_file_check_connection(self, fs): marker_file_name = self._get_marker_file_name() try: fs.info(marker_file_name) except FileNotFoundError: fs.touch(marker_file_name) + + def _ls_check_connection(self, fs): + fs.ls("", detail=False) + + def _check_connection_with_method(self, fs, check_connection_method): + if check_connection_method == "marker_file": + self._marker_file_check_connection(fs) + elif check_connection_method == "ls": + self._ls_check_connection(fs) return True + def _check_connection(self, fs): + check_connection_method = self.check_connection_method or self.env.context.get( + "force_connection_method", "" + ) + return self._check_connection_with_method(fs, check_connection_method) + @property def fs(self) -> fsspec.AbstractFileSystem: """Get the fsspec filesystem for this backend.""" @@ -450,10 +481,22 @@ def move_files(self, files, destination_path, **kw) -> None: def delete(self, relative_path) -> None: self.fs.rm_file(relative_path) - def action_test_config(self) -> None: + def action_test_config(self): + self.ensure_one() + if self.check_connection_method: + return self._test_config() + else: + action = self.env["ir.actions.actions"]._for_xml_id( + "fs_storage.act_open_fs_test_connection_view" + ) + action["context"] = {"active_model": "fs.storage", "active_id": self.id} + return action + + def _test_config(self): try: # Accessing the property will check the connection # pylint: disable=W0104 + # ruff: noqa: B018 self.fs title = _("Connection Test Succeeded!") message = _("Everything seems properly set up!") diff --git a/fs_storage/security/ir.model.access.csv b/fs_storage/security/ir.model.access.csv index 9c079b2eef..c1a81aae11 100644 --- a/fs_storage/security/ir.model.access.csv +++ b/fs_storage/security/ir.model.access.csv @@ -1,2 +1,3 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_fs_storage_edit,fs_storage edit,model_fs_storage,base.group_system,1,1,1,1 +access_fs_test_connection,fs.test.connection.access,model_fs_test_connection,base.group_system,1,1,1,1 diff --git a/fs_storage/views/fs_storage_view.xml b/fs_storage/views/fs_storage_view.xml index 3e25196230..712bb2a656 100644 --- a/fs_storage/views/fs_storage_view.xml +++ b/fs_storage/views/fs_storage_view.xml @@ -42,6 +42,7 @@ options="{'mode': 'python'}" placeholder="Enter you fsspec options here." /> + diff --git a/fs_storage/wizards/__init__.py b/fs_storage/wizards/__init__.py new file mode 100644 index 0000000000..197ee33cb7 --- /dev/null +++ b/fs_storage/wizards/__init__.py @@ -0,0 +1 @@ +from . import fs_test_connection diff --git a/fs_storage/wizards/fs_test_connection.py b/fs_storage/wizards/fs_test_connection.py new file mode 100644 index 0000000000..b5bc86a2ca --- /dev/null +++ b/fs_storage/wizards/fs_test_connection.py @@ -0,0 +1,28 @@ +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). + +from odoo import api, fields, models + + +class FSTestConnection(models.TransientModel): + _name = "fs.test.connection" + _description = "FS Test Connection Wizard" + + def _get_check_connection_method_selection(self): + return self.env["fs.storage"]._get_check_connection_method_selection() + + storage_id = fields.Many2one("fs.storage") + check_connection_method = fields.Selection( + selection="_get_check_connection_method_selection", + required=True, + ) + + @api.model + def default_get(self, field_list): + res = super().default_get(field_list) + res["storage_id"] = self.env.context.get("active_id", False) + return res + + def action_test_config(self): + return self.storage_id.with_context( + force_connection_method=self.check_connection_method + )._test_config() diff --git a/fs_storage/wizards/fs_test_connection.xml b/fs_storage/wizards/fs_test_connection.xml new file mode 100644 index 0000000000..2abed80164 --- /dev/null +++ b/fs_storage/wizards/fs_test_connection.xml @@ -0,0 +1,31 @@ + + + + fs.test.connection.form + fs.test.connection + +
+ + + + +
+
+
+ + + + FS Test Connection + ir.actions.act_window + fs.test.connection + form + new + + From 089a115ed0967900e9dcb4ea28ca78359988db3a Mon Sep 17 00:00:00 2001 From: Florian da Costa Date: Mon, 8 Apr 2024 18:39:31 +0200 Subject: [PATCH 09/10] [FIX] Avoid use of context to pass the check_connection_method --- fs_storage/models/fs_storage.py | 19 +++++-------------- fs_storage/wizards/fs_test_connection.py | 4 +--- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index 99391bc5da..1961a50bd2 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -293,19 +293,13 @@ def _marker_file_check_connection(self, fs): def _ls_check_connection(self, fs): fs.ls("", detail=False) - def _check_connection_with_method(self, fs, check_connection_method): + def _check_connection(self, fs, check_connection_method): if check_connection_method == "marker_file": self._marker_file_check_connection(fs) elif check_connection_method == "ls": self._ls_check_connection(fs) return True - def _check_connection(self, fs): - check_connection_method = self.check_connection_method or self.env.context.get( - "force_connection_method", "" - ) - return self._check_connection_with_method(fs, check_connection_method) - @property def fs(self) -> fsspec.AbstractFileSystem: """Get the fsspec filesystem for this backend.""" @@ -316,7 +310,7 @@ def fs(self) -> fsspec.AbstractFileSystem: # Check whether we need to invalidate FS cache or not. # Use a marker file to limit the scope of the LS command for performance. try: - self._check_connection(self.__fs) + self._check_connection(self.__fs, self.check_connection_method) except Exception as e: self.__fs.clear_instance_cache() self.__fs = None @@ -484,7 +478,7 @@ def delete(self, relative_path) -> None: def action_test_config(self): self.ensure_one() if self.check_connection_method: - return self._test_config() + return self._test_config(self.check_connection_method) else: action = self.env["ir.actions.actions"]._for_xml_id( "fs_storage.act_open_fs_test_connection_view" @@ -492,12 +486,9 @@ def action_test_config(self): action["context"] = {"active_model": "fs.storage", "active_id": self.id} return action - def _test_config(self): + def _test_config(self, connection_method): try: - # Accessing the property will check the connection - # pylint: disable=W0104 - # ruff: noqa: B018 - self.fs + self._check_connection(self.fs, connection_method) title = _("Connection Test Succeeded!") message = _("Everything seems properly set up!") msg_type = "success" diff --git a/fs_storage/wizards/fs_test_connection.py b/fs_storage/wizards/fs_test_connection.py index b5bc86a2ca..ebaf6154cc 100644 --- a/fs_storage/wizards/fs_test_connection.py +++ b/fs_storage/wizards/fs_test_connection.py @@ -23,6 +23,4 @@ def default_get(self, field_list): return res def action_test_config(self): - return self.storage_id.with_context( - force_connection_method=self.check_connection_method - )._test_config() + return self.storage_id._test_config(self.check_connection_method) From 0356313271cd97f1681e2efd1052bd7d3817b454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Fri, 12 Jul 2024 16:10:01 +0200 Subject: [PATCH 10/10] [FIX] fs_storage: add missing sudo when creating FilsSystem This is necessary for non admin users to access file systems, for instance for attachments. This went unnoticed so far presumably because the filesystem is cached and likely accessed by the server during server bootstrap. --- fs_storage/models/fs_storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs_storage/models/fs_storage.py b/fs_storage/models/fs_storage.py index 1961a50bd2..ebbb51149c 100644 --- a/fs_storage/models/fs_storage.py +++ b/fs_storage/models/fs_storage.py @@ -305,7 +305,7 @@ def fs(self) -> fsspec.AbstractFileSystem: """Get the fsspec filesystem for this backend.""" self.ensure_one() if not self.__fs: - self.__fs = self._get_filesystem() + self.__fs = self.sudo()._get_filesystem() if not tools.config["test_enable"]: # Check whether we need to invalidate FS cache or not. # Use a marker file to limit the scope of the LS command for performance.