feat(fastapi): let FilterConfig control generated query parameter names - #785
Open
ruicleite96 wants to merge 2 commits into
Open
feat(fastapi): let FilterConfig control generated query parameter names#785ruicleite96 wants to merge 2 commits into
ruicleite96 wants to merge 2 commits into
Conversation
`_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.
The names `provide_filters` generates were hardcoded: eleven literals (`currentPage`, `searchString`, `orderBy`, `createdBefore`, ...) plus `camelize()` applied to model field names for the per-field filters. An application whose API is snake_case had no way to reach them, and no way to express suffix conventions such as `issue_date__gte`. Adds an optional `alias_generator` to `FilterConfig`, receiving each parameter's snake_case name and returning the query parameter to expose. Every generated name is now derived from a snake_case canonical form, and the default generator is `camelize` — which reproduces the previous names exactly (`created_before` -> `createdBefore`, `page_size` -> `pageSize`), so output is unchanged unless a generator is supplied. `alias_generator=lambda name: name` keeps snake_case throughout. The dependency cache key is now the hashable tuple rather than `hash()` of it. A config may now carry a function, functions hash by identity, and identity is the address — reducing the key to an int drops the last reference, letting CPython hand the same address to the next generator so the cache returns the wrong providers for it. Keeping the tuple retains the reference. `make_hashable` preserves hashable callables for the same reason, instead of stringifying them into an address.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #784 — the first commit here is that fix, which this depends on to be demonstrable. Review #784 first; this diff is the second commit.
Problem
The query parameter names
provide_filtersgenerates are hardcoded. Eleven string literals:plus
camelize()applied to the model's own field names for the per-field filters (statusIn,isActive,expenseCategoryId, …).There is no hook to change them. An application whose public API is snake_case cannot use the generator at all, and one using suffix conventions (
issue_date__gte,parent_id__is_null) has no way to express them. The only escape is to hand-roll the parameter layer, which means reimplementing theinspect.Parameterassembly this module already does well.Change
An optional
alias_generatoronFilterConfig, receiving each parameter's snake_case name and returning the query parameter to expose:Every generated name now derives from a snake_case canonical form, and the default generator is
camelize, which reproduces the existing names exactly:camelize(default)created_beforecreatedBeforepage_sizepageSizesearch_ignore_casesearchIgnoreCasesort_ordersortOrderSo output is byte-identical unless a generator is supplied.
test_default_names_are_unchangedpins that.The cache had to change with it
Putting a callable in the config exposed a real hazard. The key was
hash((_CACHE_NAMESPACE, make_hashable(config)))— anint. Functions hash by identity, identity is the address, and reducing the key to an int drops the last reference to the function. CPython then reuses that address for the next generator, so two different generators produce the same key andDependencyCachereturns the wrong providers.This is not theoretical — I hit it while testing:
Two changes fix it:
hash()of it, so the tuple retains the callable;make_hashablekeeps hashable callables as-is instead ofstr()-ing them into an address.test_distinct_generators_get_distinct_dependenciescovers it.tests/unit/test_extensions/test_fastapi/test_providers.py::test_create_filter_dependencies_cache_missasserted the key washash(...); it now asserts the tuple. That is the one intentional test change.Tests
tests/unit/test_extensions/test_fastapi/test_provider_alias_generator.py:Full
tests/unit/test_extensions/suite passes.ruff check,ruff formatandmypyclean.Scope
FastAPI only. The Litestar provider names its parameters the same way and could take the same hook — I left it out to keep this reviewable, and am happy to add it here or in a follow-up if you want parity.
I am also happy to change the generator's signature if you would prefer something richer than
Callable[[str], str]— for example receiving(field, operator)separately, which is whatfastapi-filtersdoes. One string keeps the fixed parameters and the per-field ones under a single transform, which is why I started there.