Allow repeated filter query parameters (#782)

Fixes #718

Author: ro70

AUTHORS

@@ -28,4 +28,5 @@ Nathanael Gordon <[email protected]>
 Charlie Allatson <[email protected]>
 Joseba Mendivil <[email protected]>
 Felix Viernickel <[email protected]>
+René Kälin <[email protected]>
 Tom Glowka <[email protected]>

CHANGELOG.md

@@ -22,6 +22,7 @@ any parts of the framework not mentioned in the documentation should generally b
 
 * `SerializerMethodResourceRelatedField` is now consistent with DRF `SerializerMethodField`:
    * Pass `method_name` argument to specify method name. If no value is provided, it defaults to `get_{field_name}`
+* Allowed repeated filter query parameters.
 
 ### Deprecated
 

docs/usage.md

@@ -64,17 +64,17 @@ You can configure fixed values for the page size or limit -- or allow the client
 via query parameters.
 
 Two pagination classes are available:
-- `JsonApiPageNumberPagination` breaks a response up into pages that start at a given page number with a given size 
+- `JsonApiPageNumberPagination` breaks a response up into pages that start at a given page number with a given size
   (number of items per page). It can be configured with the following attributes:
   - `page_query_param` (default `page[number]`)
-  - `page_size_query_param` (default `page[size]`) Set this to `None` if you don't want to allow the client 
+  - `page_size_query_param` (default `page[size]`) Set this to `None` if you don't want to allow the client
      to specify the size.
   - `page_size` (default `REST_FRAMEWORK['PAGE_SIZE']`) default number of items per page unless overridden by
      `page_size_query_param`.
   - `max_page_size` (default `100`) enforces an upper bound on the `page_size_query_param`.
      Set it to `None` if you don't want to enforce an upper bound.
 
-- `JsonApiLimitOffsetPagination` breaks a response up into pages that start from an item's offset in the viewset for 
+- `JsonApiLimitOffsetPagination` breaks a response up into pages that start from an item's offset in the viewset for
   a given number of items (the limit).
   It can be configured with the following attributes:
   - `offset_query_param` (default `page[offset]`).
@@ -177,7 +177,8 @@ Filters can be:
 - Membership in a list of values:
     `?filter[name.in]=abc,123,zzz (name in ['abc','123','zzz'])`
 - Filters can be combined for intersection (AND):
-    `?filter[qty]=123&filter[name.in]=abc,123,zzz&filter[...]`
+    `?filter[qty]=123&filter[name.in]=abc,123,zzz&filter[...]` or
+    `?filter[authors.id]=1&filter[authors.id]=2`
 - A related resource path can be used:
     `?filter[inventory.item.partNum]=123456` (where `inventory.item` is the relationship path)
 

example/fixtures/blogentry.json

@@ -69,6 +69,28 @@
     "tagline": "BIOLOGICAL SCIENCES (BARNARD)"
   }
 },
+{
+  "model": "example.author",
+  "pk": 1,
+  "fields": {
+    "created_at": "2016-05-02T10:09:48.277",
+    "modified_at": "2016-05-02T10:09:48.277",
+    "name": "Alice",
+    "email": "[email protected]",
+    "type": null
+  }
+},
+{
+  "model": "example.author",
+  "pk": 2,
+  "fields": {
+    "created_at": "2016-05-02T10:09:57.133",
+    "modified_at": "2016-05-02T10:09:57.133",
+    "name": "Bob",
+    "email": "[email protected]",
+    "type": null
+  }
+},
 {
   "model": "example.entry",
   "pk": 1,
@@ -83,7 +105,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [1]
   }
 },
 {
@@ -100,7 +122,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [2]
   }
 },
 {
@@ -117,7 +139,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [2]
   }
 },
 {
@@ -134,7 +156,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [1,2]
   }
 },
 {
@@ -151,7 +173,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [1,2]
   }
 },
 {
@@ -168,7 +190,7 @@
     "n_comments": 0,
     "n_pingbacks": 0,
     "rating": 0,
-    "authors": []
+    "authors": [1,2]
   }
 },
 {

example/tests/test_filters.py

@@ -323,7 +323,7 @@ def test_filter_missing_rvalue(self):
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
         self.assertEqual(dja_response['errors'][0]['detail'],
-                         "missing filter[headline] test value")
+                         "missing value for query parameter filter[headline]")
 
     def test_filter_missing_rvalue_equal(self):
         """
@@ -335,7 +335,61 @@ def test_filter_missing_rvalue_equal(self):
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
         self.assertEqual(dja_response['errors'][0]['detail'],
-                         "missing filter[headline] test value")
+                         "missing value for query parameter filter[headline]")
+
+    def test_filter_single_relation(self):
+        """
+        test for filter with a single relation
+        e.g. filterset-entries?filter[authors.id]=1
+        looks for entries written by (at least) author.id=1
+        """
+        response = self.client.get(self.fs_url, data={'filter[authors.id]': 1})
+
+        self.assertEqual(response.status_code, 200,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+
+        ids = [k['id'] for k in dja_response['data']]
+
+        expected_ids = [str(k.id) for k in self.entries.filter(authors__id=1)]
+
+        self.assertEqual(set(ids), set(expected_ids))
+
+    def test_filter_repeated_relations(self):
+        """
+        test for filters with repeated relations
+        e.g. filterset-entries?filter[authors.id]=1&filter[authors.id]=2
+        looks for entries written by (at least) author.id=1 AND author.id=2
+        """
+        response = self.client.get(self.fs_url, data={'filter[authors.id]': [1, 2]})
+
+        self.assertEqual(response.status_code, 200,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+
+        ids = [k['id'] for k in dja_response['data']]
+
+        expected_ids = [str(k.id) for k in self.entries.filter(authors__id=1).filter(authors__id=2)]
+
+        self.assertEqual(set(ids), set(expected_ids))
+
+    def test_filter_in(self):
+        """
+        test for the in filter
+        e.g. filterset-entries?filter[authors.id.in]=1,2
+        looks for entries written by (at least) author.id=1 OR author.id=2
+        """
+        response = self.client.get(self.fs_url, data={'filter[authors.id.in]': '1,2'})
+
+        self.assertEqual(response.status_code, 200,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+
+        ids = [k['id'] for k in dja_response['data']]
+
+        expected_ids = [str(k.id) for k in self.entries.filter(authors__id__in=[1, 2])]
+
+        self.assertEqual(set(ids), set(expected_ids))
 
     def test_search_keywords(self):
         """
@@ -488,7 +542,7 @@ def test_param_invalid(self):
         self.assertEqual(dja_response['errors'][0]['detail'],
                          "invalid query parameter: garbage")
 
-    def test_param_duplicate(self):
+    def test_param_duplicate_sort(self):
         """
         Test a duplicated query parameter:
         `?sort=headline&page[size]=3&sort=bodyText` is not allowed.
@@ -504,6 +558,17 @@ def test_param_duplicate(self):
         self.assertEqual(dja_response['errors'][0]['detail'],
                          "repeated query parameter not allowed: sort")
 
+    def test_param_duplicate_page(self):
+        """
+        test a duplicated page[size] query parameter
+        """
+        response = self.client.get(self.fs_url, data={'page[size]': [1, 2]})
+        self.assertEqual(response.status_code, 400,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+        self.assertEqual(dja_response['errors'][0]['detail'],
+                         "repeated query parameter not allowed: page[size]")
+
     def test_many_params(self):
         """
         Test that filter params aren't ignored when many params are present

example/views.py

@@ -146,9 +146,21 @@ class EntryFilter(filters.FilterSet):
     bname = filters.CharFilter(field_name="blog__name",
                                lookup_expr="exact")
 
+    authors__id = filters.ModelMultipleChoiceFilter(
+        field_name='authors',
+        to_field_name='id',
+        conjoined=True,  # to "and" the ids
+        queryset=Author.objects.all(),
+    )
+
     class Meta:
         model = Entry
-        fields = ['id', 'headline', 'body_text']
+        fields = {
+            'id': ('exact',),
+            'headline': ('exact',),
+            'body_text': ('exact',),
+            'authors__id': ('in',),
+        }
 
 
 class FiltersetEntryViewSet(EntryViewSet):
@@ -158,6 +170,7 @@ class FiltersetEntryViewSet(EntryViewSet):
     pagination_class = NoPagination
     filterset_fields = None
     filterset_class = EntryFilter
+    filter_backends = (QueryParameterValidationFilter, DjangoFilterBackend,)
 
 
 class NoFiltersetEntryViewSet(EntryViewSet):

rest_framework_json_api/django_filters/backends.py

@@ -101,19 +101,19 @@ def get_filterset_kwargs(self, request, queryset, view):
         filter_keys = []
         # rewrite filter[field] query params to make DjangoFilterBackend work.
         data = request.query_params.copy()
-        for qp, val in request.query_params.items():
+        for qp, val in request.query_params.lists():
             m = self.filter_regex.match(qp)
             if m and (not m.groupdict()['assoc'] or
                       m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'):
                 raise ValidationError("invalid query parameter: {}".format(qp))
             if m and qp != self.search_param:
-                if not val:
-                    raise ValidationError("missing {} test value".format(qp))
+                if not all(val):
+                    raise ValidationError("missing value for query parameter {}".format(qp))
                 # convert jsonapi relationship path to Django ORM's __ notation
                 key = m.groupdict()['assoc'].replace('.', '__')
                 # undo JSON_API_FORMAT_FIELD_NAMES conversion:
                 key = format_value(key, 'underscore')
-                data[key] = val
+                data.setlist(key, val)
                 filter_keys.append(key)
                 del data[qp]
         return {

rest_framework_json_api/filters.py

@@ -70,7 +70,7 @@ class QueryParameterValidationFilter(BaseFilterBackend):
     """
     #: compiled regex that matches the allowed http://jsonapi.org/format/#query-parameters:
     #: `sort` and `include` stand alone; `filter`, `fields`, and `page` have []'s
-    query_regex = re.compile(r'^(sort|include)$|^(filter|fields|page)(\[[\w\.\-]+\])?$')
+    query_regex = re.compile(r'^(sort|include)$|^(?P<type>filter|fields|page)(\[[\w\.\-]+\])?$')
 
     def validate_query_params(self, request):
         """
@@ -82,9 +82,10 @@ def validate_query_params(self, request):
         # TODO: For jsonapi error object conformance, must set jsonapi errors "parameter" for
         # the ValidationError. This requires extending DRF/DJA Exceptions.
         for qp in request.query_params.keys():
-            if not self.query_regex.match(qp):
+            m = self.query_regex.match(qp)
+            if not m:
                 raise ValidationError('invalid query parameter: {}'.format(qp))
-            if len(request.query_params.getlist(qp)) > 1:
+            if not m.group('type') == 'filter' and len(request.query_params.getlist(qp)) > 1:
                 raise ValidationError(
                     'repeated query parameter not allowed: {}'.format(qp))