Add support for fastapi's lazy routers (#379)

Support FastAPI 0.137.1+ included-router trees, including routes added
after inclusion, for both versioned and unversioned Cadwyn routing

Author: zmievsa

CHANGELOG.md

@@ -5,6 +5,12 @@ Please follow [the Keep a Changelog standard](https://keepachangelog.com/en/1.0.
 
 ## [Unreleased]
 
+## [7.1.0]
+
+### Added
+
+- Support FastAPI 0.137.1+ included-router trees, including routes added to included routers after inclusion, for Cadwyn's versioned and unversioned routing
+
 ## [7.0.0]
 
 - Breaking change if you relied on lifespan running multiple times: Fixed duplicated lifecycle callbacks in Cadwyn routers, see [this issue](https://github.com/zmievsa/cadwyn/issues/372) for more details

cadwyn/applications.py

@@ -18,6 +18,7 @@
 from fastapi.openapi.utils import get_openapi
 from fastapi.params import Depends
 from fastapi.responses import HTMLResponse
+from fastapi.routing import _EffectiveRouteContext, _IncludedRouter, _iter_routes_with_context
 from fastapi.templating import Jinja2Templates
 from fastapi.utils import generate_unique_id
 from starlette.middleware import Middleware
@@ -38,7 +39,7 @@
     VersionPickingMiddleware,
     _generate_api_version_dependency,
 )
-from cadwyn.route_generation import generate_versioned_routers
+from cadwyn.route_generation import copy_route, generate_versioned_routers
 from cadwyn.routing import _RootCadwynAPIRouter
 from cadwyn.structure import VersionBundle
 
@@ -51,6 +52,22 @@
 logger = getLogger(__name__)
 
 
+def _get_effective_include_in_schema(route: BaseRoute, effective_route_context: _EffectiveRouteContext | None) -> bool:
+    if effective_route_context is not None:
+        starlette_route = effective_route_context.starlette_route
+        if starlette_route is not None:
+            return bool(getattr(starlette_route, "include_in_schema", False))
+        return bool(effective_route_context.include_in_schema)
+    return bool(getattr(route, "include_in_schema", False))
+
+
+def _materialize_routes(routes: Sequence[BaseRoute]) -> list[BaseRoute]:
+    return [
+        copy_route(route, effective_route_context) if effective_route_context is not None else route
+        for route, effective_route_context in _iter_routes_with_context(routes)
+    ]
+
+
 @dataclasses.dataclass(**DATACLASS_SLOTS)
 class FakeDependencyOverridesProvider:
     dependency_overrides: dict[Callable[..., Any], Callable[..., Any]]
@@ -255,7 +272,13 @@ def __init__(
         unversioned_router = APIRouter(**self._kwargs_to_router)
         self._add_utility_endpoints(unversioned_router)
         self._add_default_versioned_routers()
+        route_count_before_include = len(self.router.routes)
+        unversioned_route_count_before_include = len(self.router.unversioned_routes)
         self.include_router(unversioned_router)
+        utility_routes = _materialize_routes(self.router.routes[route_count_before_include:])
+        self.router.routes[route_count_before_include:] = utility_routes
+        self.router.unversioned_routes[unversioned_route_count_before_include:] = utility_routes
+        self.router._mark_routes_changed()
         self.add_middleware(
             versioning_middleware_class,
             api_version_parameter_name=api_version_parameter_name,
@@ -411,15 +434,19 @@ async def openapi_jsons(self, req: Request) -> JSONResponse:
         )
 
     def _there_are_public_unversioned_routes(self):
-        return any(isinstance(route, Route) and route.include_in_schema for route in self.router.unversioned_routes)
+        return any(
+            isinstance(route, Route) and _get_effective_include_in_schema(route, effective_route_context)
+            for route, effective_route_context in _iter_routes_with_context(self.router.unversioned_routes)
+        )
 
     def _filter_openapi_tags(self, routes: list) -> Union[list[dict[str, Any]], None]:
         if not self.openapi_tags:
             return self.openapi_tags
         used_tags: set[str | Enum] = set()
-        for route in routes:
-            if isinstance(route, routing.APIRoute) and route.include_in_schema:
-                used_tags.update(route.tags)
+        for route, effective_route_context in _iter_routes_with_context(routes):
+            route_data = cast("Any", effective_route_context or route)
+            if isinstance(route, routing.APIRoute) and route_data.include_in_schema:
+                used_tags.update(route_data.tags)
         return [tag for tag in self.openapi_tags if tag.get("name") in used_tags]
 
     async def swagger_dashboard(self, req: Request) -> Response:
@@ -505,8 +532,8 @@ def _add_versioned_routers(
             )
             added_routes.append(versioned_router.routes[-1])
 
-        added_route_count = 0
         for router in (first_router, *other_routers):
+            route_count_before_include = len(versioned_router.routes)
             self.router.versioned_routers[version].include_router(
                 router,
                 dependencies=[
@@ -522,9 +549,15 @@ def _add_versioned_routers(
                     )
                 ],
             )
-            added_route_count += len(router.routes)
-
-        added_routes.extend(versioned_router.routes[-added_route_count:])
-        self.router.routes.extend(added_routes)
+            newly_added_routes = versioned_router.routes[route_count_before_include:]
+            if not any(isinstance(route, _IncludedRouter) for route in router.routes):
+                # There is no nested FastAPI include tree to preserve, so Cadwyn can materialize the plain
+                # routes here and keep the root router's version bookkeeping stable.
+                newly_added_routes = _materialize_routes(newly_added_routes)
+                versioned_router.routes[route_count_before_include:] = newly_added_routes
+                versioned_router._mark_routes_changed()
+            added_routes.extend(newly_added_routes)
+
+        self.router.extend_routes(added_routes)
 
         return added_routes

cadwyn/route_generation.py

@@ -11,12 +11,9 @@
     cast,
 )
 
-import fastapi.params
 import fastapi.routing
-import fastapi.security.base
-import fastapi.utils
 from fastapi import APIRouter
-from fastapi.routing import APIRoute
+from fastapi.routing import APIRoute, _EffectiveRouteContext, _iter_routes_with_context
 from pydantic import BaseModel
 from starlette.routing import BaseRoute
 from typing_extensions import TypeVar, assert_never
@@ -104,11 +101,29 @@ def only_exists_in_older_versions(self, endpoint: _Call) -> _Call:
 
 def copy_router(router: _R) -> _R:
     router = copy(router)
-    router.routes = [copy_route(r) for r in router.routes]
+    router.routes = [
+        copy_route(route, effective_route_context)
+        for route, effective_route_context in _iter_routes_with_context(router.routes)
+    ]
+    router._mark_routes_changed()
     return router
 
 
-def copy_route(route: _RouteT) -> _RouteT:
+def copy_route(route: _RouteT, effective_route_context: _EffectiveRouteContext | None = None) -> _RouteT:
+    """Copy a route and materialize FastAPI's include-router context into the copy.
+
+    FastAPI 0.137+ keeps included routers as tree nodes. Its _EffectiveRouteContext is the merged
+    route state produced by that tree: prefix, dependencies, tags, responses, schema flags, etc.
+    Cadwyn mutates copied routes per API version, so it must copy the original route and apply that
+    effective state to the copy. The original router tree is left intact.
+    """
+    if (
+        effective_route_context is not None
+        and not isinstance(route, APIRoute)
+        and effective_route_context.starlette_route is not None
+    ):
+        return cast("_RouteT", copy(effective_route_context.starlette_route))
+
     if not isinstance(route, APIRoute):
         return copy(route)
 
@@ -120,19 +135,46 @@ def copy_route(route: _RouteT) -> _RouteT:
     # These can hold TypeAdapters for recursive types (e.g. JsonValue) that cause
     # infinite recursion during deepcopy.
     memo: dict[int, Any] = {}
-    for attr in ("dependant", "_flat_dependant", "body_field", "response_model"):
+    for attr in ("dependant", "_flat_dependant", "body_field", "response_model", "dependency_overrides_provider"):
         obj = getattr(route, attr, None)
         if obj is not None:
             memo[id(obj)] = obj
     new_route = deepcopy(route, memo)
-    new_route.dependant = copy(route.dependant)
-    if getattr(route, "_flat_dependant", None) is not None:
-        new_route._flat_dependant = copy(route._flat_dependant)
-    new_route.body_field = route.body_field
-    new_route.dependencies = copy(route.dependencies)
+    if effective_route_context is not None:
+        _apply_effective_route_context_to_route(new_route, effective_route_context)
+        _refresh_route_app(new_route)
+    else:
+        new_route.dependant = copy(route.dependant)
+        if getattr(route, "_flat_dependant", None) is not None:
+            new_route._flat_dependant = copy(route._flat_dependant)
+        new_route.body_field = route.body_field
+        new_route.dependencies = copy(route.dependencies)
     return new_route
 
 
+def _apply_effective_route_context_to_route(route: APIRoute, effective_route_context: _EffectiveRouteContext) -> None:
+    for attr_name, attr_value in vars(effective_route_context).items():
+        if attr_name in {"original_route", "starlette_route"}:
+            continue
+        setattr(route, attr_name, _copy_effective_route_context_attr(attr_name, attr_value))
+
+
+def _copy_effective_route_context_attr(attr_name: str, attr_value: Any) -> Any:
+    if attr_name in {"dependant", "_flat_dependant"} and attr_value is not None:
+        return copy(attr_value)
+    if isinstance(attr_value, (dict, list, set)):
+        return copy(attr_value)
+    return attr_value
+
+
+def _refresh_route_app(route: APIRoute) -> None:
+    route.app = fastapi.routing.request_response(route.get_route_handler())
+
+
+def _route_methods(route: APIRoute) -> set[str]:
+    return route.methods or set()
+
+
 class _EndpointTransformer(Generic[_R, _WR]):
     def __init__(
         self,
@@ -149,9 +191,12 @@ def __init__(
         self.api_version_parameter_name = api_version_parameter_name
         self.api_version_location: APIVersionLocation = api_version_location
         self.schema_generators = generate_versioned_models(versions)
+        self.head_router = copy_router(parent_router)
 
         self.routes_that_never_existed = [
-            route for route in parent_router.routes if isinstance(route, APIRoute) and _DELETED_ROUTE_TAG in route.tags
+            route
+            for route in self.head_router.routes
+            if isinstance(route, APIRoute) and _DELETED_ROUTE_TAG in route.tags
         ]
 
     def transform(self) -> GeneratedRouters[_R, _WR]:
@@ -165,7 +210,7 @@ def transform(self) -> GeneratedRouters[_R, _WR]:
             self.schema_generators[str(version.value)].annotation_transformer.migrate_router_to_version(router)
             self.schema_generators[str(version.value)].annotation_transformer.migrate_router_to_version(webhook_router)
 
-            self._attach_routes_to_data_converters(router, self.parent_router, version)
+            self._attach_routes_to_data_converters(router, self.head_router, version)
 
             routers[version.value] = router
             webhook_routers[version.value] = webhook_router
@@ -183,7 +228,7 @@ def transform(self) -> GeneratedRouters[_R, _WR]:
                 f"{self.routes_that_never_existed}",
             )
 
-        for route_index, head_route in enumerate(self.parent_router.routes):
+        for route_index, head_route in enumerate(self.head_router.routes):
             if not isinstance(head_route, APIRoute):
                 continue
             _add_request_and_response_params(head_route)
@@ -326,7 +371,7 @@ def _extract_all_routes_identifiers_for_route_to_converter_matching(
                     annotation = route.body_field.field_info.annotation
                     if annotation is not None and lenient_issubclass(annotation, BaseModel):
                         request_bodies.add(annotation)
-                for method in route.methods:
+                for method in _route_methods(route):
                     path_to_route_methods_mapping[route.path][method].add(index)
 
         head_response_models = {model.__cadwyn_original_model__ for model in response_models}
@@ -363,7 +408,7 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                     if deleted_routes:
                         method_union = set()
                         for deleted_route in deleted_routes:
-                            method_union |= deleted_route.methods
+                            method_union |= _route_methods(deleted_route)
                         raise RouterGenerationError(
                             f'Endpoint "{list(method_union)} {instruction.endpoint_path}" you tried to delete in '
                             f'"{version_change.__name__}" was already deleted in a newer version. If you really have '
@@ -372,7 +417,7 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                             f"{[r.endpoint.__name__ for r in deleted_routes]}",
                         )
                     for original_route in original_routes:
-                        methods_to_which_we_applied_changes |= original_route.methods
+                        methods_to_which_we_applied_changes |= _route_methods(original_route)
                         original_route.tags.append(_DELETED_ROUTE_TAG)
                     err = (
                         'Endpoint "{endpoint_methods} {endpoint_path}" you tried to delete in'
@@ -382,7 +427,7 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                     if original_routes:
                         method_union = set()
                         for original_route in original_routes:
-                            method_union |= original_route.methods
+                            method_union |= _route_methods(original_route)
                         raise RouterGenerationError(
                             f'Endpoint "{list(method_union)} {instruction.endpoint_path}" you tried to restore in'
                             f' "{version_change.__name__}" already existed in a newer version. If you really have two '
@@ -408,13 +453,13 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                             f"endpoints that can be restored: {[r.endpoint.__name__ for r in e.routes]}",
                         ) from e
                     for deleted_route in deleted_routes:
-                        methods_to_which_we_applied_changes |= deleted_route.methods
+                        methods_to_which_we_applied_changes |= _route_methods(deleted_route)
                         deleted_route.tags.remove(_DELETED_ROUTE_TAG)
 
                         routes_that_never_existed = _get_routes(
                             self.routes_that_never_existed,
                             deleted_route.path,
-                            deleted_route.methods,
+                            _route_methods(deleted_route),
                             deleted_route.endpoint.__name__,
                             is_deleted=True,
                         )
@@ -425,7 +470,8 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                             # to remove it because I like its clarity very much
                             routes = routes_that_never_existed
                             raise RouterGenerationError(
-                                f'Endpoint "{list(deleted_route.methods)} {deleted_route.path}" you tried to restore '
+                                f'Endpoint "{list(_route_methods(deleted_route))} {deleted_route.path}" '
+                                "you tried to restore "
                                 f'in "{version_change.__name__}" has {len(routes_that_never_existed)} applicable '
                                 f"routes with the same function name and path that could be restored. This can cause "
                                 f"problems during version generation. Specifically, Cadwyn won't be able to warn "
@@ -439,7 +485,7 @@ def _apply_endpoint_changes_to_router(  # noqa: C901
                     )
                 elif isinstance(instruction, EndpointHadInstruction):
                     for original_route in original_routes:
-                        methods_to_which_we_applied_changes |= original_route.methods
+                        methods_to_which_we_applied_changes |= _route_methods(original_route)
                         _apply_endpoint_had_instruction(
                             version_change.__name__,
                             instruction,
@@ -468,7 +514,7 @@ def _validate_no_repetitions_in_routes(routes: list[fastapi.routing.APIRoute]):
     route_map = {}
 
     for route in routes:
-        route_info = _EndpointInfo(route.path, frozenset(route.methods))
+        route_info = _EndpointInfo(route.path, frozenset(_route_methods(route)))
         if route_info in route_map:
             raise RouteAlreadyExistsError(route, route_map[route_info])
         route_map[route_info] = route
@@ -485,7 +531,8 @@ def _add_data_migrations_to_route(
     if not (route.dependant.request_param_name and route.dependant.response_param_name):  # pragma: no cover
         raise CadwynError(
             f"{route.dependant.request_param_name=}, {route.dependant.response_param_name=} "
-            f"for route {list(route.methods)} {route.path} which should not be possible. Please, contact my author.",
+            f"for route {list(_route_methods(route))} {route.path} which should not be possible. "
+            "Please, contact my author.",
         )
 
     route.endpoint = versions._versioned(
@@ -514,7 +561,7 @@ def _apply_endpoint_had_instruction(
             if getattr(original_route, attr_name) == attr:
                 raise RouterGenerationError(
                     f'Expected attribute "{attr_name}" of endpoint'
-                    f' "{list(original_route.methods)} {original_route.path}"'
+                    f' "{list(_route_methods(original_route))} {original_route.path}"'
                     f' to be different in "{version_change_name}", but it was the same.'
                     " It means that your version change has no effect on the attribute"
                     " and can be removed.",
@@ -526,7 +573,7 @@ def _apply_endpoint_had_instruction(
                     new_path_params.discard(api_version_parameter_name)
                 if new_path_params != original_path_params:
                     raise RouterPathParamsModifiedError(
-                        f'When altering the path of "{list(original_route.methods)} {original_route.path}" '
+                        f'When altering the path of "{list(_route_methods(original_route))} {original_route.path}" '
                         f'in "{version_change_name}", you have tried to change its path params '
                         f'from "{list(original_path_params)}" to "{list(new_path_params)}". It is not allowed to '
                         "change the path params of a route because the endpoint was created to handle the old path "
@@ -552,7 +599,7 @@ def _get_routes(
         if (
             isinstance(route, fastapi.routing.APIRoute)
             and route.path.rstrip("/") == endpoint_path
-            and set(route.methods).issubset(endpoint_methods)
+            and _route_methods(route).issubset(endpoint_methods)
             and (endpoint_func_name is None or route.endpoint.__name__ == endpoint_func_name)
             and (_DELETED_ROUTE_TAG in route.tags) == is_deleted
         )
@@ -563,7 +610,7 @@ def _get_route_from_func(
     routes: Sequence[BaseRoute],
     endpoint: Endpoint,
 ) -> Union[fastapi.routing.APIRoute, None]:
-    for route in routes:
+    for route, _effective_route_context in _iter_routes_with_context(routes):
         if isinstance(route, fastapi.routing.APIRoute) and (route.endpoint == endpoint):
             return route
     return None