Repository: vitalik/django-ninja
Pull request #1212 (2024-06-28 22:50:22)

Allow generic filter types
This MR allows creating custom filter type with pydantic to reuse across filter classes, by making the field_name implicit.

like:
```py
IContainsField = Annotated[Optional[str], Field(None, q='__icontains')]

class BookFilterSchema(FilterSchema):
    name: IContainsField


Patch:
diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
deleted file mode 100644
index 2282521..0000000
--- a/.github/workflows/docs.yml
+++ /dev/null
@@ -1,15 +0,0 @@
-name: Docs
-
-on:
-  workflow_dispatch:
-
-jobs:
-  docs:
-    runs-on: ubuntu-latest
-    
-    steps:
-      - uses: actions/checkout@v3
-        with:
-          fetch-depth: 0
-      - name: Docs update
-        run: git push origin master:docs
diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
index 336290d..9efe9f0 100644
--- a/.github/workflows/publish.yml
+++ b/.github/workflows/publish.yml
@@ -23,3 +23,14 @@ jobs:
           FLIT_USERNAME: __token__
           FLIT_PASSWORD: ${{ secrets.PYPI_TOKEN }}
         run: flit publish
+
+  docs:
+    runs-on: ubuntu-latest
+    
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+      - name: Docs update
+        # run: git push origin master:docs
+        run: git branch -a
diff --git a/docs/docs/guides/input/filtering.md b/docs/docs/guides/input/filtering.md
index f7de698..eb46c8b 100644
--- a/docs/docs/guides/input/filtering.md
+++ b/docs/docs/guides/input/filtering.md
@@ -75,14 +75,21 @@ The `name` field will be converted into `Q(name=...)` expression.
 When your database lookups are more complicated than that, you can explicitly specify them in the field definition using a `"q"` kwarg:
 ```python hl_lines="2"
 class BookFilterSchema(FilterSchema):
