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

Bugfix/basepath #1716

Merged
merged 7 commits into from
Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions connexion/middleware/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ def _build_middleware_stack(self) -> t.Tuple[ASGIApp, t.Iterable[ASGIApp]]:
app = middleware(app) # type: ignore
apps.append(app)

# We sort the APIs by base path so that the most specific APIs are registered first.
Ruwann marked this conversation as resolved.
Show resolved Hide resolved
# This is due to the way Starlette matches routes.
self.apis = utils.sort_apis_by_basepath(self.apis)
for app in apps:
if isinstance(app, SpecMiddleware):
for api in self.apis:
Expand Down
45 changes: 45 additions & 0 deletions connexion/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
import typing as t

import yaml
from starlette.routing import compile_path

from connexion.exceptions import TypeValidationError

if t.TYPE_CHECKING:
from connexion.middleware.main import API


def boolean(s):
"""
Expand Down Expand Up @@ -423,3 +427,44 @@ def inspect_function_arguments(function: t.Callable) -> t.Tuple[t.List[str], boo
]
has_kwargs = any(p.kind == p.VAR_KEYWORD for p in parameters.values())
return list(bound_arguments), has_kwargs


T = t.TypeVar("T")


def sort_routes(routes: t.List[T], *, key: t.Optional[t.Callable] = None) -> t.List[T]:
"""Sorts a list of routes from most specific to least specific.

See Starlette routing documentation and implementation as this function
is aimed to sort according to that logic.
- https://www.starlette.io/routing/#route-priority

For example:
- /users/me
- /users/{username}
- /users/{username}/projects/{project}

:param routes: List of routes to sort

:return: List of routes sorted from most specific to least specific
"""

class SortableRoute:
def __init__(self, path: str) -> None:
self.path = path
self.path_regex, _, _ = compile_path(path)

def __lt__(self, other: "SortableRoute") -> bool:
return bool(other.path_regex.match(self.path))

return sorted(routes, key=lambda r: SortableRoute(key(r) if key else r))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart 🤯



def sort_apis_by_basepath(apis: t.List["API"]) -> t.List["API"]:
"""Sorts a list of APIs by basepath.

:param apis: List of APIs to sort

:return: List of APIs sorted by basepath
"""
return sort_routes(apis, key=lambda api: api.base_path or "/")
65 changes: 65 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,68 @@ def test_is_json_mimetype():
"application/vnd.com.myEntreprise.v6+json; charset=UTF-8"
)
assert not utils.is_json_mimetype("text/html")


def test_sort_routes():
routes = ["/users/me", "/users/{username}"]
expected = ["/users/me", "/users/{username}"]
assert utils.sort_routes(routes) == expected

# Empty basepath case for Mount
# https://github.com/encode/starlette/blob/1c1043ca0ab7126419948b27f9d0a78270fd74e6/starlette/routing.py#L388
routes = ["/{path:path}", "/basepath/{path:path}"]
expected = ["/basepath/{path:path}", "/{path:path}"]
assert utils.sort_routes(routes) == expected

routes = ["/basepath/{path:path}", "/basepath/v2/{path:path}"]
expected = ["/basepath/v2/{path:path}", "/basepath/{path:path}"]
assert utils.sort_routes(routes) == expected

routes = ["/users/{username}", "/users/me"]
expected = ["/users/me", "/users/{username}"]
assert utils.sort_routes(routes) == expected

routes = [
"/users/{username}",
"/users/me",
"/users/{username}/items",
"/users/{username}/items/{item}",
]
expected = [
"/users/me",
"/users/{username}",
"/users/{username}/items",
"/users/{username}/items/{item}",
]
assert utils.sort_routes(routes) == expected

routes = [
"/users/{username}",
"/users/me",
"/users/{username}/items/{item}",
"/users/{username}/items/special",
]
expected = [
"/users/me",
"/users/{username}",
"/users/{username}/items/special",
"/users/{username}/items/{item}",
]
assert utils.sort_routes(routes) == expected


def test_sort_apis_by_basepath():
api1 = MagicMock(base_path="/{path:path}")
api2 = MagicMock(base_path="/basepath/{path:path}")
assert utils.sort_apis_by_basepath([api1, api2]) == [api2, api1]

api3 = MagicMock(base_path="/basepath/v2/{path:path}")
assert utils.sort_apis_by_basepath([api1, api2, api3]) == [api3, api2, api1]

api4 = MagicMock(base_path="/healthz")
assert utils.sort_apis_by_basepath([api1, api2, api3, api4]) == [
api3,
api2,
api4,
api1,
]