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

Rework validation, add queryset filter method #788

Merged
merged 11 commits into from
Oct 24, 2017
1 change: 0 additions & 1 deletion django_filters/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# flake8: noqa
import pkgutil

from .constants import STRICTNESS
from .filterset import FilterSet
from .filters import *

Expand Down
3 changes: 0 additions & 3 deletions django_filters/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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'),
Expand Down
19 changes: 0 additions & 19 deletions django_filters/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
84 changes: 45 additions & 39 deletions django_filters/filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions django_filters/rest_framework/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I noticed that even if I do not want to raise exception here, then it is not raised, but queryset returned without filtering. I guess in case if invalid filterset and do not raise exception it should return qs.none()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It keeps the partial queryset, yet - I think this is tested for in https://github.com/carltongibson/django-filter/pull/788/files#diff-b18add7202661e9eacece0aded8e619bR301.

return queryset

def to_html(self, request, queryset, view):
Expand Down
12 changes: 1 addition & 11 deletions django_filters/rest_framework/filterset.py
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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))
31 changes: 13 additions & 18 deletions django_filters/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import warnings
from collections import OrderedDict

import django
from django.conf import settings
Expand All @@ -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
Expand Down Expand Up @@ -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)
11 changes: 10 additions & 1 deletion django_filters/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class FilterMixin(object):
"""
filterset_class = None
filter_fields = ALL_FIELDS
strict = True

def get_filterset_class(self):
"""
Expand Down Expand Up @@ -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)
Expand Down