Skip to content

Commit c5535fe

Browse files
committed
Fixed dependencies and added uncommited tests
1 parent 1ffb7b8 commit c5535fe

32 files changed

Lines changed: 2709 additions & 149 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ FastOpenAPI follows the [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
3333

3434
### Fixed
3535

36+
- **Generator (`yield`) dependencies stay open while the endpoint runs** — previously the cleanup code after `yield` executed before the endpoint was called, so a yielded resource (e.g. a DB session) was already closed when used. Cleanup now runs after the response is built, in reverse creation order, on both sync and async adapters, including error paths
37+
- **`Security` scopes are part of the dependency cache key** — two `Security(dep, scopes=...)` declarations with different scopes on one endpoint no longer share a cached result (the second check silently received the first one's scopes)
38+
- **OpenAPI schema documents parameters declared inside `Depends`/`Security` dependencies**`Query`/`Header`/`Cookie`/form/body parameters of (nested) dependency functions and class dependencies now appear in the operation, matching what the runtime actually requires; `SecurityScopes` injections and `Header(alias="Authorization")` stay hidden
39+
- **Parameters inside dependencies follow HTTP method semantics** — a bare Pydantic model in a dependency on GET/HEAD/DELETE resolves from query parameters, as it does on the endpoint itself (previously it always tried the request body)
3640
- **aiohttp multipart uploads** no longer fail with 500 — the JSON body reader drained the stream before the multipart parser could run
3741
- **Falcon ASGI urlencoded forms and multipart** no longer fail with 500 — async extractors got real async implementations instead of inheriting WSGI-only code
3842
- **Falcon multipart with text fields** no longer rejected as malformed (`secure_filename` probing on non-file parts)

fastopenapi/core/dependency_resolver.py

Lines changed: 90 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -47,45 +47,71 @@ def resolve_dependencies(
4747
self,
4848
endpoint: Callable[..., Any],
4949
request_data: RequestData,
50+
method: str | None = None,
5051
) -> dict[str, Any]:
5152
"""
5253
Resolve all dependencies for an endpoint
5354
55+
Generator dependencies stay open so the endpoint can use the
56+
yielded value; the adapter must call ``close(request_data)``
57+
after the endpoint returns to run their cleanup code.
58+
5459
Args:
5560
endpoint: The endpoint function
5661
request_data: Request data container
62+
method: HTTP method of the current request
5763
5864
Returns:
5965
Dict mapping parameter names to resolved dependency values
6066
"""
61-
# Initialize request-scoped tracking
62-
is_top_level = False
67+
self._open_request_scope(request_data, method)
68+
return self._resolve_endpoint_dependencies(endpoint, request_data)
69+
70+
def _open_request_scope(
71+
self, request_data: RequestData, method: str | None
72+
) -> None:
73+
"""Create the request-scoped cache entry if it does not exist yet"""
6374
with self._request_cache_lock:
6475
if request_data not in self._request_cache:
65-
is_top_level = True
6676
self._request_cache[request_data] = {
6777
"resolved": {},
6878
"resolving": set(),
6979
"generators": [],
80+
"method": method,
7081
}
7182

72-
try:
73-
return self._resolve_endpoint_dependencies(endpoint, request_data)
74-
finally:
75-
if is_top_level:
76-
# Get generators before deleting cache
77-
with self._request_cache_lock:
78-
cache = self._request_cache.get(request_data, {})
79-
generators = list(cache.get("generators", []))
80-
# Close generators (triggers finally blocks)
81-
for gen in generators:
82-
try:
83-
gen.close()
84-
except Exception:
85-
pass
86-
# Clean up request cache
87-
with self._request_cache_lock:
88-
self._request_cache.pop(request_data, None)
83+
def close(self, request_data: RequestData) -> None:
84+
"""
85+
Close generator dependencies opened for a request
86+
87+
Runs code after ``yield`` (via ``gen.close()``) in reverse creation
88+
order and drops the request cache entry. No-op when the request has
89+
no cache entry.
90+
"""
91+
with self._request_cache_lock:
92+
cache = self._request_cache.pop(request_data, None)
93+
if cache is None:
94+
return
95+
for gen in reversed(cache["generators"]):
96+
try:
97+
gen.close()
98+
except Exception:
99+
pass
100+
101+
async def aclose(self, request_data: RequestData) -> None:
102+
"""Async variant of ``close`` (also handles async generators)"""
103+
with self._request_cache_lock:
104+
cache = self._request_cache.pop(request_data, None)
105+
if cache is None:
106+
return
107+
for gen in reversed(cache["generators"]):
108+
try:
109+
if inspect.isasyncgen(gen):
110+
await gen.aclose()
111+
else:
112+
gen.close()
113+
except Exception:
114+
pass
89115

90116
def _resolve_endpoint_dependencies(
91117
self, endpoint: Callable[..., Any], request_data: RequestData
@@ -181,7 +207,7 @@ def _execute_dependency_function(
181207
"""
182208
Execute dependency function with caching and circular dependency detection
183209
"""
184-
cache_key = self._make_cache_key(dependency_func, request_data)
210+
cache_key = self._make_cache_key(dependency_func, request_data, security_scopes)
185211
request_cache = self._get_request_cache(request_data)
186212

187213
# The cache is request-scoped and a request is handled by a single
@@ -306,19 +332,25 @@ def _resolve_sub_dependencies(
306332

307333
return sub_dependencies
308334

309-
@staticmethod
310335
def _resolve_regular_params(
336+
self,
311337
dependency_func: Callable[..., Any],
312338
regular_params: dict[str, inspect.Parameter],
313339
request_data: RequestData,
314340
) -> dict[str, Any]:
315341
"""Resolve non-dependency parameters of a dependency function"""
316342
from fastopenapi.resolution.resolver import ParameterResolver
317343

344+
# Dependency params follow the same method-specific rules as
345+
# endpoint params (e.g. bare models map to query on GET)
346+
cache = self._request_cache.get(request_data)
347+
method = cache.get("method") if cache else None
348+
318349
try:
319350
return ParameterResolver.resolve_params(
320351
regular_params,
321352
request_data,
353+
method=method,
322354
owner=(
323355
getattr(dependency_func, "__module__", "fastopenapi"),
324356
getattr(dependency_func, "__qualname__", repr(dependency_func)),
@@ -349,50 +381,25 @@ async def resolve_dependencies_async(
349381
self,
350382
endpoint: Callable[..., Any],
351383
request_data: RequestData,
384+
method: str | None = None,
352385
) -> dict[str, Any]:
353386
"""
354387
Resolve all dependencies for an endpoint (async version)
355388
389+
Generator dependencies stay open so the endpoint can use the
390+
yielded value; the adapter must call ``aclose(request_data)``
391+
after the endpoint returns to run their cleanup code.
392+
356393
Args:
357394
endpoint: The endpoint function
358395
request_data: Request data container
396+
method: HTTP method of the current request
359397
360398
Returns:
361399
Dict mapping parameter names to resolved dependency values
362400
"""
363-
# Initialize request-scoped tracking
364-
is_top_level = False
365-
with self._request_cache_lock:
366-
if request_data not in self._request_cache:
367-
is_top_level = True
368-
self._request_cache[request_data] = {
369-
"resolved": {},
370-
"resolving": set(),
371-
"generators": [],
372-
}
373-
374-
try:
375-
return await self._resolve_endpoint_dependencies_async(
376-
endpoint, request_data
377-
)
378-
finally:
379-
if is_top_level:
380-
# Get generators before deleting cache
381-
with self._request_cache_lock:
382-
cache = self._request_cache.get(request_data, {})
383-
generators = list(cache.get("generators", []))
384-
# Close generators (triggers finally blocks)
385-
for gen in generators:
386-
try:
387-
if inspect.isasyncgen(gen):
388-
await gen.aclose()
389-
else:
390-
gen.close()
391-
except Exception:
392-
pass
393-
# Clean up request cache
394-
with self._request_cache_lock:
395-
self._request_cache.pop(request_data, None)
401+
self._open_request_scope(request_data, method)
402+
return await self._resolve_endpoint_dependencies_async(endpoint, request_data)
396403

397404
async def _resolve_endpoint_dependencies_async(
398405
self, endpoint: Callable[..., Any], request_data: RequestData
@@ -478,7 +485,7 @@ async def _execute_dependency_function_async(
478485
"""
479486
Execute dependency function with caching and circular dependency detection
480487
"""
481-
cache_key = self._make_cache_key(dependency_func, request_data)
488+
cache_key = self._make_cache_key(dependency_func, request_data, security_scopes)
482489
request_cache = self._get_request_cache(request_data)
483490

484491
hit, value = self._try_get_cached(cache_key, request_cache)
@@ -573,17 +580,25 @@ def _get_dependency_func(
573580
return dependency_func
574581

575582
def _make_cache_key(
576-
self, dependency_func: Callable[..., Any], request_data: RequestData
577-
) -> tuple[int, int]:
578-
"""Create cache key for request-scoped cache"""
579-
return (id(dependency_func), id(request_data))
583+
self,
584+
dependency_func: Callable[..., Any],
585+
request_data: RequestData,
586+
security_scopes: SecurityScopes | None = None,
587+
) -> tuple[int, int, tuple[str, ...]]:
588+
"""Create cache key for request-scoped cache
589+
590+
Scopes are part of the key: the same Security dependency requested
591+
with different scopes must be executed once per scope set.
592+
"""
593+
scopes = tuple(sorted(security_scopes.scopes)) if security_scopes else ()
594+
return (id(dependency_func), id(request_data), scopes)
580595

581596
def _get_request_cache(self, request_data: RequestData) -> dict[str, Any]:
582597
"""Get cache dictionary for current request"""
583598
return self._request_cache[request_data]
584599

585600
def _try_get_cached(
586-
self, cache_key: tuple[int, int], request_cache: dict[str, Any]
601+
self, cache_key: tuple[int, int, tuple[str, ...]], request_cache: dict[str, Any]
587602
) -> tuple[bool, Any]:
588603
"""Try to get cached value from request-scoped cache"""
589604
with self._request_cache_lock:
@@ -593,7 +608,10 @@ def _try_get_cached(
593608
return False, None
594609

595610
def _cache_result(
596-
self, cache_key: tuple[int, int], result: Any, request_cache: dict[str, Any]
611+
self,
612+
cache_key: tuple[int, int, tuple[str, ...]],
613+
result: Any,
614+
request_cache: dict[str, Any],
597615
) -> None:
598616
"""Store result in request-scoped cache"""
599617
with self._request_cache_lock:
@@ -647,17 +665,23 @@ def get_cache_stats(self) -> dict[str, int]:
647665

648666
# Convenience functions
649667
def resolve_dependencies(
650-
endpoint: Callable[..., Any], request_data: RequestData
668+
endpoint: Callable[..., Any],
669+
request_data: RequestData,
670+
method: str | None = None,
651671
) -> dict[str, Any]:
652672
"""Convenience function to resolve dependencies (sync)"""
653-
return dependency_resolver.resolve_dependencies(endpoint, request_data)
673+
return dependency_resolver.resolve_dependencies(endpoint, request_data, method)
654674

655675

656676
async def resolve_dependencies_async(
657-
endpoint: Callable[..., Any], request_data: RequestData
677+
endpoint: Callable[..., Any],
678+
request_data: RequestData,
679+
method: str | None = None,
658680
) -> dict[str, Any]:
659681
"""Convenience function to resolve dependencies (async)"""
660-
return await dependency_resolver.resolve_dependencies_async(endpoint, request_data)
682+
return await dependency_resolver.resolve_dependencies_async(
683+
endpoint, request_data, method
684+
)
661685

662686

663687
def get_dependency_stats() -> dict[str, int]:

fastopenapi/openapi/generator.py

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import threading
44
import types
55
import typing
6+
from collections.abc import Iterable, Iterator
67
from dataclasses import dataclass
78
from functools import lru_cache
89
from typing import Any
@@ -24,6 +25,7 @@
2425
Header,
2526
Param,
2627
Security,
28+
SecurityScopes,
2729
is_body_model_annotation,
2830
is_pydantic_model,
2931
unwrap_annotated_parameter,
@@ -301,8 +303,7 @@ def process_route_parameters(
301303
form_required: list[str] = []
302304
has_explicit_embed = False
303305

304-
for param_name, param in sig.parameters.items():
305-
param = unwrap_annotated_parameter(param)
306+
for param_name, param in self._flatten_route_parameters(sig):
306307
if self._should_skip_parameter(param):
307308
continue
308309

@@ -333,7 +334,24 @@ def process_route_parameters(
333334
has_explicit_embed,
334335
form_required,
335336
)
336-
return parameters, request_body
337+
return self._dedupe_parameters(parameters), request_body
338+
339+
@staticmethod
340+
def _dedupe_parameters(parameters: list[dict[str, Any]]) -> list[dict[str, Any]]:
341+
"""Drop duplicate wire parameters (same name and location)
342+
343+
Python names may repeat across dependency scopes while mapping to
344+
distinct wire params (different alias/location), so dedup happens
345+
on the final ("name", "in") identity; the first occurrence wins.
346+
"""
347+
seen: set[tuple[str, str]] = set()
348+
deduped = []
349+
for param in parameters:
350+
key = (param["name"], param["in"])
351+
if key not in seen:
352+
seen.add(key)
353+
deduped.append(param)
354+
return deduped
337355

338356
def _classify_parameter_result(
339357
self,
@@ -388,11 +406,46 @@ def _extract_path_parameters(self, path: str) -> set[str]:
388406
openapi_path = PATH_PARAM_PATTERN.sub(r"{\1}", path)
389407
return set(OPENAPI_PATH_PATTERN.findall(openapi_path))
390408

409+
def _flatten_route_parameters(
410+
self, sig: inspect.Signature
411+
) -> Iterator[tuple[str, inspect.Parameter]]:
412+
"""Yield endpoint parameters with Depends/Security expanded
413+
414+
Dependency functions require their own Query/Header/... parameters
415+
at runtime, so they belong to the operation. Repeated dependencies
416+
are walked once; SecurityScopes injections are runtime-internal and
417+
skipped. Python names may repeat across dependency scopes — wire-level
418+
dedup happens later, in ``_dedupe_parameters``, once alias and
419+
location are known.
420+
"""
421+
seen_deps: set[Any] = set()
422+
423+
def walk(
424+
params: Iterable[tuple[str, inspect.Parameter]],
425+
) -> Iterator[tuple[str, inspect.Parameter]]:
426+
for param_name, param in params:
427+
param = unwrap_annotated_parameter(param)
428+
if isinstance(param.default, (Depends, Security)):
429+
func = param.default.dependency
430+
if func is None and param.annotation is not inspect.Parameter.empty:
431+
func = param.annotation
432+
if func is None or func in seen_deps:
433+
continue
434+
seen_deps.add(func)
435+
try:
436+
sub_sig = inspect.signature(func)
437+
except (TypeError, ValueError):
438+
continue
439+
yield from walk(sub_sig.parameters.items())
440+
elif param.annotation is SecurityScopes:
441+
continue
442+
else:
443+
yield param_name, param
444+
445+
yield from walk(sig.parameters.items())
446+
391447
def _should_skip_parameter(self, param: inspect.Parameter) -> bool:
392448
"""Determine if parameter should be skipped"""
393-
if isinstance(param.default, (Depends, Security)):
394-
return True
395-
396449
# Skip authorization headers handled by security
397450
if isinstance(param.default, Header) and param.default.alias == "Authorization":
398451
return True

0 commit comments

Comments
 (0)