/docs return 404 after adding api_version_default_value #268 (#269)

zmievsa · web-flow · commit 72321a61a975 · 2025-03-30T08:51:13.000+02:00
#268
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ Please follow [the Keep a Changelog standard](https://keepachangelog.com/en/1.0.
 
 ## [Unreleased]
 
+## [5.2.1]
+
+### Fixed
+
+* [#268](https://github.com/zmievsa/cadwyn/issues/268) A bug where we received 404 for all unversioned routes when a default api version was passed to Cadwyn at initialization
+
 ## [5.2.0]
 
 ### Removed
diff --git a/cadwyn/_internal/__init__.py b/cadwyn/_internal/__init__.py
diff --git a/cadwyn/_internal/context_vars.py b/cadwyn/_internal/context_vars.py
@@ -0,0 +1,9 @@
+from contextvars import ContextVar
+
+from typing_extensions import Literal
+
+DEFAULT_API_VERSION_VAR: "ContextVar[str | None]" = ContextVar("cadwyn_default_api_version")
+CURRENT_DEPENDENCY_SOLVER_OPTIONS = Literal["cadwyn", "fastapi"]
+CURRENT_DEPENDENCY_SOLVER_VAR: ContextVar[CURRENT_DEPENDENCY_SOLVER_OPTIONS] = ContextVar(
+    "cadwyn_dependencies_current_dependency_solver"
+)
diff --git a/cadwyn/applications.py b/cadwyn/applications.py
@@ -133,6 +133,12 @@ def __init__(
                 stacklevel=2,
             )
             api_version_parameter_name = api_version_header_name
+        if api_version_default_value is not None and api_version_location == "path":
+            raise CadwynStructureError(
+                "You tried to pass an api_version_default_value while putting the API version in Path. "
+                "This is not currently supported by Cadwyn. "
+                "Please, open an issue on our github if you'd like to have it."
+            )
 
         super().__init__(
             debug=debug,
@@ -231,8 +237,8 @@ def __init__(
             versioning_middleware_class,
             api_version_parameter_name=api_version_parameter_name,
             api_version_manager=self._api_version_manager,
-            api_version_default_value=api_version_default_value,
             api_version_var=self.versions.api_version_var,
+            api_version_default_value=api_version_default_value,
         )
         if self.api_version_format == "date" and (
             sorted(self.versions.versions, key=lambda v: v.value, reverse=True) != list(self.versions.versions)
diff --git a/cadwyn/dependencies.py b/cadwyn/dependencies.py
@@ -1,5 +1,5 @@
-from cadwyn.structure.versions import _CURRENT_DEPENDENCY_SOLVER_OPTIONS, _CURRENT_DEPENDENCY_SOLVER_VAR
+from cadwyn._internal.context_vars import CURRENT_DEPENDENCY_SOLVER_OPTIONS, CURRENT_DEPENDENCY_SOLVER_VAR
 
 
-async def current_dependency_solver() -> _CURRENT_DEPENDENCY_SOLVER_OPTIONS:
-    return _CURRENT_DEPENDENCY_SOLVER_VAR.get("fastapi")
+async def current_dependency_solver() -> CURRENT_DEPENDENCY_SOLVER_OPTIONS:
+    return CURRENT_DEPENDENCY_SOLVER_VAR.get("fastapi")
diff --git a/cadwyn/middleware.py b/cadwyn/middleware.py
@@ -7,10 +7,12 @@
 from contextvars import ContextVar
 from typing import Annotated, Any, Literal, Protocol, Union
 
+import fastapi
 from fastapi import Request
 from starlette.middleware.base import BaseHTTPMiddleware, DispatchFunction, RequestResponseEndpoint
 from starlette.types import ASGIApp
 
+from cadwyn._internal.context_vars import DEFAULT_API_VERSION_VAR
 from cadwyn.structure.common import VersionType
 
 
@@ -69,6 +71,8 @@ def api_version_dependency(**kwargs: Any):
                 annotation=Annotated[
                     validation_data_type, fastapi_depends_class(openapi_examples={"default": {"value": default_value}})
                 ],
+                # Path-based parameters do not support a default value in FastAPI :(
+                default=default_value if fastapi_depends_class != fastapi.Path else inspect.Signature.empty,
             ),
         ],
     )
@@ -103,10 +107,11 @@ async def dispatch(
         api_version = self._api_version_manager.get(request)
 
         if api_version is None:
-            if callable(self.api_version_default_value):  # pragma: no cover # TODO
+            if callable(self.api_version_default_value):
                 api_version = await self.api_version_default_value(request)
             else:
                 api_version = self.api_version_default_value
+            DEFAULT_API_VERSION_VAR.set(api_version)
 
         self.api_version_var.set(api_version)
         response = await call_next(request)
diff --git a/cadwyn/routing.py b/cadwyn/routing.py
@@ -11,6 +11,7 @@
 from starlette.routing import BaseRoute, Match
 from starlette.types import Receive, Scope, Send
 
+from cadwyn._internal.context_vars import DEFAULT_API_VERSION_VAR
 from cadwyn._utils import same_definition_as_in
 from cadwyn.middleware import APIVersionFormat
 from cadwyn.structure.common import VersionType
@@ -70,8 +71,8 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
         if scope["type"] == "lifespan":
             await self.lifespan(scope, receive, send)
             return
-
         version = self.api_version_var.get(None)
+        default_version_that_was_picked = DEFAULT_API_VERSION_VAR.get(None)
 
         # if version is None, then it's an unversioned request and we need to use the unversioned routes
         # if there will be a value, we search for the most suitable version
@@ -81,6 +82,14 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
             routes = self.versioned_routers[version].routes
         else:
             routes = await self._get_routes_from_closest_suitable_version(version)
+        if default_version_that_was_picked:
+            # We add unversioned routes to versioned routes because otherwise unversioned routes
+            # will be completely unavailable when a default version is passed. So routes such as
+            # /docs will not be accessible at all.
+
+            # We use this order because if versioned routes go first and there is a versioned route that is
+            # the same as an unversioned route -- the unversioned one becomes impossible to match.
+            routes = self.unversioned_routes + routes
         await self.process_request(scope=scope, receive=receive, send=send, routes=routes)
 
     @cached_property
diff --git a/cadwyn/structure/versions.py b/cadwyn/structure/versions.py
@@ -24,8 +24,9 @@
 from pydantic import BaseModel
 from pydantic_core import PydanticUndefined
 from starlette._utils import is_async_callable
-from typing_extensions import Any, Literal, ParamSpec, TypeAlias, TypeVar, assert_never, deprecated, get_args
+from typing_extensions import Any, ParamSpec, TypeAlias, TypeVar, assert_never, deprecated, get_args
 
+from cadwyn._internal.context_vars import CURRENT_DEPENDENCY_SOLVER_VAR
 from cadwyn._utils import classproperty
 from cadwyn.exceptions import (
     CadwynError,
@@ -52,10 +53,6 @@
 _CADWYN_RESPONSE_PARAM_NAME = "cadwyn_response_param"
 _P = ParamSpec("_P")
 _R = TypeVar("_R")
-_CURRENT_DEPENDENCY_SOLVER_OPTIONS = Literal["cadwyn", "fastapi"]
-_CURRENT_DEPENDENCY_SOLVER_VAR: ContextVar[_CURRENT_DEPENDENCY_SOLVER_OPTIONS] = ContextVar(
-    "cadwyn_dependencies_dry_run"
-)
 
 PossibleInstructions: TypeAlias = Union[
     AlterSchemaSubInstruction, AlterEndpointSubInstruction, AlterEnumSubInstruction, SchemaHadInstruction, staticmethod
@@ -387,7 +384,7 @@ async def _migrate_request(
         request.scope["headers"] = tuple((key.encode(), value.encode()) for key, value in request_info.headers.items())
         del request._headers
         # This gives us the ability to tell the user whether cadwyn is running its dependencies or FastAPI
-        _CURRENT_DEPENDENCY_SOLVER_VAR.set("cadwyn")
+        CURRENT_DEPENDENCY_SOLVER_VAR.set("cadwyn")
         # Remember this: if len(body_params) == 1, then route.body_schema == route.dependant.body_params[0]
         result = await solve_dependencies(
             request=request,
diff --git a/pyproject.toml b/pyproject.toml
@@ -1,6 +1,6 @@
 [project]
 name = "cadwyn"
-version = "5.2.0"
+version = "5.2.1"
 description = "Production-ready community-driven modern Stripe-like API versioning in FastAPI"
 authors = [{ name = "Stanislav Zmiev", email = "zmievsa@gmail.com" }]
 license = "MIT"
diff --git a/tests/test_applications.py b/tests/test_applications.py
@@ -8,6 +8,7 @@
 from pydantic import BaseModel
 
 from cadwyn import Cadwyn
+from cadwyn.exceptions import CadwynStructureError
 from cadwyn.route_generation import VersionedAPIRouter
 from cadwyn.structure.endpoints import endpoint
 from cadwyn.structure.schemas import schema
@@ -405,3 +406,15 @@ def test__api_version_header_name_is_deprecated_and_translates_to_api_version_pa
     with pytest.warns(DeprecationWarning):
         cadwyn = Cadwyn(api_version_header_name="x-api-version", versions=VersionBundle(Version("2022-11-16")))
     assert cadwyn.api_version_parameter_name == "x-api-version"
+
+
+def test__api_version_default_value_with_path_location__should_raise_error():
+    with pytest.raises(
+        CadwynStructureError,
+        match="You tried to pass an api_version_default_value while putting the API version in Path",
+    ):
+        Cadwyn(
+            versions=VersionBundle(HeadVersion(), Version("2022-11-16")),
+            api_version_default_value="2022-11-16",
+            api_version_location="path",
+        )
diff --git a/tests/test_routing.py b/tests/test_routing.py
@@ -1,11 +1,14 @@
+from typing import Callable
+
 import pytest
 from starlette.requests import Request
 from starlette.responses import PlainTextResponse
 from starlette.routing import Match, NoMatchFound
 from starlette.testclient import TestClient
 
 from cadwyn import Cadwyn
-from cadwyn.structure.versions import Version, VersionBundle
+from cadwyn.route_generation import VersionedAPIRouter
+from cadwyn.structure.versions import HeadVersion, Version, VersionBundle
 from tests._resources.app_for_testing_routing import mixed_hosts_app
 
 
@@ -153,3 +156,53 @@ def test__host_routing__partial_match__404():
 
     response = client.get("/v1/doggies/tom")
     assert response.status_code == 200
+
+
+async def get_default_version(req: Request):
+    return "2023-04-14"
+
+
+@pytest.mark.parametrize("default_version", ["2023-04-14", get_default_version])
+def test__get_unversioned_endpoints__with_default_version(default_version: "str | Callable"):
+    app = Cadwyn(
+        versions=VersionBundle(HeadVersion(), Version("2023-04-14"), Version("2022-11-16")),
+        api_version_default_value=default_version,
+    )
+
+    router = VersionedAPIRouter()
+
+    @app.get("/my_duplicated_route")
+    def get_my_unversioned_number():
+        return 11
+
+    @router.get("/my_duplicated_route")
+    def get_my_versioned_number():
+        return 83
+
+    @router.get("/my_single_route")
+    def get_my_versioned_number_2():
+        return 52
+
+    app.generate_and_include_versioned_routers(router)
+
+    with TestClient(app) as client:
+        resp = client.get("/docs")
+        assert resp.status_code == 200, resp.json()
+
+        resp = client.get("/docs?version=2023-04-14")
+        assert resp.status_code == 200, resp.json()
+
+        resp = client.get("/docs?version=2022-11-16")
+        assert resp.status_code == 200, resp.json()
+
+        resp = client.get("/my_duplicated_route")
+        assert resp.status_code == 200, resp.json()
+        assert resp.json() == 11
+
+        resp = client.get("/my_duplicated_route", headers={"X-API-VERSION": "2023-04-14"})
+        assert resp.status_code == 200, resp.json()
+        assert resp.json() == 83
+
+        resp = client.get("/my_single_route")
+        assert resp.status_code == 200, resp.json()
+        assert resp.json() == 52
diff --git a/uv.lock b/uv.lock
