Skip to content

Allow repeated filter query parameters #782

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

Merged
merged 10 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ Nathanael Gordon <[email protected]>
Charlie Allatson <[email protected]>
Joseba Mendivil <[email protected]>
Felix Viernickel <[email protected]>
René Kälin <[email protected]>
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ any parts of the framework not mentioned in the documentation should generally b

## [Unreleased]

### Changed

* Allowed repeated filter query parameters.

### Fixed

* Avoid `AttributeError` for PUT and PATCH methods when using `APIView`
Expand Down
9 changes: 5 additions & 4 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]`).
Expand Down Expand Up @@ -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)

Expand Down
34 changes: 28 additions & 6 deletions example/fixtures/blogentry.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -83,7 +105,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [1]
}
},
{
Expand All @@ -100,7 +122,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [2]
}
},
{
Expand All @@ -117,7 +139,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [2]
}
},
{
Expand All @@ -134,7 +156,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [1,2]
}
},
{
Expand All @@ -151,7 +173,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [1,2]
}
},
{
Expand All @@ -168,7 +190,7 @@
"n_comments": 0,
"n_pingbacks": 0,
"rating": 0,
"authors": []
"authors": [1,2]
}
},
{
Expand Down
67 changes: 66 additions & 1 deletion example/tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,60 @@ def test_filter_missing_rvalue_equal(self):
self.assertEqual(dja_response['errors'][0]['detail'],
"missing filter[headline] test value")

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):
"""
test for `filter[search]="keywords"` where some of the keywords are in the entry and
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
15 changes: 14 additions & 1 deletion example/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,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):
Expand All @@ -159,6 +171,7 @@ class FiltersetEntryViewSet(EntryViewSet):
pagination_class = NoPagination
filterset_fields = None
filterset_class = EntryFilter
filter_backends = (QueryParameterValidationFilter, DjangoFilterBackend,)


class NoFiltersetEntryViewSet(EntryViewSet):
Expand Down
6 changes: 3 additions & 3 deletions rest_framework_json_api/django_filters/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
if not val[0]:
raise ValidationError("missing {} test value".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 {
Expand Down
7 changes: 4 additions & 3 deletions rest_framework_json_api/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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))

Expand Down