-    name: Optional[str] = Field(None, q='name__icontains') 
+    name: Optional[str] = Field(None, q='name__icontains')
 ```
 You can even specify multiple lookup keyword argument names as a list:
 ```python hl_lines="2 3 4"
 class BookFilterSchema(FilterSchema):
     search: Optional[str] = Field(None, q=['name__icontains',
                                      'author__name__icontains',
-                                     'publisher__name__icontains']) 
+                                     'publisher__name__icontains'])
+```
+And to make generic fields, you can make the field name implicit by skipping it:
+```python hl_lines="2"
+IContainsField = Annotated[Optional[str], Field(None, q='__icontains')]
+
+class BookFilterSchema(FilterSchema):
+    name: IContainsField
 ```
 By default, field-level expressions are combined using `"OR"` connector, so with the above setup, a query parameter `?search=foobar` will search for books that have "foobar" in either of their name, author or publisher.
 
diff --git a/ninja/__init__.py b/ninja/__init__.py
index 81f4a6d..5cad423 100644
--- a/ninja/__init__.py
+++ b/ninja/__init__.py
@@ -1,6 +1,6 @@
 """Django Ninja - Fast Django REST framework"""
 
-__version__ = "1.2.2"
+__version__ = "1.2.0"
 
 
 from pydantic import Field
diff --git a/ninja/filter_schema.py b/ninja/filter_schema.py
index 3cc33b9..fe8f3b4 100644
--- a/ninja/filter_schema.py
+++ b/ninja/filter_schema.py
@@ -68,6 +68,8 @@ class FilterSchema(Schema):
         if not q_expression:
             return Q(**{field_name: field_value})
         elif isinstance(q_expression, str):
+            if q_expression.startswith("__"):
+                q_expression = f"{field_name}{q_expression}"
             return Q(**{q_expression: field_value})
         elif isinstance(q_expression, list):
             expression_connector = field_extra.get(  # type: ignore
@@ -75,8 +77,11 @@ class FilterSchema(Schema):
             )
             q = Q()
             for q_expression_part in q_expression:
+                q_expression_part = str(q_expression_part)
+                if q_expression_part.startswith("__"):
+                    q_expression_part = f"{field_name}{q_expression_part}"
                 q = q._combine(  # type: ignore
-                    Q(**{q_expression_part: field_value}),  # type: ignore
+                    Q(**{q_expression_part: field_value}),
                     expression_connector,
                 )
             return q
@@ -87,6 +92,7 @@ class FilterSchema(Schema):
                 f"  {field_name}: {field.annotation} = Field(..., q='<here>')\n"
                 f"or\n"
                 f"  {field_name}: {field.annotation} = Field(..., q=['lookup1', 'lookup2', ...])\n"
+                f"You can omit the field name and make it implicit by starting the lookup directly by '__'."
                 f"Alternatively, you can implement {self.__class__.__name__}.filter_{field_name} that must return a Q expression for that field"
             )
 
diff --git a/ninja/openapi/schema.py b/ninja/openapi/schema.py
index 494e4e1..72226e7 100644
--- a/ninja/openapi/schema.py
+++ b/ninja/openapi/schema.py
@@ -1,10 +1,9 @@
 import itertools
 import re
+import warnings
 from http.client import responses
 from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Set, Tuple
 
-from django.utils.termcolors import make_style
-
 from ninja.constants import NOT_SET
 from ninja.operation import Operation
 from ninja.params.models import TModel, TModels
@@ -29,9 +28,6 @@ def get_schema(api: "NinjaAPI", path_prefix: str = "") -> "OpenAPISchema":
     return openapi
 
 
-bold_red_style = make_style(opts=("bold",), fg="red")
-
-
 class OpenAPISchema(dict):
     def __init__(self, api: "NinjaAPI", path_prefix: str) -> None:
         self.api = api
@@ -107,10 +103,9 @@ class OpenAPISchema(dict):
     def operation_details(self, operation: Operation) -> DictStrAny:
         op_id = operation.operation_id or self.api.get_openapi_operation_id(operation)
         if op_id in self.all_operation_ids:
-            print(
-                bold_red_style(
-                    f'Warning: operation_id "{op_id}" is already used (Try giving a different name to: {operation.view_func.__module__}.{operation.view_func.__name__})'
-                )
+            warnings.warn(
+                f'operation_id "{op_id}" is already used (func: {operation.view_func})',
+                stacklevel=2,
             )
         self.all_operation_ids.add(op_id)
         result = {
diff --git a/ninja/operation.py b/ninja/operation.py
index b47eb4f..6b017c1 100644
--- a/ninja/operation.py
+++ b/ninja/operation.py
@@ -161,7 +161,6 @@ class Operation:
 
     def _run_checks(self, request: HttpRequest) -> Optional[HttpResponse]:
         "Runs security/throttle checks for each operation"
-        # NOTE: if you change anything in this function - do this also in AsyncOperation
 
         # csrf:
         if self.api.csrf:
@@ -337,8 +336,6 @@ class AsyncOperation(Operation):
 
     async def _run_checks(self, request: HttpRequest) -> Optional[HttpResponse]:  # type: ignore
         "Runs security checks for each operation"
-        # NOTE: if you change anything in this function - do this also in Sync Operation
-
         # auth:
         if self.auth_callbacks:
             error = await self._run_authentication(request)
@@ -351,12 +348,6 @@ class AsyncOperation(Operation):
             if error:
                 return error
 
-        # Throttling:
-        if self.throttle_objects:
-            error = self._check_throttles(request)
-            if error:
-                return error
-
         return None
 
     async def _run_authentication(self, request: HttpRequest) -> Optional[HttpResponse]:  # type: ignore
diff --git a/ninja/schema.py b/ninja/schema.py
index e51dd43..55a60a8 100644
--- a/ninja/schema.py
+++ b/ninja/schema.py
@@ -215,13 +215,9 @@ class Schema(BaseModel, metaclass=ResolverMetaclass):
     def _run_root_validator(
         cls, values: Any, handler: ModelWrapValidatorHandler[S], info: ValidationInfo
     ) -> Any:
-        # If Pydantic intends to validate against the __dict__ of the immediate Schema
-        # object, then we need to call `handler` directly on `values` before the conversion
-        # to DjangoGetter, since any checks or modifications on DjangoGetter's __dict__
-        # will not persist to the original object.
-        forbids_extra = cls.model_config.get("extra") == "forbid"
-        should_validate_assignment = cls.model_config.get("validate_assignment", False)
-        if forbids_extra or should_validate_assignment:
+        # when extra is "forbid" we need to perform default pydantic validation
+        # as DjangoGetter does not act as dict and pydantic will not be able to validate it
+        if cls.model_config.get("extra") == "forbid":
             handler(values)
 
         values = DjangoGetter(values, cls, info.context)


Test patch:
diff --git a/tests/test_filter_schema.py b/tests/test_filter_schema.py
index 1a77169..6c32d04 100644
--- a/tests/test_filter_schema.py
+++ b/tests/test_filter_schema.py
@@ -46,9 +46,15 @@ def test_empty_q_when_none_ignored():
     assert q == Q()
 
 
-def test_q_expressions2():
+@pytest.mark.parametrize("implicit_field_name", [False, True])
+def test_q_expressions2(implicit_field_name):
+    if implicit_field_name:
+        q = "__icontains"
+    else:
+        q = "name__icontains"
+
     class DummyFilterSchema(FilterSchema):
-        name: Optional[str] = Field(None, q="name__icontains")
+        name: Optional[str] = Field(None, q=q)
         tag: Optional[str] = Field(None, q="tag")
 
     filter_instance = DummyFilterSchema(name="John", tag=None)
@@ -66,11 +72,15 @@ def test_q_expressions3():
     assert q == Q(name__icontains="John") & Q(tag="active")
 
 
-def test_q_is_a_list():
+@pytest.mark.parametrize("implicit_field_name", [False, True])
+def test_q_is_a_list(implicit_field_name):
+    if implicit_field_name:
+        q__name = "__icontains"
+    else:
+        q__name = "name__icontains"
+
     class DummyFilterSchema(FilterSchema):
-        name: Optional[str] = Field(
-            None, q=["name__icontains", "user__username__icontains"]
-        )
+        name: Optional[str] = Field(None, q=[q__name, "user__username__icontains"])
         tag: Optional[str] = Field(None, q="tag")
 
     filter_instance = DummyFilterSchema(name="foo", tag="bar")
diff --git a/tests/test_openapi_schema.py b/tests/test_openapi_schema.py
index f7bf5d6..aec67f4 100644
--- a/tests/test_openapi_schema.py
+++ b/tests/test_openapi_schema.py
@@ -792,7 +792,7 @@ def test_get_openapi_urls():
         get_openapi_urls(api)
 
 
-def test_unique_operation_ids(capsys):
+def test_unique_operation_ids():
     api = NinjaAPI()
 
     @api.get("/1")
@@ -803,9 +803,9 @@ def test_unique_operation_ids(capsys):
     def same_name(request):  # noqa: F811
         pass
 
-    api.get_openapi_schema()
-    captured = capsys.readouterr()
-    assert '"test_openapi_schema_same_name" is already used ' in captured.out
+    match = 'operation_id "test_openapi_schema_same_name" is already used'
+    with pytest.warns(UserWarning, match=match):
+        api.get_openapi_schema()
 
 
 def test_docs_decorator():
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 28b2ad0..8f92d52 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -1,4 +1,4 @@
-from typing import List, Optional, Union
+from typing import List, Optional
 from unittest.mock import Mock
 
 import pytest
@@ -197,33 +197,21 @@ def test_django_getter():
     assert repr(dg) == "<DjangoGetter: {'i': 1}>"
 
 
-def test_schema_validates_assignment_and_reassigns_the_value():
+def test_django_getter_validates_assignment():
     class ValidateAssignmentSchema(Schema):
         str_var: str
+
         model_config = {"validate_assignment": True}
 
     schema_inst = ValidateAssignmentSchema(str_var="test_value")
+
+    # Validate we can re-assign the value, this is a test for
+    # a bug where validate_assignment would cause an AttributeError
+    # for __dict__ on the target schema.
     schema_inst.str_var = "reassigned_value"
-    assert schema_inst.str_var == "reassigned_value"
     try:
         schema_inst.str_var = 5
         raise AssertionError()
     except ValidationError:
         # We expect this error, all is okay
         pass
-
-
-@pytest.mark.parametrize("test_validate_assignment", [False, None])
-def test_schema_skips_validation_when_validate_assignment_False(
-    test_validate_assignment: Union[bool, None],
-):
-    class ValidateAssignmentSchema(Schema):
-        str_var: str
-        model_config = {"validate_assignment": test_validate_assignment}
-
-    schema_inst = ValidateAssignmentSchema(str_var="test_value")
-    try:
-        schema_inst.str_var = 5
-        assert schema_inst.str_var == 5
-    except ValidationError as ve:
-        raise AssertionError() from ve
diff --git a/tests/test_throttling.py b/tests/test_throttling.py
index b5ad50d..e3c93b6 100644
--- a/tests/test_throttling.py
+++ b/tests/test_throttling.py
@@ -3,7 +3,7 @@ from django.core.cache import cache
 from django.core.exceptions import ImproperlyConfigured
 
 from ninja import NinjaAPI, Router
-from ninja.testing import TestAsyncClient, TestClient
+from ninja.testing import TestClient
 from ninja.throttling import (
     AnonRateThrottle,
     AuthRateThrottle,
@@ -114,26 +114,6 @@ def test_operation_throttling():
     assert resp.json() == {"detail": "Too many requests."}
 
 
-@pytest.mark.asyncio
-async def test_async_throttling():
-    th = AnonRateThrottle("1/s")
-    set_throttle_timer(th, 0)
-
-    api = NinjaAPI(throttle=th)
-
-    @api.get("/check-async")
-    async def check(request):
-        return "OK"
-
-    client = TestAsyncClient(api)
-
-    resp = await client.get("/check-async")
-    assert resp.status_code == 200
-
-    resp = await client.get("/check-async")
-    assert resp.status_code == 429
-
-
 # "Unit tests" for the throttling module
 
 _client = TestClient(NinjaAPI())