From b26b552dd9d4ce3aba4c01b92d04b686e94b4968 Mon Sep 17 00:00:00 2001 From: rk Date: Mon, 20 Apr 2020 12:03:01 +0200 Subject: [PATCH 1/9] Allowed repeated filter query parameters. --- rest_framework_json_api/filters.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/filters.py b/rest_framework_json_api/filters.py index f0b95a35..eafcdb76 100644 --- a/rest_framework_json_api/filters.py +++ b/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)$|^(?Pfilter|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)) From b0ec6ad46ac9a3c633cc868e81688f8f727aef4c Mon Sep 17 00:00:00 2001 From: rk Date: Mon, 20 Apr 2020 12:08:04 +0200 Subject: [PATCH 2/9] Assigned list instead of last-value only to filterset kwargs. --- rest_framework_json_api/django_filters/backends.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/django_filters/backends.py b/rest_framework_json_api/django_filters/backends.py index 0c4b80d3..e2db7a33 100644 --- a/rest_framework_json_api/django_filters/backends.py +++ b/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: + 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 { From e48ce8c9ff0b6b378d18916ade22706eb64fbdad Mon Sep 17 00:00:00 2001 From: rk Date: Mon, 20 Apr 2020 12:09:45 +0200 Subject: [PATCH 3/9] Updated example and tests. --- example/fixtures/blogentry.json | 34 ++++++++++++++--- example/tests/test_filters.py | 67 ++++++++++++++++++++++++++++++++- example/views.py | 15 +++++++- 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/example/fixtures/blogentry.json b/example/fixtures/blogentry.json index 44b70d97..464573e4 100644 --- a/example/fixtures/blogentry.json +++ b/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": "alice@example.com", + "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": "bob@example.com", + "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] } }, { diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index a42480c2..1cd24d2c 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -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 @@ -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 diff --git a/example/views.py b/example/views.py index 33393be9..8c80d145 100644 --- a/example/views.py +++ b/example/views.py @@ -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): @@ -159,6 +171,7 @@ class FiltersetEntryViewSet(EntryViewSet): pagination_class = NoPagination filterset_fields = None filterset_class = EntryFilter + filter_backends = (QueryParameterValidationFilter, DjangoFilterBackend,) class NoFiltersetEntryViewSet(EntryViewSet): From 7143259ac365888f9af54fba0a3c3999b13e4733 Mon Sep 17 00:00:00 2001 From: rk Date: Mon, 20 Apr 2020 12:19:20 +0200 Subject: [PATCH 4/9] Added author name. --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 8ecb85d2..a5d51cb6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -28,3 +28,4 @@ Nathanael Gordon Charlie Allatson Joseba Mendivil Felix Viernickel +René Kälin From 4feaa251ec5fd1adc9e9712cd1e2ad138345bcd0 Mon Sep 17 00:00:00 2001 From: rk Date: Mon, 20 Apr 2020 12:28:47 +0200 Subject: [PATCH 5/9] Updated documentation. --- docs/usage.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index fc6aa6ad..f7152c6e 100644 --- a/docs/usage.md +++ b/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) From ab61575d1d0baae1f039e0718634829a16eff56b Mon Sep 17 00:00:00 2001 From: rk Date: Thu, 14 May 2020 17:09:57 +0200 Subject: [PATCH 6/9] Updated changelog. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85942770..7ad7c94a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` From b26b8980146325a5e56fab467216a93b350920c8 Mon Sep 17 00:00:00 2001 From: rk Date: Wed, 20 May 2020 09:26:47 +0200 Subject: [PATCH 7/9] Checked all values for emptiness. --- rest_framework_json_api/django_filters/backends.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest_framework_json_api/django_filters/backends.py b/rest_framework_json_api/django_filters/backends.py index e2db7a33..d18bec54 100644 --- a/rest_framework_json_api/django_filters/backends.py +++ b/rest_framework_json_api/django_filters/backends.py @@ -107,8 +107,9 @@ def get_filterset_kwargs(self, request, queryset, view): m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): raise ValidationError("invalid query parameter: {}".format(qp)) if m and qp != self.search_param: - if not val[0]: - raise ValidationError("missing {} test value".format(qp)) + for v in val: + if not v: + 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: From 8d8d97752c69bcd44b246d11600a15d729eda8c5 Mon Sep 17 00:00:00 2001 From: rk Date: Wed, 20 May 2020 11:31:15 +0200 Subject: [PATCH 8/9] Adjusted tests. --- example/tests/test_filters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index 1cd24d2c..b422ed11 100644 --- a/example/tests/test_filters.py +++ b/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,7 @@ 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): """ From ee29228e5a882c885f842d22942056627e5c5273 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 20 May 2020 11:58:10 +0200 Subject: [PATCH 9/9] Simplify emptiness check --- rest_framework_json_api/django_filters/backends.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/django_filters/backends.py b/rest_framework_json_api/django_filters/backends.py index d18bec54..29acfa5c 100644 --- a/rest_framework_json_api/django_filters/backends.py +++ b/rest_framework_json_api/django_filters/backends.py @@ -107,9 +107,8 @@ def get_filterset_kwargs(self, request, queryset, view): m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): raise ValidationError("invalid query parameter: {}".format(qp)) if m and qp != self.search_param: - for v in val: - if not v: - raise ValidationError("missing value for query parameter {}".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: