Only instantiate specification once (#1819)

Fixes #1801 

I had to make quite a few additional changes to satisfy mypy.
This commit is contained in:
Robbe Sneyders
2023-11-30 23:59:26 +01:00
committed by GitHub
parent bbd085bd39
commit 0857710147
9 changed files with 108 additions and 49 deletions

View File

@@ -43,7 +43,7 @@ repos:
args: ["tests"] args: ["tests"]
- repo: https://github.com/pre-commit/mirrors-mypy - repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961 rev: v0.981
hooks: hooks:
- id: mypy - id: mypy
files: "^connexion/" files: "^connexion/"

View File

@@ -1,6 +1,5 @@
import abc import abc
import logging import logging
import pathlib
import typing as t import typing as t
from collections import defaultdict from collections import defaultdict
@@ -22,9 +21,7 @@ class SpecMiddleware(abc.ABC):
base class""" base class"""
@abc.abstractmethod @abc.abstractmethod
def add_api( def add_api(self, specification: Specification, **kwargs) -> t.Any:
self, specification: t.Union[pathlib.Path, str, dict], **kwargs
) -> t.Any:
""" """
Register an API represented by a single OpenAPI specification on this middleware. Register an API represented by a single OpenAPI specification on this middleware.
Multiple APIs can be registered on a single middleware. Multiple APIs can be registered on a single middleware.
@@ -40,15 +37,14 @@ class AbstractSpecAPI:
def __init__( def __init__(
self, self,
specification: t.Union[pathlib.Path, str, dict], specification: Specification,
base_path: t.Optional[str] = None, base_path: t.Optional[str] = None,
resolver: t.Optional[Resolver] = None, resolver: t.Optional[Resolver] = None,
arguments: t.Optional[dict] = None,
uri_parser_class=None, uri_parser_class=None,
*args, *args,
**kwargs, **kwargs,
): ):
self.specification = Specification.load(specification, arguments=arguments) self.specification = specification
self.uri_parser_class = uri_parser_class self.uri_parser_class = uri_parser_class
self._set_base_path(base_path) self._set_base_path(base_path)
@@ -88,7 +84,7 @@ class AbstractRoutingAPI(AbstractSpecAPI, t.Generic[OP]):
""" """
Adds the paths defined in the specification as operations. 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(): for path, methods in paths.items():
logger.debug("Adding %s%s...", self.base_path, path) logger.debug("Adding %s%s...", self.base_path, path)
@@ -176,7 +172,7 @@ class AbstractRoutingAPI(AbstractSpecAPI, t.Generic[OP]):
class RoutedAPI(AbstractSpecAPI, t.Generic[OP]): class RoutedAPI(AbstractSpecAPI, t.Generic[OP]):
def __init__( def __init__(
self, self,
specification: t.Union[pathlib.Path, str, dict], specification: Specification,
*args, *args,
next_app: ASGIApp, next_app: ASGIApp,
**kwargs, **kwargs,
@@ -235,7 +231,7 @@ class RoutedMiddleware(SpecMiddleware, t.Generic[API]):
self.app = app self.app = app
self.apis: t.Dict[str, t.List[API]] = defaultdict(list) 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) api = self.api_cls(specification, next_app=self.app, **kwargs)
self.apis[api.base_path].append(api) self.apis[api.base_path].append(api)
return api return api

View File

@@ -24,6 +24,7 @@ from connexion.middleware.security import SecurityMiddleware
from connexion.middleware.swagger_ui import SwaggerUIMiddleware from connexion.middleware.swagger_ui import SwaggerUIMiddleware
from connexion.options import SwaggerUIOptions from connexion.options import SwaggerUIOptions
from connexion.resolver import Resolver from connexion.resolver import Resolver
from connexion.spec import Specification
from connexion.types import MaybeAwaitable from connexion.types import MaybeAwaitable
from connexion.uri_parsing import AbstractURIParser from connexion.uri_parsing import AbstractURIParser
from connexion.utils import inspect_function_arguments from connexion.utils import inspect_function_arguments
@@ -390,18 +391,18 @@ class ConnexionMiddleware:
if self.middleware_stack is not None: if self.middleware_stack is not None:
raise RuntimeError("Cannot add api after an application has started") raise RuntimeError("Cannot add api after an application has started")
if isinstance(specification, dict): if isinstance(specification, (pathlib.Path, str)):
specification = specification
else:
specification = t.cast(pathlib.Path, self.specification_dir / specification) specification = t.cast(pathlib.Path, self.specification_dir / specification)
# Add specification as file to watch for reloading # Add specification as file to watch for reloading
if pathlib.Path.cwd() in specification.parents: if pathlib.Path.cwd() in specification.parents:
self.extra_files.append( self.extra_files.append(
str(specification.relative_to(pathlib.Path.cwd())) str(specification.relative_to(pathlib.Path.cwd()))
) )
specification = Specification.load(specification, arguments=arguments)
options = self.options.replace( options = self.options.replace(
arguments=arguments,
auth_all_paths=auth_all_paths, auth_all_paths=auth_all_paths,
jsonifier=jsonifier, jsonifier=jsonifier,
swagger_ui_options=swagger_ui_options, swagger_ui_options=swagger_ui_options,

View File

@@ -1,4 +1,3 @@
import pathlib
import typing as t import typing as t
from contextvars import ContextVar from contextvars import ContextVar
@@ -14,6 +13,7 @@ from connexion.middleware.abstract import (
) )
from connexion.operations import AbstractOperation from connexion.operations import AbstractOperation
from connexion.resolver import Resolver from connexion.resolver import Resolver
from connexion.spec import Specification
_scope: ContextVar[dict] = ContextVar("SCOPE") _scope: ContextVar[dict] = ContextVar("SCOPE")
@@ -50,7 +50,7 @@ class RoutingOperation:
class RoutingAPI(AbstractRoutingAPI): class RoutingAPI(AbstractRoutingAPI):
def __init__( def __init__(
self, self,
specification: t.Union[pathlib.Path, str, dict], specification: Specification,
*, *,
next_app: ASGIApp, next_app: ASGIApp,
base_path: t.Optional[str] = None, base_path: t.Optional[str] = None,
@@ -110,14 +110,14 @@ class RoutingMiddleware(SpecMiddleware):
def add_api( def add_api(
self, self,
specification: t.Union[pathlib.Path, str, dict], specification: Specification,
base_path: t.Optional[str] = None, base_path: t.Optional[str] = None,
arguments: t.Optional[dict] = None, arguments: t.Optional[dict] = None,
**kwargs, **kwargs,
) -> None: ) -> None:
"""Add an API to the router based on a OpenAPI spec. """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 base_path: Base path where to add this API.
:param arguments: Jinja arguments to replace in the spec. :param arguments: Jinja arguments to replace in the spec.
""" """

View File

@@ -9,6 +9,7 @@ from connexion.lifecycle import ConnexionRequest
from connexion.middleware.abstract import RoutedAPI, RoutedMiddleware from connexion.middleware.abstract import RoutedAPI, RoutedMiddleware
from connexion.operations import AbstractOperation from connexion.operations import AbstractOperation
from connexion.security import SecurityHandlerFactory from connexion.security import SecurityHandlerFactory
from connexion.spec import Specification
logger = logging.getLogger("connexion.middleware.security") logger = logging.getLogger("connexion.middleware.security")
@@ -31,11 +32,21 @@ class SecurityOperation:
@classmethod @classmethod
def from_operation( def from_operation(
cls, cls,
operation: AbstractOperation, operation: t.Union[AbstractOperation, Specification],
*, *,
next_app: ASGIApp, next_app: ASGIApp,
security_handler_factory: SecurityHandlerFactory, security_handler_factory: SecurityHandlerFactory,
) -> "SecurityOperation": ) -> "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( return cls(
next_app=next_app, next_app=next_app,
security_handler_factory=security_handler_factory, security_handler_factory=security_handler_factory,
@@ -120,7 +131,16 @@ class SecurityAPI(RoutedAPI[SecurityOperation]):
default_operation = self.make_operation(self.specification) default_operation = self.make_operation(self.specification)
self.operations = defaultdict(lambda: default_operation) 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( return SecurityOperation.from_operation(
operation, operation,
next_app=self.next_app, next_app=self.next_app,

View File

@@ -1,6 +1,5 @@
import json import json
import logging import logging
import pathlib
import re import re
import typing as t import typing as t
from contextvars import ContextVar from contextvars import ContextVar
@@ -17,6 +16,7 @@ from connexion.jsonifier import Jsonifier
from connexion.middleware import SpecMiddleware from connexion.middleware import SpecMiddleware
from connexion.middleware.abstract import AbstractSpecAPI from connexion.middleware.abstract import AbstractSpecAPI
from connexion.options import SwaggerUIConfig, SwaggerUIOptions from connexion.options import SwaggerUIConfig, SwaggerUIOptions
from connexion.spec import Specification
from connexion.utils import yamldumper from connexion.utils import yamldumper
logger = logging.getLogger("connexion.middleware.swagger_ui") logger = logging.getLogger("connexion.middleware.swagger_ui")
@@ -191,14 +191,14 @@ class SwaggerUIMiddleware(SpecMiddleware):
def add_api( def add_api(
self, self,
specification: t.Union[pathlib.Path, str, dict], specification: Specification,
base_path: t.Optional[str] = None, base_path: t.Optional[str] = None,
arguments: t.Optional[dict] = None, arguments: t.Optional[dict] = None,
**kwargs **kwargs
) -> None: ) -> None:
"""Add an API to the router based on a OpenAPI spec. """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 base_path: Base path where to add this API.
:param arguments: Jinja arguments to replace in the spec. :param arguments: Jinja arguments to replace in the spec.
""" """

View File

@@ -76,6 +76,11 @@ class AbstractOperation(metaclass=abc.ABCMeta):
self._responses = self._operation.get("responses", {}) self._responses = self._operation.get("responses", {})
@classmethod
@abc.abstractmethod
def from_spec(cls, spec, *args, path, method, resolver, **kwargs):
pass
@property @property
def method(self): def method(self):
""" """

View File

@@ -8,6 +8,7 @@ import json
import os import os
import pathlib import pathlib
import pkgutil import pkgutil
import typing as t
from collections.abc import Mapping from collections.abc import Mapping
from urllib.parse import urlsplit from urllib.parse import urlsplit
@@ -19,7 +20,7 @@ from jsonschema.validators import extend as extend_validator
from .exceptions import InvalidSpecification from .exceptions import InvalidSpecification
from .json_schema import NullableTypeValidator, resolve_refs from .json_schema import NullableTypeValidator, resolve_refs
from .operations import OpenAPIOperation, Swagger2Operation from .operations import AbstractOperation, OpenAPIOperation, Swagger2Operation
from .utils import deep_get from .utils import deep_get
validate_properties = Draft4Validator.VALIDATORS["properties"] validate_properties = Draft4Validator.VALIDATORS["properties"]
@@ -72,6 +73,9 @@ def canonical_base_path(base_path):
class Specification(Mapping): class Specification(Mapping):
operation_cls: t.Type[AbstractOperation]
def __init__(self, raw_spec, *, base_uri=""): def __init__(self, raw_spec, *, base_uri=""):
self._raw_spec = copy.deepcopy(raw_spec) self._raw_spec = copy.deepcopy(raw_spec)
self._set_defaults(raw_spec) self._set_defaults(raw_spec)
@@ -206,6 +210,16 @@ class Specification(Mapping):
new_spec.base_path = base_path new_spec.base_path = base_path
return new_spec 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): class Swagger2Specification(Specification):
"""Python interface for a Swagger 2 specification.""" """Python interface for a Swagger 2 specification."""

View File

@@ -20,41 +20,52 @@ def test_canonical_base_path():
def test_api(): 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.name == "/api/v1_0"
assert api.blueprint.url_prefix == "/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.name == "/v1_0"
assert api2.blueprint.url_prefix == "/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.name == "/api/v1_0"
assert api3.blueprint.url_prefix == "/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.name == "/v1_0"
assert api4.blueprint.url_prefix == "/v1.0" assert api4.blueprint.url_prefix == "/v1.0"
def test_api_base_path_slash(): 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.name == "/"
assert api.blueprint.url_prefix == "" assert api.blueprint.url_prefix == ""
def test_template(): def test_template():
api1 = FlaskApi( api1 = FlaskApi(
TEST_FOLDER / "fixtures/simple/swagger.yaml", Specification.load(
TEST_FOLDER / "fixtures/simple/swagger.yaml", arguments={"title": "test"}
),
base_path="/api/v1.0", base_path="/api/v1.0",
arguments={"title": "test"},
) )
assert api1.specification["info"]["title"] == "test" assert api1.specification["info"]["title"] == "test"
api2 = FlaskApi( api2 = FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/simple/swagger.yaml", TEST_FOLDER / "fixtures/simple/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "other test"}, arguments={"title": "other test"},
),
base_path="/api/v1.0",
) )
assert api2.specification["info"]["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(): def test_invalid_operation_does_stop_application_to_setup():
with pytest.raises(ResolverError): with pytest.raises(ResolverError):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/op_error_api/swagger.yaml", TEST_FOLDER / "fixtures/op_error_api/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
with pytest.raises(ResolverError): with pytest.raises(ResolverError):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/missing_op_id/swagger.yaml", TEST_FOLDER / "fixtures/missing_op_id/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
with pytest.raises(ResolverError): with pytest.raises(ResolverError):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/module_not_implemented/swagger.yaml", TEST_FOLDER / "fixtures/module_not_implemented/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
with pytest.raises(ResolverError): with pytest.raises(ResolverError):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/user_module_loading_error/swagger.yaml", TEST_FOLDER / "fixtures/user_module_loading_error/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
@@ -93,18 +112,22 @@ def test_other_errors_stop_application_to_setup():
# Errors should still result exceptions! # Errors should still result exceptions!
with pytest.raises(InvalidSpecification): with pytest.raises(InvalidSpecification):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/bad_specs/swagger.yaml", TEST_FOLDER / "fixtures/bad_specs/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
def test_invalid_schema_file_structure(): def test_invalid_schema_file_structure():
with pytest.raises(InvalidSpecification): with pytest.raises(InvalidSpecification):
FlaskApi( FlaskApi(
Specification.load(
TEST_FOLDER / "fixtures/invalid_schema/swagger.yaml", TEST_FOLDER / "fixtures/invalid_schema/swagger.yaml",
base_path="/api/v1.0",
arguments={"title": "OK"}, arguments={"title": "OK"},
),
base_path="/api/v1.0",
) )
@@ -115,7 +138,7 @@ def test_invalid_encoding():
"gbk" "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) 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: with tempfile.NamedTemporaryFile(delete=False) as f:
f.write(b"!!python/object:object {}\n") f.write(b"!!python/object:object {}\n")
try: 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) os.unlink(f.name)
except InvalidSpecification: except InvalidSpecification:
pytest.fail("Could load invalid YAML file, use yaml.safe_load!") 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: with tempfile.NamedTemporaryFile(delete=False) as f:
f.write(b"[1]\n") f.write(b"[1]\n")
with pytest.raises(InvalidSpecification): 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) os.unlink(f.name)