Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor routing into middleware-api-operation model #1533

Merged
merged 2 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion connexion/apis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
"""


from .abstract import (AbstractAPI, AbstractMinimalAPI, # NOQA
from .abstract import (AbstractAPI, AbstractRoutingAPI, # NOQA
AbstractSwaggerUIAPI)
4 changes: 2 additions & 2 deletions connexion/apis/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def add_swagger_ui(self):
"""


class AbstractMinimalAPI(AbstractSpecAPI):
class AbstractRoutingAPI(AbstractSpecAPI):

def __init__(
self,
Expand Down Expand Up @@ -203,7 +203,7 @@ def _handle_add_operation_error(self, path: str, method: str, exc_info: tuple):
raise value.with_traceback(traceback)


class AbstractAPI(AbstractMinimalAPI, metaclass=AbstractAPIMeta):
class AbstractAPI(AbstractRoutingAPI, metaclass=AbstractAPIMeta):
"""
Defines an abstract interface for a Swagger API
"""
Expand Down
7 changes: 7 additions & 0 deletions connexion/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,10 @@ def randomize_endpoint(self):

def get_path_parameter_types(self):
return {}

async def __call__(self, *args, **kwargs):
Ruwann marked this conversation as resolved.
Show resolved Hide resolved
raise ResolverProblem(
title='Not Implemented',
detail=self.exception.reason,
status=self.status_code
)
120 changes: 37 additions & 83 deletions connexion/middleware/routing.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,19 @@
import pathlib
import typing as t
from contextlib import contextmanager
from contextvars import ContextVar

from starlette.requests import Request as StarletteRequest
from starlette.routing import Router
from starlette.types import ASGIApp, Receive, Scope, Send

from connexion.apis import AbstractMinimalAPI
from connexion.apis import AbstractRoutingAPI
from connexion.exceptions import NotFoundProblem
from connexion.middleware import AppMiddleware
from connexion.operations import AbstractOperation, make_operation
from connexion.resolver import Resolver

ROUTING_CONTEXT = 'connexion_routing'


_scope_receive_send: ContextVar[tuple] = ContextVar('SCOPE_RECEIVE_SEND')


class MiddlewareResolver(Resolver):

def __init__(self, call_next: t.Callable) -> None:
"""Resolver that resolves each operation to the provided call_next function."""
super().__init__()
self.call_next = call_next

def resolve_function_from_operation_id(self, operation_id: str) -> t.Callable:
return self.call_next
_scope: ContextVar[dict] = ContextVar('SCOPE')


class RoutingMiddleware(AppMiddleware):
Expand All @@ -40,7 +26,7 @@ def __init__(self, app: ASGIApp) -> None:
"""
self.app = app
# Pass unknown routes to next app
self.router = Router(default=self.default_fn)
self.router = Router(default=RoutingOperation(None, self.app))

def add_api(
self,
Expand All @@ -55,10 +41,8 @@ def add_api(
:param base_path: Base path where to add this API.
:param arguments: Jinja arguments to replace in the spec.
"""
kwargs.pop("resolver", None)
resolver = MiddlewareResolver(self.create_call_next())
api = MiddlewareAPI(specification, base_path=base_path, arguments=arguments,
resolver=resolver, default=self.default_fn, **kwargs)
api = RoutingAPI(specification, base_path=base_path, arguments=arguments,
next_app=self.app, **kwargs)
self.router.mount(api.base_path, app=api.router)

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
Expand All @@ -68,7 +52,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
await self.app(scope, receive, send)
return

_scope_receive_send.set((scope.copy(), receive, send))
_scope.set(scope.copy())

# Needs to be set so starlette router throws exceptions instead of returning error responses
scope['app'] = self
Expand All @@ -77,56 +61,23 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
except ValueError:
raise NotFoundProblem

async def default_fn(self, scope: Scope, receive: Receive, send: Send) -> None:
"""Callback to call next app as default when no matching route is found."""
original_scope, *_ = _scope_receive_send.get()

api_base_path = scope.get('root_path', '')[len(original_scope.get('root_path', '')):]

extensions = original_scope.setdefault('extensions', {})
connexion_routing = extensions.setdefault(ROUTING_CONTEXT, {})
connexion_routing.update({
'api_base_path': api_base_path
})
await self.app(original_scope, receive, send)

def create_call_next(self):

async def call_next(
operation: AbstractOperation,
request: StarletteRequest = None
) -> None:
"""Attach operation to scope and pass it to the next app"""
scope, receive, send = _scope_receive_send.get()

api_base_path = request.scope.get('root_path', '')[len(scope.get('root_path', '')):]

extensions = scope.setdefault('extensions', {})
connexion_routing = extensions.setdefault(ROUTING_CONTEXT, {})
connexion_routing.update({
'api_base_path': api_base_path,
'operation_id': operation.operation_id
})
return await self.app(scope, receive, send)

return call_next


class MiddlewareAPI(AbstractMinimalAPI):
class RoutingAPI(AbstractRoutingAPI):

def __init__(
self,
specification: t.Union[pathlib.Path, str, dict],
base_path: t.Optional[str] = None,
arguments: t.Optional[dict] = None,
resolver: t.Optional[Resolver] = None,
default: ASGIApp = None,
next_app: ASGIApp = None,
resolver_error_handler: t.Optional[t.Callable] = None,
debug: bool = False,
**kwargs
) -> None:
"""API implementation on top of Starlette Router for Connexion middleware."""
self.router = Router(default=default)
self.next_app = next_app
self.router = Router(default=RoutingOperation(None, next_app))

super().__init__(
specification,
Expand All @@ -138,28 +89,31 @@ def __init__(
)

def add_operation(self, path: str, method: str) -> None:
operation = make_operation(
self.specification,
self,
path,
method,
self.resolver
)
operation_cls = self.specification.operation_cls
operation = operation_cls.from_spec(self.specification, self, path, method, self.resolver)
routing_operation = RoutingOperation(operation.operation_id, next_app=self.next_app)
self._add_operation_internal(method, path, routing_operation)

def _add_operation_internal(self, method: str, path: str, operation: 'RoutingOperation') -> None:
self.router.add_route(path, operation, methods=[method])

@contextmanager
def patch_operation_function():
"""Patch the operation function so no decorators are set in the middleware. This
should be cleaned up by separating the APIs and Operations between the App and
middleware"""
original_operation_function = AbstractOperation.function
AbstractOperation.function = operation._resolution.function
try:
yield
finally:
AbstractOperation.function = original_operation_function

with patch_operation_function():
self._add_operation_internal(method, path, operation)

def _add_operation_internal(self, method: str, path: str, operation: AbstractOperation) -> None:
self.router.add_route(path, operation.function, methods=[method])

class RoutingOperation:

def __init__(self, operation_id: t.Optional[str], next_app: ASGIApp) -> None:
self.operation_id = operation_id
self.next_app = next_app

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
"""Attach operation to scope and pass it to the next app"""
original_scope = _scope.get()

api_base_path = scope.get('root_path', '')[len(original_scope.get('root_path', '')):]

extensions = original_scope.setdefault('extensions', {})
connexion_routing = extensions.setdefault(ROUTING_CONTEXT, {})
connexion_routing.update({
'api_base_path': api_base_path,
'operation_id': self.operation_id
})
await self.next_app(original_scope, receive, send)