From ba326171995183ca9a7e23e2fefa24759740f387 Mon Sep 17 00:00:00 2001 From: Stefan Rijnhart Date: Wed, 7 Dec 2022 16:17:29 +0100 Subject: [PATCH] [RFR] base_rest: use routing attribute constant throughout --- .../apispec/base_rest_service_apispec.py | 7 +- base_rest/apispec/rest_method_param_plugin.py | 3 +- .../apispec/rest_method_security_plugin.py | 4 +- base_rest/components/service.py | 5 +- base_rest/models/rest_service_registration.py | 10 +-- base_rest/restapi.py | 4 +- base_rest/tests/common.py | 4 +- base_rest/tests/test_controller_builder.py | 68 ++++++++++++------- base_rest/tools.py | 2 + 9 files changed, 63 insertions(+), 44 deletions(-) diff --git a/base_rest/apispec/base_rest_service_apispec.py b/base_rest/apispec/base_rest_service_apispec.py index 02f30ffe9..72276d40a 100644 --- a/base_rest/apispec/base_rest_service_apispec.py +++ b/base_rest/apispec/base_rest_service_apispec.py @@ -7,6 +7,7 @@ from apispec import APISpec from ..core import _rest_services_databases +from ..tools import ROUTING_DECORATOR_ATTR from .rest_method_param_plugin import RestMethodParamPlugin from .rest_method_security_plugin import RestMethodSecurityPlugin from .restapi_method_route_plugin import RestApiMethodRoutePlugin @@ -62,18 +63,18 @@ def _get_plugins(self): def _add_method_path(self, method): description = textwrap.dedent(method.__doc__ or "") - routing = method.original_routing + routing = getattr(method, ROUTING_DECORATOR_ATTR) for paths, method in routing["routes"]: for path in paths: self.path( path, operations={method.lower(): {"summary": description}}, - original_routing=routing, + **{ROUTING_DECORATOR_ATTR: routing}, ) def generate_paths(self): for _name, method in inspect.getmembers(self._service, inspect.ismethod): - routing = getattr(method, "original_routing", None) + routing = getattr(method, ROUTING_DECORATOR_ATTR, None) if not routing: continue self._add_method_path(method) diff --git a/base_rest/apispec/rest_method_param_plugin.py b/base_rest/apispec/rest_method_param_plugin.py index 68746ef1f..dde70782e 100644 --- a/base_rest/apispec/rest_method_param_plugin.py +++ b/base_rest/apispec/rest_method_param_plugin.py @@ -4,6 +4,7 @@ from apispec import BasePlugin from ..restapi import RestMethodParam +from ..tools import ROUTING_DECORATOR_ATTR class RestMethodParamPlugin(BasePlugin): @@ -25,7 +26,7 @@ def init_spec(self, spec): self.openapi_version = spec.openapi_version def operation_helper(self, path=None, operations=None, **kwargs): - routing = kwargs.get("original_routing") + routing = kwargs.get(ROUTING_DECORATOR_ATTR) if not routing: super(RestMethodParamPlugin, self).operation_helper( path, operations, **kwargs diff --git a/base_rest/apispec/rest_method_security_plugin.py b/base_rest/apispec/rest_method_security_plugin.py index 632f7cc76..bdc3d0e5c 100644 --- a/base_rest/apispec/rest_method_security_plugin.py +++ b/base_rest/apispec/rest_method_security_plugin.py @@ -3,6 +3,8 @@ from apispec import BasePlugin +from ..tools import ROUTING_DECORATOR_ATTR + class RestMethodSecurityPlugin(BasePlugin): """ @@ -23,7 +25,7 @@ def init_spec(self, spec): spec.components.security_scheme("user", user_scheme) def operation_helper(self, path=None, operations=None, **kwargs): - routing = kwargs.get("original_routing") + routing = kwargs.get(ROUTING_DECORATOR_ATTR) if not routing: super(RestMethodSecurityPlugin, self).operation_helper( path, operations, **kwargs diff --git a/base_rest/components/service.py b/base_rest/components/service.py index e04a39da1..ec9d1ba9f 100644 --- a/base_rest/components/service.py +++ b/base_rest/components/service.py @@ -11,6 +11,7 @@ from odoo.addons.component.core import AbstractComponent from ..apispec.base_rest_service_apispec import BaseRestServiceAPISpec +from ..tools import ROUTING_DECORATOR_ATTR _logger = logging.getLogger(__name__) @@ -93,7 +94,7 @@ def _prepare_input_params(self, method, params): method_name = method.__name__ if hasattr(method, "skip_secure_params"): return params - routing = getattr(method, "original_routing", None) + routing = getattr(method, ROUTING_DECORATOR_ATTR, None) if not routing: _logger.warning( "Method %s is not a public method of service %s", @@ -122,7 +123,7 @@ def _prepare_response(self, method, result): method_name = method.__name__ if hasattr(method, "skip_secure_response"): return result - routing = getattr(method, "original_routing", None) + routing = getattr(method, ROUTING_DECORATOR_ATTR, None) output_param = routing["output_param"] if not output_param: _logger.warning( diff --git a/base_rest/models/rest_service_registration.py b/base_rest/models/rest_service_registration.py index 3f34971eb..ee44c3dd2 100644 --- a/base_rest/models/rest_service_registration.py +++ b/base_rest/models/rest_service_registration.py @@ -29,11 +29,7 @@ _rest_services_databases, _rest_services_routes, ) -from ..tools import _inspect_methods - -# Decorator attribute added on a route function (cfr Odoo's route) -ROUTING_DECORATOR_ATTR = "original_routing" - +from ..tools import ROUTING_DECORATOR_ATTR, _inspect_methods _logger = logging.getLogger(__name__) @@ -397,9 +393,9 @@ def _generate_methods(self): path_sep = "/" root_path = "{}{}{}".format(root_path, path_sep, self._service._usage) for name, method in _inspect_methods(self._service.__class__): - if not hasattr(method, "original_routing"): + routing = getattr(method, ROUTING_DECORATOR_ATTR, None) + if routing is None: continue - routing = method.original_routing for routes, http_method in routing["routes"]: method_name = "{}_{}".format(http_method.lower(), name) default_route = routes[0] diff --git a/base_rest/restapi.py b/base_rest/restapi.py index e7dfbe7d2..ce61e7020 100644 --- a/base_rest/restapi.py +++ b/base_rest/restapi.py @@ -10,7 +10,7 @@ from odoo import _, http from odoo.exceptions import UserError, ValidationError -from .tools import cerberus_to_json +from .tools import ROUTING_DECORATOR_ATTR, cerberus_to_json def method(routes, input_param=None, output_param=None, **kw): @@ -104,7 +104,7 @@ def response_wrap(*args, **kw): response = f(*args, **kw) return response - response_wrap.original_routing = routing + setattr(response_wrap, ROUTING_DECORATOR_ATTR, routing) response_wrap.original_func = f return response_wrap diff --git a/base_rest/tests/common.py b/base_rest/tests/common.py index 4d7e9f5fb..cd2426781 100644 --- a/base_rest/tests/common.py +++ b/base_rest/tests/common.py @@ -26,7 +26,7 @@ _rest_controllers_per_module, _rest_services_databases, ) -from ..tools import _inspect_methods +from ..tools import ROUTING_DECORATOR_ATTR, _inspect_methods class RegistryMixin(object): @@ -187,7 +187,7 @@ def _get_controller_for(service, addon="base_rest"): def _get_controller_route_methods(controller): methods = {} for name, method in _inspect_methods(controller): - if hasattr(method, "original_routing"): + if hasattr(method, ROUTING_DECORATOR_ATTR): methods[name] = method return methods diff --git a/base_rest/tests/test_controller_builder.py b/base_rest/tests/test_controller_builder.py index 05fc7eb1d..3768527fc 100644 --- a/base_rest/tests/test_controller_builder.py +++ b/base_rest/tests/test_controller_builder.py @@ -5,6 +5,7 @@ from odoo.addons.component.core import Component from .. import restapi +from ..tools import ROUTING_DECORATOR_ATTR from .common import TransactionRestServiceRegistryCase @@ -109,7 +110,7 @@ def _validator_my_instance_method(self): # the generated method_name is always the {http_method}_{method_name} method = routes["get_get"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -126,7 +127,7 @@ def _validator_my_instance_method(self): method = routes["get_search"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -140,7 +141,7 @@ def _validator_my_instance_method(self): method = routes["post_update"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "public", @@ -157,7 +158,7 @@ def _validator_my_instance_method(self): method = routes["put_update"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["PUT"], "auth": "public", @@ -171,7 +172,7 @@ def _validator_my_instance_method(self): method = routes["post_create"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "public", @@ -185,7 +186,7 @@ def _validator_my_instance_method(self): method = routes["post_delete"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "public", @@ -199,7 +200,7 @@ def _validator_my_instance_method(self): method = routes["delete_delete"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["DELETE"], "auth": "public", @@ -213,7 +214,7 @@ def _validator_my_instance_method(self): method = routes["post_my_method"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "public", @@ -227,7 +228,7 @@ def _validator_my_instance_method(self): method = routes["post_my_instance_method"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "public", @@ -295,7 +296,7 @@ def _get_partner_schema(self): method = routes["get_get"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -312,7 +313,7 @@ def _get_partner_schema(self): method = routes["get_get_name"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -326,7 +327,7 @@ def _get_partner_schema(self): method = routes["post_update_name"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "user", @@ -392,7 +393,7 @@ def update_name(self, _id, **params): method = routes["get_get"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -409,7 +410,7 @@ def update_name(self, _id, **params): method = routes["get_get_name"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["GET"], "auth": "public", @@ -423,7 +424,7 @@ def update_name(self, _id, **params): method = routes["post_update_name"] self.assertDictEqual( - method.original_routing, + getattr(method, ROUTING_DECORATOR_ATTR), { "methods": ["POST"], "auth": "user", @@ -510,26 +511,33 @@ def _validator_get(self): ("save_session", default_save_session), ]: self.assertEqual( - routes["get_new_api_method_without"].original_routing[attr], + getattr(routes["get_new_api_method_without"], ROUTING_DECORATOR_ATTR)[ + attr + ], default, "wrong %s" % attr, ) self.assertEqual( - routes["get_new_api_method_with"].original_routing["auth"], "public" + getattr(routes["get_new_api_method_with"], ROUTING_DECORATOR_ATTR)["auth"], + "public", ) self.assertEqual( - routes["get_new_api_method_with"].original_routing["cors"], "http://my_site" + getattr(routes["get_new_api_method_with"], ROUTING_DECORATOR_ATTR)["cors"], + "http://my_site", ) self.assertEqual( - routes["get_new_api_method_with"].original_routing["csrf"], not default_csrf + getattr(routes["get_new_api_method_with"], ROUTING_DECORATOR_ATTR)["csrf"], + not default_csrf, ) self.assertEqual( - routes["get_new_api_method_with"].original_routing["save_session"], + getattr(routes["get_new_api_method_with"], ROUTING_DECORATOR_ATTR)[ + "save_session" + ], not default_save_session, ) self.assertEqual( - routes["get_get"].original_routing["auth"], + getattr(routes["get_get"], ROUTING_DECORATOR_ATTR)["auth"], default_auth, "wrong auth for get_get", ) @@ -541,12 +549,14 @@ def _validator_get(self): ("save_session", default_save_session), ]: self.assertEqual( - routes["my_controller_route_without"].original_routing[attr], + getattr(routes["my_controller_route_without"], ROUTING_DECORATOR_ATTR)[ + attr + ], default, "wrong %s" % attr, ) - routing = routes["my_controller_route_with"].original_routing + routing = getattr(routes["my_controller_route_with"], ROUTING_DECORATOR_ATTR) for attr, value in [ ("auth", "public"), ("cors", "http://with_cors"), @@ -560,7 +570,9 @@ def _validator_get(self): "wrong %s" % attr, ) self.assertEqual( - routes["my_controller_route_without_auth_2"].original_routing["auth"], + getattr( + routes["my_controller_route_without_auth_2"], ROUTING_DECORATOR_ATTR + )["auth"], None, "wrong auth for my_controller_route_without_auth_2", ) @@ -605,7 +617,9 @@ def _validator_get(self): routes = self._get_controller_route_methods(controller) self.assertEqual( - routes["get_new_api_method_with_public_or"].original_routing["auth"], + getattr( + routes["get_new_api_method_with_public_or"], ROUTING_DECORATOR_ATTR + )["auth"], "public_or_my_default_auth", ) @@ -643,7 +657,9 @@ def _validator_get(self): routes = self._get_controller_route_methods(controller) self.assertEqual( - routes["get_new_api_method_with_public_or"].original_routing["auth"], + getattr( + routes["get_new_api_method_with_public_or"], ROUTING_DECORATOR_ATTR + )["auth"], "my_default_auth", ) diff --git a/base_rest/tools.py b/base_rest/tools.py index 6449fcf7a..92e78e85d 100644 --- a/base_rest/tools.py +++ b/base_rest/tools.py @@ -6,6 +6,8 @@ _logger = logging.getLogger(__name__) +# Decorator attribute added on a route function (cfr Odoo's route) +ROUTING_DECORATOR_ATTR = "original_routing" SUPPORTED_META = ["title", "description", "example", "examples"]