Skip to content

Conversation

l1b3r
Copy link

@l1b3r l1b3r commented Jul 18, 2025

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 in Field method which the current solution relies on.

Issue 3: Pydantic V2 Behavior Change
Pydantic V2 does not merge **extra and json_schema_extra if the latter is defined. That means, if you define a Field like this:

Field(None, q="name__icontains", json_schema_extra={"something": "else"})

You will not be able to extract "q" from json_schema_extra because the **extra arguments are ignored when json_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 in json_schema_extra will end up in your openapi.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 called FilterLookup, which is used within Annotated blocks:

class BookFilterSchema(FilterSchema):
    name: Annotated[str | None, FilterLookup("name__icontains")] = None

FilterLookup supports everything that was supported previously, namely:

  • Having multiple lookups for the same field
  • expression_connector to specify how to join multiple lookups
  • ignore_none to control whether to treat Nones as valid filter values

Addressing Stated Issues

  • FilterLookup is a separate vanilla Python class whose arguments are clearly typed. This enables IDE autocompletion and improved DX
    • ✅ Solves issue 1
  • FilterLookup is orthogonal to Field and does not rely on its features anymore. You can use FilterLookup with or without Field, there is no dependency from the former on the latter.
    • ✅ Solves issue 2
    • ✅ Solves issue 3
    • ✅ Solves issue 4

Alternative Solutions Considered

We could introduce a FilterField function that would wrap around Pydantic's Filter function and return an augmented FieldInfo. The FilterField could be used exactly as a regular Field:

class BookFilterSchema(FilterSchema):
    name: str | None = FilterField(None, q="name__icontains")

The problem of wrapping Field lies in how complexly Field 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 and django-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 new FilterLookup-annotated fields and with old-style Field-centric fields, even within a single schema class:

class BookFilterSchema(FilterSchema):
    name: Annotated[str | None, FilterLookup("name__icontains")] = None
    author: str | None = Field(None, q="author__name")  # Still works but deprecated

The old Field(q=...) approach is still functional but considered deprecated and not recommended for new code.

Changes in this PR

  1. Added FilterLookup class with full typing support and validation
  2. Enhanced FilterSchema to support FilterLookup-based annotations while preserving original behavior
  3. Comprehensive test coverage:
    • Renamed existing tests using Field approach to "*_deprecated" suffix
    • Added corresponding tests with FilterLookup, named "*_annotated" suffix
    • Added validation tests for edge cases
  4. Fixed typing issues in FilterSchema (previously ignored):
  5. Updated exports: Added FilterLookup to ninja/__init__.py
  6. Updated documentation: rewrite of filtering guide to showcase the new approach with backward compatibility notes

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:

# Old (deprecated but still works)
name: str | None = Field(None, q="name__icontains")

# New (recommended)  
name: Annotated[str | None, FilterLookup("name__icontains")] = None

l1b3r referenced this pull request Aug 12, 2025
* Removed Deprecated Config class (on Schema) support
* FilterField instead of Field (+ deprecation warning)
@l1b3r
Copy link
Author

l1b3r commented Aug 12, 2025

Updated PR by merging with latest master:

  • Removed FilterField from master in favor of FilterLookup suggested by this PR
  • Replaced deprecated Config with preferred model_config = ConfigDict() approach in FilterSchema. Introduced FilterConfigDict (inherits from vanilla Pydantic's ConfigDict ) for better IDE support.
  • Preserved (with modifications) a custom warning when a deprecated filtering configuration is used.
  • Adjusted tests and docs

Apologies for the commit noise. Let me know if you want me to squash them on my end before (and if) we proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant