From ed9d8bc23a81cda171010e77f759353a613fb123 Mon Sep 17 00:00:00 2001 From: PicchiSeba Date: Thu, 27 Feb 2025 12:42:09 +0100 Subject: [PATCH 1/4] [IMP]fastapi: obtain endpoint via common method --- fastapi/fastapi_dispatcher.py | 6 +++--- fastapi/models/fastapi_endpoint.py | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fastapi/fastapi_dispatcher.py b/fastapi/fastapi_dispatcher.py index c41beb2d2..63aedd4f4 100644 --- a/fastapi/fastapi_dispatcher.py +++ b/fastapi/fastapi_dispatcher.py @@ -27,11 +27,11 @@ def dispatch(self, endpoint, args): # don't parse the httprequest let starlette parse the stream self.request.params = {} # dict(self.request.get_http_params(), **args) environ = self._get_environ() - root_path = "/" + environ["PATH_INFO"].split("/")[1] + path = environ["PATH_INFO"] # TODO store the env into contextvar to be used by the odoo_env # depends method - with fastapi_app_pool.get_app(env=request.env, root_path=root_path) as app: - uid = request.env["fastapi.endpoint"].sudo().get_uid(root_path) + with fastapi_app_pool.get_app(env=request.env, root_path=path) as app: + uid = request.env["fastapi.endpoint"].sudo().get_uid(path) data = BytesIO() with self._manage_odoo_env(uid): for r in app(environ, self._make_response): diff --git a/fastapi/models/fastapi_endpoint.py b/fastapi/models/fastapi_endpoint.py index 25e3dc468..45dd13319 100644 --- a/fastapi/models/fastapi_endpoint.py +++ b/fastapi/models/fastapi_endpoint.py @@ -211,8 +211,15 @@ def _reset_app_cache_marker(self): """ @api.model - def get_app(self, root_path): - record = self.search([("root_path", "=", root_path)]) + @tools.ormcache("path") + def get_endpoint(self, path): + root_path = "/" + path.split("/")[1] + endpoint = self.search([("root_path", "=", root_path)])[:1] or False + return endpoint + + @api.model + def get_app(self, path): + record = self.get_endpoint(path) if not record: return None app = FastAPI() @@ -236,9 +243,9 @@ def _clear_fastapi_exception_handlers(self, app: FastAPI) -> None: self._clear_fastapi_exception_handlers(route.app) @api.model - @tools.ormcache("root_path") - def get_uid(self, root_path): - record = self.search([("root_path", "=", root_path)]) + @tools.ormcache("path") + def get_uid(self, path): + record = self.get_endpoint(path) if not record: return None return record.user_id.id From 4926f48470e833f31cc242a9fa66b764e6b72099 Mon Sep 17 00:00:00 2001 From: PicchiSeba Date: Mon, 5 May 2025 14:51:21 +0200 Subject: [PATCH 2/4] [IMP]fastapi: enable multi-slash routes --- fastapi/demo/fastapi_endpoint_demo.xml | 11 +++++++++++ fastapi/models/fastapi_endpoint.py | 13 +++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/fastapi/demo/fastapi_endpoint_demo.xml b/fastapi/demo/fastapi_endpoint_demo.xml index a1c34e34b..7016b807c 100644 --- a/fastapi/demo/fastapi_endpoint_demo.xml +++ b/fastapi/demo/fastapi_endpoint_demo.xml @@ -44,4 +44,15 @@ methods. See documentation to learn more about how to create a new app. http_basic + + + Fastapi Multi-Slash Demo Endpoint + + Like the other demo endpoint but with multi-slash + + demo + /fastapi/demo + http_basic + + diff --git a/fastapi/models/fastapi_endpoint.py b/fastapi/models/fastapi_endpoint.py index 45dd13319..9f3b256c4 100644 --- a/fastapi/models/fastapi_endpoint.py +++ b/fastapi/models/fastapi_endpoint.py @@ -213,8 +213,17 @@ def _reset_app_cache_marker(self): @api.model @tools.ormcache("path") def get_endpoint(self, path): - root_path = "/" + path.split("/")[1] - endpoint = self.search([("root_path", "=", root_path)])[:1] or False + # try to match the request url with the most similar endpoint + endpoints_by_length = self.search([]).sorted( + lambda fe: len(fe.root_path), reverse=True + ) + endpoint = False + while endpoints_by_length: + candidate_endpoint = endpoints_by_length[0] + if path.startswith(candidate_endpoint.root_path): + endpoint = candidate_endpoint + break + endpoints_by_length -= candidate_endpoint return endpoint @api.model From 55adbd2ad17db342efee51b2b8b119bddd27c1f3 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Wed, 14 May 2025 15:23:59 +0200 Subject: [PATCH 3/4] [IMP] fastapi: Ensures endpoint matching is based on path parts When we lookup the fastapi.endpoint to use to process a request at a given path, we must ensure that the matching mechanism on the path and the endpoint root_path is bases on the path parts. e.g. /a/b is not prefix of /a/bc. In the same time improves robustness of the matching mechanism and avoid useless SQL queries --- fastapi/models/fastapi_endpoint.py | 79 ++++++++++++++++++++++++------ fastapi/tests/test_fastapi.py | 20 ++++++++ 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/fastapi/models/fastapi_endpoint.py b/fastapi/models/fastapi_endpoint.py index 9f3b256c4..93130c69d 100644 --- a/fastapi/models/fastapi_endpoint.py +++ b/fastapi/models/fastapi_endpoint.py @@ -198,6 +198,8 @@ def _endpoint_registry_route_unique_key(self, routing: Dict[str, Any]): return f"{self._name}:{self.id}:{path}" def _reset_app(self): + self._get_id_by_root_path_map.clear_cache(self) + self._get_id_for_path.clear_cache(self) self._reset_app_cache_marker.clear_cache(self) @tools.ormcache() @@ -211,24 +213,71 @@ def _reset_app_cache_marker(self): """ @api.model - @tools.ormcache("path") - def get_endpoint(self, path): - # try to match the request url with the most similar endpoint - endpoints_by_length = self.search([]).sorted( - lambda fe: len(fe.root_path), reverse=True + def _normalize_url_path(self, path) -> str: + """ + Normalize a URL path: + * Remove redundant slashes, + * Remove trailing slash (unless it's the root), + * Lowercase for case-insensitive matching + """ + parts = [part.lower() for part in path.strip().split("/") if part] + return "/" + "/".join(parts) + + @api.model + def _is_suburl(self, path, prefix) -> bool: + """ + Check if 'path' is a subpath of 'prefix' in URL logic: + * Must start with the prefix followed by a slash + This will ensure that the matching is done one the path + parts and ensures that e.g. /a/b is not prefix of /a/bc. + """ + path = self._normalize_url_path(path) + prefix = self._normalize_url_path(prefix) + + if path == prefix: + return True + if path.startswith(prefix + "/"): + return True + return False + + @api.model + def _find_first_matching_url_path(self, paths, prefix) -> str | None: + """ + Return the first path that is a subpath of 'prefix', + ordered by longest URL path first (most number of segments). + """ + # Sort by number of segments (shallowest first) + sorted_paths = sorted( + paths, + key=lambda p: len(self._normalize_url_path(p).split("/")), + reverse=True, ) - endpoint = False - while endpoints_by_length: - candidate_endpoint = endpoints_by_length[0] - if path.startswith(candidate_endpoint.root_path): - endpoint = candidate_endpoint - break - endpoints_by_length -= candidate_endpoint - return endpoint + + for path in sorted_paths: + if self._is_suburl(prefix, path): + return path + return None + + @api.model + @tools.ormcache() + def _get_id_by_root_path_map(self): + return {r.root_path: r.id for r in self.search([])} + + @api.model + @tools.ormcache("path") + def _get_id_for_path(self, path): + id_by_path = self._get_id_by_root_path_map() + root_path = self._find_first_matching_url_path(id_by_path.keys(), path) + return id_by_path.get(root_path) + + @api.model + def _get_endpoint(self, path): + id_ = self._get_id_for_path(path) + return self.browse(id_) if id_ else None @api.model def get_app(self, path): - record = self.get_endpoint(path) + record = self._get_endpoint(path) if not record: return None app = FastAPI() @@ -254,7 +303,7 @@ def _clear_fastapi_exception_handlers(self, app: FastAPI) -> None: @api.model @tools.ormcache("path") def get_uid(self, path): - record = self.get_endpoint(path) + record = self._get_endpoint(path) if not record: return None return record.user_id.id diff --git a/fastapi/tests/test_fastapi.py b/fastapi/tests/test_fastapi.py index 37b11a961..91d4c9063 100644 --- a/fastapi/tests/test_fastapi.py +++ b/fastapi/tests/test_fastapi.py @@ -169,3 +169,23 @@ def test_no_commit_on_exception(self) -> None: expected_message="test", expected_status_code=status.HTTP_409_CONFLICT, ) + + def test_url_matching(self): + # Test the URL mathing method on the endpoint + paths = ["/fastapi", "/fastapi_demo", "/fastapi/v1"] + EndPoint = self.env["fastapi.endpoint"] + self.assertEqual( + EndPoint._find_first_matching_url_path(paths, "/fastapi_demo/test"), + "/fastapi_demo", + ) + self.assertEqual( + EndPoint._find_first_matching_url_path(paths, "/fastapi/test"), "/fastapi" + ) + self.assertEqual( + EndPoint._find_first_matching_url_path(paths, "/fastapi/v2/test"), + "/fastapi", + ) + self.assertEqual( + EndPoint._find_first_matching_url_path(paths, "/fastapi/v1/test"), + "/fastapi/v1", + ) From 666a6b1d9b3d9a89f905c38890cacdb16956d0ef Mon Sep 17 00:00:00 2001 From: PicchiSeba Date: Wed, 11 Jun 2025 09:13:59 +0200 Subject: [PATCH 4/4] [IMP]fastapi: add test for multi-slash scenario --- fastapi/demo/fastapi_endpoint_demo.xml | 2 +- fastapi/tests/test_fastapi.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/fastapi/demo/fastapi_endpoint_demo.xml b/fastapi/demo/fastapi_endpoint_demo.xml index 7016b807c..ad3fd9da0 100644 --- a/fastapi/demo/fastapi_endpoint_demo.xml +++ b/fastapi/demo/fastapi_endpoint_demo.xml @@ -51,7 +51,7 @@ methods. See documentation to learn more about how to create a new app. Like the other demo endpoint but with multi-slash demo - /fastapi/demo + /fastapi/demo-multi http_basic diff --git a/fastapi/tests/test_fastapi.py b/fastapi/tests/test_fastapi.py index 91d4c9063..fbda932fb 100644 --- a/fastapi/tests/test_fastapi.py +++ b/fastapi/tests/test_fastapi.py @@ -20,7 +20,11 @@ class FastAPIHttpCase(HttpCase): def setUpClass(cls): super().setUpClass() cls.fastapi_demo_app = cls.env.ref("fastapi.fastapi_endpoint_demo") - cls.fastapi_demo_app._handle_registry_sync() + cls.fastapi_multi_demo_app = cls.env.ref( + "fastapi.fastapi_endpoint_multislash_demo" + ) + cls.fastapi_apps = cls.fastapi_demo_app + cls.fastapi_multi_demo_app + cls.fastapi_apps._handle_registry_sync() lang = ( cls.env["res.lang"] .with_context(active_test=False) @@ -189,3 +193,9 @@ def test_url_matching(self): EndPoint._find_first_matching_url_path(paths, "/fastapi/v1/test"), "/fastapi/v1", ) + + def test_multi_slash(self): + route = "/fastapi/demo-multi/demo/" + response = self.url_open(route, timeout=20) + self.assertEqual(response.status_code, 200) + self.assertIn(self.fastapi_multi_demo_app.root_path, str(response.url))