From 08577101473360c99d8cdd0dbc56af659562025c Mon Sep 17 00:00:00 2001 From: Robbe Sneyders Date: Thu, 30 Nov 2023 23:59:26 +0100 Subject: [PATCH] Only instantiate specification once (#1819) Fixes #1801 I had to make quite a few additional changes to satisfy mypy. --- .pre-commit-config.yaml | 2 +- connexion/middleware/abstract.py | 16 +++---- connexion/middleware/main.py | 9 ++-- connexion/middleware/routing.py | 8 ++-- connexion/middleware/security.py | 24 +++++++++- connexion/middleware/swagger_ui.py | 6 +-- connexion/operations/abstract.py | 5 +++ connexion/spec.py | 16 ++++++- tests/test_api.py | 71 ++++++++++++++++++++---------- 9 files changed, 108 insertions(+), 49 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 25e4ef40c..ba54705e3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,7 +43,7 @@ repos: args: ["tests"] - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.961 + rev: v0.981 hooks: - id: mypy files: "^connexion/" diff --git a/connexion/middleware/abstract.py b/connexion/middleware/abstract.py index 9be98fe6e..a5fd58aa1 100644 --- a/connexion/middleware/abstract.py +++ b/connexion/middleware/abstract.py @@ -1,6 +1,5 @@ import abc import logging -import pathlib import typing as t from collections import defaultdict @@ -22,9 +21,7 @@ class SpecMiddleware(abc.ABC): base class""" @abc.abstractmethod - def add_api( - self, specification: t.Union[pathlib.Path, str, dict], **kwargs - ) -> t.Any: + def add_api(self, specification: Specification, **kwargs) -> t.Any: """ Register an API represented by a single OpenAPI specification on this middleware. Multiple APIs can be registered on a single middleware. @@ -40,15 +37,14 @@ class AbstractSpecAPI: def __init__( self, - specification: t.Union[pathlib.Path, str, dict], + specification: Specification, base_path: t.Optional[str] = None, resolver: t.Optional[Resolver] = None, - arguments: t.Optional[dict] = None, uri_parser_class=None, *args, **kwargs, ): - self.specification = Specification.load(specification, arguments=arguments) + self.specification = specification self.uri_parser_class = uri_parser_class self._set_base_path(base_path) @@ -88,7 +84,7 @@ def add_paths(self, paths: t.Optional[dict] = None) -> None: """ Adds the paths defined in the specification as operations. """ - paths = paths or self.specification.get("paths", dict()) + paths = t.cast(dict, paths or self.specification.get("paths", dict())) for path, methods in paths.items(): logger.debug("Adding %s%s...", self.base_path, path) @@ -176,7 +172,7 @@ def _handle_add_operation_error( class RoutedAPI(AbstractSpecAPI, t.Generic[OP]): def __init__( self, - specification: t.Union[pathlib.Path, str, dict], + specification: Specification, *args, next_app: ASGIApp, **kwargs, @@ -235,7 +231,7 @@ def __init__(self, app: ASGIApp) -> None: self.app = app self.apis: t.Dict[str, t.List[API]] = defaultdict(list) - def add_api(self, specification: t.Union[pathlib.Path, str, dict], **kwargs) -> API: + def add_api(self, specification: Specification, **kwargs) -> API: api = self.api_cls(specification, next_app=self.app, **kwargs) self.apis[api.base_path].append(api) return api diff --git a/connexion/middleware/main.py b/connexion/middleware/main.py index c3079aad6..48b261bd7 100644 --- a/connexion/middleware/main.py +++ b/connexion/middleware/main.py @@ -24,6 +24,7 @@ from connexion.middleware.swagger_ui import SwaggerUIMiddleware from connexion.options import SwaggerUIOptions from connexion.resolver import Resolver +from connexion.spec import Specification from connexion.types import MaybeAwaitable from connexion.uri_parsing import AbstractURIParser from connexion.utils import inspect_function_arguments @@ -390,18 +391,18 @@ def add_api( if self.middleware_stack is not None: raise RuntimeError("Cannot add api after an application has started") - if isinstance(specification, dict): - specification = specification - else: + if isinstance(specification, (pathlib.Path, str)): specification = t.cast(pathlib.Path, self.specification_dir / specification) + # Add specification as file to watch for reloading if pathlib.Path.cwd() in specification.parents: self.extra_files.append( str(specification.relative_to(pathlib.Path.cwd())) ) + specification = Specification.load(specification, arguments=arguments) + options = self.options.replace( - arguments=arguments, auth_all_paths=auth_all_paths, jsonifier=jsonifier, swagger_ui_options=swagger_ui_options, diff --git a/connexion/middleware/routing.py b/connexion/middleware/routing.py index bb3221b31..7f4be932f 100644 --- a/connexion/middleware/routing.py +++ b/connexion/middleware/routing.py @@ -1,4 +1,3 @@ -import pathlib import typing as t from contextvars import ContextVar @@ -14,6 +13,7 @@ ) from connexion.operations import AbstractOperation from connexion.resolver import Resolver +from connexion.spec import Specification _scope: ContextVar[dict] = ContextVar("SCOPE") @@ -50,7 +50,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: class RoutingAPI(AbstractRoutingAPI): def __init__( self, - specification: t.Union[pathlib.Path, str, dict], + specification: Specification, *, next_app: ASGIApp, base_path: t.Optional[str] = None, @@ -110,14 +110,14 @@ def __init__(self, app: ASGIApp) -> None: def add_api( self, - specification: t.Union[pathlib.Path, str, dict], + specification: Specification, base_path: t.Optional[str] = None, arguments: t.Optional[dict] = None, **kwargs, ) -> None: """Add an API to the router based on a OpenAPI spec. - :param specification: OpenAPI spec as dict or path to file. + :param specification: OpenAPI spec. :param base_path: Base path where to add this API. :param arguments: Jinja arguments to replace in the spec. """ diff --git a/connexion/middleware/security.py b/connexion/middleware/security.py index 7180c56e6..f6d150fcf 100644 --- a/connexion/middleware/security.py +++ b/connexion/middleware/security.py @@ -9,6 +9,7 @@ from connexion.middleware.abstract import RoutedAPI, RoutedMiddleware from connexion.operations import AbstractOperation from connexion.security import SecurityHandlerFactory +from connexion.spec import Specification logger = logging.getLogger("connexion.middleware.security") @@ -31,11 +32,21 @@ def __init__( @classmethod def from_operation( cls, - operation: AbstractOperation, + operation: t.Union[AbstractOperation, Specification], *, next_app: ASGIApp, security_handler_factory: SecurityHandlerFactory, ) -> "SecurityOperation": + """Create a SecurityOperation from an Operation of Specification instance + + :param operation: The operation can be both an Operation or Specification instance here + since security is defined at both levels in the OpenAPI spec. Creating a + SecurityOperation based on a Specification can be used to create a SecurityOperation + for routes not explicitly defined in the specification. + :param next_app: The next ASGI app to call. + :param security_handler_factory: The factory to be used to generate security handlers for + the different security schemes. + """ return cls( next_app=next_app, security_handler_factory=security_handler_factory, @@ -120,7 +131,16 @@ def add_auth_on_not_found(self) -> None: default_operation = self.make_operation(self.specification) self.operations = defaultdict(lambda: default_operation) - def make_operation(self, operation: AbstractOperation) -> SecurityOperation: + def make_operation( + self, operation: t.Union[AbstractOperation, Specification] + ) -> SecurityOperation: + """Create a SecurityOperation from an Operation of Specification instance + + :param operation: The operation can be both an Operation or Specification instance here + since security is defined at both levels in the OpenAPI spec. Creating a + SecurityOperation based on a Specification can be used to create a SecurityOperation + for routes not explicitly defined in the specification. + """ return SecurityOperation.from_operation( operation, next_app=self.next_app, diff --git a/connexion/middleware/swagger_ui.py b/connexion/middleware/swagger_ui.py index 61d43ac36..90d67fa45 100644 --- a/connexion/middleware/swagger_ui.py +++ b/connexion/middleware/swagger_ui.py @@ -1,6 +1,5 @@ import json import logging -import pathlib import re import typing as t from contextvars import ContextVar @@ -17,6 +16,7 @@ from connexion.middleware import SpecMiddleware from connexion.middleware.abstract import AbstractSpecAPI from connexion.options import SwaggerUIConfig, SwaggerUIOptions +from connexion.spec import Specification from connexion.utils import yamldumper logger = logging.getLogger("connexion.middleware.swagger_ui") @@ -191,14 +191,14 @@ def __init__(self, app: ASGIApp) -> None: def add_api( self, - specification: t.Union[pathlib.Path, str, dict], + specification: Specification, base_path: t.Optional[str] = None, arguments: t.Optional[dict] = None, **kwargs ) -> None: """Add an API to the router based on a OpenAPI spec. - :param specification: OpenAPI spec as dict or path to file. + :param specification: OpenAPI spec. :param base_path: Base path where to add this API. :param arguments: Jinja arguments to replace in the spec. """ diff --git a/connexion/operations/abstract.py b/connexion/operations/abstract.py index 9a79c293e..ddd5196d4 100644 --- a/connexion/operations/abstract.py +++ b/connexion/operations/abstract.py @@ -76,6 +76,11 @@ def __init__( self._responses = self._operation.get("responses", {}) + @classmethod + @abc.abstractmethod + def from_spec(cls, spec, *args, path, method, resolver, **kwargs): + pass + @property def method(self): """ diff --git a/connexion/spec.py b/connexion/spec.py index a142ed306..91ff3cd93 100644 --- a/connexion/spec.py +++ b/connexion/spec.py @@ -8,6 +8,7 @@ import os import pathlib import pkgutil +import typing as t from collections.abc import Mapping from urllib.parse import urlsplit @@ -19,7 +20,7 @@ from .exceptions import InvalidSpecification from .json_schema import NullableTypeValidator, resolve_refs -from .operations import OpenAPIOperation, Swagger2Operation +from .operations import AbstractOperation, OpenAPIOperation, Swagger2Operation from .utils import deep_get validate_properties = Draft4Validator.VALIDATORS["properties"] @@ -72,6 +73,9 @@ def canonical_base_path(base_path): class Specification(Mapping): + + operation_cls: t.Type[AbstractOperation] + def __init__(self, raw_spec, *, base_uri=""): self._raw_spec = copy.deepcopy(raw_spec) self._set_defaults(raw_spec) @@ -206,6 +210,16 @@ def with_base_path(self, base_path): new_spec.base_path = base_path return new_spec + @property + @abc.abstractmethod + def base_path(self): + pass + + @base_path.setter + @abc.abstractmethod + def base_path(self, base_path): + pass + class Swagger2Specification(Specification): """Python interface for a Swagger 2 specification.""" diff --git a/tests/test_api.py b/tests/test_api.py index 4c9a40887..388429792 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -20,41 +20,52 @@ def test_canonical_base_path(): def test_api(): - api = FlaskApi(TEST_FOLDER / "fixtures/simple/swagger.yaml", base_path="/api/v1.0") + api = FlaskApi( + Specification.load(TEST_FOLDER / "fixtures/simple/swagger.yaml"), + base_path="/api/v1.0", + ) assert api.blueprint.name == "/api/v1_0" assert api.blueprint.url_prefix == "/api/v1.0" - api2 = FlaskApi(TEST_FOLDER / "fixtures/simple/swagger.yaml") + api2 = FlaskApi(Specification.load(TEST_FOLDER / "fixtures/simple/swagger.yaml")) assert api2.blueprint.name == "/v1_0" assert api2.blueprint.url_prefix == "/v1.0" - api3 = FlaskApi(TEST_FOLDER / "fixtures/simple/openapi.yaml", base_path="/api/v1.0") + api3 = FlaskApi( + Specification.load(TEST_FOLDER / "fixtures/simple/openapi.yaml"), + base_path="/api/v1.0", + ) assert api3.blueprint.name == "/api/v1_0" assert api3.blueprint.url_prefix == "/api/v1.0" - api4 = FlaskApi(TEST_FOLDER / "fixtures/simple/openapi.yaml") + api4 = FlaskApi(Specification.load(TEST_FOLDER / "fixtures/simple/openapi.yaml")) assert api4.blueprint.name == "/v1_0" assert api4.blueprint.url_prefix == "/v1.0" def test_api_base_path_slash(): - api = FlaskApi(TEST_FOLDER / "fixtures/simple/basepath-slash.yaml") + api = FlaskApi( + Specification.load(TEST_FOLDER / "fixtures/simple/basepath-slash.yaml") + ) assert api.blueprint.name == "/" assert api.blueprint.url_prefix == "" def test_template(): api1 = FlaskApi( - TEST_FOLDER / "fixtures/simple/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/simple/swagger.yaml", arguments={"title": "test"} + ), base_path="/api/v1.0", - arguments={"title": "test"}, ) assert api1.specification["info"]["title"] == "test" api2 = FlaskApi( - TEST_FOLDER / "fixtures/simple/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/simple/swagger.yaml", + arguments={"title": "other test"}, + ), base_path="/api/v1.0", - arguments={"title": "other test"}, ) assert api2.specification["info"]["title"] == "other test" @@ -62,30 +73,38 @@ def test_template(): def test_invalid_operation_does_stop_application_to_setup(): with pytest.raises(ResolverError): FlaskApi( - TEST_FOLDER / "fixtures/op_error_api/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/op_error_api/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) with pytest.raises(ResolverError): FlaskApi( - TEST_FOLDER / "fixtures/missing_op_id/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/missing_op_id/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) with pytest.raises(ResolverError): FlaskApi( - TEST_FOLDER / "fixtures/module_not_implemented/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/module_not_implemented/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) with pytest.raises(ResolverError): FlaskApi( - TEST_FOLDER / "fixtures/user_module_loading_error/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/user_module_loading_error/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) @@ -93,18 +112,22 @@ def test_other_errors_stop_application_to_setup(): # Errors should still result exceptions! with pytest.raises(InvalidSpecification): FlaskApi( - TEST_FOLDER / "fixtures/bad_specs/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/bad_specs/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) def test_invalid_schema_file_structure(): with pytest.raises(InvalidSpecification): FlaskApi( - TEST_FOLDER / "fixtures/invalid_schema/swagger.yaml", + Specification.load( + TEST_FOLDER / "fixtures/invalid_schema/swagger.yaml", + arguments={"title": "OK"}, + ), base_path="/api/v1.0", - arguments={"title": "OK"}, ) @@ -115,7 +138,7 @@ def test_invalid_encoding(): "gbk" ) ) - FlaskApi(pathlib.Path(f.name), base_path="/api/v1.0") + FlaskApi(Specification.load(pathlib.Path(f.name)), base_path="/api/v1.0") os.unlink(f.name) @@ -124,7 +147,7 @@ def test_use_of_safe_load_for_yaml_swagger_specs(): with tempfile.NamedTemporaryFile(delete=False) as f: f.write(b"!!python/object:object {}\n") try: - FlaskApi(pathlib.Path(f.name), base_path="/api/v1.0") + FlaskApi(Specification.load(pathlib.Path(f.name)), base_path="/api/v1.0") os.unlink(f.name) except InvalidSpecification: pytest.fail("Could load invalid YAML file, use yaml.safe_load!") @@ -134,7 +157,7 @@ def test_validation_error_on_completely_invalid_swagger_spec(): with tempfile.NamedTemporaryFile(delete=False) as f: f.write(b"[1]\n") with pytest.raises(InvalidSpecification): - FlaskApi(pathlib.Path(f.name), base_path="/api/v1.0") + FlaskApi(Specification.load(pathlib.Path(f.name)), base_path="/api/v1.0") os.unlink(f.name)