From e5a56b7f7bb47fe268402605d9f9d1cd128ccae5 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Tue, 24 Oct 2017 04:59:43 -0400 Subject: [PATCH] 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 - django_filters/conf.py | 3 - django_filters/constants.py | 19 ---- django_filters/filterset.py | 84 ++++++++------- django_filters/rest_framework/backends.py | 9 +- django_filters/rest_framework/filterset.py | 12 +-- django_filters/utils.py | 31 +++--- django_filters/views.py | 11 +- docs/ref/filterset.txt | 65 +++--------- tests/rest_framework/test_backends.py | 26 +++++ tests/rest_framework/test_integration.py | 25 ++++- tests/settings.py | 4 - tests/test_conf.py | 44 -------- tests/test_filtering.py | 27 ++--- tests/test_filterset.py | 114 ++++++++++----------- tests/test_forms.py | 32 ++++++ tests/test_utils.py | 38 ++++--- tests/test_views.py | 20 ++++ 18 files changed, 275 insertions(+), 290 deletions(-) diff --git a/django_filters/__init__.py b/django_filters/__init__.py index d552d509a..35d11398d 100644 --- a/django_filters/__init__.py +++ b/django_filters/__init__.py @@ -1,7 +1,6 @@ # flake8: noqa import pkgutil -from .constants import STRICTNESS from .filterset import FilterSet from .filters import * diff --git a/django_filters/conf.py b/django_filters/conf.py index 985db486f..81d710785 100644 --- a/django_filters/conf.py +++ b/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'), diff --git a/django_filters/constants.py b/django_filters/constants.py index 2f5fb3dbd..795d6ccca 100644 --- a/django_filters/constants.py +++ b/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, - } diff --git a/django_filters/filterset.py b/django_filters/filterset.py index ba53310e5..597f87bc9 100644 --- a/django_filters/filterset.py +++ b/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,16 +149,6 @@ 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 @@ -168,42 +156,60 @@ def __init__(self, data=None, queryset=None, *, request=None, prefix=None, stric 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: diff --git a/django_filters/rest_framework/backends.py b/django_filters/rest_framework/backends.py index a08b440d2..aecc34cb1 100644 --- a/django_filters/rest_framework/backends.py +++ b/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): diff --git a/django_filters/rest_framework/filterset.py b/django_filters/rest_framework/filterset.py index 48d2d66c3..e90f5d247 100644 --- a/django_filters/rest_framework/filterset.py +++ b/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)) diff --git a/django_filters/utils.py b/django_filters/utils.py index f94c3aa50..fe34fc3d6 100644 --- a/django_filters/utils.py +++ b/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) diff --git a/django_filters/views.py b/django_filters/views.py index 2303ed9df..916f4f433 100644 --- a/django_filters/views.py +++ b/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) diff --git a/docs/ref/filterset.txt b/docs/ref/filterset.txt index 329816546..dcdf668ad 100644 --- a/docs/ref/filterset.txt +++ b/docs/ref/filterset.txt @@ -11,9 +11,7 @@ Meta options - :ref:`fields ` - :ref:`exclude ` - :ref:`form
` -- :ref:`together ` - :ref:`filter_overrides ` -- :ref:`strict ` .. _model: @@ -101,26 +99,6 @@ form class from which ``FilterSet.form`` will subclass. This works similar to the ``form`` option on a ``ModelAdmin.`` -.. _together: - -Group fields with ``together`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The inner ``Meta`` class also takes an optional ``together`` argument. This -is a list of lists, each containing field names. For convenience can be a -single list/tuple when dealing with a single set of fields. Fields within a -field set must either be all or none present in the request for -``FilterSet.form`` to be valid:: - - import django_filters - - class ProductFilter(django_filters.FilterSet): - class Meta: - model = Product - fields = ['price', 'release_date', 'rating'] - together = ['rating', 'price'] - - .. _filter_overrides: Customise filter generation with ``filter_overrides`` @@ -149,39 +127,28 @@ This is a map of model fields to filter classes with options:: }, } +Overriding ``FilterSet`` methods +-------------------------------- -.. _strict: +When overriding classmethods, calling ``super(MyFilterSet, cls)`` may result +in a ``NameError`` exception. This is due to the ``FilterSetMetaclass`` calling +these classmethods before the ``FilterSet`` class has been fully created. +There are two recommmended workarounds: -Handling validation errors with ``strict`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +1. If using python 3.6 or newer, use the argumentless ``super()`` syntax. +2. For older versions of python, use an intermediate class. Ex:: -The ``strict`` option determines the filterset's behavior when filters fail to validate. Example use: + class Intermediate(django_filters.FilterSet): -.. code-block:: python - - from django_filters import FilterSet, STRICTNESS + @classmethod + def method(cls, arg): + super(Intermediate, cls).method(arg) + ... - class ProductFilter(FilterSet): + class ProductFilter(Intermediate): class Meta: model = Product - fields = ['name', 'release_date'] - strict = STRICTNESS.RETURN_NO_RESULTS - -Currently, there are three different behaviors: - -- ``STRICTNESS.RETURN_NO_RESULTS`` (default) This returns an empty queryset. The - filterset form can then be rendered to display the input errors. -- ``STRICTNESS.IGNORE`` Instead of returning an empty queryset, invalid filters - effectively become a noop. Valid filters are applied to the queryset however. -- ``STRICTNESS.RAISE_VALIDATION_ERROR`` This raises a ``ValidationError`` for - all invalid filters. This behavior is generally useful with APIs. - -If the ``strict`` option is not provided, then the filterset will default to the -value of the ``FILTERS_STRICTNESS`` setting. - - -Overriding ``FilterSet`` methods --------------------------------- + fields = ['...'] ``filter_for_lookup()`` ~~~~~~~~~~~~~~~~~~~~~~~ @@ -212,4 +179,4 @@ filters for a model field, you can override ``filter_for_lookup()``. Ex:: return django_filters.DateRangeFilter, {} # use default behavior otherwise - return super(ProductFilter, cls).filter_for_lookup(f, lookup_type) + return super().filter_for_lookup(f, lookup_type) diff --git a/tests/rest_framework/test_backends.py b/tests/rest_framework/test_backends.py index 339c62457..dcf713dfd 100644 --- a/tests/rest_framework/test_backends.py +++ b/tests/rest_framework/test_backends.py @@ -14,6 +14,7 @@ backends ) +from ..models import Article from .models import FilterableItem factory = APIRequestFactory() @@ -303,3 +304,28 @@ class Backend(DjangoFilterBackend): # derived filter_class.Meta should inherit from default_filter_set.Meta self.assertIn(BooleanField, filter_overrides) self.assertDictEqual(filter_overrides[BooleanField], {}) + + +class ValidationErrorTests(TestCase): + + def test_errors(self): + class F(FilterSet): + class Meta: + model = Article + fields = ['id', 'author', 'name'] + + view = FilterFieldsRootView() + backend = DjangoFilterBackend() + request = factory.get('/?id=foo&author=bar&name=baz') + request = view.initialize_request(request) + queryset = Article.objects.all() + view.filter_class = F + + with self.assertRaises(serializers.ValidationError) as exc: + backend.filter_queryset(request, queryset, view) + + # test output, does not include error code + self.assertDictEqual(exc.exception.detail, { + 'id': ['Enter a number.'], + 'author': ['Select a valid choice. That choice is not one of the available choices.'], + }) diff --git a/tests/rest_framework/test_integration.py b/tests/rest_framework/test_integration.py index c5bb2998d..fba96497d 100644 --- a/tests/rest_framework/test_integration.py +++ b/tests/rest_framework/test_integration.py @@ -9,7 +9,7 @@ from rest_framework import generics, serializers, status from rest_framework.test import APIRequestFactory -from django_filters import STRICTNESS, filters +from django_filters import filters from django_filters.rest_framework import DjangoFilterBackend, FilterSet from .models import ( @@ -287,8 +287,7 @@ def test_html_rendering(self): response = view(request).render() self.assertEqual(response.status_code, status.HTTP_200_OK) - @override_settings(FILTERS_STRICTNESS=STRICTNESS.RAISE_VALIDATION_ERROR) - def test_strictness_validation_error(self): + def test_raise_validation_error(self): """ Ensure validation errors return a proper error response instead of an internal server error. @@ -299,6 +298,26 @@ def test_strictness_validation_error(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data, {'decimal': ['Enter a number.']}) + def test_permissive(self): + """ + Permissive handling should return a partially filtered result set. + """ + FilterableItem.objects.create(decimal=Decimal('1.23'), date='2017-01-01') + FilterableItem.objects.create(decimal=Decimal('1.23'), date='2016-01-01') + + class Backend(DjangoFilterBackend): + raise_exception = False + + class View(FilterFieldsRootView): + filter_backends = (Backend,) + + view = View.as_view() + request = factory.get('/?decimal=foobar&date=2017-01-01') + response = view(request).render() + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data[0]['date'], '2017-01-01') + self.assertEqual(len(response.data), 1) + @override_settings(ROOT_URLCONF='tests.rest_framework.test_integration') class IntegrationTestDetailFiltering(CommonFilteringTestCase): diff --git a/tests/settings.py b/tests/settings.py index 6eaa58e3f..3892cbf8c 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,6 +1,5 @@ # ensure package/conf is importable -from django_filters import STRICTNESS from django_filters.conf import DEFAULTS DATABASES = { @@ -40,6 +39,3 @@ # help verify that DEFAULTS is importable from conf. def FILTERS_VERBOSE_LOOKUPS(): return DEFAULTS['VERBOSE_LOOKUPS'] - - -FILTERS_STRICTNESS = STRICTNESS.RETURN_NO_RESULTS diff --git a/tests/test_conf.py b/tests/test_conf.py index c41a28512..45a24e826 100644 --- a/tests/test_conf.py +++ b/tests/test_conf.py @@ -1,9 +1,7 @@ from django.test import TestCase, override_settings -from django_filters import STRICTNESS, FilterSet from django_filters.conf import is_callable, settings -from tests.models import User class DefaultSettingsTests(TestCase): @@ -15,9 +13,6 @@ def test_verbose_lookups(self): def test_disable_help_text(self): self.assertFalse(settings.DISABLE_HELP_TEXT) - def test_strictness(self): - self.assertEqual(settings.STRICTNESS, STRICTNESS.RETURN_NO_RESULTS) - def test_empty_choice_label(self): self.assertEqual(settings.EMPTY_CHOICE_LABEL, '---------') @@ -28,45 +23,6 @@ def test_null_choice_value(self): self.assertEqual(settings.NULL_CHOICE_VALUE, 'null') -class StrictnessTests(TestCase): - class F(FilterSet): - class Meta: - model = User - fields = [] - - def test_settings_default(self): - self.assertEqual(self.F().strict, STRICTNESS.RETURN_NO_RESULTS) - - def test_ignore(self): - with override_settings(FILTERS_STRICTNESS=STRICTNESS.IGNORE): - self.assertEqual(self.F().strict, STRICTNESS.IGNORE) - - def test_return_no_results(self): - with override_settings(FILTERS_STRICTNESS=STRICTNESS.RETURN_NO_RESULTS): - self.assertEqual(self.F().strict, STRICTNESS.RETURN_NO_RESULTS) - - def test_raise_validation_error(self): - with override_settings(FILTERS_STRICTNESS=STRICTNESS.RAISE_VALIDATION_ERROR): - self.assertEqual(self.F().strict, STRICTNESS.RAISE_VALIDATION_ERROR) - - def test_legacy_ignore(self): - with override_settings(FILTERS_STRICTNESS=False): - self.assertEqual(self.F().strict, STRICTNESS.IGNORE) - - def test_legacy_return_no_results(self): - with override_settings(FILTERS_STRICTNESS=True): - self.assertEqual(self.F().strict, STRICTNESS.RETURN_NO_RESULTS) - - def test_legacy_raise_validation_error(self): - with override_settings(FILTERS_STRICTNESS='RAISE'): - self.assertEqual(self.F().strict, STRICTNESS.RAISE_VALIDATION_ERROR) - - def test_legacy_differentiation(self): - self.assertNotEqual(STRICTNESS.IGNORE, False) - self.assertNotEqual(STRICTNESS.RETURN_NO_RESULTS, True) - self.assertNotEqual(STRICTNESS.RAISE_VALIDATION_ERROR, 'RAISE') - - class OverrideSettingsTests(TestCase): def test_attribute_override(self): diff --git a/tests/test_filtering.py b/tests/test_filtering.py index fa0066a71..bd6ede604 100644 --- a/tests/test_filtering.py +++ b/tests/test_filtering.py @@ -1086,25 +1086,9 @@ class Meta: self.assertEqual(list(F().qs), list(User.objects.all())) self.assertEqual(list(F({'username': 'alex'}).qs), [User.objects.get(username='alex')]) - self.assertEqual(list(F({'username': 'jose'}).qs), - list()) - - def test_filtering_without_strict(self): - User.objects.create(username='alex') - User.objects.create(username='jacob') - User.objects.create(username='aaron') - - class F(FilterSet): - username = AllValuesFilter() - - class Meta: - model = User - fields = ['username'] - strict = False - self.assertEqual(list(F().qs), list(User.objects.all())) - self.assertEqual(list(F({'username': 'alex'}).qs), - [User.objects.get(username='alex')]) + # invalid choice + self.assertFalse(F({'username': 'jose'}).is_valid()) self.assertEqual(list(F({'username': 'jose'}).qs), list(User.objects.all())) @@ -1128,8 +1112,11 @@ class Meta: [User.objects.get(username='alex')]) self.assertEqual(list(F({'username': ['alex', 'jacob']}).qs), list(User.objects.filter(username__in=['alex', 'jacob']))) - self.assertEqual(list(F({'username': ['jose']}).qs), - list()) + + # invalid choice + self.assertFalse(F({'username': 'jose'}).is_valid()) + self.assertEqual(list(F({'username': 'jose'}).qs), + list(User.objects.all())) class FilterMethodTests(TestCase): diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 434933ce2..3b431b025 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -2,9 +2,8 @@ import unittest from django.db import models -from django.test import TestCase, override_settings +from django.test import TestCase -from django_filters.constants import STRICTNESS from django_filters.exceptions import FieldLookupError from django_filters.filters import ( BaseInFilter, @@ -601,16 +600,16 @@ class Grandchild(Child): class FilterSetInstantiationTests(TestCase): - def test_creating_instance(self): - class F(FilterSet): - class Meta: - model = User - fields = ['username'] + class F(FilterSet): + class Meta: + model = User + fields = ['username'] - f = F() + def test_creating_instance(self): + f = self.F() self.assertFalse(f.is_bound) self.assertIsNotNone(f.queryset) - self.assertEqual(len(f.filters), len(F.base_filters)) + self.assertEqual(len(f.filters), len(self.F.base_filters)) for name, filter_ in f.filters.items(): self.assertEqual( filter_.model, @@ -618,76 +617,69 @@ class Meta: "%s does not have model set correctly" % name) def test_creating_bound_instance(self): - class F(FilterSet): - class Meta: - model = User - fields = ['username'] - - f = F({'username': 'username'}) + f = self.F({'username': 'username'}) self.assertTrue(f.is_bound) def test_creating_with_queryset(self): - class F(FilterSet): - class Meta: - model = User - fields = ['username'] - m = mock.Mock() - f = F(queryset=m) + f = self.F(queryset=m) self.assertEqual(f.queryset, m) def test_creating_with_request(self): - class F(FilterSet): - class Meta: - model = User - fields = ['username'] - m = mock.Mock() - f = F(request=m) + f = self.F(request=m) self.assertEqual(f.request, m) -class FilterSetStrictnessTests(TestCase): - - def test_settings_default(self): - class F(FilterSet): - class Meta: - model = User - fields = [] - - # Ensure default is not IGNORE - self.assertEqual(F().strict, STRICTNESS.RETURN_NO_RESULTS) +class FilterSetQuerysetTests(TestCase): - # override and test - with override_settings(FILTERS_STRICTNESS=STRICTNESS.IGNORE): - self.assertEqual(F().strict, STRICTNESS.IGNORE) + class F(FilterSet): + class Meta: + model = User + fields = ['username'] - def test_meta_value(self): - class F(FilterSet): - class Meta: - model = User - fields = [] - strict = STRICTNESS.IGNORE + def test_filter_queryset_called_once(self): + m = mock.Mock() + f = self.F({'username': 'bob'}, queryset=m) + + with mock.patch.object(f, 'filter_queryset', + wraps=f.filter_queryset) as fn: + f.qs + fn.assert_called_once_with(m.all()) + f.qs + fn.assert_called_once_with(m.all()) + + def test_get_form_class_called_once(self): + f = self.F() + + with mock.patch.object(f, 'get_form_class', + wraps=f.get_form_class) as fn: + f.form + fn.assert_called_once() + f.form + fn.assert_called_once() + + def test_qs_caching(self): + m = mock.Mock() + f = self.F(queryset=m) - self.assertEqual(F().strict, STRICTNESS.IGNORE) + self.assertIs(f.qs, m.all()) + self.assertIs(f.qs, f.qs) - def test_init_default(self): - class F(FilterSet): - class Meta: - model = User - fields = [] - strict = STRICTNESS.IGNORE + def test_form_caching(self): + f = self.F() - strict = STRICTNESS.RAISE_VALIDATION_ERROR - self.assertEqual(F(strict=strict).strict, strict) + self.assertIs(f.form, f.form) - def test_legacy_value(self): - class F(FilterSet): - class Meta: - model = User - fields = [] + def test_qs_triggers_form_validation(self): + m = mock.Mock() + f = self.F({'username': 'bob'}, queryset=m) - self.assertEqual(F(strict=False).strict, STRICTNESS.IGNORE) + with mock.patch.object(f.form, 'full_clean', + wraps=f.form.full_clean) as fn: + fn.assert_not_called() + f.qs + fn.assert_called() # test filter.method here, as it depends on its parent FilterSet diff --git a/tests/test_forms.py b/tests/test_forms.py index 8b1dfbe22..13ca50daa 100644 --- a/tests/test_forms.py +++ b/tests/test_forms.py @@ -223,3 +223,35 @@ class Meta: F().form.fields['title__in'].help_text, '' ) + + +class FilterSetValidityTests(TestCase): + + class F(FilterSet): + class Meta: + model = Book + fields = ['title', 'price'] + + def test_not_bound(self): + f = self.F() + + self.assertFalse(f.is_bound) + self.assertFalse(f.is_valid()) + self.assertEqual(f.data, {}) + self.assertEqual(f.errors, {}) + + def test_is_bound_and_valid(self): + f = self.F({'title': 'Some book'}) + + self.assertTrue(f.is_bound) + self.assertTrue(f.is_valid()) + self.assertEqual(f.data, {'title': 'Some book'}) + self.assertEqual(f.errors, {}) + + def test_is_bound_and_not_valid(self): + f = self.F({'price': 'four dollars'}) + + self.assertTrue(f.is_bound) + self.assertFalse(f.is_valid()) + self.assertEqual(f.data, {'price': 'four dollars'}) + self.assertEqual(f.errors, {'price': ['Enter a number.']}) diff --git a/tests/test_utils.py b/tests/test_utils.py index dd7c6438d..d31f5b7b2 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -3,20 +3,19 @@ from django.db import models from django.db.models.constants import LOOKUP_SEP from django.db.models.fields.related import ForeignObjectRel -from django.forms import ValidationError from django.test import TestCase, override_settings from django.utils.functional import Promise from django.utils.timezone import get_default_timezone -from django_filters import STRICTNESS, FilterSet +from django_filters import FilterSet from django_filters.exceptions import FieldLookupError from django_filters.utils import ( get_field_parts, get_model_field, handle_timezone, label_for_filter, - raw_validation, resolve_field, + translate_validation, verbose_field_name, verbose_lookup_expr ) @@ -342,19 +341,30 @@ def test_handle_dst_starting(self): self.assertEqual(handled, get_default_timezone().localize(dst_starting_date, True)) -class RawValidationDataTests(TestCase): - def test_simple(self): - class F(FilterSet): - class Meta: - model = Article - fields = ['id', 'author', 'name'] - strict = STRICTNESS.RAISE_VALIDATION_ERROR +class TranslateValidationDataTests(TestCase): - f = F(data={'id': 'foo', 'author': 'bar', 'name': 'baz'}) - with self.assertRaises(ValidationError) as exc: - f.qs + class F(FilterSet): + class Meta: + model = Article + fields = ['id', 'author', 'name'] - self.assertDictEqual(raw_validation(exc.exception), { + def test_error_detail(self): + f = self.F(data={'id': 'foo', 'author': 'bar', 'name': 'baz'}) + exc = translate_validation(f.errors) + + self.assertDictEqual(exc.detail, { 'id': ['Enter a number.'], 'author': ['Select a valid choice. That choice is not one of the available choices.'], }) + + def test_full_error_details(self): + f = self.F(data={'id': 'foo', 'author': 'bar', 'name': 'baz'}) + exc = translate_validation(f.errors) + + self.assertEqual(exc.get_full_details(), { + 'id': [{'message': 'Enter a number.', 'code': 'invalid'}], + 'author': [{ + 'message': 'Select a valid choice. That choice is not one of the available choices.', + 'code': 'invalid_choice', + }], + }) diff --git a/tests/test_views.py b/tests/test_views.py index 9204e9a33..dc7282b48 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -71,6 +71,26 @@ def test_view_with_model_and_fields_no_filterset(self): for b in ['Ender's Game', 'Rainbow Six', 'Snowcrash']: self.assertContains(response, b) + def test_view_with_strict_errors(self): + factory = RequestFactory() + request = factory.get(self.base_url + '?title=Snowcrash&price=four dollars') + view = FilterView.as_view(model=Book) + response = view(request) + titles = [o.title for o in response.context_data['object_list']] + + self.assertEqual(response.status_code, 200) + self.assertEqual(titles, []) + + def test_view_with_non_strict_errors(self): + factory = RequestFactory() + request = factory.get(self.base_url + '?title=Snowcrash&price=four dollars') + view = FilterView.as_view(model=Book, strict=False) + response = view(request) + titles = [o.title for o in response.context_data['object_list']] + + self.assertEqual(response.status_code, 200) + self.assertEqual(titles, ['Snowcrash'],) + def test_view_without_filterset_or_model(self): factory = RequestFactory() request = factory.get(self.base_url)