Merge pull request #66 from rpkilby/df-update

django-filter update

Author: Ryan P Kilby

CHANGELOG.rst

@@ -1,6 +1,20 @@
 Unreleased
 ----------
 
+This release is tied to a major update of django-filter (more details in #66),
+which fixes how lookup expressions are resolved. 'in', 'range', and 'isnull'
+lookups no longer require special handling by django-rest-framework-filters.
+This has the following effects:
+
+  * Deprecates ArrayDecimalField/InSetNumberFilter
+  * Deprecates ArrayCharField/InSetCharFilter
+  * Deprecates FilterSet.fix_filter_field
+  * Deprecates ALL_LOOKUPS in favor of '__all__' constant.
+  * AllLookupsFilter now generates only valid lookup expressions.
+
+* #2 'range' lookup types do not work
+* #15 Date lookup types do not work (year, day, ...)
+* #16 'in' lookup types do not work
 * #64 Fix browsable API filter form
 * #69 Fix compatibility with base django-filter `FilterSet`s
 * #70 Refactor related filter handling, fixing some edge cases

rest_framework_filters/fields.py

@@ -1,4 +1,7 @@
+
+import warnings
 from django import forms
+
 from django_filters.widgets import BooleanWidget
 
 
@@ -7,6 +10,14 @@ class BooleanField(forms.BooleanField):
 
 
 class ArrayDecimalField(forms.DecimalField):
+    def __init__(self, *args, **kwargs):
+        super(ArrayDecimalField, self).__init__(*args, **kwargs)
+        warnings.warn(
+            'ArrayDecimalField is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/62',
+            DeprecationWarning, stacklevel=3
+        )
+
     def clean(self, value):
         if value is None:
             return None
@@ -18,6 +29,14 @@ def clean(self, value):
 
 
 class ArrayCharField(forms.CharField):
+    def __init__(self, *args, **kwargs):
+        super(ArrayCharField, self).__init__(*args, **kwargs)
+        warnings.warn(
+            'ArrayCharField is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/62',
+            DeprecationWarning, stacklevel=3
+        )
+
     def clean(self, value):
         if value is None:
             return None

rest_framework_filters/filters.py

@@ -1,14 +1,16 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import warnings
 from django.utils import six
 
 from django_filters.filters import *
-from django_filters.filters import LOOKUP_TYPES
 
 from . import fields
 
-ALL_LOOKUPS = LOOKUP_TYPES
+
+class ALL_LOOKUPS(object):
+    pass
 
 
 def _import_class(path):
@@ -49,17 +51,36 @@ class AllLookupsFilter(Filter):
 ###################################################
 # Fixed-up versions of some of the default filters
 ###################################################
+
+# This class is necessary, as directly django-filter's BooleanFilter
+# is using the incorrect form widget.
 class BooleanFilter(BooleanFilter):
     field_class = fields.BooleanField
 
 
 class InSetNumberFilter(Filter):
     field_class = fields.ArrayDecimalField
 
+    def __init__(self, *args, **kwargs):
+        super(InSetNumberFilter, self).__init__(*args, **kwargs)
+        warnings.warn(
+            'InSetNumberFilter is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/62',
+            DeprecationWarning, stacklevel=2
+        )
+
 
 class InSetCharFilter(Filter):
     field_class = fields.ArrayCharField
 
+    def __init__(self, *args, **kwargs):
+        super(InSetCharFilter, self).__init__(*args, **kwargs)
+        warnings.warn(
+            'InSetCharFilter is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/62',
+            DeprecationWarning, stacklevel=2
+        )
+
 
 class MethodFilter(Filter):
     """

rest_framework_filters/filterset.py

@@ -10,16 +10,34 @@
 from django.db.models.fields.related import ForeignObjectRel
 from django.utils import six
 
-import django_filters
-import django_filters.filters
 from django_filters import filterset
 
 from . import filters
+from . import utils
+
+
+def _base(f):
+    f._base = True
+    return f
+
+
+def _get_fix_filter_field(cls):
+    method = getattr(cls, 'fix_filter_field')
+    if not getattr(method, '_base', False):
+        warnings.warn(
+            'fix_filter_field is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/62',
+            DeprecationWarning, stacklevel=2
+        )
+    return cls.fix_filter_field
 
 
 class FilterSetMetaclass(filterset.FilterSetMetaclass):
     def __new__(cls, name, bases, attrs):
+        cls.convert__all__(attrs)
+
         new_class = super(FilterSetMetaclass, cls).__new__(cls, name, bases, attrs)
+        fix_filter_field = _get_fix_filter_field(new_class)
 
         # Populate our FilterSet fields with all the possible
         # filters for the AllLookupsFilter field.
@@ -28,24 +46,25 @@ def __new__(cls, name, bases, attrs):
                 model = new_class._meta.model
                 field = filterset.get_model_field(model, filter_.name)
 
-                for lookup_type in django_filters.filters.LOOKUP_TYPES:
+                for lookup_expr in utils.lookups_for_field(field):
                     if isinstance(field, ForeignObjectRel):
                         f = new_class.filter_for_reverse_field(field, filter_.name)
                     else:
-                        f = new_class.filter_for_field(field, filter_.name)
-                    f.lookup_type = lookup_type
-                    f = new_class.fix_filter_field(f)
+                        f = new_class.filter_for_field(field, filter_.name, lookup_expr)
+                    f = fix_filter_field(f)
 
                     # compute filter name
-                    filter_name = name
+                    filter_name = LOOKUP_SEP.join([name, lookup_expr])
+
                     # Don't add "exact" to filter names
-                    if lookup_type != 'exact':
-                        filter_name = LOOKUP_SEP.join([name, lookup_type])
+                    _exact = LOOKUP_SEP + 'exact'
+                    if filter_name.endswith(_exact):
+                        filter_name = filter_name[:-len(_exact)]
 
                     new_class.base_filters[filter_name] = f
 
             elif name not in new_class.declared_filters:
-                new_class.base_filters[name] = new_class.fix_filter_field(filter_)
+                new_class.base_filters[name] = fix_filter_field(filter_)
 
         return new_class
 
@@ -61,9 +80,33 @@ def related_filters(self):
             ])
         return self._related_filters
 
+    @staticmethod
+    def convert__all__(attrs):
+        """
+        Extract Meta.fields and convert any fields w/ `__all__`
+        to a declared AllLookupsFilter.
+        """
+        meta = attrs.get('Meta', None)
+        fields = getattr(meta, 'fields', None)
+
+        if isinstance(fields, dict):
+            for name, lookups in six.iteritems(fields.copy()):
+                if lookups == filters.ALL_LOOKUPS:
+                    warnings.warn(
+                        "ALL_LOOKUPS has been deprecated in favor of '__all__'. See: "
+                        "https://github.com/philipn/django-rest-framework-filters/issues/62",
+                        DeprecationWarning, stacklevel=2
+                    )
+                    lookups = '__all__'
+
+                if lookups == '__all__':
+                    del fields[name]
+                    attrs[name] = filters.AllLookupsFilter()
+
 
 class FilterSet(six.with_metaclass(FilterSetMetaclass, filterset.FilterSet)):
     filter_overrides = {
+        # uses API-friendly django_filters.BooleanWidget
         models.BooleanField: {
             'filter_class': filters.BooleanFilter,
         },
@@ -91,7 +134,7 @@ def __init__(self, *args, **kwargs):
                 # Add an 'isnull' filter to allow checking if the relation is empty.
                 filter_name = "%s%sisnull" % (filter_.name, LOOKUP_SEP)
                 if filter_name not in self.filters:
-                    self.filters[filter_name] = filters.BooleanFilter(name=filter_.name, lookup_type='isnull')
+                    self.filters[filter_name] = filters.BooleanFilter(name=filter_.name, lookup_expr='isnull')
 
             elif isinstance(filter_, filters.MethodFilter):
                 filter_.resolve_action()
@@ -276,15 +319,6 @@ def qs(self):
         return qs
 
     @classmethod
+    @_base
     def fix_filter_field(cls, f):
-        """
-        Fix the filter field based on the lookup type.
-        """
-        lookup_type = f.lookup_type
-        if lookup_type == 'isnull':
-            return filters.BooleanFilter(name=f.name, lookup_type='isnull')
-        if lookup_type == 'in' and type(f) == filters.NumberFilter:
-            return filters.InSetNumberFilter(name=f.name, lookup_type='in')
-        if lookup_type == 'in' and type(f) == filters.CharFilter:
-            return filters.InSetCharFilter(name=f.name, lookup_type='in')
         return f

rest_framework_filters/utils.py

@@ -0,0 +1,40 @@
+
+from collections import OrderedDict
+
+from django.db.models.constants import LOOKUP_SEP
+from django.db.models.lookups import Transform
+from django.utils import six
+
+
+def lookups_for_field(model_field):
+    """
+    Generates a list of all possible lookup expressions for a model field.
+    """
+    lookups = []
+
+    for expr, lookup in six.iteritems(class_lookups(model_field)):
+        if issubclass(lookup, Transform):
+            lookups += [
+                LOOKUP_SEP.join([expr, transform]) for transform
+                in lookups_for_field(lookup(model_field).output_field)
+            ]
+        else:
+            lookups.append(expr)
+
+    return lookups
+
+
+def class_lookups(model_field):
+    """
+    Get a compiled set of class_lookups for a model field.
+    """
+    field_class = model_field.__class__
+    class_lookups = OrderedDict()
+
+    # traverse MRO in reverse, as this puts standard
+    # lookups before subclass transforms/lookups
+    for cls in field_class.mro()[::-1]:
+        if hasattr(cls, 'class_lookups'):
+            class_lookups.update(getattr(cls, 'class_lookups'))
+
+    return class_lookups

setup.py

@@ -43,7 +43,7 @@ def get_package_data(package):
     zip_safe=False,
     install_requires=[
         'djangorestframework',
-        'django-filter>=0.12.0',
+        'django-filter>=0.13.0',
     ],
     classifiers=[
         'Development Status :: 5 - Production/Stable',