Skip to content

Commit

Permalink
Get rid of enrich_swagger (#188)
Browse files Browse the repository at this point in the history
  • Loading branch information
zmievsa authored May 29, 2024
1 parent 74f24f6 commit 327656d
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 199 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,26 @@ Please follow [the Keep a Changelog standard](https://keepachangelog.com/en/1.0.

## [Unreleased]

## [3.15.4]

### Changed

* `Cadwyn.enrich_swagger` is now completely unnecessary: openapi is now generated at runtime. It also now does not do anything, is deprecated, and will be removed in a future version

### Fixed

* fastapi-pagination now does not require an explicit call to `Cadwyn.enrich_swagger`



## [3.15.3]

### Changed

* `Cadwyn.router.routes` now includes all existing routers within the application instead of just unversioned routes

### Fixed

* Compatibility with fastapi-pagination
* High cardinality of metrics for routers with path variables in starlette-exporter

Expand Down
168 changes: 40 additions & 128 deletions cadwyn/applications.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
from collections.abc import Callable, Coroutine, Sequence
from datetime import date
from logging import getLogger
Expand All @@ -24,7 +25,6 @@
from starlette.types import Lifespan
from typing_extensions import Self, deprecated

from cadwyn._utils import same_definition_as_in
from cadwyn.middleware import HeaderVersioningMiddleware, _get_api_version_dependency
from cadwyn.route_generation import generate_versioned_routers
from cadwyn.routing import _RootHeaderAPIRouter
Expand Down Expand Up @@ -155,11 +155,10 @@ def __init__(
self.redoc_url = redoc_url
self.openapi_url = openapi_url
self.redoc_url = redoc_url
self.swaggers = {}

unversioned_router = APIRouter(**self._kwargs_to_router)
self._add_openapi_endpoints(unversioned_router)
self.add_unversioned_routers(unversioned_router)
self.include_router(unversioned_router)
self.add_middleware(
HeaderVersioningMiddleware,
api_version_header_name=self.router.api_version_header_name,
Expand Down Expand Up @@ -220,55 +219,45 @@ def generate_and_include_versioned_routers(self, *routers: APIRouter) -> None:
for version, router in router_versions.items():
self.add_header_versioned_routers(router, header_value=version.isoformat())

def enrich_swagger(self):
"""
This method goes through all header-based apps and collect a dict[openapi_version, openapi_json]
For each route a `X-API-VERSION` header with value is added
"""
unversioned_routes_openapi = get_openapi(
title=self.title,
version=self.version,
openapi_version=self.openapi_version,
description=self.description,
terms_of_service=self.terms_of_service,
contact=self.contact,
license_info=self.license_info,
routes=self.router.unversioned_routes,
tags=self.openapi_tags,
servers=self.servers,
async def openapi_jsons(self, req: Request) -> JSONResponse:
raw_version = req.query_params.get("version") or req.headers.get(self.router.api_version_header_name)
not_found_error = HTTPException(
status_code=404,
detail=f"OpenApi file of with version `{raw_version}` not found",
)
if unversioned_routes_openapi["paths"]:
self.swaggers["unversioned"] = unversioned_routes_openapi

for header_value, router in self.router.versioned_routers.items():
header_value_str = header_value.isoformat()
openapi = get_openapi(
try:
version = datetime.date.fromisoformat(raw_version) # pyright: ignore[reportArgumentType]
# TypeError when raw_version is None
# ValueError when raw_version is of the non-iso format
except (ValueError, TypeError):
version = raw_version

if version in self.router.versioned_routers:
routes = self.router.versioned_routers[version].routes
formatted_version = version.isoformat()
elif version == "unversioned" and self._there_are_public_unversioned_routes():
routes = self.router.unversioned_routes
formatted_version = "unversioned"
else:
raise not_found_error

return JSONResponse(
get_openapi(
title=self.title,
version=header_value.isoformat(),
version=formatted_version,
openapi_version=self.openapi_version,
description=self.description,
terms_of_service=self.terms_of_service,
contact=self.contact,
license_info=self.license_info,
routes=router.routes,
routes=routes,
tags=self.openapi_tags,
servers=self.servers,
)
# in current implementation we expect header_value to be a date
self.swaggers[header_value_str] = openapi

async def openapi_jsons(self, req: Request) -> JSONResponse:
version = req.query_params.get("version") or req.headers.get(self.router.api_version_header_name)
openapi_of_a_version = self.swaggers.get(version)
if not openapi_of_a_version:
raise HTTPException(
status_code=404,
detail=f"OpenApi file of with version `{version}` not found",
)
)

return JSONResponse(openapi_of_a_version)
def _there_are_public_unversioned_routes(self):
return any(isinstance(route, Route) and route.include_in_schema for route in self.router.unversioned_routes)

async def swagger_dashboard(self, req: Request) -> Response:
version = req.query_params.get("version")
Expand Down Expand Up @@ -303,12 +292,12 @@ def _extract_root_path(self, req: Request):

def _render_docs_dashboard(self, req: Request, docs_url: str):
base_url = str(req.base_url).rstrip("/")
table = {version: f"{base_url}{docs_url}?version={version}" for version in self.router.sorted_versions}
if self._there_are_public_unversioned_routes():
table |= {"unversioned": f"{base_url}{docs_url}?version=unversioned"}
return self._templates.TemplateResponse(
"docs.html",
{
"request": req,
"table": {version: f"{base_url}{docs_url}?version={version}" for version in sorted(self.swaggers)},
},
{"request": req, "table": table},
)

def add_header_versioned_routers(
Expand Down Expand Up @@ -347,94 +336,17 @@ def add_header_versioned_routers(
added_routes.extend(versioned_router.routes[-added_route_count:])
self.router.routes.extend(added_routes)

self.enrich_swagger()
return added_routes

@deprecated("Use builtin FastAPI methods such as include_router instead")
def add_unversioned_routers(self, *routers: APIRouter):
for router in routers:
self.router.include_router(router)
self.enrich_swagger()
self.include_router(router)

@deprecated("Use add add_unversioned_routers instead")
@deprecated("Use builtin FastAPI methods such as add_api_route instead")
def add_unversioned_routes(self, *routes: Route):
router = APIRouter(routes=list(routes))
self.include_router(router)
self.enrich_swagger()

@same_definition_as_in(FastAPI.include_router)
def include_router(self, *args: Any, **kwargs: Any):
route = super().include_router(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.post)
def post(self, *args: Any, **kwargs: Any):
route = super().post(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.get)
def get(self, *args: Any, **kwargs: Any):
route = super().get(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.patch)
def patch(self, *args: Any, **kwargs: Any):
route = super().patch(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.delete)
def delete(self, *args: Any, **kwargs: Any):
route = super().delete(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.put)
def put(self, *args: Any, **kwargs: Any):
route = super().put(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.trace)
def trace(self, *args: Any, **kwargs: Any): # pragma: no cover
route = super().trace(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.options)
def options(self, *args: Any, **kwargs: Any):
route = super().options(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.head)
def head(self, *args: Any, **kwargs: Any):
route = super().head(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.add_api_route)
def add_api_route(self, *args: Any, **kwargs: Any):
route = super().add_api_route(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.api_route)
def api_route(self, *args: Any, **kwargs: Any):
route = super().api_route(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.add_api_websocket_route)
def add_api_websocket_route(self, *args: Any, **kwargs: Any): # pragma: no cover
route = super().add_api_websocket_route(*args, **kwargs)
self.enrich_swagger()
return route

@same_definition_as_in(FastAPI.websocket)
def websocket(self, *args: Any, **kwargs: Any): # pragma: no cover
route = super().websocket(*args, **kwargs)
self.enrich_swagger()
return route

@deprecated("It no longer does anything")
def enrich_swagger(self): ...
6 changes: 3 additions & 3 deletions cadwyn/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
from fastapi.routing import APIRouter
from starlette.datastructures import URL
from starlette.responses import RedirectResponse
from starlette.routing import BaseRoute, Match, Route
from starlette.routing import BaseRoute, Match
from starlette.types import Receive, Scope, Send

from cadwyn._utils import same_definition_as_in

from .route_generation import (
InternalRepresentationOf,
InternalRepresentationOf, # pyright: ignore[reportDeprecated]
generate_versioned_routers,
)

Expand Down Expand Up @@ -51,7 +51,7 @@ def __init__(
self.versioned_routers: dict[date, APIRouter] = {}
self.api_version_header_name = api_version_header_name.lower()
self.api_version_var = api_version_var
self.unversioned_routes: list[Route] = []
self.unversioned_routes: list[BaseRoute] = []

@cached_property
def sorted_versions(self):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "cadwyn"
version = "3.15.3"
version = "3.15.4"
description = "Production-ready community-driven modern Stripe-like API versioning in FastAPI"
authors = ["Stanislav Zmiev <zmievsa@gmail.com>"]
license = "MIT"
Expand Down
4 changes: 2 additions & 2 deletions tests/_resources/versioned_app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ async def lifespan(app: FastAPI):
versioned_app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))), lifespan=lifespan)
versioned_app.add_header_versioned_routers(v2021_01_01_router, header_value="2021-01-01")
versioned_app.add_header_versioned_routers(v2022_01_02_router, header_value="2022-02-02")
versioned_app.add_unversioned_routers(webhooks_router)
versioned_app.add_unversioned_routers(webhooks_router) # pyright: ignore[reportDeprecated]

versioned_app_with_custom_api_version_var = Cadwyn(
versions=VersionBundle(Version(date(2022, 11, 16))), lifespan=lifespan, api_version_var=ContextVar("My api version")
)
versioned_app_with_custom_api_version_var.add_header_versioned_routers(v2021_01_01_router, header_value="2021-01-01")
versioned_app_with_custom_api_version_var.add_header_versioned_routers(v2022_01_02_router, header_value="2022-02-02")
versioned_app_with_custom_api_version_var.add_unversioned_routers(webhooks_router)
versioned_app_with_custom_api_version_var.add_unversioned_routers(webhooks_router) # pyright: ignore[reportDeprecated]

# TODO: We should not have any clients that are run like this. Instead, all of them must run using "with"
client = TestClient(versioned_app, raise_server_exceptions=False, headers=BASIC_HEADERS)
Expand Down
66 changes: 63 additions & 3 deletions tests/test_applications.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
)


def test__cadwyn_enrich_swagger__still_exists_and_is_deprecated():
app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
with pytest.deprecated_call():
app.enrich_swagger() # pyright: ignore[reportDeprecated]


def test__header_routing__invalid_version_format__error():
main_app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
main_app.add_header_versioned_routers(APIRouter(), header_value=DEFAULT_API_VERSION)
Expand Down Expand Up @@ -65,6 +71,30 @@ def test__header_routing_fastapi_init__changing_openapi_url__docs_still_return_2
assert client.get("/openapi.json?version=2021-01-01").status_code == 404


def test__header_routing_fastapi__calling_openapi_incorrectly__docs_should_return_404():
app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
app.add_header_versioned_routers(v2021_01_01_router, header_value="2021-01-01")
app.add_header_versioned_routers(v2022_01_02_router, header_value="2022-02-02")
with TestClient(app) as client:
assert client.get("/openapi.json?version=2021-01-01").status_code == 200
# - Nonexisting version
assert client.get("/openapi.json?version=2019-01-01").status_code == 404
# - Nonexisting but compatible version
assert client.get("/openapi.json?version=2024-01-01").status_code == 404
# - version = null
assert client.get("/openapi.json?version=").status_code == 404
# - version not passed at all
assert client.get("/openapi.json").status_code == 404
# - Unversioned when we haven't added any
assert client.get("/openapi.json?version=unversioned").status_code == 404

@app.post("/my_unversioned_route")
def my_unversioned_route():
raise NotImplementedError

assert client.get("/openapi.json?version=unversioned").status_code == 200


def test__header_routing_fastapi_add_header_versioned_routers__apirouter_is_empty__version_should_not_have_any_routes():
app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
app.add_header_versioned_routers(APIRouter(), header_value="2022-11-16")
Expand Down Expand Up @@ -118,14 +148,44 @@ def test__get_openapi__nonexisting_version__error():
assert resp.json() == {"detail": "OpenApi file of with version `2023-01-01` not found"}


def test__get_docs__all_versions():
resp = client_without_headers.get("/docs")
def test__get_docs__without_unversioned_routes__should_return_all_versioned_doc_urls():
app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
app.add_header_versioned_routers(v2021_01_01_router, header_value="2021-01-01")
app.add_header_versioned_routers(v2022_01_02_router, header_value="2022-02-02")

client = TestClient(app)

resp = client.get("/docs")
assert resp.status_code == 200
assert "http://testserver/docs?version=2021-01-01" in resp.text
assert "http://testserver/docs?version=2022-02-02" in resp.text
assert "http://testserver/docs?version=unversioned" not in resp.text

resp = client.get("/redoc")
assert resp.status_code == 200
assert "http://testserver/redoc?version=2021-01-01" in resp.text
assert "http://testserver/redoc?version=2022-02-02" in resp.text
assert "http://testserver/redoc?version=unversioned" not in resp.text


def test__get_docs__with_unversioned_routes__should_return_all_versioned_doc_urls():
app = Cadwyn(versions=VersionBundle(Version(date(2022, 11, 16))))
app.add_header_versioned_routers(v2021_01_01_router, header_value="2021-01-01")
app.add_header_versioned_routers(v2022_01_02_router, header_value="2022-02-02")

@app.post("/my_unversioned_route")
def my_unversioned_route():
raise NotImplementedError

client = TestClient(app)

resp = client.get("/docs")
assert resp.status_code == 200
assert "http://testserver/docs?version=2022-02-02" in resp.text
assert "http://testserver/docs?version=2021-01-01" in resp.text
assert "http://testserver/docs?version=unversioned" in resp.text

resp = client_without_headers.get("/redoc")
resp = client.get("/redoc")
assert resp.status_code == 200
assert "http://testserver/redoc?version=2022-02-02" in resp.text
assert "http://testserver/redoc?version=2021-01-01" in resp.text
Expand Down
Loading

0 comments on commit 327656d

Please sign in to comment.