From 0dccef3cd38d29d2d57a12467100cad08d6c15ce Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Sun, 1 Dec 2024 22:24:53 +0000 Subject: [PATCH 1/5] refactor: openApi made lazy. This is a staging post so there are extra tests (in all the decorators). Fixes a potential issue in __init__ where the default argument creates a shared object at parse time rather than a separate one for each instance. As only one instance, not normally an issue but not good practice. One FIXME is an edge case in init_openapi where neither if clause evaluates to True for openapi_file_path (should we log an error or what?) --- robyn/__init__.py | 59 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/robyn/__init__.py b/robyn/__init__.py index 929c0971..332d5877 100644 --- a/robyn/__init__.py +++ b/robyn/__init__.py @@ -43,7 +43,7 @@ def __init__( file_object: str, config: Config = Config(), openapi_file_path: Optional[str] = None, - openapi: OpenAPI = OpenAPI(), + openapi: Optional[OpenAPI] = None, dependencies: DependencyMap = DependencyMap(), ) -> None: directory_path = os.path.dirname(os.path.abspath(file_object)) @@ -51,12 +51,10 @@ def __init__( self.directory_path = directory_path self.config = config self.dependencies = dependencies + self.openapi_file_path = openapi_file_path self.openapi = openapi - if openapi_file_path: - openapi.override_openapi(Path(self.directory_path).joinpath(openapi_file_path)) - elif Path(self.directory_path).joinpath("openapi.json").exists(): - openapi.override_openapi(Path(self.directory_path).joinpath("openapi.json")) + self.init_openapi() if not bool(os.environ.get("ROBYN_CLI", False)): # the env variables are already set when are running through the cli @@ -83,6 +81,19 @@ def __init__( self.exception_handler: Optional[Callable] = None self.authentication_handler: Optional[AuthenticationHandler] = None + def init_openapi(self) -> None: + if self.config.disable_openapi: + return + + if self.openapi is None: + self.openapi = OpenAPI() + + if self.openapi_file_path: + self.openapi.override_openapi(Path(self.directory_path).joinpath(self.openapi_file_path)) + elif Path(self.directory_path).joinpath("openapi.json").exists(): + self.openapi.override_openapi(Path(self.directory_path).joinpath("openapi.json")) + # TODO! what about when the elif fails? + def _handle_dev_mode(self): cli_dev_mode = self.config.dev # --dev env_dev_mode = os.getenv("ROBYN_DEV_MODE", "False").lower() == "true" # ROBYN_DEV_MODE=True @@ -240,6 +251,13 @@ def is_port_in_use(self, port: int) -> bool: raise Exception(f"Invalid port number: {port}") def _add_openapi_routes(self, auth_required: bool = False): + if self.config.disable_openapi: + return + + if self.openapi is None: + logger.error("No openAPI") + return + self.add_route( route_type=HttpMethod.GET, endpoint="/openapi.json", @@ -369,7 +387,8 @@ def get( """ def inner(handler): - self.openapi.add_openapi_path_obj("get", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("get", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.GET, endpoint, handler, const, auth_required) @@ -392,7 +411,8 @@ def post( """ def inner(handler): - self.openapi.add_openapi_path_obj("post", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("post", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.POST, endpoint, handler, auth_required=auth_required) @@ -415,7 +435,8 @@ def put( """ def inner(handler): - self.openapi.add_openapi_path_obj("put", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("put", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.PUT, endpoint, handler, auth_required=auth_required) @@ -438,7 +459,8 @@ def delete( """ def inner(handler): - self.openapi.add_openapi_path_obj("delete", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("delete", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.DELETE, endpoint, handler, auth_required=auth_required) @@ -461,7 +483,8 @@ def patch( """ def inner(handler): - self.openapi.add_openapi_path_obj("patch", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("patch", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.PATCH, endpoint, handler, auth_required=auth_required) @@ -484,7 +507,8 @@ def head( """ def inner(handler): - self.openapi.add_openapi_path_obj("head", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("head", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.HEAD, endpoint, handler, auth_required=auth_required) @@ -507,7 +531,8 @@ def options( """ def inner(handler): - self.openapi.add_openapi_path_obj("options", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("options", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.OPTIONS, endpoint, handler, auth_required=auth_required) @@ -530,7 +555,9 @@ def connect( """ def inner(handler): - self.openapi.add_openapi_path_obj("connect", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("connect", endpoint, openapi_name, openapi_tags, handler) + return self.add_route(HttpMethod.CONNECT, endpoint, handler, auth_required=auth_required) return inner @@ -552,7 +579,8 @@ def trace( """ def inner(handler): - self.openapi.add_openapi_path_obj("trace", endpoint, openapi_name, openapi_tags, handler) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_openapi_path_obj("trace", endpoint, openapi_name, openapi_tags, handler) return self.add_route(HttpMethod.TRACE, endpoint, handler, auth_required=auth_required) @@ -568,7 +596,8 @@ def include_router(self, router): self.middleware_router.global_middlewares.extend(router.middleware_router.global_middlewares) self.middleware_router.route_middlewares.extend(router.middleware_router.route_middlewares) - self.openapi.add_subrouter_paths(router.openapi) + if not self.config.disable_openapi and self.openapi is not None: + self.openapi.add_subrouter_paths(router.openapi) # extend the websocket routes prefix = router.prefix From 642eac9c6f98e955d0a383fd8fe8de4d4fec5444 Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Mon, 2 Dec 2024 04:32:06 +0000 Subject: [PATCH 2/5] refactor: openApi made lazy. All add_route methods have auth_required, openapi_name and openapi_tags which get stored in the route. Instead of incrementally adding them to openapi routes they are added all at once in app.start include_routes now additionally tracks the list of routers whose routes have been added to the main app. This will be used more later to avoid merging routes until app.start for more flexibility --- robyn/__init__.py | 61 +++++++++++++++++--------------------------- robyn/processpool.py | 2 +- robyn/router.py | 24 +++++++++++++++-- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/robyn/__init__.py b/robyn/__init__.py index 332d5877..2cecf614 100644 --- a/robyn/__init__.py +++ b/robyn/__init__.py @@ -80,6 +80,7 @@ def __init__( self.event_handlers: dict = {} self.exception_handler: Optional[Callable] = None self.authentication_handler: Optional[AuthenticationHandler] = None + self.included_routers: List[Router] = [] def init_openapi(self) -> None: if self.config.disable_openapi: @@ -113,6 +114,8 @@ def add_route( handler: Callable, is_const: bool = False, auth_required: bool = False, + openapi_name: str = "", + openapi_tags: Union[List[str], None] = None, ): """ Connect a URI to a handler @@ -128,6 +131,10 @@ def add_route( """ injected_dependencies = self.dependencies.get_dependency_map(self) + list_openapi_tags: List[str] = [] + if not self.config.disable_openapi and openapi_tags is not None: + list_openapi_tags = openapi_tags + if auth_required: self.middleware_router.add_auth_middleware(endpoint)(handler) @@ -148,6 +155,9 @@ def add_route( endpoint=endpoint, handler=handler, is_const=is_const, + auth_required=auth_required, + openapi_name=openapi_name, + openapi_tags=list_openapi_tags, exception_handler=self.exception_handler, injected_dependencies=injected_dependencies, ) @@ -258,6 +268,8 @@ def _add_openapi_routes(self, auth_required: bool = False): logger.error("No openAPI") return + self.router.prepare_routes_openapi(self.openapi, self.included_routers) + self.add_route( route_type=HttpMethod.GET, endpoint="/openapi.json", @@ -387,10 +399,7 @@ def get( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("get", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.GET, endpoint, handler, const, auth_required) + return self.add_route(HttpMethod.GET, endpoint, handler, const, auth_required, openapi_name, openapi_tags) return inner @@ -411,10 +420,7 @@ def post( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("post", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.POST, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.POST, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -435,10 +441,7 @@ def put( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("put", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.PUT, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.PUT, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -459,10 +462,7 @@ def delete( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("delete", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.DELETE, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.DELETE, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -483,10 +483,7 @@ def patch( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("patch", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.PATCH, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.PATCH, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -507,10 +504,7 @@ def head( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("head", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.HEAD, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.HEAD, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -531,10 +525,7 @@ def options( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("options", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.OPTIONS, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.OPTIONS, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -555,10 +546,7 @@ def connect( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("connect", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.CONNECT, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.CONNECT, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -579,10 +567,7 @@ def trace( """ def inner(handler): - if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_openapi_path_obj("trace", endpoint, openapi_name, openapi_tags, handler) - - return self.add_route(HttpMethod.TRACE, endpoint, handler, auth_required=auth_required) + return self.add_route(HttpMethod.TRACE, endpoint, handler, auth_required=auth_required, openapi_name=openapi_name, openapi_tags=openapi_tags) return inner @@ -592,12 +577,14 @@ def include_router(self, router): :param router Robyn: the router object to include the routes from """ + self.included_routers.append(router) + self.router.routes.extend(router.router.routes) self.middleware_router.global_middlewares.extend(router.middleware_router.global_middlewares) self.middleware_router.route_middlewares.extend(router.middleware_router.route_middlewares) if not self.config.disable_openapi and self.openapi is not None: - self.openapi.add_subrouter_paths(router.openapi) + self.openapi.add_subrouter_paths(self.openapi) # extend the websocket routes prefix = router.prefix diff --git a/robyn/processpool.py b/robyn/processpool.py index f1b0839f..7669ec85 100644 --- a/robyn/processpool.py +++ b/robyn/processpool.py @@ -182,7 +182,7 @@ def spawn_process( server.set_response_headers_exclude_paths(excluded_response_headers_paths) for route in routes: - route_type, endpoint, function, is_const = route + route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags = route server.add_route(route_type, endpoint, function, is_const) for middleware_type, middleware_function in global_middlewares: diff --git a/robyn/router.py b/robyn/router.py index 655aa525..52ea7547 100644 --- a/robyn/router.py +++ b/robyn/router.py @@ -11,6 +11,7 @@ from robyn.authentication import AuthenticationHandler, AuthenticationNotConfiguredError from robyn.dependency_injection import DependencyMap from robyn.jsonify import jsonify +from robyn.openapi import OpenAPI from robyn.responses import FileResponse from robyn.robyn import FunctionInfo, Headers, HttpMethod, Identity, MiddlewareType, QueryParams, Request, Response, Url from robyn.types import Body, Files, FormData, IPAddress, Method, PathParams @@ -19,11 +20,18 @@ _logger = logging.getLogger(__name__) +def lower_http_method(method: HttpMethod): + return str(method).lower()[11:] + + class Route(NamedTuple): route_type: HttpMethod route: str function: FunctionInfo is_const: bool + auth_required: bool + openapi_name: str + openapi_tags: List[str] class RouteMiddleware(NamedTuple): @@ -108,6 +116,9 @@ def add_route( # type: ignore endpoint: str, handler: Callable, is_const: bool, + auth_required: bool, + openapi_name: str, + openapi_tags: List[str], exception_handler: Optional[Callable], injected_dependencies: dict, ) -> Union[Callable, CoroutineType]: @@ -226,7 +237,7 @@ def inner_handler(*args, **kwargs): params, new_injected_dependencies, ) - self.routes.append(Route(route_type, endpoint, function, is_const)) + self.routes.append(Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags)) return async_inner_handler else: function = FunctionInfo( @@ -236,9 +247,18 @@ def inner_handler(*args, **kwargs): params, new_injected_dependencies, ) - self.routes.append(Route(route_type, endpoint, function, is_const)) + self.routes.append(Route(route_type, endpoint, function, is_const, auth_required, openapi_name, openapi_tags)) return inner_handler + def prepare_routes_openapi(self, openapi: OpenAPI, included_routers: List) -> None: + for route in self.routes: + openapi.add_openapi_path_obj(lower_http_method(route.route_type), route.route, route.openapi_name, route.openapi_tags, route.function.handler) + + # TODO! after include_routes does not immediatelly merge all the routes + # for router in included_routers: + # for route in router: + # openapi.add_openapi_path_obj(lower_http_method(route.route_type), route.route, route.openapi_name, route.openapi_tags, route.function.handler) + def get_routes(self) -> List[Route]: return self.routes From 2ae5e7f581fac474ec748d7f52c237f0b01255e2 Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Mon, 2 Dec 2024 04:51:36 +0000 Subject: [PATCH 3/5] refactor: try to solve the speed issue by improving handling moving from allowing None for openApi_tags to not --- robyn/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/robyn/__init__.py b/robyn/__init__.py index 2cecf614..756acd9a 100644 --- a/robyn/__init__.py +++ b/robyn/__init__.py @@ -131,9 +131,7 @@ def add_route( """ injected_dependencies = self.dependencies.get_dependency_map(self) - list_openapi_tags: List[str] = [] - if not self.config.disable_openapi and openapi_tags is not None: - list_openapi_tags = openapi_tags + list_openapi_tags: List[str] = openapi_tags if openapi_tags else [] if auth_required: self.middleware_router.add_auth_middleware(endpoint)(handler) From f9ebbee1a789a6dec8df7ba452e7d3057bba15e0 Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Mon, 2 Dec 2024 20:56:19 +0000 Subject: [PATCH 4/5] refactor: slightly optimised lower_http_method I've tested with 100,000 loops. This is marginally faster. This PR lowers slows full test suite by 0.004s on my system. However, those benchmarks are mostly setup. Once we are making calls to a running server none of this code is being executed. --- robyn/router.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robyn/router.py b/robyn/router.py index 52ea7547..774237fe 100644 --- a/robyn/router.py +++ b/robyn/router.py @@ -21,7 +21,7 @@ def lower_http_method(method: HttpMethod): - return str(method).lower()[11:] + return (str(method))[11:].lower() class Route(NamedTuple): From b1dae0bba5711ca950e49ea1ca930ca735988e6e Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Mon, 2 Dec 2024 21:51:23 +0000 Subject: [PATCH 5/5] refactor: remove extra Robyn instance variable to avoid extra side effects in the future. No consistent peformance impact on 100,000 Robyn(__file__) calls --- robyn/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/robyn/__init__.py b/robyn/__init__.py index 756acd9a..464739a6 100644 --- a/robyn/__init__.py +++ b/robyn/__init__.py @@ -51,10 +51,9 @@ def __init__( self.directory_path = directory_path self.config = config self.dependencies = dependencies - self.openapi_file_path = openapi_file_path self.openapi = openapi - self.init_openapi() + self.init_openapi(openapi_file_path) if not bool(os.environ.get("ROBYN_CLI", False)): # the env variables are already set when are running through the cli @@ -82,15 +81,15 @@ def __init__( self.authentication_handler: Optional[AuthenticationHandler] = None self.included_routers: List[Router] = [] - def init_openapi(self) -> None: + def init_openapi(self, openapi_file_path: Optional[str]) -> None: if self.config.disable_openapi: return if self.openapi is None: self.openapi = OpenAPI() - if self.openapi_file_path: - self.openapi.override_openapi(Path(self.directory_path).joinpath(self.openapi_file_path)) + if openapi_file_path: + self.openapi.override_openapi(Path(self.directory_path).joinpath(openapi_file_path)) elif Path(self.directory_path).joinpath("openapi.json").exists(): self.openapi.override_openapi(Path(self.directory_path).joinpath("openapi.json")) # TODO! what about when the elif fails?