Rework validation, add queryset filter method (#788)

* Extract validation and filtering from caching

* Remove STRICTNESS, handle strictness in views

- Remove 'FILTERS_STRICTNESS' setting.
- Remove 'strict' option from FilterSet.
- Add 'raise_exception' class attribute to DjangoFilterBackend.
- Add 'strict' and 'get_strict' to FilterMixin.

* Add 'is_valid()' and 'errors' to FilterSet API

* Add FilterSet.get_form_class()

* Remove strictness from tests

* Drop FilterSet strict/together docs

* Add note on overriding FilterSet classmethods

* Add FilterSet API tests

* Add tests for view strictness

* Rework 'raw_validation' util

* Update filter backend+tests

django_filters/__init__.py

@@ -1,7 +1,6 @@
 # flake8: noqa
 import pkgutil
 
-from .constants import STRICTNESS
 from .filterset import FilterSet
 from .filters import *
 

django_filters/conf.py

@@ -2,7 +2,6 @@
 from django.core.signals import setting_changed
 from django.utils.translation import ugettext_lazy as _
 
-from .constants import STRICTNESS
 from .utils import deprecate
 
 DEFAULTS = {
@@ -13,8 +12,6 @@
     'NULL_CHOICE_LABEL': None,
     'NULL_CHOICE_VALUE': 'null',
 
-    'STRICTNESS': STRICTNESS.RETURN_NO_RESULTS,
-
     'VERBOSE_LOOKUPS': {
         # transforms don't need to be verbose, since their expressions are chained
         'date': _('date'),

django_filters/constants.py

@@ -3,22 +3,3 @@
 
 
 EMPTY_VALUES = ([], (), {}, '', None)
-
-
-class STRICTNESS(object):
-    class IGNORE(object):
-        pass
-
-    class RETURN_NO_RESULTS(object):
-        pass
-
-    class RAISE_VALIDATION_ERROR(object):
-        pass
-
-    # Values of False & True chosen for backward compatability reasons.
-    # Originally, these were the only options.
-    _LEGACY = {
-        False: IGNORE,
-        True: RETURN_NO_RESULTS,
-        "RAISE": RAISE_VALIDATION_ERROR,
-    }

django_filters/filterset.py

@@ -7,7 +7,7 @@
 from django.db.models.fields.related import ForeignObjectRel
 
 from .conf import settings
-from .constants import ALL_FIELDS, STRICTNESS
+from .constants import ALL_FIELDS
 from .filters import (
     BaseInFilter,
     BaseRangeFilter,
@@ -47,8 +47,6 @@ def __init__(self, options=None):
 
         self.filter_overrides = getattr(options, 'filter_overrides', {})
 
-        self.strict = getattr(options, 'strict', None)
-
         self.form = getattr(options, 'form', forms.Form)
 
 
@@ -140,7 +138,7 @@ def get_declared_filters(cls, bases, attrs):
 class BaseFilterSet(object):
     FILTER_DEFAULTS = FILTER_FOR_DBFIELD_DEFAULTS
 
-    def __init__(self, data=None, queryset=None, *, request=None, prefix=None, strict=None):
+    def __init__(self, data=None, queryset=None, *, request=None, prefix=None):
         if queryset is None:
             queryset = self._meta.model._default_manager.all()
         model = queryset.model
@@ -151,59 +149,67 @@ def __init__(self, data=None, queryset=None, *, request=None, prefix=None, stric
         self.request = request
         self.form_prefix = prefix
 
-        # What to do on on validation errors
-        # Fallback to meta, then settings strictness
-        if strict is None:
-            strict = self._meta.strict
-        if strict is None:
-            strict = settings.STRICTNESS
-
-        # transform legacy values
-        self.strict = STRICTNESS._LEGACY.get(strict, strict)
-
         self.filters = copy.deepcopy(self.base_filters)
 
         # propagate the model and filterset to the filters
         for filter_ in self.filters.values():
             filter_.model = model
             filter_.parent = self
 
+    def is_valid(self):
+        """
+        Return True if the underlying form has no errors, or False otherwise.
+        """
+        return self.is_bound and self.form.is_valid()
+
+    @property
+    def errors(self):
+        """
+        Return an ErrorDict for the data provided for the underlying form.
+        """
+        return self.form.errors
+
+    def filter_queryset(self, queryset):
+        """
+        Filter the queryset with the underlying form's `cleaned_data`. You must
+        call `is_valid()` or `errors` before calling this method.
+
+        This method should be overridden if additional filtering needs to be
+        applied to the queryset before it is cached.
+        """
+        for name, value in self.form.cleaned_data.items():
+            queryset = self.filters[name].filter(queryset, value)
+        return queryset
+
     @property
     def qs(self):
         if not hasattr(self, '_qs'):
-            if not self.is_bound:
-                self._qs = self.queryset.all()
-                return self._qs
-
-            if not self.form.is_valid():
-                if self.strict == STRICTNESS.RAISE_VALIDATION_ERROR:
-                    raise forms.ValidationError(self.form.errors)
-                elif self.strict == STRICTNESS.RETURN_NO_RESULTS:
-                    self._qs = self.queryset.none()
-                    return self._qs
-                # else STRICTNESS.IGNORE...  ignoring
-
-            # start with all the results and filter from there
             qs = self.queryset.all()
-            for name, filter_ in self.filters.items():
-                value = self.form.cleaned_data.get(name)
+            if self.is_bound:
+                # ensure form validation before filtering
+                self.errors
+                qs = self.filter_queryset(qs)
+            self._qs = qs
+        return self._qs
 
-                if value is not None:  # valid & clean data
-                    qs = filter_.filter(qs, value)
+    def get_form_class(self):
+        """
+        Returns a django Form suitable of validating the filterset data.
 
-            self._qs = qs
+        This method should be overridden if the form class needs to be
+        customized relative to the filterset instance.
+        """
+        fields = OrderedDict([
+            (name, filter_.field)
+            for name, filter_ in self.filters.items()])
 
-        return self._qs
+        return type(str('%sForm' % self.__class__.__name__),
+                    (self._meta.form,), fields)
 
     @property
     def form(self):
         if not hasattr(self, '_form'):
-            fields = OrderedDict([
-                (name, filter_.field)
-                for name, filter_ in self.filters.items()])
-
-            Form = type(str('%sForm' % self.__class__.__name__),
-                        (self._meta.form,), fields)
+            Form = self.get_form_class()
             if self.is_bound:
                 self._form = Form(self.data, prefix=self.form_prefix)
             else:

django_filters/rest_framework/backends.py

@@ -3,11 +3,12 @@
 from django.template import loader
 
 from . import filters, filterset
-from .. import compat
+from .. import compat, utils
 
 
 class DjangoFilterBackend(object):
     default_filter_set = filterset.FilterSet
+    raise_exception = True
 
     @property
     def template(self):
@@ -49,8 +50,10 @@ def filter_queryset(self, request, queryset, view):
         filter_class = self.get_filter_class(view, queryset)
 
         if filter_class:
-            return filter_class(request.query_params, queryset=queryset, request=request).qs
-
+            filterset = filter_class(request.query_params, queryset=queryset, request=request)
+            if not filterset.is_valid() and self.raise_exception:
+                raise utils.translate_validation(filterset.errors)
+            return filterset.qs
         return queryset
 
     def to_html(self, request, queryset, view):

django_filters/rest_framework/filterset.py

@@ -1,12 +1,11 @@
 from copy import deepcopy
 
-from django import forms
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 
 from django_filters import filterset
 
-from .. import compat, utils
+from .. import compat
 from .filters import BooleanFilter, IsoDateTimeFilter
 
 FILTER_FOR_DBFIELD_DEFAULTS = deepcopy(filterset.FILTER_FOR_DBFIELD_DEFAULTS)
@@ -38,12 +37,3 @@ def form(self):
             form.helper = helper
 
         return form
-
-    @property
-    def qs(self):
-        from rest_framework.exceptions import ValidationError
-
-        try:
-            return super(FilterSet, self).qs
-        except forms.ValidationError as e:
-            raise ValidationError(utils.raw_validation(e))

django_filters/utils.py

@@ -1,4 +1,5 @@
 import warnings
+from collections import OrderedDict
 
 import django
 from django.conf import settings
@@ -8,7 +9,6 @@
 from django.db.models.expressions import Expression
 from django.db.models.fields import FieldDoesNotExist
 from django.db.models.fields.related import ForeignObjectRel, RelatedField
-from django.forms import ValidationError
 from django.utils import timezone
 from django.utils.encoding import force_text
 from django.utils.text import capfirst
@@ -233,22 +233,17 @@ def label_for_filter(model, field_name, lookup_expr, exclude=False):
     return verbose_expression
 
 
-def raw_validation(error):
+def translate_validation(error_dict):
     """
-    Deconstruct a django.forms.ValidationError into a primitive structure
-    eg, plain dicts and lists.
+    Translate a Django ErrorDict into its DRF ValidationError.
     """
-    if isinstance(error, ValidationError):
-        if hasattr(error, 'error_dict'):
-            error = error.error_dict
-        elif not hasattr(error, 'message'):
-            error = error.error_list
-        else:
-            error = error.message
-
-    if isinstance(error, dict):
-        return {key: raw_validation(value) for key, value in error.items()}
-    elif isinstance(error, list):
-        return [raw_validation(value) for value in error]
-    else:
-        return error
+    # it's necessary to lazily import the exception, as it can otherwise create
+    # an import loop when importing django_filters inside the project settings.
+    from rest_framework.exceptions import ValidationError, ErrorDetail
+
+    exc = OrderedDict(
+        (key, [ErrorDetail(e.message, code=e.code) for e in error_list])
+        for key, error_list in error_dict.as_data().items()
+    )
+
+    return ValidationError(exc)

django_filters/views.py

@@ -15,6 +15,7 @@ class FilterMixin(object):
     """
     filterset_class = None
     filter_fields = ALL_FIELDS
+    strict = True
 
     def get_filterset_class(self):
         """
@@ -58,13 +59,21 @@ def get_filterset_kwargs(self, filterset_class):
                 raise ImproperlyConfigured(msg % args)
         return kwargs
 
+    def get_strict(self):
+        return self.strict
+
 
 class BaseFilterView(FilterMixin, MultipleObjectMixin, View):
 
     def get(self, request, *args, **kwargs):
         filterset_class = self.get_filterset_class()
         self.filterset = self.get_filterset(filterset_class)
-        self.object_list = self.filterset.qs
+
+        if self.filterset.is_valid() or not self.get_strict():
+            self.object_list = self.filterset.qs
+        else:
+            self.object_list = self.filterset.queryset.none()
+
         context = self.get_context_data(filter=self.filterset,
                                         object_list=self.object_list)
         return self.render_to_response(context)