Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FilterMethod.__call__ calling method with value=<Queryset []> #1550

Open
CelestialGuru opened this issue Jan 19, 2023 · 2 comments
Open

FilterMethod.__call__ calling method with value=<Queryset []> #1550

CelestialGuru opened this issue Jan 19, 2023 · 2 comments

Comments

@CelestialGuru
Copy link

Here's my filterset:

class LockFilterSet(filterset.FilterSet):
    class Meta:
        model = models.Lock
        fields = ()

    grate_widths = filterset.ModelMultipleChoiceFilter(
        field_name="grate_width",
        queryset=configuration_models.SizeClass.objects.all(),
        method="filter_grate_widths",
    )

    def filter_grate_widths(self, queryset, name, value):
        return queryset.filter(Q(**{f"{name}__in": value}) | Q(**{f"{name}__isnull": True}))

If I GET /api/locks/, /api/locks/?, or /api/locks/?grate_widths= my filter_grate_widths method is called. The value is <Queryset []>.

Examining the call stack we see that it came from

# django_filters/filters.py
class FilterMethod:
    def __init__(self, filter_instance):
        self.f = filter_instance

    def __call__(self, qs, value):
        if value in EMPTY_VALUES:
            return qs

        return self.method(qs, self.f.field_name, value)

EMPTY_VALUES is ([], (), {}, "", None).

But of course the issue is that value is <Queryset []>, which is not in EMPTY_VALUES.

I did some digging and this value is generated by the form. The form is django.forms.Form looking at the metaclass code. So I am not sure if this bug is django-filter or django itself.

Yes I can fix this by adding if not value: return queryset, but the source code in FilteMethod suggests that my method should only be called when there is some non-empty value, which my url parameters are (empty).

What am I using

  • Python 3.11
  • django 4.1.3
  • djangorestframework 3.14.0
  • django-filter 22.1
@CelestialGuru
Copy link
Author

Further study about Django forms:

Setup

from django import forms
from django.contrib.auth import get_user_model

User = get_user_model()

class MyForm(forms.Form):
   users = forms.ModelMultipleChoiceField(queryset=User.objects.all(), required=False)

Empty form

form = MyForm(data={"users":[]})
form.is_valid()
print(form.cleaned_data)  # {'users': <Queryset []>}

Non-empty form

form = MyForm(data={"users":[1]})
form.is_valid()
print(form.cleaned_data)  # {'users': <Queryset [<User: [email protected]>]>}

It appears that vanilla Django forms return Queryset instances for the ModelMultipleChoiceField. So that leaves the question of why does FilterMethod mentioned earlier return sometimes a Queryset (empty) and other times a list?

The metaclass FilterSetOptions has this line: self.form = getattr(options, "form", forms.Form) which would just be the vanilla Django form class. That leaves django_filters.fields.ModelMultipleChoiceField in question.

Setup, but with django-filter fields

from django import forms
from django.contrib.auth import get_user_model
from django_filters import fields as django_filter_fields

User = get_user_model()

class MyForm(forms.Form):
   users = django_filter_fields.ModelMultipleChoiceField(queryset=User.objects.all(), required=False)

Empty form

form = MyForm(data={"users":[]})
form.is_valid()
print(form.validated_data)  # dict_values([<QuerySet []>])

Non-empty form

form = MyForm(data={"users":[1]})
form.is_valid()
print(form.validated_data)  # dict_values([[<User: [email protected]>]])

There, I found it. A minimal case demonstrating the difference in django.forms.ModelMultipleChoiceField and django_filters.fields.ModelMultipleChoiceField. Why does the former always return Queryset instances while the latter does not? I do not know yet.

@CelestialGuru
Copy link
Author

CelestialGuru commented Jan 19, 2023

I think I found out where the problem is. In django.forms.models.py:ModelMultipleChoiceField.clean it has

def clean(self, value):
    value = self.prepare_value(value)
    if self.required and not value:
        raise ValidationError(self.error_messages["required"], code="required")
    elif not self.required and not value:
        return self.queryset.none()
    qs = self._check_values(value)
    self.run_validators(value)
    return qs

The two return paths are either self.queryset.none() or self._check_values(value).

The former is where the empty queryset comes from.

self._check_values(value) calls django_filters.fields.py:ModelMultipleChoiceField._check_values which is

def _check_values(self, value):
    ...
    result = list(super()._check_values(value))  # This is where the list vs queryset comes from
    result += [self.null_value] if null else []
    return result

we can clearly see the list() changing the queryset to a list. I don't pretend to know what result += [self.null_value] if null else [] is for and I can see that is why you needed to change a queryset to a list to be able to append self.null_value if needed. However, like mentioned in the original post, this does not work with EMPTY_VALUES in FilterMethod. value in EMPTY_VALUES is False for the empty queryset. I might suggest changing the logic to if not value: return qs, but I can see how that might not work properly with valid numeric values like 0 or 0.0.

Solution

class ModelMultipleChoiceFilter(QuerySetRequestMixin, MultipleChoiceFilter):
    field_class = ModelMultipleChoiceField

    def clean(self, value):
        # When a 'method' argument is passed, the proxy FilterMethod class is used
        # and first checks if the value is in EMPTY_VALUES, calling 'method' only if it is not.
        # When value is empty, super() returns an empty queryset. `value in EMPTY_VALUES is False`.
        # When value is not empty, super() returns a list. `value in EMPTY_VALUES is True`.
        # 
        # The inconsistency is fixed by calling list() on whatever super() returns. That way
        # FilterMethod will always get a list and `value in EMPTY_VALUES` will work as intended.
        return list(super().clean(value))

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

No branches or pull requests

1 participant