failed WRONG_FIX UNSUBMITTED wrong_fix_unsubmitted(None) · None tool calls · 0 s · fastapi/fastapi
♻️ Refactor internals to preserve `APIRouter` and `APIRoute` instances ## Pull Request ♻️ Refactor internals to preserve `APIRouter` and `APIRoute` instances Supersedes https://github.com/fastapi/fastapi/pull/4794 Unblocks :sparkles: SO MANY THINGS :sparkles: Before this, `router.include_router(other_router)` would take each path operation from `other_router` and "clone" it, or recreate it from scratch. This would mean that in the end there was only one top level router, part of the app. The way it is structured here is that there are a few additional classes to handle intermediate metadata for router and route inclusion. That way the information of "router X includes Y and Y includes Z" is stored somewhere, without affecting (recreating / clonning) the final route. ### Non Objective Dependencies for 404 Originally in the other PR I intended to support dependencies that would be executed even for 404, but that would conflict with the fact that a router could _not_ find a match, but the next router _did_ find a match. Executing dependencies in the router that did not find a match would not make sense, they could consume the request, body, etc. This original idea was discarded. ### Breaking Change Now `router.routes` is no longer a plain list of `APIRoute` objects, it can contain these intermediate objects that can contain additional routers, forming a tree. Any logic that depended on iterating on the `router.routes` directly would be affected, that logic cannot expect to be able to extract data from a plain list of routes, as it's no longer a plain list but a tree. Additionally, any logic that iterated on `router.routes` to modify them would now also see these new objects, and would not see all the routes in the app. `router.routes` should be considered an internal implementation detail, only passed around to the FastAPI functions that need it. ### Features * Adding routes after a router is included now works, they are reflected as they are not copied. * Including `subrouter` in `mainrouter` can be done before adding routes (path operations) to `subrouter`, because now the the entire object is stored instead of copying the routes. * As routes are not copied, in some cases that might save some memory. ### Alpha Features This is not documented yet, so it's not officially supported yet and could change in the future. But, as `APIRoute` and `APIRouter` instances are now preserved, they could be customized. `APIRouter` has two new methods, `.matches()` and `.handle()`, counterpart to the existing ones in `APIRoute`. With this a router could customize how it matches and handles requests. For example, it could match only requests that include some specific header, for example for handling versions in headers. Still, for now, consider this very experimental and potentially changing and breaking in the future. ### Future Features Enabled * Custom `APIRoute` subclasses (as desccribed above) * Custom `APIRouter` subclasses (as described above) * Dependencies per router * Exception handlers per router * Middleware per router * Other features planned <!-- Please start with a GitHub Discussion. Once a team member asks you to open a PR, create it and link the discussion here. Obvious typo fixes can be made in a PR without starting a discussion. --> Discussion: <!-- Link to the GitHub Discussion --> ## Description <!-- Write the description of your PR here --> ## AI Disclaimer Codex with GPT 5.5, through a lot of planning and research iterations, for weeks, then the same for the implementation, with too many iterations to count to clean up the implementation, what's supported and not, docs, types, etc. All code and tests manually reviewed by hand. <!-- If using AI, write here the prompt and model used --> <details> <summary>AI transcript</summary> <!-- Paste here the entire AI transcript --> </details> ## Checklist - [ ] This PR is an obvious typo fix, or it links to a GitHub Discussion for the proposed code change. - [x] I added tests for the change. - [x] The new or updated tests fail on the main branch and pass on this PR. - [x] Coverage stays at 100%. - [x] The documentation explains the change if needed.
| # | Tool | Arguments | Result |
|---|---|---|---|
| No trace captured. | |||
--- a/docs_src/path_operation_advanced_configuration/tutorial002_py310.py
+++ b/docs_src/path_operation_advanced_configuration/tutorial002_py310.py
@@ -1,24 +1,14 @@
from fastapi import FastAPI
from fastapi.routing import APIRoute
-app = FastAPI()
-
-@app.get("/items/")
-async def read_items():
- return [{"item_id": "Foo"}]
+def custom_generate_unique_id(route: APIRoute) -> str:
+ return route.name
-def use_route_names_as_operation_ids(app: FastAPI) -> None:
- """
- Simplify operation IDs so that generated API clients have simpler function
- names.
+app = FastAPI(generate_unique_id_function=custom_generate_unique_id)
- Should be called only after all routes have been added.
- """
- for route in app.routes:
- if isinstance(route, APIRoute):
- route.operation_id = route.name # in this case, 'read_items'
-
-use_route_names_as_operation_ids(app)
+@app.get("/items/")
+async def read_items():
+ return [{"item_id": "Foo"}]
--- a/fastapi/applications.py
+++ b/fastapi/applications.py
@@ -921,6 +921,7 @@ class Item(BaseModel):
),
] = "3.1.0"
self.openapi_schema: dict[str, Any] | None = None
+ self._openapi_routes_version: int | None = None
if self.openapi_url:
assert self.title, "A title must be provided for OpenAPI, e.g.: 'My API'"
assert self.version, "A version must be provided for OpenAPI, e.g.: '2.1.0'"
@@ -1079,7 +1080,8 @@ def openapi(self) -> dict[str, Any]:
Read more in the
[FastAPI docs for OpenAPI](https://fastapi.tiangolo.com/how-to/extending-openapi/).
"""
- if not self.openapi_schema:
+ routes_version = self.router._get_routes_version()
+ if not self.openapi_schema or self._openapi_routes_version != routes_version:
self.openapi_schema = get_openapi(
title=self.title,
version=self.version,
@@ -1096,6 +1098,7 @@ def openapi(self) -> dict[str, Any]:
separate_input_output_schemas=self.separate_input_output_schemas,
external_docs=self.openapi_external_docs,
)
+ self._openapi_routes_version = routes_version
return self.openapi_schema
def setup(self) -> None:
--- a/fastapi/openapi/utils.py
+++ b/fastapi/openapi/utils.py
@@ -213,7 +213,7 @@ def get_openapi_operation_request_body(
def generate_operation_id(
- *, route: routing.APIRoute, method: str
+ *, route: routing._APIRouteLike, method: str
) -> str: # pragma: nocover
warnings.warn(
message="fastapi.openapi.utils.generate_operation_id() was deprecated, "
@@ -227,14 +227,14 @@ def generate_operation_id(
return generate_operation_id_for_path(name=route.name, path=path, method=method)
-def generate_operation_summary(*, route: routing.APIRoute, method: str) -> str:
+def generate_operation_summary(*, route: routing._APIRouteLike, method: str) -> str:
if route.summary:
return route.summary
return route.name.replace("_", " ").title()
def get_openapi_operation_metadata(
- *, route: routing.APIRoute, method: str, operation_ids: set[str]
+ *, route: routing._APIRouteLike, method: str, operation_ids: set[str]
) -> dict[str, Any]:
operation: dict[str, Any] = {}
if route.tags:
@@ -259,7 +259,7 @@ def get_openapi_operation_metadata(
def get_openapi_path(
*,
- route: routing.APIRoute,
+ route: routing._APIRouteLike,
operation_ids: set[str],
model_name_map: ModelNameMap,
field_mapping: dict[
@@ -329,7 +329,7 @@ def get_openapi_path(
cb_security_schemes,
cb_definitions,
) = get_openapi_path(
- route=callback,
+ route=cast(routing._APIRouteLike, callback),
operation_ids=operation_ids,
model_name_map=model_name_map,
field_mapping=field_mapping,
@@ -478,31 +478,44 @@ def get_openapi_path(
return path, security_schemes, definitions
+def _get_api_route_for_openapi(
+ route: BaseRoute, route_context: routing._EffectiveRouteContext | None
+) -> routing._APIRouteLike | None:
+ if route_context is not None and isinstance(
+ route_context.original_route, routing.APIRoute
+ ):
+ return cast(routing._APIRouteLike, route_context)
+ if isinstance(route, routing.APIRoute):
+ return cast(routing._APIRouteLike, route)
+ return None
+
+
def get_fields_from_routes(
routes: Sequence[BaseRoute],
) -> list[ModelField]:
body_fields_from_routes: list[ModelField] = []
responses_from_routes: list[ModelField] = []
request_fields_from_routes: list[ModelField] = []
callback_flat_models: list[ModelField] = []
- for route in routes:
- if not isinstance(route, routing.APIRoute):
+ for route, route_context in routing._iter_routes_with_context(routes):
+ api_route = _get_api_route_for_openapi(route, route_context)
+ if api_route is None:
continue
- if route.include_in_schema:
- if route.body_field:
- assert isinstance(route.body_field, ModelField), (
+ if api_route.include_in_schema:
+ if api_route.body_field:
+ assert isinstance(api_route.body_field, ModelField), (
"A request body must be a Pydantic Field"
)
- body_fields_from_routes.append(route.body_field)
- if route.response_field:
- responses_from_routes.append(route.response_field)
- if route.response_fields:
- responses_from_routes.extend(route.response_fields.values())
- if route.stream_item_field:
- responses_from_routes.append(route.stream_item_field)
- if route.callbacks:
- callback_flat_models.extend(get_fields_from_routes(route.callbacks))
- params = get_flat_params(route.dependant)
+ body_fields_from_routes.append(api_route.body_field)
+ if api_route.response_field:
+ responses_from_routes.append(api_route.response_field)
+ if api_route.response_fields:
+ responses_from_routes.extend(api_route.response_fields.values())
+ if api_route.stream_item_field:
+ responses_from_routes.append(api_route.stream_item_field)
+ if api_route.callbacks:
+ callback_flat_models.extend(get_fields_from_routes(api_route.callbacks))
+ params = get_flat_params(api_route.dependant)
request_fields_from_routes.extend(params)
flat_models = callback_flat_models + list(
@@ -546,18 +559,19 @@ def get_openapi(
paths: dict[str, dict[str, Any]] = {}
webhook_paths: dict[str, dict[str, Any]] = {}
operation_ids: set[str] = set()
- all_fields = get_fields_from_routes(list(routes or []) + list(webhooks or []))
+ all_fields = get_fields_from_routes(list(routes) + list(webhooks or []))
flat_models = get_flat_models_from_fields(all_fields, known_models=set())
model_name_map = get_model_name_map(flat_models)
field_mapping, definitions = get_definitions(
fields=all_fields,
model_name_map=model_name_map,
separate_input_output_schemas=separate_input_output_schemas,
)
- for route in routes or []:
- if isinstance(route, routing.APIRoute):
+ for route, route_context in routing._iter_routes_with_context(routes):
+ api_route = _get_api_route_for_openapi(route, route_context)
+ if api_route is not None:
result = get_openapi_path(
- route=route,
+ route=api_route,
operation_ids=operation_ids,
model_name_map=model_name_map,
field_mapping=field_mapping,
@@ -566,17 +580,18 @@ def get_openapi(
if result:
path, security_schemes, path_definitions = result
if path:
- paths.setdefault(route.path_format, {}).update(path)
+ paths.setdefault(api_route.path_format, {}).update(path)
if security_schemes:
components.setdefault("securitySchemes", {}).update(
security_schemes
)
if path_definitions:
definitions.update(path_definitions)
- for webhook in webhooks or []:
- if isinstance(webhook, routing.APIRoute):
+ for webhook, webhook_context in routing._iter_routes_with_context(webhooks or []):
+ api_webhook = _get_api_route_for_openapi(webhook, webhook_context)
+ if api_webhook is not None:
result = get_openapi_path(
- route=webhook,
+ route=api_webhook,
operation_ids=operation_ids,
model_name_map=model_name_map,
field_mapping=field_mapping,
@@ -585,7 +600,7 @@ def get_openapi(
if result:
path, security_schemes, path_definitions = result
if path:
- webhook_paths.setdefault(webhook.path_format, {}).update(path)
+ webhook_paths.setdefault(api_webhook.path_format, {}).update(path)
if security_schemes:
components.setdefault("securitySchemes", {}).update(
security_schemes
--- a/fastapi/routing.py
+++ b/fastapi/routing.py
@@ -1,4 +1,5 @@
import contextlib
+import copy
import email.message
import functools
import inspect
@@ -21,10 +22,13 @@
AsyncExitStack,
asynccontextmanager,
)
+from contextvars import ContextVar
+from dataclasses import dataclass, field
from enum import Enum, IntEnum
from typing import (
Annotated,
Any,
+ Protocol,
TypeVar,
cast,
)
@@ -74,12 +78,17 @@
)
from starlette import routing
from starlette._exception_handler import wrap_app_handling_exceptions
-from starlette._utils import is_async_callable
+from starlette._utils import get_route_path, is_async_callable
from starlette.concurrency import iterate_in_threadpool, run_in_threadpool
-from starlette.datastructures import FormData
+from starlette.datastructures import FormData, URLPath
from starlette.exceptions import HTTPException
from starlette.requests import Request
-from starlette.responses import JSONResponse, Response, StreamingResponse
+from starlette.responses import (
+ JSONResponse,
+ PlainTextResponse,
+ Response,
+ StreamingResponse,
+)
from starlette.routing import (
BaseRoute,
Match,
@@ -808,6 +817,250 @@ def matches(self, scope: Scope) -> tuple[Match, Scope]:
return match, child_scope
+_FASTAPI_SCOPE_KEY = "fastapi"
+_FASTAPI_EFFECTIVE_ROUTE_CONTEXT_KEY = "effective_route_context"
+_FASTAPI_INCLUDED_ROUTER_KEY = "included_router"
+_effective_route_context_var: ContextVar[Any | None] = ContextVar(
+ "fastapi_effective_route_context", default=None
+)
+_SCOPE_MISSING = object()
+
+
+def _get_fastapi_scope(scope: Scope) -> dict[str, Any]:
+ fastapi_scope = scope.setdefault(_FASTAPI_SCOPE_KEY, {})
+ assert isinstance(fastapi_scope, dict)
+ return fastapi_scope
+
+
+def _get_scope_effective_route_context(scope: Scope) -> Any | None:
+ return scope.get(_FASTAPI_SCOPE_KEY, {}).get(_FASTAPI_EFFECTIVE_ROUTE_CONTEXT_KEY)
+
+
+def _get_scope_included_router(scope: Scope) -> Any | None:
+ return scope.get(_FASTAPI_SCOPE_KEY, {}).get(_FASTAPI_INCLUDED_ROUTER_KEY)
+
+
+def _restore_fastapi_scope_key(scope: Scope, key: str, previous: Any) -> None:
+ fastapi_scope = scope.get(_FASTAPI_SCOPE_KEY)
+ if not isinstance(fastapi_scope, dict):
+ return
+ if previous is _SCOPE_MISSING:
+ fastapi_scope.pop(key, None)
+ else:
+ fastapi_scope[key] = previous
+
+
+class _APIRouteLike(Protocol):
+ path: str
+ endpoint: Callable[..., Any]
+ stream_item_type: Any | None
+ response_model: Any
+ summary: str | None
+ response_description: str
+ deprecated: bool | None
+ operation_id: str | None
+ response_model_include: IncEx | None
+ response_model_exclude: IncEx | None
+ response_model_by_alias: bool
+ response_model_exclude_unset: bool
+ response_model_exclude_defaults: bool
+ response_model_exclude_none: bool
+ include_in_schema: bool
+ response_class: type[Response] | DefaultPlaceholder
+ dependency_overrides_provider: Any | None
+ callbacks: list[BaseRoute] | None
+ openapi_extra: dict[str, Any] | None
+ generate_unique_id_function: Callable[[Any], str] | DefaultPlaceholder
+ strict_content_type: bool | DefaultPlaceholder
+ tags: list[str | Enum]
+ responses: dict[int | str, dict[str, Any]]
+ name: str
+ path_regex: Any
+ path_format: str
+ param_convertors: dict[str, Any]
+ methods: set[str]
+ unique_id: str
+ status_code: int | None
+ response_field: ModelField | None
+ stream_item_field: ModelField | None
+ dependencies: list[params.Depends]
+ description: str
+ response_fields: dict[int | str, ModelField]
+ dependant: Dependant
+ _flat_dependant: Dependant
+ _embed_body_fields: bool
+ body_field: ModelField | None
+ is_sse_stream: bool
+ is_json_stream: bool
+
+
+def _populate_api_route_state(
+ route: _APIRouteLike,
+ path: str,
+ endpoint: Callable[..., Any],
+ *,
+ response_model: Any = Default(None),
+ status_code: int | None = None,
+ tags: list[str | Enum] | None = None,
+ dependencies: Sequence[params.Depends] | None = None,
+ summary: str | None = None,
+ description: str | None = None,
+ response_description: str = "Successful Response",
+ responses: dict[int | str, dict[str, Any]] | None = None,
+ deprecated: bool | None = None,
+ name: str | None = None,
+ methods: set[str] | list[str] | None = None,
+ operation_id: str | None = None,
+ response_model_include: IncEx | None = None,
+ response_model_exclude: IncEx | None = None,
+ response_model_by_alias: bool = True,
+ response_model_exclude_unset: bool = False,
+ response_model_exclude_defaults: bool = False,
+ response_model_exclude_none: bool = False,
+ include_in_schema: bool = True,
+ response_class: type[Response] | DefaultPlaceholder = Default(JSONResponse),
+ dependency_overrides_provider: Any | None = None,
+ callbacks: list[BaseRoute] | None = None,
+ openapi_extra: dict[str, Any] | None = None,
+ generate_unique_id_function: Callable[[Any], str] | DefaultPlaceholder = Default(
+ generate_unique_id
+ ),
+ strict_content_type: bool | DefaultPlaceholder = Default(True),
+) -> None:
+ route.path = path
+ route.endpoint = endpoint
+ route.stream_item_type = None
+ if isinstance(response_model, DefaultPlaceholder):
+ return_annotation = get_typed_return_annotation(endpoint)
+ if lenient_issubclass(return_annotation, Response):
+ response_model = None
+ else:
+ stream_item = get_stream_item_type(return_annotation)
+ if stream_item is not None:
+ # Extract item type for JSONL or SSE streaming when
+ # response_class is DefaultPlaceholder (JSONL) or
+ # EventSourceResponse (SSE).
+ # ServerSentEvent is excluded: it's a transport
+ # wrapper, not a data model, so it shouldn't feed
+ # into validation or OpenAPI schema generation.
+ if (
+ isinstance(response_class, DefaultPlaceholder)
+ or lenient_issubclass(response_class, EventSourceResponse)
+ ) and not lenient_issubclass(stream_item, ServerSentEvent):
+ route.stream_item_type = stream_item
+ response_model = None
+ else:
+ response_model = return_annotation
+ route.response_model = response_model
+ route.summary = summary
+ route.response_description = response_description
+ route.deprecated = deprecated
+ route.operation_id = operation_id
+ route.response_model_include = response_model_include
+ route.response_model_exclude = response_model_exclude
+ route.response_model_by_alias = response_model_by_alias
+ route.response_model_exclude_unset = response_model_exclude_unset
+ route.response_model_exclude_defaults = response_model_exclude_defaults
+ route.response_model_exclude_none = response_model_exclude_none
+ route.include_in_schema = include_in_schema
+ route.response_class = response_class
+ route.dependency_overrides_provider = dependency_overrides_provider
+ route.callbacks = callbacks
+ route.openapi_extra = openapi_extra
+ route.generate_unique_id_function = generate_unique_id_function
+ route.strict_content_type = strict_content_type
+ route.tags = tags or []
+ route.responses = responses or {}
+ route.name = get_name(endpoint) if name is None else name
+ route.path_regex, route.path_format, route.param_convertors = compile_path(path)
+ if methods is None:
+ methods = ["GET"]
+ route.methods = {method.upper() for method in methods}
+ if isinstance(generate_unique_id_function, DefaultPlaceholder):
+ current_generate_unique_id: Callable[[Any], str] = (
+ generate_unique_id_function.value
+ )
+ else:
+ current_generate_unique_id = generate_unique_id_function
+ route.unique_id = route.operation_id or current_generate_unique_id(route)
+ # normalize enums e.g. http.HTTPStatus
+ if isinstance(status_code, IntEnum):
+ status_code = int(status_code)
+ route.status_code = status_code
+ if route.response_model:
+ assert is_body_allowed_for_status_code(status_code), (
+ f"Status code {status_code} must not have a response body"
+ )
+ response_name = "Response_" + route.unique_id
+ route.response_field = create_model_field(
+ name=response_name,
+ type_=route.response_model,
+ mode="serialization",
+ )
+ else:
+ route.response_field = None
+ if route.stream_item_type:
+ stream_item_name = "StreamItem_" + route.unique_id
+ route.stream_item_field = create_model_field(
+ name=stream_item_name,
+ type_=route.stream_item_type,
+ mode="serialization",
+ )
+ else:
+ route.stream_item_field = None
+ route.dependencies = list(dependencies or [])
+ route.description = description or inspect.cleandoc(route.endpoint.__doc__ or "")
+ # if a "form feed" character (page break) is found in the description text,
+ # truncate description text to the content preceding the first "form feed"
+ route.description = route.description.split("\f")[0].strip()
+ response_fields = {}
+ for additional_status_code, response in route.responses.items():
+ assert isinstance(response, dict), "An additional response must be a dict"
+ model = response.get("model")
+ if model:
+ assert is_body_allowed_for_status_code(additional_status_code), (
+ f"Status code {additional_status_code} must not have a response body"
+ )
+ response_name = f"Response_{additional_status_code}_{route.unique_id}"
+ response_field = create_model_field(
+ name=response_name, type_=model, mode="serialization"
+ )
+ response_fields[additional_status_code] = response_field
+ if response_fields:
+ route.response_fields = response_fields
+ else:
+ route.response_fields = {}
+
+ assert callable(endpoint), "An endpoint must be a callable"
+ route.dependant = get_dependant(
+ path=r............................F
=================================== FAILURES ===================================
________ test_included_api_route_without_app_scope_returns_405_response ________
async def functions are not natively supported.
You need to install a suitable plugin for your async framework, for example:
- anyio
- pytest-asyncio
- pytest-tornasync
- pytest-trio
- pytest-twisted
=============================== warnings summary ===============================
../../../../../../../Users/jp/repos/kaggle-gemini-coding-agent-post-training/.envs/overlays/starlette-1.6.0-py3-none-any/starlette/testclient.py:53
/Users/jp/repos/kaggle-gemini-coding-agent-post-training/.envs/overlays/starlette-1.6.0-py3-none-any/starlette/testclient.py:53: DeprecationWarning: The anyio.abc.BlockingPortal alias is deprecated, use anyio.from_thread.BlockingPortal instead.
_PortalFactoryType = Callable[[], AbstractContextManager[anyio.abc.BlockingPortal]]
tests/test_router_include_context.py:648
/private/tmp/swe_work/oracle_refix/fastapi_15745/b/workspace/tests/test_router_include_context.py:648: PytestUnknownMarkWarning: Unknown pytest.mark.anyio - is this a typo? You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/how-to/mark.html
@pytest.mark.anyio
tests/test_router_include_context.py:762
/private/tmp/swe_work/oracle_refix/fastapi_15745/b/workspace/tests/test_router_include_context.py:762: PytestUnknownMarkWarning: Unknown pytest.mark.anyio - is this a typo? You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/how-to/mark.html
@pytest.mark.anyio
tests/test_router_include_context.py:824
/private/tmp/swe_work/oracle_refix/fastapi_15745/b/workspace/tests/test_router_include_context.py:824: PytestUnknownMarkWarning: Unknown pytest.mark.anyio - is this a typo? You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/how-to/mark.html
@pytest.mark.anyio
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
1 failed, 28 passed, 4 warnings in 0.79s