Annotation-based filtering in FilterSchema #1514
Open
+477
−200
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.
Disclaimer: I am the guy who suggested
FilterSchema
in the first place.Problem Statement
There are several issues with how fields are configured for
FilterSchema
:Issue 1: Poor IDE Support
IDEs are unaware of custom arguments placed into a
Field
, which degrades developer experience (no autocompletion, no type checking).Issue 2: Deprecated Pydantic Features
Pydantic V2 has deprecated usage of
**extra
kwarg inField
method which the current solution relies on.Issue 3: Pydantic V2 Behavior Change
Pydantic V2 does not merge
**extra
andjson_schema_extra
if the latter is defined. That means, if you define aField
like this:You will not be able to extract
"q"
fromjson_schema_extra
because the**extra
arguments are ignored whenjson_schema_extra
is present.Issue 4: OpenAPI Schema Pollution
Pydantic V2 suggests using
json_schema_extra
instead of**extra
. While we could use it this way, this would mean that anything placed injson_schema_extra
will end up in youropenapi.json
document. While having extra data in the openapi.json does not break renderers like Swagger UI, exposing internal DB lookup configuration in the public-facing document is undesirable.Suggested Solution
This PR changes the concept of specifying lookup metadata from using
Field
arguments to using a separate annotation class calledFilterLookup
, which is used withinAnnotated
blocks:FilterLookup
supports everything that was supported previously, namely:expression_connector
to specify how to join multiple lookupsignore_none
to control whether to treatNone
s as valid filter valuesAddressing Stated Issues
FilterLookup
is a separate vanilla Python class whose arguments are clearly typed. This enables IDE autocompletion and improved DXFilterLookup
is orthogonal toField
and does not rely on its features anymore. You can useFilterLookup
with or withoutField
, there is no dependency from the former on the latter.Alternative Solutions Considered
We could introduce a
FilterField
function that would wrap around Pydantic'sFilter
function and return an augmentedFieldInfo
. TheFilterField
could be used exactly as a regularField
:The problem of wrapping
Field
lies in how complexlyField
is defined in Pydantic's code base. In order to enable proper static type checking support,FilterField
would need to repeat all those overloads with all those arguments anddjango-ninja
's maintainers would need to make sure the definitions stay aligned with every new version of Pydantic. This, in my opinion, is a considerable amount of technical debt to maintain.Backward Compatibility
FilterSchema
now has a layer of backwards compatibility which allows it to work with newFilterLookup
-annotated fields and with old-styleField
-centric fields, even within a single schema class:The old
Field(q=...)
approach is still functional but considered deprecated and not recommended for new code.Changes in this PR
FilterLookup
class with full typing support and validationFilterSchema
to supportFilterLookup
-based annotations while preserving original behaviorField
approach to "*_deprecated" suffixFilterLookup
, named "*_annotated" suffixFilterSchema
(previously ignored):FilterLookup
toninja/__init__.py
Migration Path
For existing users, no immediate action is required - the old approach continues to work. However, for better IDE support and future compatibility, migration to
FilterLookup
is recommended: