Wrong django-filter types

[markopy]

django-filter fields can have a different type from the underlying model field they operate against. drf-spectacular incorrectly makes the schema type match the model field instead of the filter.

Take the following filter for example:

class UserFilter(django_filters.FilterSet):
    has_logged_in = django_filters.BooleanFilter(field_name='last_login', lookup_expr='isnull')

last_login is a datetime model field but the filter field is boolean. Currently the schema has the type for has_logged_in as datetime which is incorrect.

The reason, I believe, is this part of the code:

drf-spectacular/drf_spectacular/contrib/django_filters.py

Lines 57 to 60 in 243445f

	if isinstance(model_field, models.Field):
	schema = auto_schema._map_model_field(model_field, direction=None)
	else:
	schema = self.map_filter_field(filter_field)

The model field type is preferred over the actual filter type.

I don't understand why the model field is even used here. Is it to get additional information? Since the filter itself has a clear type I don't think the mode field type is ever relevant.

[tfranzel]

i see your point. this has been criticized before and you are absolutely right. however you also have to notice that the map_filter_fields method below list some commented out filters that don't have a clear type. the whole reason i did it that way was because fields were under-specified for me at several points. we should definitely look into this again.

[markopy]

I don't fully understand all the intricacies of this but I saw that map_filter_fields doesn't implement all types. Maybe the logic can be inverted so map_filter_fields is used for the types it supports and we only fall back to the model otherwise?

[tfranzel]

inversion is the way to go. actually already started working on it.

[tfranzel]

although still not perfect, this refactoring should be a massive improvement. @markopy please have a look and test it, thanks.

[markopy]

This solves my problem of using BooleanFilter on model fields of other types. Seems like a good solution and the currently unsupported filters can be added in the future as needed.

Thanks for the quick fix!

[lerela]

Hey @tfranzel, these changes broke drf-spectacular here with a custom DateTimeFilter.

Traceback:

192.168.0.48 - - [01/Mar/2021 14:51:52] "GET /api/schema/ HTTP/1.1" 500 -
Traceback (most recent call last):
  File "venv/lib/python3.8/site-packages/django/contrib/staticfiles/handlers.py", line 65, in __call__
    return self.application(environ, start_response)
  File "venv/lib/python3.8/site-packages/django/core/handlers/wsgi.py", line 141, in __call__
    response = self.get_response(request)
  File "venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 75, in get_response
    response = self._middleware_chain(request)
  File "venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 36, in inner
    response = response_for_exception(request, exc)
  File "venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 90, in response_for_exception
    response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
  File "venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 125, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)
  File "venv/lib/python3.8/site-packages/django_extensions/management/technical_response.py", line 37, in null_technical_500_response
    six.reraise(exc_type, exc_value, tb)
  File "venv/lib/python3.8/site-packages/six.py", line 702, in reraise
    raise value.with_traceback(tb)
  File "venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "venv/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "venv/lib/python3.8/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "venv/lib/python3.8/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "venv/lib/python3.8/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "venv/lib/python3.8/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "venv/lib/python3.8/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "venv/lib/python3.8/site-packages/drf_spectacular/views.py", line 67, in get
    return self._get_schema_response(request)
  File "venv/lib/python3.8/site-packages/drf_spectacular/views.py", line 71, in _get_schema_response
    return Response(generator.get_schema(request=request, public=self.serve_public))
  File "venv/lib/python3.8/site-packages/drf_spectacular/generators.py", line 219, in get_schema
    paths=self.parse(request, public),
  File "venv/lib/python3.8/site-packages/drf_spectacular/generators.py", line 196, in parse
    operation = view.schema.get_operation(path, path_regex, method, self.registry)
  File "venv/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 61, in get_operation
    parameters = self._get_parameters()
  File "venv/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 190, in _get_parameters
    **dict_helper(self._get_filter_parameters()),
  File "venv/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 365, in _get_filter_parameters
    parameters += filter_extension.get_schema_operation_parameters(self)
  File "venv/lib/python3.8/site-packages/drf_spectacular/contrib/django_filters.py", line 43, in get_schema_operation_parameters
    result += self.resolve_filter_field(
  File "venv/lib/python3.8/site-packages/drf_spectacular/contrib/django_filters.py", line 67, in resolve_filter_field
    schema = build_basic_type(unambiguous_mapping[filter_field.__class__])
KeyError: <class 'myapp.filters.generics.CustomDateTimeFilter'>

Filter:

class CustomDateTimeFilter(filters.DateTimeFilter):
    field_class = CustomDateTimeField

class MyFilter(filters.FilterSet):
    from_date = CustomDateTimeFilter(field_name="date", lookup_expr="gte")

[tfranzel]

@lerela yeah absolutely missed that one. 🤦

[tfranzel]

@lerela that should do it

[lerela]

It does 👍
Thanks!

[elonzh]

I think this issue should not been closed, I upgrade to 0.17.0, and it still exists.

❯ pip show drf_spectacular
Name: drf-spectacular
Version: 0.17.0
Summary: Sane and flexible OpenAPI 3 schema generation for Django REST framework
Home-page: https://github.com/tfranzel/drf-spectacular
Author: T. Franzel
Author-email: tfranzel@gmail.com
License: BSD
Location: /home/elonzh/miniconda/envs/unob/lib/python3.8/site-packages
Requires: inflection, Django, PyYAML, jsonschema, uritemplate, djangorestframework

seen = django_filters.BooleanFilter(field_name="seen", lookup_expr="isnull")

[tfranzel]

@elonzh not quite sure how you get there. since that change BooleanFilter is directly resolved to bool. are you 100% certain this is the most recent version. i cannot imagine how this should be possible. also this is covered multiple times in the tests.

drf-spectacular/drf_spectacular/contrib/django_filters.py

Line 54 in 1246bce

filters.BooleanFilter: OpenApiTypes.BOOL,

[elonzh]

🤣 I'm sure I'm using version 0.17.0 because I upgrade it because of issue #427.

[tfranzel]

then we would need a reproduction of this somehow, as i can't see how it should be possible. please try to reproduce it in a minimal way so i can tackle this.

[elonzh]

I just start a new Django project and reproduce the issue easily.

import django_filters
from django.db import models
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import serializers, routers
from rest_framework.viewsets import ModelViewSet


class Dummy(models.Model):
    seen = models.DateTimeField(null=True)


class DummySerializer(serializers.ModelSerializer):
    class Meta:
        model = Dummy
        fields = "__all__"


class DummyFilterSet(django_filters.FilterSet):
    class Meta:
        model = Dummy
        fields = []

    seen = django_filters.BooleanFilter(field_name="seen", lookup_expr="isnull")


class DummyViewSet(ModelViewSet):
    queryset = Dummy.objects.all()
    serializer_class = DummySerializer
    filterset_class = DummyFilterSet
    filter_backends = [DjangoFilterBackend]


router = routers.SimpleRouter()
router.register("dummy", DummyViewSet)
urlpatterns = router.urls

I also print the package version.

Django version 3.1.11, using settings 'demo.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
[11/Jun/2021 09:14:28] "GET /api/v1/schema/swagger-ui/ HTTP/1.1" 200 1020
[11/Jun/2021 09:14:29] "GET /api/v1/schema/ HTTP/1.1" 200 17360
/home/elonzh/proj/ivysci/unob/demo/demo/settings.py changed, reloading.
drf_spectacular.__version__='0.17.0'
...

I replace filterset_class to filterset_fields and it seems gives the right schema.

class DummyViewSet(ModelViewSet):
    queryset = Dummy.objects.all()
    serializer_class = DummySerializer
-   filterset_class = DummyFilterSet
+   filterset_fields = {
+       "seen": ["isnull"]
+   }
    filter_backends = [DjangoFilterBackend]

[tfranzel]

awesome. thank you. i can work with that

[tfranzel]

turns out django_filters.rest_framework.filters.BooleanFilter != django_filters.filters.BooleanFilter. this is the only field where there is a subclassing happening in django_filters.filters

@elonzh. easy fix for you would be to use the other import path. i'll evaluate if and how we should fix this.

[tfranzel]

i'll close the issue again as it was a simple fix and i'm pretty sure your case is now covered @elonzh