entrouvert
/
debian-django-filter
17
0
Fork 0
Code Issues Pull Requests Releases Activity

Merge pull request #451 from rpkilby/warn-unrecognized-field 

Resolve 450, add error message for unrecognized field types
This commit is contained in:
Carlton Gibson 2016-08-14 20:44:23 +02:00 committed by GitHub
parent 2f7fec71bf 1aac7c96d8
commit 2715d6f315
9 changed files with 268 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
django_filters/filterset.py
View File

@ -3,7 +3,6 @@ from __future__ import unicode_literals
import copy
import re
import warnings
from collections import OrderedDict
from django import forms
@ -21,18 +20,12 @@ from .filters import (Filter, CharFilter, BooleanFilter, BaseInFilter, BaseRange
ChoiceFilter, DateFilter, DateTimeFilter, TimeFilter, ModelChoiceFilter,
ModelMultipleChoiceFilter, NumberFilter, UUIDFilter,
DurationFilter)
from .utils import try_dbfield, get_model_field, resolve_field
from .utils import try_dbfield, get_all_model_fields, get_model_field, resolve_field, deprecate
ORDER_BY_FIELD = 'o'
def deprecate(msg):
warnings.warn(
"%s See: https://django-filter.readthedocs.io/en/latest/migration.html" % msg,
DeprecationWarning, stacklevel=3)
class STRICTNESS(object):
"""
Values of False & True chosen for backward compatability reasons.
@ -68,13 +61,15 @@ def get_declared_filters(bases, attrs, with_base_filters=True):
def filters_for_model(model, fields=None, exclude=None, filter_for_field=None,
filter_for_reverse_field=None):
field_dict = OrderedDict()
opts = model._meta
if fields is None:
fields = [
f.name for f in sorted(opts.fields + opts.many_to_many)
if not isinstance(f, models.AutoField) and
not (getattr(remote_field(f), 'parent_link', False))
]
# Setting exclude with no fields implies all other fields.
if exclude is not None and fields is None:
fields = '__all__'
# All implies all db fields associated with a filter_class.
if fields == '__all__':
fields = get_all_model_fields(model)
# Loop through the list of fields.
for f in fields:
# Skip the field if excluded.
@ -139,6 +134,19 @@ def get_full_clean_override(together):
class FilterSetOptions(object):
def __init__(self, options=None):
if getattr(options, 'model', None) is not None:
if not hasattr(options, 'fields') and not hasattr(options, 'exclude'):
deprecate(
"Not setting Meta.fields with Meta.model is undocumented behavior "
"and may result in unintentionally exposing filter fields. This has "
"been deprecated in favor of setting Meta.fields = '__all__' or by "
"setting the Meta.exclude attribute.", 1)
elif getattr(options, 'fields', -1) is None:
deprecate(
"Setting 'Meta.fields = None' is undocumented behavior and has been "
"deprecated in favor of Meta.fields = '__all__'.", 1)
self.model = getattr(options, 'model', None)
self.fields = getattr(options, 'fields', None)
self.exclude = getattr(options, 'exclude', None)
@ -166,6 +174,9 @@ class FilterSetMetaclass(type):
opts = new_class._meta = FilterSetOptions(
getattr(new_class, 'Meta', None))
# TODO: replace with deprecations
# if opts.model and opts.fields:
if opts.model:
filters = new_class.filters_for_model(opts.model, opts)
filters.update(declared_filters)
@ -432,8 +443,15 @@ class BaseFilterSet(object):
@classmethod
def filters_for_model(cls, model, opts):
# TODO: remove with deprecations - this emulates the old behavior
fields = opts.fields
if fields is None:
DEFAULTS = dict(FILTER_FOR_DBFIELD_DEFAULTS)
DEFAULTS.update(cls.filter_overrides)
fields = get_all_model_fields(model, field_types=DEFAULTS.keys())
return filters_for_model(
model, opts.fields, opts.exclude,
model, fields, opts.exclude,
cls.filter_for_field,
cls.filter_for_reverse_field
)
@ -451,8 +469,13 @@ class BaseFilterSet(object):
filter_class, params = cls.filter_for_lookup(f, lookup_type)
default.update(params)
if filter_class is not None:
return filter_class(**default)
assert filter_class is not None, (
"%s resolved field '%s' with '%s' lookup to an unrecognized field "
"type %s. Try adding an override to 'filter_overrides'. See: "
"https://django-filter.readthedocs.io/en/latest/usage.html#overriding-default-filters"
) % (cls.__name__, name, lookup_expr, f.__class__.__name__)
return filter_class(**default)
@classmethod
def filter_for_reverse_field(cls, f, name):
@ -543,7 +566,7 @@ class FilterSet(six.with_metaclass(FilterSetMetaclass, BaseFilterSet)):
def filterset_factory(model):
meta = type(str('Meta'), (object,), {'model': model})
meta = type(str('Meta'), (object,), {'model': model, 'fields': '__all__'})
filterset = type(str('%sFilterSet' % model._meta.object_name),
(FilterSet,), {'Meta': meta})
return filterset

29
django_filters/utils.py
View File

@ -1,3 +1,5 @@
import warnings
from django.conf import settings
from django.core.exceptions import FieldError
from django.db import models
@ -7,10 +9,16 @@ from django.db.models.fields import FieldDoesNotExist
from django.db.models.fields.related import ForeignObjectRel
from django.utils import six, timezone
from .compat import remote_model
from .compat import remote_field, remote_model
from .exceptions import FieldLookupError
def deprecate(msg, level_modifier=0):
warnings.warn(
"%s See: https://django-filter.readthedocs.io/en/latest/migration.html" % msg,
DeprecationWarning, stacklevel=3 + level_modifier)
def try_dbfield(fn, field_class):
"""
Try ``fn`` with the DB ``field_class`` by walking its
@ -31,6 +39,25 @@ def try_dbfield(fn, field_class):
return data
# TODO: remove field_types arg with deprecations
def get_all_model_fields(model, field_types=None):
opts = model._meta
if field_types is not None:
return [
f.name for f in sorted(opts.fields + opts.many_to_many)
if not isinstance(f, models.AutoField) and
not (getattr(remote_field(f), 'parent_link', False)) and
f.__class__ in field_types
]
return [
f.name for f in sorted(opts.fields + opts.many_to_many)
if not isinstance(f, models.AutoField) and
not (getattr(remote_field(f), 'parent_link', False))
]
def get_model_field(model, field_name):
"""
Get a ``model`` field, traversing relationships

25
docs/migration.txt
View File

@ -27,3 +27,28 @@ no longer proxied from the queryset. To fix this, call the methods on the
# 1.0
for obj in f.qs:
...
Filters no longer autogenerated when Meta.fields is not specified
-----------------------------------------------------------------
Details: https://github.com/carltongibson/django-filter/pull/450
FilterSets had an undocumented behavior of autogenerating filters for all
model fields when either ``Meta.fields`` was not specified or when set to
``None``. This can lead to potentially unsafe data or schema exposure and
has been deprecated in favor of explicitly setting ``Meta.fields`` to the
``'__all__'`` special value. You may also blacklist fields by setting
the ``Meta.exclude`` attribute.
.. code-block:: python
class UserFilter(FilterSet):
class Meta:
model = User
fields = '__all__'
# or
class UserFilter(FilterSet):
class Meta:
model = User
exclude = ['password']

2
docs/ref/filters.txt
View File

@ -461,7 +461,7 @@ each of those is present as an option. This is similar to the default behavior
of the admin.
``AllValuesMultipleFilter``
~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is a ``MultipleChoiceFilter`` whose choices are the current values in the
database. So if in the DB for the given field you have values of 5, 7, and 9

81
docs/ref/filterset.txt
View File

@ -6,14 +6,89 @@ This document provides a guide on using additional FilterSet features.
Meta options
------------
- model
- fields
- exclude
- :ref:`model <model>`
- :ref:`fields <fields>`
- :ref:`exclude <exclude>`
- :ref:`order_by <order-by>`
- :ref:`form <form>`
- :ref:`together <together>`
.. _model:
Automatic filter generation with ``model``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The ``FilterSet`` is capable of automatically generating filters for a given
``model``'s fields. Similar to Django's ``ModelForm``, filters are created
based on the underlying model field's type. This option must be combined with
either the ``fields`` or ``exclude`` option, which is the same requirement for
Django's ``ModelForm`` class, detailed `here`__.
__ https://docs.djangoproject.com/en/dev/topics/forms/modelforms/#selecting-the-fields-to-use
.. code-block:: python
class UserFilter(django_filters.FilterSet):
class Meta:
model = User
fields = ['username', 'last_login']
.. _fields:
Declaring filterable ``fields``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The ``fields`` option is combined with ``model`` to automatically generate
filters. Note that generated filters will not overwrite filters declared on
the ``FilterSet``. The ``fields`` option accepts two syntaxes:
* a list of field names
* a dictionary of field names mapped to a list of lookups
.. code-block:: python
class UserFilter(django_filters.FilterSet):
class Meta:
model = User
fields = ['username', 'last_login']
# or
class UserFilter(django_filters.FilterSet):
class Meta:
model = User
fields = {
'username': ['exact', 'contains'],
'last_login': ['exact', 'year__gt'],
}
The list syntax will create an ``exact`` lookup filter for each field included
in ``fields``. The dictionary syntax will create a filter for each lookup
expression declared for its corresponding model field. These expressions may
include both transforms and lookups, as detailed in the `lookup reference`__.
__ https://docs.djangoproject.com/en/dev/ref/models/lookups/#module-django.db.models.lookups
.. _exclude:
Disable filter fields with ``exclude``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The ``exclude`` option accepts a blacklist of field names to exclude from
automatic filter generation. Note that this option will not disable filters
declared directly on the ``FilterSet``.
.. code-block:: python
class UserFilter(django_filters.FilterSet):
class Meta:
model = User
exclude = ['password']
.. _order-by:
Ordering using ``order_by``

59
tests/test_deprecations.py
View File

@ -3,12 +3,16 @@ import warnings
from django.test import TestCase
from django_filters import FilterSet
from django_filters.filters import CharFilter
from .models import User
from .models import NetworkSetting
from .models import SubnetMaskField
class UserFilter(FilterSet):
class Meta:
model = User
fields = '__all__'
class FilterSetContainerDeprecationTests(TestCase):
@ -47,3 +51,58 @@ class FilterSetContainerDeprecationTests(TestCase):
UserFilter().count()
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
class FilterSetMetaDeprecationTests(TestCase):
def test_fields_not_set(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
class F(FilterSet):
class Meta:
model = User
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("Not setting Meta.fields with Meta.model is undocumented behavior", str(w[-1].message))
def test_fields_is_none(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
class F(FilterSet):
class Meta:
model = User
fields = None
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("Setting 'Meta.fields = None' is undocumented behavior", str(w[-1].message))
def test_fields_not_set_ignore_unknown(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
class F(FilterSet):
class Meta:
model = NetworkSetting
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("Not setting Meta.fields with Meta.model is undocumented behavior", str(w[-1].message))
self.assertNotIn('mask', F.base_filters.keys())
def test_fields_not_set_with_override(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
class F(FilterSet):
filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter}
}
class Meta:
model = NetworkSetting
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("Not setting Meta.fields with Meta.model is undocumented behavior", str(w[-1].message))
self.assertEqual(list(F.base_filters.keys()), ['ip', 'mask'])

1
tests/test_filtering.py
View File

@ -390,6 +390,7 @@ class DurationFilterTests(TestCase):
class Meta:
model = SpacewalkRecord
fields = '__all__'
qs = SpacewalkRecord.objects.order_by('duration')

41
tests/test_filterset.py
View File

@ -153,6 +153,17 @@ class FilterSetFilterForFieldTests(TestCase):
result = FilterSet.filter_for_field(f, 'first_name')
self.assertIsInstance(result, CharFilter)
def test_unknown_field_type_error(self):
f = NetworkSetting._meta.get_field('mask')
with self.assertRaises(AssertionError) as excinfo:
FilterSet.filter_for_field(f, 'mask')
self.assertIn(
"FilterSet resolved field 'mask' with 'exact' lookup "
"to an unrecognized field type SubnetMaskField",
excinfo.exception.args[0])
def test_symmetrical_selfref_m2m_field(self):
f = Node._meta.get_field('adjacents')
result = FilterSet.filter_for_field(f, 'adjacents')
@ -301,6 +312,7 @@ class FilterSetClassCreationTests(TestCase):
class F(FilterSet):
class Meta:
model = Book
fields = '__all__'
self.assertEqual(len(F.declared_filters), 0)
self.assertEqual(len(F.base_filters), 3)
@ -313,6 +325,7 @@ class FilterSetClassCreationTests(TestCase):
class Meta:
model = Book
fields = '__all__'
self.assertEqual(len(F.declared_filters), 1)
self.assertEqual(len(F.base_filters), 4)
@ -421,10 +434,22 @@ class FilterSetClassCreationTests(TestCase):
self.assertListEqual(list(F.base_filters),
['username', 'price'])
def test_meta_exlude_with_no_fields(self):
class F(FilterSet):
class Meta:
model = Book
exclude = ('price', )
self.assertEqual(len(F.declared_filters), 0)
self.assertEqual(len(F.base_filters), 2)
self.assertListEqual(list(F.base_filters),
['title', 'average_rating'])
def test_filterset_class_inheritance(self):
class F(FilterSet):
class Meta:
model = Book
fields = '__all__'
class G(F):
pass
@ -435,6 +460,7 @@ class FilterSetClassCreationTests(TestCase):
class Meta:
model = Book
fields = '__all__'
class G(F):
pass
@ -444,6 +470,7 @@ class FilterSetClassCreationTests(TestCase):
class F(FilterSet):
class Meta:
model = Restaurant
fields = '__all__'
self.assertEqual(set(F.base_filters), set(['name', 'serves_pizza']))
@ -454,13 +481,6 @@ class FilterSetClassCreationTests(TestCase):
self.assertEqual(set(F.base_filters), set(['name', 'serves_pizza']))
def test_custom_field_ignored(self):
class F(FilterSet):
class Meta:
model = NetworkSetting
self.assertEqual(list(F.base_filters.keys()), ['ip'])
def test_custom_field_gets_filter_from_override(self):
class F(FilterSet):
filter_overrides = {
@ -468,6 +488,7 @@ class FilterSetClassCreationTests(TestCase):
class Meta:
model = NetworkSetting
fields = '__all__'
self.assertEqual(list(F.base_filters.keys()), ['ip', 'mask'])
@ -475,10 +496,12 @@ class FilterSetClassCreationTests(TestCase):
class F(FilterSet):
class Meta:
model = User
fields = '__all__'
class ProxyF(FilterSet):
class Meta:
model = AdminUser
fields = '__all__'
self.assertEqual(list(F.base_filters), list(ProxyF.base_filters))
@ -486,10 +509,12 @@ class FilterSetClassCreationTests(TestCase):
class F(FilterSet):
class Meta:
model = Account
fields = '__all__'
class FtiF(FilterSet):
class Meta:
model = BankAccount
fields = '__all__'
# fails due to 'account_ptr' getting picked up
self.assertEqual(
@ -760,7 +785,7 @@ class FilterSetTogetherTests(TestCase):
self.assertQuerysetEqual(f.qs, [self.alex.pk], lambda o: o.pk)
@unittest.skip('remove when relevant deprecations have been completed')
@unittest.skip('TODO: remove when relevant deprecations have been completed')
class MiscFilterSetTests(TestCase):
def test_no__getitem__(self):

1
tests/test_forms.py
View File

@ -39,6 +39,7 @@ class FilterSetFormTests(TestCase):
class F(FilterSet):
class Meta:
model = Book
fields = '__all__'
form = MyForm
f = F().form