Skip to content

fix(fastapi): give each filter config its own dependency signature - #784

Open
ruicleite96 wants to merge 3 commits into
litestar-org:mainfrom
ruicleite96:fix-shared-filter-signature
Open

fix(fastapi): give each filter config its own dependency signature#784
ruicleite96 wants to merge 3 commits into
litestar-org:mainfrom
ruicleite96:fix-shared-filter-signature

Conversation

@ruicleite96

@ruicleite96 ruicleite96 commented Aug 13, 2026

Copy link
Copy Markdown

Problem

_create_filter_aggregate_function_fastapi assigns __signature__ to the module-level _aggregate_filter_function and returns that same object (providers.py:763-768). Every call to provide_filters therefore rewrites the parameters of every dependency built before it — they are all the same function.

Reproducer on current main:

app = FastAPI()
authors = provide_filters({"search": "name"})
books   = provide_filters({"created_at": True})

@app.get("/authors")
async def a(f: Annotated[list, Depends(authors)]): return []

@app.get("/books")
async def b(f: Annotated[list, Depends(books)]): return []
/authors  -> ['createdBefore', 'createdAfter']
/books    -> ['createdBefore', 'createdAfter']
same object? True

/authors was configured with search and exposes no search parameter at all, while gaining two date parameters it never asked for. Whichever config is built last wins for the whole process.

This is quiet: the app starts, the schema looks plausible, and the filters simply do not do what the router asked for. It is masked in any process that only ever builds one config — which is why the existing tests do not catch it.

Change

Build a fresh function per config, delegating to the shared implementation, instead of mutating one shared object:

def aggregate_filters(**kwargs: Any) -> list[FilterTypes]:
    return _aggregate_filter_function(**kwargs)

aggregate_filters.__signature__ = inspect.Signature(...)
return aggregate_filters

The per-config DependencyCache is untouched, so repeated identical configs still return one shared object.

After the fix:

/authors  -> ['searchString', 'searchIgnoreCase']
/books    -> ['createdBefore', 'createdAfter']
same object? False

A second way the same thing went wrong

The cache key was hash((_CACHE_NAMESPACE, make_hashable(config))) — the config alone. But
dep_defaults shapes the generated signature just as much: it supplies the filter parameter names
and DEFAULT_PAGINATION_SIZE. Two callers passing the same config with different defaults therefore
shared one dependency, and whichever was built first won for the whole process:

config = {"pagination_type": "limit_offset"}

class BigPages(DependencyDefaults):
    DEFAULT_PAGINATION_SIZE = 100

provide_filters(config)             # pageSize default 20
provide_filters(config, BigPages())  # pageSize default 20  <- asked for 100

Reversing the build order flips it: the caller who wanted the stock 20 gets 100 instead.

That is the same failure this branch already fixes one level up — a router silently served a
configuration it never asked for — so it is fixed here too rather than left as a second route to it.
The key is built from the defaults' values, not the instance, so two equal DependencyDefaults
still share one dependency and the cache stays useful.

Tests

tests/unit/test_extensions/test_fastapi/test_provider_isolation.py:

  • two different configs keep their own query parameters, and are not the same object;
  • the same config is still cached to one object, so the fix does not defeat the cache;
  • one config with two different dep_defaults gets two dependencies, each with its own page size;
  • two equal DependencyDefaults still share one dependency.

test_create_filter_dependencies_cache_miss pinned the old key formula, so it now builds its
expected key through the same _filter_cache_key helper the provider uses, rather than restating
the formula and being free to drift from it.

The full tests/unit/test_extensions/test_fastapi/ suite passes; ruff check, ruff format and mypy are clean.

Note

Of the two problems above, only the second reaches the Litestar provider. I checked both by building
its dependencies rather than by reading:

  • The shared signature does not affect it. Its provide_filters is defined inside
    _create_filter_aggregate_function (providers.py:745), so it is already a fresh function per
    config — {"search": "name"} and {"created_at": True} yield ['search_filter'] and
    ['created_filter'] respectively.
  • The cache key is the same. create_filter_dependencies keys on
    hash((_CACHE_NAMESPACE, make_hashable(config))) (providers.py:322) while reading eight
    dep_defaults attributes, so a caller passing DEFAULT_PAGINATION_SIZE = 100 gets the cached
    page_size of 20, exactly as FastAPI did.

I have not touched it here to keep the diff scoped to fastapi — happy to follow up with the
cache-key fix, in this PR or a separate one, whichever you prefer.

`_create_filter_aggregate_function_fastapi` assigned `__signature__` to the
module-level `_aggregate_filter_function` and returned that same object, so
every call to `provide_filters` overwrote the parameters of every dependency
built before it.

Two routers with different configs therefore both served whichever config was
built last. Given `provide_filters({"search": "name"})` for one router and
`provide_filters({"created_at": True})` for another, both expose only
`createdBefore` and `createdAfter`: the first silently loses the search
parameter it was configured with, and gains date parameters it never asked for.
The per-config cache masks this whenever a process happens to build only one
config.

Builds a fresh function per config instead, delegating to the shared
implementation. The cache still returns one object per identical config.
`provide_filters` takes a `FilterConfig` TypedDict, and mypy accepts a dict
literal there but not a `dict[str, Any]` variable — nor `dict(config)`, which
degrades a TypedDict to `dict[str, object]`.

Build the config in a factory returning `FilterConfig`. Each call still yields a
distinct, equal dict, so the test goes on exercising the cache by value rather
than by identity.
The cache keyed on the config alone, but `dep_defaults` shapes the generated
signature just as much — it supplies the parameter names and the default page
size. Two callers passing the same config with different defaults therefore
shared one dependency, and whichever was built first won for the whole process:
a caller asking for `DEFAULT_PAGINATION_SIZE = 100` silently served 20.

This is the same failure this branch already fixes one level up, so fix it here
rather than leave a second way for a router to serve a config it never asked
for. The key uses the values rather than the instance, so two equal
`DependencyDefaults` still share one dependency and the cache stays useful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant