From 7b29f365e1ed30cb67b5484fa4ac7f3ab566bc2a Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 23 Aug 2018 16:26:30 -0400 Subject: [PATCH 01/20] initial integration of JSONAPIDjangoFilter --- example/settings/dev.py | 1 + example/tests/test_filters.py | 251 ++++++++++++++++++++++++++++- example/urls_test.py | 5 + example/views.py | 39 +++++ rest_framework_json_api/filters.py | 89 +++++++++- 5 files changed, 382 insertions(+), 3 deletions(-) diff --git a/example/settings/dev.py b/example/settings/dev.py index 6856a91b..720d2396 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -90,6 +90,7 @@ 'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata', 'DEFAULT_FILTER_BACKENDS': ( 'rest_framework_json_api.filters.JSONAPIOrderingFilter', + 'rest_framework_json_api.filters.JSONAPIDjangoFilter', ), 'TEST_REQUEST_RENDERER_CLASSES': ( 'rest_framework_json_api.renderers.JSONRenderer', diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index 2b18b5f3..d0e8974c 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -4,9 +4,9 @@ from ..models import Blog, Entry -class DJATestParameters(APITestCase): +class DJATestFilters(APITestCase): """ - tests of JSON:API backends + tests of JSON:API filter backends """ fixtures = ('blogentry',) @@ -14,6 +14,8 @@ def setUp(self): self.entries = Entry.objects.all() self.blogs = Blog.objects.all() self.url = reverse('nopage-entry-list') + self.fs_url = reverse('filterset-entry-list') + self.no_fs_url = reverse('nofilterset-entry-list') def test_sort(self): """ @@ -103,3 +105,248 @@ def test_sort_related(self): blog_ids = [c['relationships']['blog']['data']['id'] for c in dja_response['data']] sorted_blog_ids = sorted(blog_ids) self.assertEqual(blog_ids, sorted_blog_ids) + + def test_filter_exact(self): + """ + filter for an exact match + """ + response = self.client.get(self.url, data={'filter[headline]': 'CHEM3271X'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(len(dja_response['data']), 1) + + def test_filter_exact_fail(self): + """ + failed search for an exact match + """ + response = self.client.get(self.url, data={'filter[headline]': 'XXXXX'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(len(dja_response['data']), 0) + + def test_filter_isnull(self): + """ + search for null value + """ + response = self.client.get(self.url, data={'filter[bodyText.isnull]': 'true'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual( + len(dja_response['data']), + len([k for k in self.entries if k.body_text is None]) + ) + + def test_filter_not_null(self): + """ + search for not null + """ + response = self.client.get(self.url, data={'filter[bodyText.isnull]': 'false'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual( + len(dja_response['data']), + len([k for k in self.entries if k.body_text is not None]) + ) + + def test_filter_isempty(self): + """ + search for an empty value (different from null!) + the easiest way to do this is search for r'^$' + """ + response = self.client.get(self.url, data={'filter[bodyText.regex]': '^$'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(len(dja_response['data']), + len([k for k in self.entries + if k.body_text is not None and + len(k.body_text) == 0])) + + def test_filter_related(self): + """ + filter via a relationship chain + """ + response = self.client.get(self.url, data={'filter[blog.name]': 'ANTB'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(len(dja_response['data']), + len([k for k in self.entries + if k.blog.name == 'ANTB'])) + + def test_filter_related_fieldset_class(self): + """ + filter via a FilterSet class instead of filterset_fields shortcut + This tests a shortcut for a longer ORM path: `bname` is a shortcut + name for `blog.name`. + """ + response = self.client.get(self.fs_url, data={'filter[bname]': 'ANTB'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(len(dja_response['data']), + len([k for k in self.entries + if k.blog.name == 'ANTB'])) + + def test_filter_related_missing_fieldset_class(self): + """ + filter via with neither filterset_fields nor filterset_class + This should return an error for any filter[] + """ + response = self.client.get(self.no_fs_url, data={'filter[bname]': 'ANTB'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter[bname]") + + def test_filter_fields_union_list(self): + """ + test field for a list of values(ORed): ?filter[field.in]': 'val1,val2,val3 + """ + response = self.client.get(self.url, + data={'filter[headline.in]': 'CLCV2442V,XXX,BIOL3594X'}) + dja_response = response.json() + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + self.assertEqual( + len(dja_response['data']), + len([k for k in self.entries if k.headline == 'CLCV2442V']) + + len([k for k in self.entries if k.headline == 'XXX']) + + len([k for k in self.entries if k.headline == 'BIOL3594X']), + msg="filter field list (union)") + + def test_filter_fields_intersection(self): + """ + test fields (ANDed): ?filter[field1]': 'val1&filter[field2]'='val2 + """ + # + response = self.client.get(self.url, + data={'filter[headline.regex]': '^A', + 'filter[body_text.icontains]': 'in'}) + self.assertEqual(response.status_code, 200, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertGreater(len(dja_response['data']), 1) + self.assertEqual( + len(dja_response['data']), + len([k for k in self.entries if k.headline.startswith('A') and + 'in' in k.body_text.lower()])) + + def test_filter_invalid_association_name(self): + """ + test for filter with invalid filter association name + """ + response = self.client.get(self.url, data={'filter[nonesuch]': 'CHEM3271X'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter[nonesuch]") + + def test_filter_empty_association_name(self): + """ + test for filter with missing association name + """ + response = self.client.get(self.url, data={'filter[]': 'foobar'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter[]") + + def test_filter_no_brackets(self): + """ + test for `filter=foobar` with missing filter[association] name + """ + response = self.client.get(self.url, data={'filter': 'foobar'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter") + + def test_filter_no_brackets_rvalue(self): + """ + test for `filter=` with missing filter[association] and value + """ + response = self.client.get(self.url + '?filter=') + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter") + + def test_filter_no_brackets_equal(self): + """ + test for `filter` with missing filter[association] name and =value + """ + response = self.client.get(self.url +'?filter') + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter") + + def test_filter_malformed_left_bracket(self): + """ + test for filter with invalid filter syntax + """ + response = self.client.get(self.url, data={'filter[': 'foobar'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter[") + + def test_filter_missing_right_bracket(self): + """ + test for filter missing right bracket + """ + response = self.client.get(self.url, data={'filter[headline': 'foobar'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter[headline") + + def test_filter_incorrect_brackets(self): + """ + test for filter with incorrect brackets + """ + response = self.client.get(self.url, data={'filter{headline}': 'foobar'}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "invalid filter: filter{headline}") + + def test_filter_missing_rvalue(self): + """ + test for filter with missing value to test against + this should probably be an error rather than ignoring the filter: + https://django-filter.readthedocs.io/en/latest/guide/tips.html#filtering-by-an-empty-string + """ + response = self.client.get(self.url, data={'filter[headline]': ''}) + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "missing filter[headline] test value") + + def test_filter_missing_rvalue_equal(self): + """ + test for filter with missing value to test against + this should probably be an error rather than ignoring the filter: + """ + response = self.client.get(self.url + '?filter[headline]') + self.assertEqual(response.status_code, 400, + msg=response.content.decode("utf-8")) + dja_response = response.json() + self.assertEqual(dja_response['errors'][0]['detail'], + "missing filter[headline] test value") + diff --git a/example/urls_test.py b/example/urls_test.py index e7a27ce4..8475bb79 100644 --- a/example/urls_test.py +++ b/example/urls_test.py @@ -13,6 +13,8 @@ EntryRelationshipView, EntryViewSet, NonPaginatedEntryViewSet, + FiltersetEntryViewSet, + NoFiltersetEntryViewSet, ProjectViewset ) @@ -20,7 +22,10 @@ router.register(r'blogs', BlogViewSet) router.register(r'entries', EntryViewSet) +# these "flavors" of entries are used for various tests: router.register(r'nopage-entries', NonPaginatedEntryViewSet, 'nopage-entry') +router.register(r'filterset-entries', FiltersetEntryViewSet, 'filterset-entry') +router.register(r'nofilterset-entries', NoFiltersetEntryViewSet, 'nofilterset-entry') router.register(r'authors', AuthorViewSet) router.register(r'comments', CommentViewSet) router.register(r'companies', CompanyViewset) diff --git a/example/views.py b/example/views.py index 36026b17..288d17e7 100644 --- a/example/views.py +++ b/example/views.py @@ -8,6 +8,7 @@ from rest_framework_json_api.pagination import PageNumberPagination from rest_framework_json_api.utils import format_drf_errors from rest_framework_json_api.views import ModelViewSet, RelationshipView +from django_filters import rest_framework as filters from example.models import Author, Blog, Comment, Company, Entry, Project from example.serializers import ( @@ -91,6 +92,44 @@ class NoPagination(PageNumberPagination): class NonPaginatedEntryViewSet(EntryViewSet): pagination_class = NoPagination ordering_fields = ('headline', 'body_text', 'blog__name', 'blog__id') + rels = ('exact', 'iexact', + 'contains', 'icontains', + 'gt', 'gte', 'lt', 'lte', + 'in', 'regex', 'isnull',) + filterset_fields = { + 'id': ('exact', 'in'), + 'headline': rels, + 'body_text': rels, + 'blog__name': rels, + 'blog__tagline': rels, + } + + +class EntryFilter(filters.FilterSet): + bname = filters.CharFilter(field_name="blog__name", + lookup_expr="exact") + + class Meta: + model = Entry + fields = ['id', 'headline', 'body_text'] + + +class FiltersetEntryViewSet(EntryViewSet): + """ + like above but use filterset_class instead of filterset_fields + """ + pagination_class = NoPagination + filterset_fields = None + filterset_class = EntryFilter + + +class NoFiltersetEntryViewSet(EntryViewSet): + """ + like above but no filtersets + """ + pagination_class = NoPagination + filterset_fields = None + filterset_class = None class AuthorViewSet(ModelViewSet): diff --git a/rest_framework_json_api/filters.py b/rest_framework_json_api/filters.py index 748b18bf..cf3257ad 100644 --- a/rest_framework_json_api/filters.py +++ b/rest_framework_json_api/filters.py @@ -1,8 +1,10 @@ +from django_filters.rest_framework import DjangoFilterBackend from rest_framework.exceptions import ValidationError from rest_framework.filters import OrderingFilter +from rest_framework.settings import api_settings from rest_framework_json_api.utils import format_value - +import re class JSONAPIOrderingFilter(OrderingFilter): """ @@ -42,3 +44,88 @@ def remove_invalid_fields(self, queryset, fields, view, request): return super(JSONAPIOrderingFilter, self).remove_invalid_fields( queryset, underscore_fields, view, request) + + +class JSONAPIDjangoFilter(DjangoFilterBackend): + """ + A Django-style ORM filter implementation, using `django-filter`. + + This is not part of the jsonapi standard per-se, other than the requirement + to use the `filter` keyword: This is an optional implementation of style of + filtering in which each filter is an ORM expression as implemented by + DjangoFilterBackend and seems to be in alignment with an interpretation of + http://jsonapi.org/recommendations/#filtering, including relationship + chaining. + + Filters can be: + - A resource field equality test: + `?filter[foo]=123` + - Apply other relational operators: + `?filter[foo.in]=bar,baz or ?filter[name.isnull]=true...` + - Membership in a list of values (OR): + `?filter[foo]=abc,123,zzz (foo in ['abc','123','zzz'])` + - Filters can be combined for intersection (AND): + `?filter[foo]=123&filter[bar]=abc,123,zzz&filter[...]` + - A related resource path for above tests: + `?filter[foo.rel.baz]=123 (where `rel` is the relationship name)` + + If you are also using rest_framework.filters.SearchFilter you'll want to customize + the name of the query parameter for searching to make sure it doesn't conflict + with a field name defined in the filterset. + The recommended value is: `search_param="filter[search]"` but just make sure it's + `filter[]` to comply with the jsonapi spec requirement to use the filter + keyword. The default is "search" unless overriden but it's used here just to make sure + we don't complain about it being an invalid filter. + + TODO: find a better way to deal with search_param. + """ + search_param = api_settings.SEARCH_PARAM + # since 'filter' passes query parameter validation but is still invalid, + # make this regex check for it but not set `filter` regex group. + filter_regex = re.compile(r'^filter(?P\W*)(?P[\w._]*)(?P\W*$)') + + def get_filterset(self, request, queryset, view): + """ + Sometimes there's no filterset_class defined yet the client still + requests a filter. Make sure they see an error too. This means + we have to get_filterset_kwargs() even if there's no filterset_class. + + TODO: .base_filters vs. .filters attr (not always present) + """ + filterset_class = self.get_filterset_class(view, queryset) + kwargs = self.get_filterset_kwargs(request, queryset, view) + for k in self.filter_keys: + if ((not filterset_class) + or (k not in filterset_class.base_filters)): + raise ValidationError("invalid filter[{}]".format(k)) + if filterset_class is None: + return None + return filterset_class(**kwargs) + + def get_filterset_kwargs(self, request, queryset, view): + """ + Turns filter[]= into = which is what + DjangoFilterBackend expects + """ + self.filter_keys = [] + # rewrite filter[field] query params to make DjangoFilterBackend work. + data = request.query_params.copy() + for qp, val in data.items(): + m = self.filter_regex.match(qp) + if m and (not m['assoc'] or m['ldelim'] != '[' or m['rdelim'] != ']'): + raise ValidationError("invalid filter: {}".format(qp)) + if m and qp != self.search_param: + if not val: + raise ValidationError("missing {} test value".format(qp)) + # convert jsonapi relationship path to Django ORM's __ notation + key = m['assoc'].replace('.', '__') + # undo JSON_API_FORMAT_FIELD_NAMES conversion: + key = format_value(key, 'underscore') + data[key] = val + self.filter_keys.append(key) + del data[qp] + return { + 'data': data, + 'queryset': queryset, + 'request': request, + } From dc5ca3894c59bce9b65896e762f686a52df0ad98 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 15:03:40 -0400 Subject: [PATCH 02/20] documentation, isort, flake8 --- CHANGELOG.md | 2 +- docs/usage.md | 55 +++++++++++++++++++++++++++--- example/tests/test_filters.py | 7 ++-- example/urls_test.py | 2 +- example/views.py | 4 +-- requirements-development.txt | 2 +- rest_framework_json_api/filters.py | 34 +++++++++++------- setup.py | 3 +- 8 files changed, 82 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 354c1b5c..b61f677e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ * Add `HyperlinkedRelatedField` and `SerializerMethodHyperlinkedRelatedField`. See [usage docs](docs/usage.md#related-fields) * Add related urls support. See [usage docs](docs/usage.md#related-urls) * Replaced binary `drf_example` sqlite3 db with a [fixture](example/fixtures/drf_example.yaml). See [usage docs](docs/usage.md#running-the-example-app). -* Add optional [jsonapi-style](http://jsonapi.org/format/) sort filter backend. See [usage docs](docs/usage.md#filter-backends) * For naming consistency, renamed new `JsonApi`-prefix pagination classes to `JSONAPI`-prefix. * Deprecates `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` +* Add optional [jsonapi-style](http://jsonapi.org/format/) filter backends. See [usage docs](docs/usage.md#filter-backends) v2.5.0 - Released July 11, 2018 diff --git a/docs/usage.md b/docs/usage.md index b9d89ecf..010c58d4 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -34,6 +34,7 @@ REST_FRAMEWORK = { 'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata', 'DEFAULT_FILTER_BACKENDS': ( 'rest_framework_json_api.filters.JSONAPIOrderingFilter', + 'rest_framework_json_api.filters.JSONAPIDjangoFilter', ), 'TEST_REQUEST_RENDERER_CLASSES': ( 'rest_framework_json_api.renderers.JSONRenderer', @@ -92,14 +93,14 @@ class MyLimitPagination(JSONAPILimitOffsetPagination): ### Filter Backends -_This is the first of several anticipated JSON:API-specific filter backends._ +_There are several anticipated JSON:API-specific filter backends in development. The first two are described below._ #### `JSONAPIOrderingFilter` `JSONAPIOrderingFilter` implements the [JSON:API `sort`](http://jsonapi.org/format/#fetching-sorting) and uses DRF's [ordering filter](http://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#orderingfilter). Per the JSON:API specification, "If the server does not support sorting as specified in the query parameter `sort`, -it **MUST** return `400 Bad Request`." For example, for `?sort=`abc,foo,def` where `foo` is a valid +it **MUST** return `400 Bad Request`." For example, for `?sort=abc,foo,def` where `foo` is a valid field name and the other two are not valid: ```json { @@ -118,10 +119,56 @@ field name and the other two are not valid: If you want to silently ignore bad sort fields, just use `rest_framework.filters.OrderingFilter` and set `ordering_param` to `sort`. +#### `JSONAPIDjangoFilter` +`JSONAPIDjangoFilter` implements a Django ORM-style [JSON:API `filter`](http://jsonapi.org/format/#fetching-filtering) +using the [django-filter](https://django-filter.readthedocs.io/) package. + +This filter is not part of the JSON:API standard per-se, other than the requirement +to use the `filter` keyword: It is an optional implementation of a style of +filtering in which each filter is an ORM expression as implemented by +`DjangoFilterBackend` and seems to be in alignment with an interpretation of the +[JSON:API _recommendations_](http://jsonapi.org/recommendations/#filtering), including relationship +chaining. + +Filters can be: +- A resource field equality test: + `?filter[qty]=123` +- Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) operators: + `?filter[name.icontains]=bar` or `?filter[name.isnull]=true` +- 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[...]` +- A related resource path can be used: + `?filter[inventory.item.partNum]=123456` (where `inventory.item` is the relationship path) + +If you are also using [`rest_framework.filters.SearchFilter`](https://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#searchfilter) +(which performs single parameter searchs across multiple fields) you'll want to customize the name of the query +parameter for searching to make sure it doesn't conflict with a field name defined in the filterset. +The recommended value is: `search_param="filter[search]"` but just make sure it's +`filter[_something_]` to comply with the jsonapi spec requirement to use the filter +keyword. The default is "search" unless overriden. + +The filter returns a `400 Bad Request` error for invalid filter query parameters as in this example +for `GET http://127.0.0.1:8000/nopage-entries?filter[bad]=1`: +```json +{ + "errors": [ + { + "detail": "invalid filter[bad]", + "source": { + "pointer": "/data" + }, + "status": "400" + } + ] +} +``` + #### Configuring Filter Backends You can configure the filter backends either by setting the `REST_FRAMEWORK['DEFAULT_FILTER_BACKENDS']` as shown -in the [preceding](#configuration) example or individually add them as `.filter_backends` View attributes: +in the [example settings](#configuration) or individually add them as `.filter_backends` View attributes: ```python from rest_framework_json_api import filters @@ -129,7 +176,7 @@ from rest_framework_json_api import filters class MyViewset(ModelViewSet): queryset = MyModel.objects.all() serializer_class = MyModelSerializer - filter_backends = (filters.JSONAPIOrderingFilter,) + filter_backends = (filters.JSONAPIOrderingFilter, filters.JSONAPIDjangoFilter,) ``` diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index d0e8974c..203edf9f 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -208,7 +208,7 @@ def test_filter_fields_union_list(self): """ test field for a list of values(ORed): ?filter[field.in]': 'val1,val2,val3 """ - response = self.client.get(self.url, + response = self.client.get(self.url, data={'filter[headline.in]': 'CLCV2442V,XXX,BIOL3594X'}) dja_response = response.json() self.assertEqual(response.status_code, 200, @@ -225,7 +225,7 @@ def test_filter_fields_intersection(self): test fields (ANDed): ?filter[field1]': 'val1&filter[field2]'='val2 """ # - response = self.client.get(self.url, + response = self.client.get(self.url, data={'filter[headline.regex]': '^A', 'filter[body_text.icontains]': 'in'}) self.assertEqual(response.status_code, 200, @@ -285,7 +285,7 @@ def test_filter_no_brackets_equal(self): """ test for `filter` with missing filter[association] name and =value """ - response = self.client.get(self.url +'?filter') + response = self.client.get(self.url + '?filter') self.assertEqual(response.status_code, 400, msg=response.content.decode("utf-8")) dja_response = response.json() @@ -349,4 +349,3 @@ def test_filter_missing_rvalue_equal(self): dja_response = response.json() self.assertEqual(dja_response['errors'][0]['detail'], "missing filter[headline] test value") - diff --git a/example/urls_test.py b/example/urls_test.py index 8475bb79..e2b8ef27 100644 --- a/example/urls_test.py +++ b/example/urls_test.py @@ -12,9 +12,9 @@ CompanyViewset, EntryRelationshipView, EntryViewSet, - NonPaginatedEntryViewSet, FiltersetEntryViewSet, NoFiltersetEntryViewSet, + NonPaginatedEntryViewSet, ProjectViewset ) diff --git a/example/views.py b/example/views.py index 288d17e7..f3d34396 100644 --- a/example/views.py +++ b/example/views.py @@ -1,6 +1,7 @@ +import rest_framework.exceptions as exceptions import rest_framework.parsers import rest_framework.renderers -from rest_framework import exceptions +from django_filters import rest_framework as filters import rest_framework_json_api.metadata import rest_framework_json_api.parsers @@ -8,7 +9,6 @@ from rest_framework_json_api.pagination import PageNumberPagination from rest_framework_json_api.utils import format_drf_errors from rest_framework_json_api.views import ModelViewSet, RelationshipView -from django_filters import rest_framework as filters from example.models import Author, Blog, Comment, Company, Entry, Project from example.serializers import ( diff --git a/requirements-development.txt b/requirements-development.txt index e2e8aae3..d578ffb7 100644 --- a/requirements-development.txt +++ b/requirements-development.txt @@ -14,4 +14,4 @@ Sphinx sphinx_rtd_theme tox twine - +django-filter>=2.0 diff --git a/rest_framework_json_api/filters.py b/rest_framework_json_api/filters.py index cf3257ad..50e9e3ee 100644 --- a/rest_framework_json_api/filters.py +++ b/rest_framework_json_api/filters.py @@ -1,10 +1,18 @@ -from django_filters.rest_framework import DjangoFilterBackend +import re + from rest_framework.exceptions import ValidationError from rest_framework.filters import OrderingFilter from rest_framework.settings import api_settings from rest_framework_json_api.utils import format_value -import re + +try: + from django_filters.rest_framework import DjangoFilterBackend +except ImportError as e: + class DjangoFilterBackend(object): + def __init__(self): + raise ImportError("must install django-filter package to use JSONAPIDjangoFilter") + class JSONAPIOrderingFilter(OrderingFilter): """ @@ -55,19 +63,20 @@ class JSONAPIDjangoFilter(DjangoFilterBackend): filtering in which each filter is an ORM expression as implemented by DjangoFilterBackend and seems to be in alignment with an interpretation of http://jsonapi.org/recommendations/#filtering, including relationship - chaining. + chaining. It also returns a 400 error for invalid filters. Filters can be: - A resource field equality test: - `?filter[foo]=123` - - Apply other relational operators: - `?filter[foo.in]=bar,baz or ?filter[name.isnull]=true...` - - Membership in a list of values (OR): - `?filter[foo]=abc,123,zzz (foo in ['abc','123','zzz'])` + `?filter[qty]=123` + - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 + operators: + `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` + - 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[foo]=123&filter[bar]=abc,123,zzz&filter[...]` - - A related resource path for above tests: - `?filter[foo.rel.baz]=123 (where `rel` is the relationship name)` + `?filter[qty]=123&filter[name.in]=abc,123,zzz&filter[...]` + - A related resource path can be used: + `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` If you are also using rest_framework.filters.SearchFilter you'll want to customize the name of the query parameter for searching to make sure it doesn't conflict @@ -95,8 +104,7 @@ def get_filterset(self, request, queryset, view): filterset_class = self.get_filterset_class(view, queryset) kwargs = self.get_filterset_kwargs(request, queryset, view) for k in self.filter_keys: - if ((not filterset_class) - or (k not in filterset_class.base_filters)): + if ((not filterset_class) or (k not in filterset_class.base_filters)): raise ValidationError("invalid filter[{}]".format(k)) if filterset_class is None: return None diff --git a/setup.py b/setup.py index 7ff381e2..144866b9 100755 --- a/setup.py +++ b/setup.py @@ -111,7 +111,8 @@ def get_package_data(package): 'pytest-cov', 'django-polymorphic>=2.0', 'packaging', - 'django-debug-toolbar' + 'django-debug-toolbar', + 'django-filter>=2.0' ] + mock, zip_safe=False, ) From 6b0dc8c9ee7cdef4dee6e1ae9e1017e3d3bad42b Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 17:12:28 -0400 Subject: [PATCH 03/20] Forgot to add django_filters to installed_apps Not sure it's needed, but the docmentation says to do it. --- example/settings/dev.py | 1 + 1 file changed, 1 insertion(+) diff --git a/example/settings/dev.py b/example/settings/dev.py index 720d2396..c6da48a8 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -26,6 +26,7 @@ 'polymorphic', 'example', 'debug_toolbar', + 'django_filters', ] TEMPLATES = [ From d4fbf24555f1bc933f833b5f76e71def624c4823 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 17:17:29 -0400 Subject: [PATCH 04/20] backwards compatibility for py27 + django-filter --- example/views.py | 5 +++++ rest_framework_json_api/filters.py | 28 +++++++++++++++++++++++----- setup.py | 3 +-- tox.ini | 7 +++++-- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/example/views.py b/example/views.py index f3d34396..f24976fd 100644 --- a/example/views.py +++ b/example/views.py @@ -103,6 +103,7 @@ class NonPaginatedEntryViewSet(EntryViewSet): 'blog__name': rels, 'blog__tagline': rels, } + filter_fields = filterset_fields # django-filter<=1.11 (required for py27) class EntryFilter(filters.FilterSet): @@ -121,6 +122,8 @@ class FiltersetEntryViewSet(EntryViewSet): pagination_class = NoPagination filterset_fields = None filterset_class = EntryFilter + filter_fields = filterset_fields # django-filter<=1.11 + filter_class = filterset_class class NoFiltersetEntryViewSet(EntryViewSet): @@ -130,6 +133,8 @@ class NoFiltersetEntryViewSet(EntryViewSet): pagination_class = NoPagination filterset_fields = None filterset_class = None + filter_fields = filterset_fields # django-filter<=1.11 + filter_class = filterset_class class AuthorViewSet(ModelViewSet): diff --git a/rest_framework_json_api/filters.py b/rest_framework_json_api/filters.py index 50e9e3ee..7470cdfe 100644 --- a/rest_framework_json_api/filters.py +++ b/rest_framework_json_api/filters.py @@ -93,6 +93,11 @@ class JSONAPIDjangoFilter(DjangoFilterBackend): # make this regex check for it but not set `filter` regex group. filter_regex = re.compile(r'^filter(?P\W*)(?P[\w._]*)(?P\W*$)') + def validate_filter(self, keys, filterset_class): + for k in keys: + if ((not filterset_class) or (k not in filterset_class.base_filters)): + raise ValidationError("invalid filter[{}]".format(k)) + def get_filterset(self, request, queryset, view): """ Sometimes there's no filterset_class defined yet the client still @@ -103,9 +108,7 @@ def get_filterset(self, request, queryset, view): """ filterset_class = self.get_filterset_class(view, queryset) kwargs = self.get_filterset_kwargs(request, queryset, view) - for k in self.filter_keys: - if ((not filterset_class) or (k not in filterset_class.base_filters)): - raise ValidationError("invalid filter[{}]".format(k)) + self.validate_filter(self.filter_keys, filterset_class) if filterset_class is None: return None return filterset_class(**kwargs) @@ -120,13 +123,14 @@ def get_filterset_kwargs(self, request, queryset, view): data = request.query_params.copy() for qp, val in data.items(): m = self.filter_regex.match(qp) - if m and (not m['assoc'] or m['ldelim'] != '[' or m['rdelim'] != ']'): + if m and (not m.groupdict()['assoc'] or + m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): raise ValidationError("invalid filter: {}".format(qp)) if m and qp != self.search_param: if not val: raise ValidationError("missing {} test value".format(qp)) # convert jsonapi relationship path to Django ORM's __ notation - key = m['assoc'].replace('.', '__') + key = m.groupdict()['assoc'].replace('.', '__') # undo JSON_API_FORMAT_FIELD_NAMES conversion: key = format_value(key, 'underscore') data[key] = val @@ -137,3 +141,17 @@ def get_filterset_kwargs(self, request, queryset, view): 'queryset': queryset, 'request': request, } + + def filter_queryset(self, request, queryset, view): + """ + backwards compatibility to 1.1 + """ + filter_class = self.get_filter_class(view, queryset) + + kwargs = self.get_filterset_kwargs(request, queryset, view) + self.validate_filter(self.filter_keys, filter_class) + + if filter_class: + return filter_class(kwargs['data'], queryset=queryset, request=request).qs + + return queryset diff --git a/setup.py b/setup.py index 144866b9..7ff381e2 100755 --- a/setup.py +++ b/setup.py @@ -111,8 +111,7 @@ def get_package_data(package): 'pytest-cov', 'django-polymorphic>=2.0', 'packaging', - 'django-debug-toolbar', - 'django-filter>=2.0' + 'django-debug-toolbar' ] + mock, zip_safe=False, ) diff --git a/tox.ini b/tox.ini index d5cca046..e242ba2a 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,8 @@ [tox] envlist = - py{27,34,35,36}-django111-drf{36,37,38}, - py{34,35,36}-django20-drf{37,38}, + py27-df11-django111-drf{36,37,38} + py{34,35,36}-df20-django111-drf{36,37,38}, + py{34,35,36}-df20-django20-drf{37,38}, [testenv] deps = @@ -10,6 +11,8 @@ deps = drf36: djangorestframework>=3.6.3,<3.7 drf37: djangorestframework>=3.7.0,<3.8 drf38: djangorestframework>=3.8.0,<3.9 + df11: django-filter<=1.1 + df20: django-filter>=2.0 setenv = PYTHONPATH = {toxinidir} From d86d217c78af0ae8c614d555933377705a980b37 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 18:22:13 -0400 Subject: [PATCH 05/20] handle optional django-filter package - allow the example app to still run, just failing any JSONAPIDjangoFilter tests. - split the two filters into separate files for easier maintenance. --- example/settings/dev.py | 7 +- example/views.py | 20 ++- rest_framework_json_api/filters.py | 157 -------------------- rest_framework_json_api/filters/__init__.py | 2 + rest_framework_json_api/filters/filter.py | 121 +++++++++++++++ rest_framework_json_api/filters/sort.py | 44 ++++++ 6 files changed, 186 insertions(+), 165 deletions(-) delete mode 100644 rest_framework_json_api/filters.py create mode 100644 rest_framework_json_api/filters/__init__.py create mode 100644 rest_framework_json_api/filters/filter.py create mode 100644 rest_framework_json_api/filters/sort.py diff --git a/example/settings/dev.py b/example/settings/dev.py index c6da48a8..67c3a680 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -26,9 +26,14 @@ 'polymorphic', 'example', 'debug_toolbar', - 'django_filters', ] +try: + import django_filters # noqa: 401 + INSTALLED_APPS += ['django_filters'] +except ImportError: + pass + TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', diff --git a/example/views.py b/example/views.py index f24976fd..15512312 100644 --- a/example/views.py +++ b/example/views.py @@ -1,7 +1,6 @@ import rest_framework.exceptions as exceptions import rest_framework.parsers import rest_framework.renderers -from django_filters import rest_framework as filters import rest_framework_json_api.metadata import rest_framework_json_api.parsers @@ -106,13 +105,20 @@ class NonPaginatedEntryViewSet(EntryViewSet): filter_fields = filterset_fields # django-filter<=1.11 (required for py27) -class EntryFilter(filters.FilterSet): - bname = filters.CharFilter(field_name="blog__name", - lookup_expr="exact") +# While this example is used for testing with django-filter, leave the option of running it without. +# The test cases will fail, but the app will run. +try: + from django_filters import rest_framework as filters - class Meta: - model = Entry - fields = ['id', 'headline', 'body_text'] + class EntryFilter(filters.FilterSet): + bname = filters.CharFilter(field_name="blog__name", + lookup_expr="exact") + + class Meta: + model = Entry + fields = ['id', 'headline', 'body_text'] +except ImportError: + EntryFilter = None class FiltersetEntryViewSet(EntryViewSet): diff --git a/rest_framework_json_api/filters.py b/rest_framework_json_api/filters.py deleted file mode 100644 index 7470cdfe..00000000 --- a/rest_framework_json_api/filters.py +++ /dev/null @@ -1,157 +0,0 @@ -import re - -from rest_framework.exceptions import ValidationError -from rest_framework.filters import OrderingFilter -from rest_framework.settings import api_settings - -from rest_framework_json_api.utils import format_value - -try: - from django_filters.rest_framework import DjangoFilterBackend -except ImportError as e: - class DjangoFilterBackend(object): - def __init__(self): - raise ImportError("must install django-filter package to use JSONAPIDjangoFilter") - - -class JSONAPIOrderingFilter(OrderingFilter): - """ - This implements http://jsonapi.org/format/#fetching-sorting and raises 400 - if any sort field is invalid. If you prefer *not* to report 400 errors for - invalid sort fields, just use OrderingFilter with `ordering_param='sort'` - - Also applies DJA format_value() to convert (e.g. camelcase) to underscore. - (See JSON_API_FORMAT_FIELD_NAMES in docs/usage.md) - """ - ordering_param = 'sort' - - def remove_invalid_fields(self, queryset, fields, view, request): - valid_fields = [ - item[0] for item in self.get_valid_fields(queryset, view, - {'request': request}) - ] - bad_terms = [ - term for term in fields - if format_value(term.replace(".", "__").lstrip('-'), "underscore") not in valid_fields - ] - if bad_terms: - raise ValidationError('invalid sort parameter{}: {}'.format( - ('s' if len(bad_terms) > 1 else ''), ','.join(bad_terms))) - # this looks like it duplicates code above, but we want the ValidationError to report - # the actual parameter supplied while we want the fields passed to the super() to - # be correctly rewritten. - # The leading `-` has to be stripped to prevent format_value from turning it into `_`. - underscore_fields = [] - for item in fields: - item_rewritten = item.replace(".", "__") - if item_rewritten.startswith('-'): - underscore_fields.append( - '-' + format_value(item_rewritten.lstrip('-'), "underscore")) - else: - underscore_fields.append(format_value(item_rewritten, "underscore")) - - return super(JSONAPIOrderingFilter, self).remove_invalid_fields( - queryset, underscore_fields, view, request) - - -class JSONAPIDjangoFilter(DjangoFilterBackend): - """ - A Django-style ORM filter implementation, using `django-filter`. - - This is not part of the jsonapi standard per-se, other than the requirement - to use the `filter` keyword: This is an optional implementation of style of - filtering in which each filter is an ORM expression as implemented by - DjangoFilterBackend and seems to be in alignment with an interpretation of - http://jsonapi.org/recommendations/#filtering, including relationship - chaining. It also returns a 400 error for invalid filters. - - Filters can be: - - A resource field equality test: - `?filter[qty]=123` - - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 - operators: - `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` - - 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[...]` - - A related resource path can be used: - `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` - - If you are also using rest_framework.filters.SearchFilter you'll want to customize - the name of the query parameter for searching to make sure it doesn't conflict - with a field name defined in the filterset. - The recommended value is: `search_param="filter[search]"` but just make sure it's - `filter[]` to comply with the jsonapi spec requirement to use the filter - keyword. The default is "search" unless overriden but it's used here just to make sure - we don't complain about it being an invalid filter. - - TODO: find a better way to deal with search_param. - """ - search_param = api_settings.SEARCH_PARAM - # since 'filter' passes query parameter validation but is still invalid, - # make this regex check for it but not set `filter` regex group. - filter_regex = re.compile(r'^filter(?P\W*)(?P[\w._]*)(?P\W*$)') - - def validate_filter(self, keys, filterset_class): - for k in keys: - if ((not filterset_class) or (k not in filterset_class.base_filters)): - raise ValidationError("invalid filter[{}]".format(k)) - - def get_filterset(self, request, queryset, view): - """ - Sometimes there's no filterset_class defined yet the client still - requests a filter. Make sure they see an error too. This means - we have to get_filterset_kwargs() even if there's no filterset_class. - - TODO: .base_filters vs. .filters attr (not always present) - """ - filterset_class = self.get_filterset_class(view, queryset) - kwargs = self.get_filterset_kwargs(request, queryset, view) - self.validate_filter(self.filter_keys, filterset_class) - if filterset_class is None: - return None - return filterset_class(**kwargs) - - def get_filterset_kwargs(self, request, queryset, view): - """ - Turns filter[]= into = which is what - DjangoFilterBackend expects - """ - self.filter_keys = [] - # rewrite filter[field] query params to make DjangoFilterBackend work. - data = request.query_params.copy() - for qp, val in data.items(): - m = self.filter_regex.match(qp) - if m and (not m.groupdict()['assoc'] or - m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): - raise ValidationError("invalid filter: {}".format(qp)) - if m and qp != self.search_param: - if not val: - 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 - self.filter_keys.append(key) - del data[qp] - return { - 'data': data, - 'queryset': queryset, - 'request': request, - } - - def filter_queryset(self, request, queryset, view): - """ - backwards compatibility to 1.1 - """ - filter_class = self.get_filter_class(view, queryset) - - kwargs = self.get_filterset_kwargs(request, queryset, view) - self.validate_filter(self.filter_keys, filter_class) - - if filter_class: - return filter_class(kwargs['data'], queryset=queryset, request=request).qs - - return queryset diff --git a/rest_framework_json_api/filters/__init__.py b/rest_framework_json_api/filters/__init__.py new file mode 100644 index 00000000..98f93295 --- /dev/null +++ b/rest_framework_json_api/filters/__init__.py @@ -0,0 +1,2 @@ +from .sort import JSONAPIOrderingFilter # noqa: F401 +from .filter import JSONAPIDjangoFilter # noqa: F401 diff --git a/rest_framework_json_api/filters/filter.py b/rest_framework_json_api/filters/filter.py new file mode 100644 index 00000000..6d1f86ac --- /dev/null +++ b/rest_framework_json_api/filters/filter.py @@ -0,0 +1,121 @@ +import re + +from rest_framework.exceptions import ValidationError +from rest_framework.settings import api_settings + +from rest_framework_json_api.utils import format_value + +# django-filter is an optional package. Generate a dummy class if it's missing. +try: + from django_filters.rest_framework import DjangoFilterBackend +except ImportError: + class JSONAPIDjangoFilter(object): + + def filter_queryset(self, request, queryset, view): + """ + do nothing + """ + return queryset + +else: + class JSONAPIDjangoFilter(DjangoFilterBackend): + """ + A Django-style ORM filter implementation, using `django-filter`. + + This is not part of the jsonapi standard per-se, other than the requirement + to use the `filter` keyword: This is an optional implementation of style of + filtering in which each filter is an ORM expression as implemented by + DjangoFilterBackend and seems to be in alignment with an interpretation of + http://jsonapi.org/recommendations/#filtering, including relationship + chaining. It also returns a 400 error for invalid filters. + + Filters can be: + - A resource field equality test: + `?filter[qty]=123` + - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 + operators: + `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` + - 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[...]` + - A related resource path can be used: + `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` + + If you are also using rest_framework.filters.SearchFilter you'll want to customize + the name of the query parameter for searching to make sure it doesn't conflict + with a field name defined in the filterset. + The recommended value is: `search_param="filter[search]"` but just make sure it's + `filter[]` to comply with the jsonapi spec requirement to use the filter + keyword. The default is "search" unless overriden but it's used here just to make sure + we don't complain about it being an invalid filter. + + TODO: find a better way to deal with search_param. + """ + search_param = api_settings.SEARCH_PARAM + # since 'filter' passes query parameter validation but is still invalid, + # make this regex check for it but not set `filter` regex group. + filter_regex = re.compile(r'^filter(?P\W*)(?P[\w._]*)(?P\W*$)') + + def validate_filter(self, keys, filterset_class): + for k in keys: + if ((not filterset_class) or (k not in filterset_class.base_filters)): + raise ValidationError("invalid filter[{}]".format(k)) + + def get_filterset(self, request, queryset, view): + """ + Sometimes there's no filterset_class defined yet the client still + requests a filter. Make sure they see an error too. This means + we have to get_filterset_kwargs() even if there's no filterset_class. + + TODO: .base_filters vs. .filters attr (not always present) + """ + filterset_class = self.get_filterset_class(view, queryset) + kwargs = self.get_filterset_kwargs(request, queryset, view) + self.validate_filter(self.filter_keys, filterset_class) + if filterset_class is None: + return None + return filterset_class(**kwargs) + + def get_filterset_kwargs(self, request, queryset, view): + """ + Turns filter[]= into = which is what + DjangoFilterBackend expects + """ + self.filter_keys = [] + # rewrite filter[field] query params to make DjangoFilterBackend work. + data = request.query_params.copy() + for qp, val in data.items(): + m = self.filter_regex.match(qp) + if m and (not m.groupdict()['assoc'] or + m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): + raise ValidationError("invalid filter: {}".format(qp)) + if m and qp != self.search_param: + if not val: + 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 + self.filter_keys.append(key) + del data[qp] + return { + 'data': data, + 'queryset': queryset, + 'request': request, + } + + def filter_queryset(self, request, queryset, view): + """ + backwards compatibility to 1.1 + """ + filter_class = self.get_filter_class(view, queryset) + + kwargs = self.get_filterset_kwargs(request, queryset, view) + self.validate_filter(self.filter_keys, filter_class) + + if filter_class: + return filter_class(kwargs['data'], queryset=queryset, request=request).qs + + return queryset diff --git a/rest_framework_json_api/filters/sort.py b/rest_framework_json_api/filters/sort.py new file mode 100644 index 00000000..748b18bf --- /dev/null +++ b/rest_framework_json_api/filters/sort.py @@ -0,0 +1,44 @@ +from rest_framework.exceptions import ValidationError +from rest_framework.filters import OrderingFilter + +from rest_framework_json_api.utils import format_value + + +class JSONAPIOrderingFilter(OrderingFilter): + """ + This implements http://jsonapi.org/format/#fetching-sorting and raises 400 + if any sort field is invalid. If you prefer *not* to report 400 errors for + invalid sort fields, just use OrderingFilter with `ordering_param='sort'` + + Also applies DJA format_value() to convert (e.g. camelcase) to underscore. + (See JSON_API_FORMAT_FIELD_NAMES in docs/usage.md) + """ + ordering_param = 'sort' + + def remove_invalid_fields(self, queryset, fields, view, request): + valid_fields = [ + item[0] for item in self.get_valid_fields(queryset, view, + {'request': request}) + ] + bad_terms = [ + term for term in fields + if format_value(term.replace(".", "__").lstrip('-'), "underscore") not in valid_fields + ] + if bad_terms: + raise ValidationError('invalid sort parameter{}: {}'.format( + ('s' if len(bad_terms) > 1 else ''), ','.join(bad_terms))) + # this looks like it duplicates code above, but we want the ValidationError to report + # the actual parameter supplied while we want the fields passed to the super() to + # be correctly rewritten. + # The leading `-` has to be stripped to prevent format_value from turning it into `_`. + underscore_fields = [] + for item in fields: + item_rewritten = item.replace(".", "__") + if item_rewritten.startswith('-'): + underscore_fields.append( + '-' + format_value(item_rewritten.lstrip('-'), "underscore")) + else: + underscore_fields.append(format_value(item_rewritten, "underscore")) + + return super(JSONAPIOrderingFilter, self).remove_invalid_fields( + queryset, underscore_fields, view, request) From 83c4cc060a31b08c19beda646e2df097eee71ba0 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 19:00:51 -0400 Subject: [PATCH 06/20] fix travis to match new TOXENVs due to django-filter --- .travis.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9c102172..694660b9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,44 +5,44 @@ cache: pip matrix: include: - python: 2.7 - env: TOXENV=py27-django111-drf36 + env: TOXENV=py27-df11-django111-drf36 - python: 2.7 - env: TOXENV=py27-django111-drf37 + env: TOXENV=py27-df11-django111-drf37 - python: 2.7 - env: TOXENV=py27-django111-drf38 + env: TOXENV=py27-df11-django111-drf38 - python: 3.4 - env: TOXENV=py34-django111-drf36 + env: TOXENV=py34-df20-django111-drf36 - python: 3.4 - env: TOXENV=py34-django111-drf37 + env: TOXENV=py34-df20-django111-drf37 - python: 3.4 - env: TOXENV=py34-django111-drf38 + env: TOXENV=py34-df20-django111-drf38 - python: 3.4 - env: TOXENV=py34-django20-drf37 + env: TOXENV=py34-df20-django20-drf37 - python: 3.4 - env: TOXENV=py34-django20-drf38 + env: TOXENV=py34-df20-django20-drf38 - python: 3.5 - env: TOXENV=py35-django111-drf36 + env: TOXENV=py35-df20-django111-drf36 - python: 3.5 - env: TOXENV=py35-django111-drf37 + env: TOXENV=py35-df20-django111-drf37 - python: 3.5 - env: TOXENV=py35-django111-drf38 + env: TOXENV=py35-df20-django111-drf38 - python: 3.5 - env: TOXENV=py35-django20-drf37 + env: TOXENV=py35-df20-django20-drf37 - python: 3.5 - env: TOXENV=py35-django20-drf38 + env: TOXENV=py35-df20-django20-drf38 - python: 3.6 - env: TOXENV=py36-django111-drf36 + env: TOXENV=py36-df20-django111-drf36 - python: 3.6 - env: TOXENV=py36-django111-drf37 + env: TOXENV=py36-df20-django111-drf37 - python: 3.6 - env: TOXENV=py36-django111-drf38 + env: TOXENV=py36-df20-django111-drf38 - python: 3.6 - env: TOXENV=py36-django20-drf37 + env: TOXENV=py36-df20-django20-drf37 - python: 3.6 - env: TOXENV=py36-django20-drf38 +x env: TOXENV=py36-df20-django20-drf38 - python: 3.6 env: TOXENV=flake8 From f5792c1dfb84f3f26b7bd528d8761512bc7cd4bb Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 24 Aug 2018 19:04:31 -0400 Subject: [PATCH 07/20] fixed a typo --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 694660b9..8b073020 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ matrix: - python: 3.6 env: TOXENV=py36-df20-django20-drf37 - python: 3.6 -x env: TOXENV=py36-df20-django20-drf38 + env: TOXENV=py36-df20-django20-drf38 - python: 3.6 env: TOXENV=flake8 From cbc9d55aae94887908d16f89be3b8933420bc620 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Sat, 25 Aug 2018 16:28:00 -0400 Subject: [PATCH 08/20] add a warning if django-filter is missing and JSONAPIDjangoFilter is used --- rest_framework_json_api/filters/filter.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rest_framework_json_api/filters/filter.py b/rest_framework_json_api/filters/filter.py index 6d1f86ac..95b11886 100644 --- a/rest_framework_json_api/filters/filter.py +++ b/rest_framework_json_api/filters/filter.py @@ -1,4 +1,5 @@ import re +import warnings from rest_framework.exceptions import ValidationError from rest_framework.settings import api_settings @@ -11,6 +12,13 @@ except ImportError: class JSONAPIDjangoFilter(object): + def __init__(self, *args, **kwargs): + """ + complain that they need django-filter + TODO: should this be a warning or an exception? + """ + warnings.warn("must install django-filter package to use JSONAPIDjangoFilter") + def filter_queryset(self, request, queryset, view): """ do nothing From 4f2b75bc1d93b04a324babade733777b5010fa2a Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Tue, 28 Aug 2018 10:52:01 -0400 Subject: [PATCH 09/20] improve filter_regex - Had a mistake (unquoted '.') and missing '-' as an allowed character. Also '_' already in '\w' - Don't be so exhaustive in testing for invalid filters; let JSONAPIQueryValidationFilter (when available) deal with that. --- example/tests/test_filters.py | 11 ----------- rest_framework_json_api/filters/filter.py | 16 +++++++++++----- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index 203edf9f..273fe9ac 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -314,17 +314,6 @@ def test_filter_missing_right_bracket(self): self.assertEqual(dja_response['errors'][0]['detail'], "invalid filter: filter[headline") - def test_filter_incorrect_brackets(self): - """ - test for filter with incorrect brackets - """ - response = self.client.get(self.url, data={'filter{headline}': 'foobar'}) - self.assertEqual(response.status_code, 400, - msg=response.content.decode("utf-8")) - dja_response = response.json() - self.assertEqual(dja_response['errors'][0]['detail'], - "invalid filter: filter{headline}") - def test_filter_missing_rvalue(self): """ test for filter with missing value to test against diff --git a/rest_framework_json_api/filters/filter.py b/rest_framework_json_api/filters/filter.py index 95b11886..70b86cb4 100644 --- a/rest_framework_json_api/filters/filter.py +++ b/rest_framework_json_api/filters/filter.py @@ -57,13 +57,19 @@ class JSONAPIDjangoFilter(DjangoFilterBackend): `filter[]` to comply with the jsonapi spec requirement to use the filter keyword. The default is "search" unless overriden but it's used here just to make sure we don't complain about it being an invalid filter. - - TODO: find a better way to deal with search_param. """ + # TODO: find a better way to deal with search_param. search_param = api_settings.SEARCH_PARAM - # since 'filter' passes query parameter validation but is still invalid, - # make this regex check for it but not set `filter` regex group. - filter_regex = re.compile(r'^filter(?P\W*)(?P[\w._]*)(?P\W*$)') + + # Make this regex check for 'filter' as well as 'filter[...]' + # Leave other incorrect usages of 'filter' to JSONAPIQueryValidationFilter. + # See http://jsonapi.org/format/#document-member-names for allowed characters + # and http://jsonapi.org/format/#document-member-names-reserved-characters for reserved + # characters (for use in paths, lists or as delimiters). + # regex `\w` matches [a-zA-Z0-9_]. + # TODO: U+0080 and above allowed but not recommended. Leave them out for now. Fix later? + # Also, ' ' (space) is allowed within a member name but not recommended. + filter_regex = re.compile(r'^filter(?P\[?)(?P[\w\.\-]*)(?P\]?$)') def validate_filter(self, keys, filterset_class): for k in keys: From 6a8d7aef0f5a87ee0f856b959c79f122c32aac92 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Wed, 29 Aug 2018 16:20:48 -0400 Subject: [PATCH 10/20] easy changes recommended by @sliverc review --- example/requirements.txt | 2 +- example/settings/dev.py | 7 +- example/views.py | 26 +-- rest_framework_json_api/filters/filter.py | 240 ++++++++++------------ 4 files changed, 126 insertions(+), 149 deletions(-) diff --git a/example/requirements.txt b/example/requirements.txt index fe28eddc..58ce43b2 100644 --- a/example/requirements.txt +++ b/example/requirements.txt @@ -11,4 +11,4 @@ pyparsing pytz six sqlparse - +django-filter>=2.0 diff --git a/example/settings/dev.py b/example/settings/dev.py index 67c3a680..c6da48a8 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -26,14 +26,9 @@ 'polymorphic', 'example', 'debug_toolbar', + 'django_filters', ] -try: - import django_filters # noqa: 401 - INSTALLED_APPS += ['django_filters'] -except ImportError: - pass - TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', diff --git a/example/views.py b/example/views.py index 15512312..f3b9ccec 100644 --- a/example/views.py +++ b/example/views.py @@ -5,6 +5,7 @@ import rest_framework_json_api.metadata import rest_framework_json_api.parsers import rest_framework_json_api.renderers +from django_filters import rest_framework as filters from rest_framework_json_api.pagination import PageNumberPagination from rest_framework_json_api.utils import format_drf_errors from rest_framework_json_api.views import ModelViewSet, RelationshipView @@ -102,23 +103,16 @@ class NonPaginatedEntryViewSet(EntryViewSet): 'blog__name': rels, 'blog__tagline': rels, } - filter_fields = filterset_fields # django-filter<=1.11 (required for py27) + filter_fields = filterset_fields # django-filter<=1.1 (required for py27) -# While this example is used for testing with django-filter, leave the option of running it without. -# The test cases will fail, but the app will run. -try: - from django_filters import rest_framework as filters +class EntryFilter(filters.FilterSet): + bname = filters.CharFilter(field_name="blog__name", + lookup_expr="exact") - class EntryFilter(filters.FilterSet): - bname = filters.CharFilter(field_name="blog__name", - lookup_expr="exact") - - class Meta: - model = Entry - fields = ['id', 'headline', 'body_text'] -except ImportError: - EntryFilter = None + class Meta: + model = Entry + fields = ['id', 'headline', 'body_text'] class FiltersetEntryViewSet(EntryViewSet): @@ -128,7 +122,7 @@ class FiltersetEntryViewSet(EntryViewSet): pagination_class = NoPagination filterset_fields = None filterset_class = EntryFilter - filter_fields = filterset_fields # django-filter<=1.11 + filter_fields = filterset_fields # django-filter<=1.1 filter_class = filterset_class @@ -139,7 +133,7 @@ class NoFiltersetEntryViewSet(EntryViewSet): pagination_class = NoPagination filterset_fields = None filterset_class = None - filter_fields = filterset_fields # django-filter<=1.11 + filter_fields = filterset_fields # django-filter<=1.1 filter_class = filterset_class diff --git a/rest_framework_json_api/filters/filter.py b/rest_framework_json_api/filters/filter.py index 70b86cb4..42c3cfd1 100644 --- a/rest_framework_json_api/filters/filter.py +++ b/rest_framework_json_api/filters/filter.py @@ -1,135 +1,123 @@ import re -import warnings from rest_framework.exceptions import ValidationError from rest_framework.settings import api_settings +from django_filters import VERSION +from django_filters.rest_framework import DjangoFilterBackend from rest_framework_json_api.utils import format_value -# django-filter is an optional package. Generate a dummy class if it's missing. -try: - from django_filters.rest_framework import DjangoFilterBackend -except ImportError: - class JSONAPIDjangoFilter(object): - - def __init__(self, *args, **kwargs): - """ - complain that they need django-filter - TODO: should this be a warning or an exception? - """ - warnings.warn("must install django-filter package to use JSONAPIDjangoFilter") - - def filter_queryset(self, request, queryset, view): - """ - do nothing - """ - return queryset - -else: - class JSONAPIDjangoFilter(DjangoFilterBackend): + +class JSONAPIDjangoFilter(DjangoFilterBackend): + """ + A Django-style ORM filter implementation, using `django-filter`. + + This is not part of the jsonapi standard per-se, other than the requirement + to use the `filter` keyword: This is an optional implementation of style of + filtering in which each filter is an ORM expression as implemented by + DjangoFilterBackend and seems to be in alignment with an interpretation of + http://jsonapi.org/recommendations/#filtering, including relationship + chaining. It also returns a 400 error for invalid filters. + + Filters can be: + - A resource field equality test: + `?filter[qty]=123` + - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 + operators: + `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` + - 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[...]` + - A related resource path can be used: + `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` + + If you are also using rest_framework.filters.SearchFilter you'll want to customize + the name of the query parameter for searching to make sure it doesn't conflict + with a field name defined in the filterset. + The recommended value is: `search_param="filter[search]"` but just make sure it's + `filter[]` to comply with the jsonapi spec requirement to use the filter + keyword. The default is "search" unless overriden but it's used here just to make sure + we don't complain about it being an invalid filter. + """ + # TODO: find a better way to deal with search_param. + search_param = api_settings.SEARCH_PARAM + + # Make this regex check for 'filter' as well as 'filter[...]' + # Leave other incorrect usages of 'filter' to JSONAPIQueryValidationFilter. + # See http://jsonapi.org/format/#document-member-names for allowed characters + # and http://jsonapi.org/format/#document-member-names-reserved-characters for reserved + # characters (for use in paths, lists or as delimiters). + # regex `\w` matches [a-zA-Z0-9_]. + # TODO: U+0080 and above allowed but not recommended. Leave them out for now.e + # Also, ' ' (space) is allowed within a member name but not recommended. + filter_regex = re.compile(r'^filter(?P\[?)(?P[\w\.\-]*)(?P\]?$)') + + def _validate_filter(self, keys, filterset_class): + for k in keys: + if ((not filterset_class) or (k not in filterset_class.base_filters)): + raise ValidationError("invalid filter[{}]".format(k)) + + def get_filterset(self, request, queryset, view): + """ + Sometimes there's no filterset_class defined yet the client still + requests a filter. Make sure they see an error too. This means + we have to get_filterset_kwargs() even if there's no filterset_class. + + TODO: .base_filters vs. .filters attr (not always present) + """ + filterset_class = self.get_filterset_class(view, queryset) + kwargs = self.get_filterset_kwargs(request, queryset, view) + self._validate_filter(kwargs.pop('filter_keys'), filterset_class) + if filterset_class is None: + return None + return filterset_class(**kwargs) + + def get_filterset_kwargs(self, request, queryset, view): + """ + Turns filter[]= into = which is what + DjangoFilterBackend expects """ - A Django-style ORM filter implementation, using `django-filter`. - - This is not part of the jsonapi standard per-se, other than the requirement - to use the `filter` keyword: This is an optional implementation of style of - filtering in which each filter is an ORM expression as implemented by - DjangoFilterBackend and seems to be in alignment with an interpretation of - http://jsonapi.org/recommendations/#filtering, including relationship - chaining. It also returns a 400 error for invalid filters. - - Filters can be: - - A resource field equality test: - `?filter[qty]=123` - - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 - operators: - `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` - - 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[...]` - - A related resource path can be used: - `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` - - If you are also using rest_framework.filters.SearchFilter you'll want to customize - the name of the query parameter for searching to make sure it doesn't conflict - with a field name defined in the filterset. - The recommended value is: `search_param="filter[search]"` but just make sure it's - `filter[]` to comply with the jsonapi spec requirement to use the filter - keyword. The default is "search" unless overriden but it's used here just to make sure - we don't complain about it being an invalid filter. + filter_keys = [] + # rewrite filter[field] query params to make DjangoFilterBackend work. + data = request.query_params.copy() + for qp, val in data.items(): + m = self.filter_regex.match(qp) + if m and (not m.groupdict()['assoc'] or + m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): + raise ValidationError("invalid filter: {}".format(qp)) + if m and qp != self.search_param: + if not val: + 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 + filter_keys.append(key) + del data[qp] + return { + 'data': data, + 'queryset': queryset, + 'request': request, + 'filter_keys': filter_keys, + } + + def filter_queryset(self, request, queryset, view): """ - # TODO: find a better way to deal with search_param. - search_param = api_settings.SEARCH_PARAM - - # Make this regex check for 'filter' as well as 'filter[...]' - # Leave other incorrect usages of 'filter' to JSONAPIQueryValidationFilter. - # See http://jsonapi.org/format/#document-member-names for allowed characters - # and http://jsonapi.org/format/#document-member-names-reserved-characters for reserved - # characters (for use in paths, lists or as delimiters). - # regex `\w` matches [a-zA-Z0-9_]. - # TODO: U+0080 and above allowed but not recommended. Leave them out for now. Fix later? - # Also, ' ' (space) is allowed within a member name but not recommended. - filter_regex = re.compile(r'^filter(?P\[?)(?P[\w\.\-]*)(?P\]?$)') - - def validate_filter(self, keys, filterset_class): - for k in keys: - if ((not filterset_class) or (k not in filterset_class.base_filters)): - raise ValidationError("invalid filter[{}]".format(k)) - - def get_filterset(self, request, queryset, view): - """ - Sometimes there's no filterset_class defined yet the client still - requests a filter. Make sure they see an error too. This means - we have to get_filterset_kwargs() even if there's no filterset_class. - - TODO: .base_filters vs. .filters attr (not always present) - """ - filterset_class = self.get_filterset_class(view, queryset) - kwargs = self.get_filterset_kwargs(request, queryset, view) - self.validate_filter(self.filter_keys, filterset_class) - if filterset_class is None: - return None - return filterset_class(**kwargs) - - def get_filterset_kwargs(self, request, queryset, view): - """ - Turns filter[]= into = which is what - DjangoFilterBackend expects - """ - self.filter_keys = [] - # rewrite filter[field] query params to make DjangoFilterBackend work. - data = request.query_params.copy() - for qp, val in data.items(): - m = self.filter_regex.match(qp) - if m and (not m.groupdict()['assoc'] or - m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): - raise ValidationError("invalid filter: {}".format(qp)) - if m and qp != self.search_param: - if not val: - 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 - self.filter_keys.append(key) - del data[qp] - return { - 'data': data, - 'queryset': queryset, - 'request': request, - } - - def filter_queryset(self, request, queryset, view): - """ - backwards compatibility to 1.1 - """ - filter_class = self.get_filter_class(view, queryset) - - kwargs = self.get_filterset_kwargs(request, queryset, view) - self.validate_filter(self.filter_keys, filter_class) - - if filter_class: - return filter_class(kwargs['data'], queryset=queryset, request=request).qs - - return queryset + Backwards compatibility to 1.1 (required for Python 2.7) + In 1.1 filter_queryset does not call get_filterset or get_filterset_kwargs. + """ + # TODO: remove when Python 2.7 support is deprecated + if VERSION >= (2, 0, 0): + return super(JSONAPIDjangoFilter, self).filter_queryset(request, queryset, view) + + filter_class = self.get_filter_class(view, queryset) + + kwargs = self.get_filterset_kwargs(request, queryset, view) + self._validate_filter(kwargs.pop('filter_keys'), filter_class) + + if filter_class: + return filter_class(kwargs['data'], queryset=queryset, request=request).qs + + return queryset From db9e1f9b1b8fa7b961767fd15103b094bb4900e6 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Wed, 29 Aug 2018 16:38:26 -0400 Subject: [PATCH 11/20] resolve @sliverc review method of using optional django-filter. See https://github.com/django-json-api/django-rest-framework-json-api/pull/466#discussion_r213599037 --- rest_framework_json_api/filters/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/filters/__init__.py b/rest_framework_json_api/filters/__init__.py index 98f93295..66c78855 100644 --- a/rest_framework_json_api/filters/__init__.py +++ b/rest_framework_json_api/filters/__init__.py @@ -1,2 +1,6 @@ +import pkgutil from .sort import JSONAPIOrderingFilter # noqa: F401 -from .filter import JSONAPIDjangoFilter # noqa: F401 +# If django-filter is not installed, no-op. +if pkgutil.find_loader('django_filters') is not None: + from .filter import JSONAPIDjangoFilter # noqa: F401 +del pkgutil From 51b99464a7061f0c542c5eca42dfcf9fffc25467 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 30 Aug 2018 14:55:53 -0400 Subject: [PATCH 12/20] rename JSONAPIDjangoFilter to DjangoFilterBackend. Per discussion about naming, the idea is that it should be easy to updgrade from DRF to DJA by simply changing some imports, retaining the same DRF (or in this case, django-filter) class names that are extended by DJA. see https://github.com/django-json-api/django-rest-framework-json-api/issues/467#issuecomment-417338257 --- docs/usage.md | 8 ++++---- example/settings/dev.py | 2 +- rest_framework_json_api/filters/__init__.py | 2 +- .../filters/{filter.py => django_filter.py} | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename rest_framework_json_api/filters/{filter.py => django_filter.py} (98%) diff --git a/docs/usage.md b/docs/usage.md index 010c58d4..112317fb 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -34,7 +34,7 @@ REST_FRAMEWORK = { 'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata', 'DEFAULT_FILTER_BACKENDS': ( 'rest_framework_json_api.filters.JSONAPIOrderingFilter', - 'rest_framework_json_api.filters.JSONAPIDjangoFilter', + 'rest_framework_json_api.filters.DjangoFilterBackend', ), 'TEST_REQUEST_RENDERER_CLASSES': ( 'rest_framework_json_api.renderers.JSONRenderer', @@ -119,8 +119,8 @@ field name and the other two are not valid: If you want to silently ignore bad sort fields, just use `rest_framework.filters.OrderingFilter` and set `ordering_param` to `sort`. -#### `JSONAPIDjangoFilter` -`JSONAPIDjangoFilter` implements a Django ORM-style [JSON:API `filter`](http://jsonapi.org/format/#fetching-filtering) +#### `DjangoFilterBackend` +`DjangoFilterBackend` implements a Django ORM-style [JSON:API `filter`](http://jsonapi.org/format/#fetching-filtering) using the [django-filter](https://django-filter.readthedocs.io/) package. This filter is not part of the JSON:API standard per-se, other than the requirement @@ -176,7 +176,7 @@ from rest_framework_json_api import filters class MyViewset(ModelViewSet): queryset = MyModel.objects.all() serializer_class = MyModelSerializer - filter_backends = (filters.JSONAPIOrderingFilter, filters.JSONAPIDjangoFilter,) + filter_backends = (filters.JSONAPIOrderingFilter, filters.DjangoFilterBackend,) ``` diff --git a/example/settings/dev.py b/example/settings/dev.py index c6da48a8..e30ae10f 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -91,7 +91,7 @@ 'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata', 'DEFAULT_FILTER_BACKENDS': ( 'rest_framework_json_api.filters.JSONAPIOrderingFilter', - 'rest_framework_json_api.filters.JSONAPIDjangoFilter', + 'rest_framework_json_api.filters.DjangoFilterBackend', ), 'TEST_REQUEST_RENDERER_CLASSES': ( 'rest_framework_json_api.renderers.JSONRenderer', diff --git a/rest_framework_json_api/filters/__init__.py b/rest_framework_json_api/filters/__init__.py index 66c78855..7083c65d 100644 --- a/rest_framework_json_api/filters/__init__.py +++ b/rest_framework_json_api/filters/__init__.py @@ -2,5 +2,5 @@ from .sort import JSONAPIOrderingFilter # noqa: F401 # If django-filter is not installed, no-op. if pkgutil.find_loader('django_filters') is not None: - from .filter import JSONAPIDjangoFilter # noqa: F401 + from .django_filter import DjangoFilterBackend # noqa: F401 del pkgutil diff --git a/rest_framework_json_api/filters/filter.py b/rest_framework_json_api/filters/django_filter.py similarity index 98% rename from rest_framework_json_api/filters/filter.py rename to rest_framework_json_api/filters/django_filter.py index 42c3cfd1..4681c2f2 100644 --- a/rest_framework_json_api/filters/filter.py +++ b/rest_framework_json_api/filters/django_filter.py @@ -8,7 +8,7 @@ from rest_framework_json_api.utils import format_value -class JSONAPIDjangoFilter(DjangoFilterBackend): +class DjangoFilterBackend(DjangoFilterBackend): """ A Django-style ORM filter implementation, using `django-filter`. @@ -110,7 +110,7 @@ def filter_queryset(self, request, queryset, view): """ # TODO: remove when Python 2.7 support is deprecated if VERSION >= (2, 0, 0): - return super(JSONAPIDjangoFilter, self).filter_queryset(request, queryset, view) + return super(DjangoFilterBackend, self).filter_queryset(request, queryset, view) filter_class = self.get_filter_class(view, queryset) From cfe89ea7cfad6e01f5dfc311ff37c5cf584170f7 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 30 Aug 2018 15:06:59 -0400 Subject: [PATCH 13/20] Revert "deprecated JsonApi paginators class prefix to JSONAPI prefix for consistency (#463)" This reverts commit e6290af5c34bf1760cb8c79fb8648b58e9336685. --- CHANGELOG.md | 2 -- README.rst | 2 +- docs/usage.md | 16 +++++----- example/tests/unit/test_pagination.py | 16 ++-------- example/views.py | 6 ++-- rest_framework_json_api/pagination.py | 43 ++++----------------------- 6 files changed, 21 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b61f677e..cc1f2f7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,6 @@ * Add `HyperlinkedRelatedField` and `SerializerMethodHyperlinkedRelatedField`. See [usage docs](docs/usage.md#related-fields) * Add related urls support. See [usage docs](docs/usage.md#related-urls) * Replaced binary `drf_example` sqlite3 db with a [fixture](example/fixtures/drf_example.yaml). See [usage docs](docs/usage.md#running-the-example-app). -* For naming consistency, renamed new `JsonApi`-prefix pagination classes to `JSONAPI`-prefix. - * Deprecates `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` * Add optional [jsonapi-style](http://jsonapi.org/format/) filter backends. See [usage docs](docs/usage.md#filter-backends) v2.5.0 - Released July 11, 2018 diff --git a/README.rst b/README.rst index 7d345185..6fa4aee5 100644 --- a/README.rst +++ b/README.rst @@ -161,7 +161,7 @@ override ``settings.REST_FRAMEWORK`` 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.JSONAPIPageNumberPagination', + 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', diff --git a/docs/usage.md b/docs/usage.md index 112317fb..e7d3543a 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -16,7 +16,7 @@ REST_FRAMEWORK = { 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.JSONAPIPageNumberPagination', + 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', @@ -59,15 +59,15 @@ 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 (number of items per page). It can be configured with the following attributes: +- `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 to specify the size. - `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 a given number of items (the limit). +- `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]`). - `limit_query_param` (default `page[limit]`). @@ -78,14 +78,14 @@ Two pagination classes are available: These examples show how to configure the parameters to use non-standard names and different limits: ```python -from rest_framework_json_api.pagination import JSONAPIPageNumberPagination, JSONAPILimitOffsetPagination +from rest_framework_json_api.pagination import JsonApiPageNumberPagination, JsonApiLimitOffsetPagination -class MyPagePagination(JSONAPIPageNumberPagination): +class MyPagePagination(JsonApiPageNumberPagination): page_query_param = 'page_number' page_size_query_param = 'page_size' max_page_size = 1000 -class MyLimitPagination(JSONAPILimitOffsetPagination): +class MyLimitPagination(JsonApiLimitOffsetPagination): offset_query_param = 'offset' limit_query_param = 'limit' max_limit = None diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index 5fdcade6..f6e95db0 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -13,11 +13,11 @@ class TestLimitOffset: """ - Unit tests for `pagination.JSONAPILimitOffsetPagination`. + Unit tests for `pagination.JsonApiLimitOffsetPagination`. """ def setup(self): - class ExamplePagination(pagination.JSONAPILimitOffsetPagination): + class ExamplePagination(pagination.JsonApiLimitOffsetPagination): default_limit = 10 max_limit = 15 @@ -85,18 +85,13 @@ def test_limit_offset_deprecation(self): assert len(record) == 1 assert 'LimitOffsetPagination' in str(record[0].message) - with pytest.warns(DeprecationWarning) as record: - pagination.JsonApiLimitOffsetPagination() - assert len(record) == 1 - assert 'JsonApiLimitOffsetPagination' in str(record[0].message) - # TODO: This test fails under py27 but it's not clear why so just leave it out for now. @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails for unknown reason") class TestPageNumber: """ - Unit tests for `pagination.JSONAPIPageNumberPagination`. + Unit tests for `pagination.JsonApiPageNumberPagination`. TODO: add unit tests for changing query parameter names, limits, etc. """ def test_page_number_deprecation(self): @@ -104,8 +99,3 @@ def test_page_number_deprecation(self): pagination.PageNumberPagination() assert len(record) == 1 assert 'PageNumberPagination' in str(record[0].message) - - with pytest.warns(DeprecationWarning) as record: - pagination.JsonApiPageNumberPagination() - assert len(record) == 1 - assert 'JsonApiPageNumberPagination' in str(record[0].message) diff --git a/example/views.py b/example/views.py index f3b9ccec..98907902 100644 --- a/example/views.py +++ b/example/views.py @@ -35,7 +35,7 @@ def get_object(self): return super(BlogViewSet, self).get_object() -class JSONAPIViewSet(ModelViewSet): +class JsonApiViewSet(ModelViewSet): """ This is an example on how to configure DRF-jsonapi from within a class. It allows using DRF-jsonapi alongside @@ -59,12 +59,12 @@ def handle_exception(self, exc): exc.status_code = HTTP_422_UNPROCESSABLE_ENTITY # exception handler can't be set on class so you have to # override the error response in this method - response = super(JSONAPIViewSet, self).handle_exception(exc) + response = super(JsonApiViewSet, self).handle_exception(exc) context = self.get_exception_handler_context() return format_drf_errors(response, context, exc) -class BlogCustomViewSet(JSONAPIViewSet): +class BlogCustomViewSet(JsonApiViewSet): queryset = Blog.objects.all() serializer_class = BlogSerializer diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index b150aa83..00873c99 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -9,7 +9,7 @@ from rest_framework.views import Response -class JSONAPIPageNumberPagination(PageNumberPagination): +class JsonApiPageNumberPagination(PageNumberPagination): """ A json-api compatible pagination format """ @@ -50,7 +50,7 @@ def get_paginated_response(self, data): }) -class JSONAPILimitOffsetPagination(LimitOffsetPagination): +class JsonApiLimitOffsetPagination(LimitOffsetPagination): """ A limit/offset based style. For example: http://api.example.org/accounts/?page[limit]=100 @@ -100,23 +100,7 @@ def get_paginated_response(self, data): }) -class JsonApiPageNumberPagination(JSONAPIPageNumberPagination): - """ - Deprecated due to desire to use `JSONAPI` prefix for all classes. - """ - page_query_param = 'page' - page_size_query_param = 'page_size' - - def __init__(self): - warnings.warn( - 'JsonApiPageNumberPagination is deprecated. Use JSONAPIPageNumberPagination ' - 'or create custom pagination. See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) - super(JsonApiPageNumberPagination, self).__init__() - - -class PageNumberPagination(JSONAPIPageNumberPagination): +class PageNumberPagination(JsonApiPageNumberPagination): """ Deprecated paginator that uses different query parameters """ @@ -125,29 +109,14 @@ class PageNumberPagination(JSONAPIPageNumberPagination): def __init__(self): warnings.warn( - 'PageNumberPagination is deprecated. Use JSONAPIPageNumberPagination ' + 'PageNumberPagination is deprecated. Use JsonApiPageNumberPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) super(PageNumberPagination, self).__init__() -class JsonApiLimitOffsetPagination(JSONAPILimitOffsetPagination): - """ - Deprecated due to desire to use `JSONAPI` prefix for all classes. - """ - max_limit = None - - def __init__(self): - warnings.warn( - 'JsonApiLimitOffsetPagination is deprecated. Use JSONAPILimitOffsetPagination ' - 'or create custom pagination. See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) - super(JsonApiLimitOffsetPagination, self).__init__() - - -class LimitOffsetPagination(JSONAPILimitOffsetPagination): +class LimitOffsetPagination(JsonApiLimitOffsetPagination): """ Deprecated paginator that uses a different max_limit """ @@ -155,7 +124,7 @@ class LimitOffsetPagination(JSONAPILimitOffsetPagination): def __init__(self): warnings.warn( - 'LimitOffsetPagination is deprecated. Use JSONAPILimitOffsetPagination ' + 'LimitOffsetPagination is deprecated. Use JsonApiLimitOffsetPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) From 00dcf526a4ad0789c2e3beba16b507d31b19930b Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 30 Aug 2018 19:29:24 -0400 Subject: [PATCH 14/20] revert JSONAPI prefix from paginators - see https://github.com/django-json-api/django-rest-framework-json-api/issues/467#issuecomment-417338257 - JSONAPIPageNumberPagination goes back to PageNumberPagination - JSONAPILimitOffsetPagination goas back to LimitOffsetPagination - requires new setting: `JSON_API_STANDARD_PAGINATION=True` to get `page[number]` and `page[size]` instead of `page` and `page_size`. This is a breaking change no matter which is the default value. --- CHANGELOG.md | 3 + README.rst | 22 +++++-- docs/usage.md | 54 ++++++++++------ example/tests/unit/test_pagination.py | 93 ++++++++++++++++++++++++--- rest_framework_json_api/pagination.py | 40 ++++++++---- rest_framework_json_api/settings.py | 1 + 6 files changed, 164 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc1f2f7f..655e9bb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ * Add related urls support. See [usage docs](docs/usage.md#related-urls) * Replaced binary `drf_example` sqlite3 db with a [fixture](example/fixtures/drf_example.yaml). See [usage docs](docs/usage.md#running-the-example-app). * Add optional [jsonapi-style](http://jsonapi.org/format/) filter backends. See [usage docs](docs/usage.md#filter-backends) +* **breaking change**: Reverted v2.5.0's `JsonApiPagenumberPagination` back to `PageNumberPagination` and `JsonApiLimitOffsetPagination` back to `LimitOffsetPagination` + You have to set `JSON_API_STANDARD_PAGINATION = True` (default is False) to get the "new" JSON:API-style query parameters. This retains the deprecated + behavior for now. See [usage docs](docs/usage.md#pagination). v2.5.0 - Released July 11, 2018 diff --git a/README.rst b/README.rst index 6fa4aee5..ff2307ea 100644 --- a/README.rst +++ b/README.rst @@ -118,10 +118,15 @@ Running the example app :: - $ git clone https://github.com/django-json-api/django-rest-framework-json-api.git - $ cd django-rest-framework-json-api - $ pip install -e . - $ django-admin.py runserver --settings=example.settings + $ git clone https://github.com/django-json-api/django-rest-framework-json-api.git + $ cd django-rest-framework-json-api + $ python3 -m venv env + $ source env/bin/activate + $ pip install -r example/requirements.txt + $ pip install -e . + $ django-admin migrate --settings=example.settings + $ django-admin loaddata drf_example --settings=example.settings + $ django-admin runserver --settings=example.settings Browse to http://localhost:8000 @@ -136,7 +141,7 @@ installed and activated: $ pip install -e . $ pip install -r requirements-development.txt - $ py.test + $ DJANGO_SETTINGS_MODULE=example.settings.test py.test ----- @@ -161,7 +166,7 @@ override ``settings.REST_FRAMEWORK`` 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', + 'rest_framework_json_api.pagination.PageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', @@ -173,7 +178,8 @@ override ``settings.REST_FRAMEWORK`` ), 'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata', 'DEFAULT_FILTER_BACKENDS': ( - 'rest_framework_json_api.backends.JSONAPIOrderingFilter', + 'rest_framework_json_api.filters.JSONAPIOrderingFilter', + 'rest_framework_json_api.filters.DjangoFilterBackend', ), 'TEST_REQUEST_RENDERER_CLASSES': ( 'rest_framework_json_api.renderers.JSONRenderer', @@ -181,4 +187,6 @@ override ``settings.REST_FRAMEWORK`` 'TEST_REQUEST_DEFAULT_FORMAT': 'vnd.api+json' } + JSON_API_STANDARD_PAGINATION = True + This package provides much more including automatic inflection of JSON keys, extra top level data (using nested serializers), relationships, links, and handy shortcuts like MultipleIDMixin. Read more at http://django-rest-framework-json-api.readthedocs.org/ diff --git a/docs/usage.md b/docs/usage.md index e7d3543a..667664f1 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -4,7 +4,7 @@ The DJA package implements a custom renderer, parser, exception handler, query filter backends, and pagination. To get started enable the pieces in `settings.py` that you want to use. -Many features of the [JSON:API](http://jsonapi.org/format) format standard have been implemented using +Many features of the [JSON:API](http://jsonapi.org/format) format standard have been implemented using Mixin classes in `serializers.py`. The easiest way to make use of those features is to import ModelSerializer variants from `rest_framework_json_api` instead of the usual `rest_framework` @@ -16,7 +16,7 @@ REST_FRAMEWORK = { 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', + 'rest_framework_json_api.pagination.PageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', @@ -41,6 +41,8 @@ REST_FRAMEWORK = { ), 'TEST_REQUEST_DEFAULT_FORMAT': 'vnd.api+json' } + +JSON_API_STANDARD_PAGINATION = True ``` ### Pagination @@ -50,6 +52,8 @@ DJA pagination is based on [DRF pagination](https://www.django-rest-framework.or When pagination is enabled, the renderer will return a `meta` object with record count and a `links` object with the next, previous, first, and last links. +Optional query parameters can also be provided to customize the page size or offset limit. + #### Configuring the Pagination Style Pagination style can be set on a particular viewset with the `pagination_class` attribute or by default for all viewsets @@ -58,36 +62,46 @@ by setting `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` and by setting `REST_FRA You can configure fixed values for the page size or limit -- or allow the client to choose the size or limit via query parameters. +You should also set `JSON_API_STANDARD_PAGINATION=True` in order to get up to date JSON:API-style default +query parameters and pagination size limits. The default value (False) sets deprecated values that will +be eliminated in the future. See below. + Two pagination classes are available: -- `JsonApiPageNumberPagination` breaks a response up into pages that start at a given page number with a given size +- `PageNumberPagination` 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 - to specify the size. - - `max_page_size` (default `100`) enforces an upper bound on the `page_size_query_param`. + - `page_size` (default `REST_FRAMEWORK['PAGE_SIZE']`) number of items per page. + - `max_page_size` (default `100`) enforces an upper bound on the page size. 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 + - `page_query_param` (if `JSON_API_STANDARD_PAGINATION is True`: `page[number]` otherwise `page`) is the query + parameter for the page number. + - `page_size_query_param` (if `JSON_API_STANDARD_PAGINATION is True`: `page[size]` otherwise `page_size`) is the + query parameter for the page size. No more than `max_page_size` items will be returned. + Set this to `None` if you don't want to allow the client to specify the size. +- `LimitOffsetPagination` 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]`). - - `limit_query_param` (default `page[limit]`). - - `max_limit` (default `100`) enforces an upper bound on the limit. - Set it to `None` if you don't want to enforce an upper bound. - + - `default_limit` (default `REST_FRAMEWORK['PAGE_SIZE']`) is the number of items per page. + - `offset_query_param` (default `page[offset]`) is the query parameter that sets the offset (item number) to return. + - `limit_query_param` (default `page[limit]`) is the query parameter that sets the limit (number of items per page). + No more than `max_limit` items will be returned. + - `max_limit` (if `JSON_API_STANDARD_PAGINATION is True`: `100` otherwise `None`) enforces an upper bound on the + limit. Set it to `None` if you don't want to enforce an upper bound. These examples show how to configure the parameters to use non-standard names and different limits: ```python -from rest_framework_json_api.pagination import JsonApiPageNumberPagination, JsonApiLimitOffsetPagination +from rest_framework_json_api.pagination import PageNumberPagination, LimitOffsetPagination -class MyPagePagination(JsonApiPageNumberPagination): +class MyPagePagination(PageNumberPagination): page_query_param = 'page_number' - page_size_query_param = 'page_size' + page_size_query_param = 'page_length' + page_size = 3 max_page_size = 1000 -class MyLimitPagination(JsonApiLimitOffsetPagination): +class MyLimitPagination(LimitOffsetPagination): offset_query_param = 'offset' limit_query_param = 'limit' + default_limit = 3 max_limit = None ``` @@ -146,7 +160,7 @@ If you are also using [`rest_framework.filters.SearchFilter`](https://django-res (which performs single parameter searchs across multiple fields) you'll want to customize the name of the query parameter for searching to make sure it doesn't conflict with a field name defined in the filterset. The recommended value is: `search_param="filter[search]"` but just make sure it's -`filter[_something_]` to comply with the jsonapi spec requirement to use the filter +`filter[_something_]` to comply with the JSON:API spec requirement to use the filter keyword. The default is "search" unless overriden. The filter returns a `400 Bad Request` error for invalid filter query parameters as in this example @@ -445,7 +459,7 @@ class OrderSerializer(serializers.ModelSerializer): ``` -In the [JSON API spec](http://jsonapi.org/format/#document-resource-objects), +In the [JSON:API spec](http://jsonapi.org/format/#document-resource-objects), relationship objects contain links to related objects. To make this work on a serializer we need to tell the `ResourceRelatedField` about the corresponding view. Use the `HyperlinkedModelSerializer` and instantiate @@ -583,7 +597,7 @@ class OrderSerializer(serializers.HyperlinkedModelSerializer): ### RelationshipView `rest_framework_json_api.views.RelationshipView` is used to build relationship views (see the -[JSON API spec](http://jsonapi.org/format/#fetching-relationships)). +[JSON:API spec](http://jsonapi.org/format/#fetching-relationships)). The `self` link on a relationship object should point to the corresponding relationship view. diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index f6e95db0..ea0bd8ac 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -13,11 +13,11 @@ class TestLimitOffset: """ - Unit tests for `pagination.JsonApiLimitOffsetPagination`. + Unit tests for `pagination.LimitOffsetPagination`. """ def setup(self): - class ExamplePagination(pagination.JsonApiLimitOffsetPagination): + class ExamplePagination(pagination.LimitOffsetPagination): default_limit = 10 max_limit = 15 @@ -81,9 +81,9 @@ def test_valid_offset_limit(self): def test_limit_offset_deprecation(self): with pytest.warns(DeprecationWarning) as record: - pagination.LimitOffsetPagination() + pagination.JsonApiLimitOffsetPagination() assert len(record) == 1 - assert 'LimitOffsetPagination' in str(record[0].message) + assert 'JsonApiLimitOffsetPagination' in str(record[0].message) # TODO: This test fails under py27 but it's not clear why so just leave it out for now. @@ -91,11 +91,88 @@ def test_limit_offset_deprecation(self): reason="python2.7 fails for unknown reason") class TestPageNumber: """ - Unit tests for `pagination.JsonApiPageNumberPagination`. - TODO: add unit tests for changing query parameter names, limits, etc. + Unit tests for `pagination.PageNumberPagination`. + TODO: add unit tests for changing query parameter names, max limits, etc. """ + + def setup(self): + class ExamplePagination(pagination.PageNumberPagination): + default_page_size = 10 + max_page_size = 15 + + self.pagination = ExamplePagination() + self.queryset = range(1, 401) + self.base_url = 'http://testserver/' + + def paginate_queryset(self, request): + return list(self.pagination.paginate_queryset(self.queryset, request)) + + def get_paginated_content(self, queryset): + response = self.pagination.get_paginated_response(queryset) + return response.data + + def get_test_request(self, arguments): + return Request(factory.get('/', arguments)) + + def test_valid_page_size(self): + """ + Basic test, assumes page and size are given. + """ + page = 10 + size = 5 + count = len(self.queryset) + last_page = (count // size) + next_page = 11 + prev_page = 9 + + request = self.get_test_request({ + self.pagination.page_size_query_param: size, + self.pagination.page_query_param: page + }) + base_url = replace_query_param(self.base_url, self.pagination.page_size_query_param, size) + first_url = replace_query_param(base_url, self.pagination.page_query_param, 1) + last_url = replace_query_param(base_url, self.pagination.page_query_param, last_page) + next_url = replace_query_param(base_url, self.pagination.page_query_param, next_page) + prev_url = replace_query_param(base_url, self.pagination.page_query_param, prev_page) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + + expected_content = { + 'results': list(range((page - 1) * size + 1, (next_page - 1) * size + 1)), + 'links': OrderedDict([ + ('first', first_url), + ('last', last_url), + ('next', next_url), + ('prev', prev_url), + ]), + 'meta': { + 'pagination': OrderedDict([ + ('page', page), + ('pages', count // size), + ('count', count), + ]) + } + } + + assert queryset == list(range((page - 1) * size + 1, (next_page - 1) * size + 1)) + assert content == expected_content + + def test_page_size_too_big(self): + """ + ask for a page size that's more than the max + """ + page = 3 + size = 20 + request = self.get_test_request({ + self.pagination.page_size_query_param: size, + self.pagination.page_query_param: page + }) + queryset = self.paginate_queryset(request) + assert len(queryset) == self.pagination.max_page_size + assert len(queryset) < size + def test_page_number_deprecation(self): with pytest.warns(DeprecationWarning) as record: - pagination.PageNumberPagination() + pagination.JsonApiPageNumberPagination() assert len(record) == 1 - assert 'PageNumberPagination' in str(record[0].message) + assert 'JsonApiPageNumberPagination' in str(record[0].message) diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index 00873c99..e2a8cf02 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -8,13 +8,24 @@ from rest_framework.utils.urls import remove_query_param, replace_query_param from rest_framework.views import Response +from rest_framework_json_api.settings import json_api_settings -class JsonApiPageNumberPagination(PageNumberPagination): + +class PageNumberPagination(PageNumberPagination): """ A json-api compatible pagination format + + An older version of this used `page` and `page_size` so + use a setting to enable the "standard" query param names. """ - page_query_param = 'page[number]' - page_size_query_param = 'page[size]' + if json_api_settings.STANDARD_PAGINATION: + page_query_param = 'page[number]' + page_size_query_param = 'page[size]' + else: + page_query_param = 'page' + page_size_query_param = 'page_size' + warnings.warn("'page' and 'page_size' parameters are deprecated. " + "Set JSON_API_STANDARD_PAGINATION=True", DeprecationWarning) max_page_size = 100 def build_link(self, index): @@ -50,7 +61,7 @@ def get_paginated_response(self, data): }) -class JsonApiLimitOffsetPagination(LimitOffsetPagination): +class LimitOffsetPagination(LimitOffsetPagination): """ A limit/offset based style. For example: http://api.example.org/accounts/?page[limit]=100 @@ -58,7 +69,11 @@ class JsonApiLimitOffsetPagination(LimitOffsetPagination): """ limit_query_param = 'page[limit]' offset_query_param = 'page[offset]' - max_limit = 100 + if json_api_settings.STANDARD_PAGINATION: + max_limit = 100 + else: + warnings.warn("'max_limit = None' is deprecated. " + "Set JSON_API_STANDARD_PAGINATION=True", DeprecationWarning) def get_last_link(self): if self.count == 0: @@ -100,31 +115,28 @@ def get_paginated_response(self, data): }) -class PageNumberPagination(JsonApiPageNumberPagination): +class JsonApiPageNumberPagination(PageNumberPagination): """ - Deprecated paginator that uses different query parameters + Changed our minds about the naming scheme. Removed JsonApi prefx. """ - page_query_param = 'page' - page_size_query_param = 'page_size' def __init__(self): warnings.warn( - 'PageNumberPagination is deprecated. Use JsonApiPageNumberPagination ' + 'JsonApiPageNumberPagination is deprecated. Use PageNumberPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) super(PageNumberPagination, self).__init__() -class LimitOffsetPagination(JsonApiLimitOffsetPagination): +class JsonApiLimitOffsetPagination(LimitOffsetPagination): """ - Deprecated paginator that uses a different max_limit + Changed our minds about the naming scheme. Removed JsonApi prefx. """ - max_limit = None def __init__(self): warnings.warn( - 'LimitOffsetPagination is deprecated. Use JsonApiLimitOffsetPagination ' + 'JsonApiLimitOffsetPagination is deprecated. Use LimitOffsetPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) diff --git a/rest_framework_json_api/settings.py b/rest_framework_json_api/settings.py index 6c7eeffe..2c3cd022 100644 --- a/rest_framework_json_api/settings.py +++ b/rest_framework_json_api/settings.py @@ -14,6 +14,7 @@ 'FORMAT_TYPES': False, 'PLURALIZE_TYPES': False, 'UNIFORM_EXCEPTIONS': False, + 'STANDARD_PAGINATION': False, # deprecated settings to be removed in the future 'FORMAT_KEYS': None, From fb17d784bd04a89abdf584c2c78523540497c57d Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 30 Aug 2018 19:44:56 -0400 Subject: [PATCH 15/20] override JSON_API_STANDARD_PAGINATION=False for test suite for now --- example/settings/dev.py | 1 + example/settings/test.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/example/settings/dev.py b/example/settings/dev.py index e30ae10f..56613856 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -68,6 +68,7 @@ JSON_API_FORMAT_FIELD_NAMES = 'camelize' JSON_API_FORMAT_TYPES = 'camelize' +JSON_API_STANDARD_PAGINATION = True REST_FRAMEWORK = { 'PAGE_SIZE': 5, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', diff --git a/example/settings/test.py b/example/settings/test.py index bbf6e400..0cbb6b6b 100644 --- a/example/settings/test.py +++ b/example/settings/test.py @@ -12,6 +12,9 @@ JSON_API_FIELD_NAMES = 'camelize' JSON_API_FORMAT_TYPES = 'camelize' JSON_API_PLURALIZE_TYPES = True +# TODO: 13 tests fail when this is True because they use `page` and `page_size`. Fix them. +JSON_API_STANDARD_PAGINATION = False + REST_FRAMEWORK.update({ 'PAGE_SIZE': 1, }) From 6145d33749389b40e864214067fcd3ccdca371b1 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 6 Sep 2018 13:52:17 -0400 Subject: [PATCH 16/20] Revert "revert JSONAPI prefix from paginators" We've decided to stick with JsonApi (as found in release 2.5.0) as the paginator prefix for now. This reverts commit 00dcf526a4ad0789c2e3beba16b507d31b19930b. --- README.rst | 19 ++---- docs/usage.md | 54 ++++++---------- example/tests/unit/test_pagination.py | 93 +++------------------------ rest_framework_json_api/pagination.py | 40 ++++-------- rest_framework_json_api/settings.py | 1 - 5 files changed, 48 insertions(+), 159 deletions(-) diff --git a/README.rst b/README.rst index 8159e8ed..cb0647fa 100644 --- a/README.rst +++ b/README.rst @@ -118,15 +118,10 @@ Running the example app :: - $ git clone https://github.com/django-json-api/django-rest-framework-json-api.git - $ cd django-rest-framework-json-api - $ python3 -m venv env - $ source env/bin/activate - $ pip install -r example/requirements.txt - $ pip install -e . - $ django-admin migrate --settings=example.settings - $ django-admin loaddata drf_example --settings=example.settings - $ django-admin runserver --settings=example.settings + $ git clone https://github.com/django-json-api/django-rest-framework-json-api.git + $ cd django-rest-framework-json-api + $ pip install -e . + $ django-admin.py runserver --settings=example.settings Browse to http://localhost:8000 @@ -141,7 +136,7 @@ installed and activated: $ pip install -e . $ pip install -r requirements-development.txt - $ DJANGO_SETTINGS_MODULE=example.settings.test py.test + $ py.test ----- @@ -166,7 +161,7 @@ override ``settings.REST_FRAMEWORK`` 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.PageNumberPagination', + 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', @@ -187,6 +182,4 @@ override ``settings.REST_FRAMEWORK`` 'TEST_REQUEST_DEFAULT_FORMAT': 'vnd.api+json' } - JSON_API_STANDARD_PAGINATION = True - This package provides much more including automatic inflection of JSON keys, extra top level data (using nested serializers), relationships, links, and handy shortcuts like MultipleIDMixin. Read more at http://django-rest-framework-json-api.readthedocs.org/ diff --git a/docs/usage.md b/docs/usage.md index 591da96b..dc1430f0 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -4,7 +4,7 @@ The DJA package implements a custom renderer, parser, exception handler, query filter backends, and pagination. To get started enable the pieces in `settings.py` that you want to use. -Many features of the [JSON:API](http://jsonapi.org/format) format standard have been implemented using +Many features of the [JSON:API](http://jsonapi.org/format) format standard have been implemented using Mixin classes in `serializers.py`. The easiest way to make use of those features is to import ModelSerializer variants from `rest_framework_json_api` instead of the usual `rest_framework` @@ -16,7 +16,7 @@ REST_FRAMEWORK = { 'PAGE_SIZE': 10, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', 'DEFAULT_PAGINATION_CLASS': - 'rest_framework_json_api.pagination.PageNumberPagination', + 'rest_framework_json_api.pagination.JsonApiPageNumberPagination', 'DEFAULT_PARSER_CLASSES': ( 'rest_framework_json_api.parsers.JSONParser', 'rest_framework.parsers.FormParser', @@ -41,8 +41,6 @@ REST_FRAMEWORK = { ), 'TEST_REQUEST_DEFAULT_FORMAT': 'vnd.api+json' } - -JSON_API_STANDARD_PAGINATION = True ``` ### Pagination @@ -52,8 +50,6 @@ DJA pagination is based on [DRF pagination](https://www.django-rest-framework.or When pagination is enabled, the renderer will return a `meta` object with record count and a `links` object with the next, previous, first, and last links. -Optional query parameters can also be provided to customize the page size or offset limit. - #### Configuring the Pagination Style Pagination style can be set on a particular viewset with the `pagination_class` attribute or by default for all viewsets @@ -62,46 +58,36 @@ by setting `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` and by setting `REST_FRA You can configure fixed values for the page size or limit -- or allow the client to choose the size or limit via query parameters. -You should also set `JSON_API_STANDARD_PAGINATION=True` in order to get up to date JSON:API-style default -query parameters and pagination size limits. The default value (False) sets deprecated values that will -be eliminated in the future. See below. - Two pagination classes are available: -- `PageNumberPagination` 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_size` (default `REST_FRAMEWORK['PAGE_SIZE']`) number of items per page. - - `max_page_size` (default `100`) enforces an upper bound on the page size. + - `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 + to specify the size. + - `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. - - `page_query_param` (if `JSON_API_STANDARD_PAGINATION is True`: `page[number]` otherwise `page`) is the query - parameter for the page number. - - `page_size_query_param` (if `JSON_API_STANDARD_PAGINATION is True`: `page[size]` otherwise `page_size`) is the - query parameter for the page size. No more than `max_page_size` items will be returned. - Set this to `None` if you don't want to allow the client to specify the size. -- `LimitOffsetPagination` 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: - - `default_limit` (default `REST_FRAMEWORK['PAGE_SIZE']`) is the number of items per page. - - `offset_query_param` (default `page[offset]`) is the query parameter that sets the offset (item number) to return. - - `limit_query_param` (default `page[limit]`) is the query parameter that sets the limit (number of items per page). - No more than `max_limit` items will be returned. - - `max_limit` (if `JSON_API_STANDARD_PAGINATION is True`: `100` otherwise `None`) enforces an upper bound on the - limit. Set it to `None` if you don't want to enforce an upper bound. + - `offset_query_param` (default `page[offset]`). + - `limit_query_param` (default `page[limit]`). + - `max_limit` (default `100`) enforces an upper bound on the limit. + Set it to `None` if you don't want to enforce an upper bound. + These examples show how to configure the parameters to use non-standard names and different limits: ```python -from rest_framework_json_api.pagination import PageNumberPagination, LimitOffsetPagination +from rest_framework_json_api.pagination import JsonApiPageNumberPagination, JsonApiLimitOffsetPagination -class MyPagePagination(PageNumberPagination): +class MyPagePagination(JsonApiPageNumberPagination): page_query_param = 'page_number' - page_size_query_param = 'page_length' - page_size = 3 + page_size_query_param = 'page_size' max_page_size = 1000 -class MyLimitPagination(LimitOffsetPagination): +class MyLimitPagination(JsonApiLimitOffsetPagination): offset_query_param = 'offset' limit_query_param = 'limit' - default_limit = 3 max_limit = None ``` @@ -160,7 +146,7 @@ If you are also using [`rest_framework.filters.SearchFilter`](https://django-res (which performs single parameter searchs across multiple fields) you'll want to customize the name of the query parameter for searching to make sure it doesn't conflict with a field name defined in the filterset. The recommended value is: `search_param="filter[search]"` but just make sure it's -`filter[_something_]` to comply with the JSON:API spec requirement to use the filter +`filter[_something_]` to comply with the jsonapi spec requirement to use the filter keyword. The default is "search" unless overriden. The filter returns a `400 Bad Request` error for invalid filter query parameters as in this example @@ -460,7 +446,7 @@ class OrderSerializer(serializers.ModelSerializer): ``` -In the [JSON:API spec](http://jsonapi.org/format/#document-resource-objects), +In the [JSON API spec](http://jsonapi.org/format/#document-resource-objects), relationship objects contain links to related objects. To make this work on a serializer we need to tell the `ResourceRelatedField` about the corresponding view. Use the `HyperlinkedModelSerializer` and instantiate @@ -598,7 +584,7 @@ class OrderSerializer(serializers.HyperlinkedModelSerializer): ### RelationshipView `rest_framework_json_api.views.RelationshipView` is used to build relationship views (see the -[JSON:API spec](http://jsonapi.org/format/#fetching-relationships)). +[JSON API spec](http://jsonapi.org/format/#fetching-relationships)). The `self` link on a relationship object should point to the corresponding relationship view. diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index ea0bd8ac..f6e95db0 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -13,11 +13,11 @@ class TestLimitOffset: """ - Unit tests for `pagination.LimitOffsetPagination`. + Unit tests for `pagination.JsonApiLimitOffsetPagination`. """ def setup(self): - class ExamplePagination(pagination.LimitOffsetPagination): + class ExamplePagination(pagination.JsonApiLimitOffsetPagination): default_limit = 10 max_limit = 15 @@ -81,9 +81,9 @@ def test_valid_offset_limit(self): def test_limit_offset_deprecation(self): with pytest.warns(DeprecationWarning) as record: - pagination.JsonApiLimitOffsetPagination() + pagination.LimitOffsetPagination() assert len(record) == 1 - assert 'JsonApiLimitOffsetPagination' in str(record[0].message) + assert 'LimitOffsetPagination' in str(record[0].message) # TODO: This test fails under py27 but it's not clear why so just leave it out for now. @@ -91,88 +91,11 @@ def test_limit_offset_deprecation(self): reason="python2.7 fails for unknown reason") class TestPageNumber: """ - Unit tests for `pagination.PageNumberPagination`. - TODO: add unit tests for changing query parameter names, max limits, etc. + Unit tests for `pagination.JsonApiPageNumberPagination`. + TODO: add unit tests for changing query parameter names, limits, etc. """ - - def setup(self): - class ExamplePagination(pagination.PageNumberPagination): - default_page_size = 10 - max_page_size = 15 - - self.pagination = ExamplePagination() - self.queryset = range(1, 401) - self.base_url = 'http://testserver/' - - def paginate_queryset(self, request): - return list(self.pagination.paginate_queryset(self.queryset, request)) - - def get_paginated_content(self, queryset): - response = self.pagination.get_paginated_response(queryset) - return response.data - - def get_test_request(self, arguments): - return Request(factory.get('/', arguments)) - - def test_valid_page_size(self): - """ - Basic test, assumes page and size are given. - """ - page = 10 - size = 5 - count = len(self.queryset) - last_page = (count // size) - next_page = 11 - prev_page = 9 - - request = self.get_test_request({ - self.pagination.page_size_query_param: size, - self.pagination.page_query_param: page - }) - base_url = replace_query_param(self.base_url, self.pagination.page_size_query_param, size) - first_url = replace_query_param(base_url, self.pagination.page_query_param, 1) - last_url = replace_query_param(base_url, self.pagination.page_query_param, last_page) - next_url = replace_query_param(base_url, self.pagination.page_query_param, next_page) - prev_url = replace_query_param(base_url, self.pagination.page_query_param, prev_page) - queryset = self.paginate_queryset(request) - content = self.get_paginated_content(queryset) - - expected_content = { - 'results': list(range((page - 1) * size + 1, (next_page - 1) * size + 1)), - 'links': OrderedDict([ - ('first', first_url), - ('last', last_url), - ('next', next_url), - ('prev', prev_url), - ]), - 'meta': { - 'pagination': OrderedDict([ - ('page', page), - ('pages', count // size), - ('count', count), - ]) - } - } - - assert queryset == list(range((page - 1) * size + 1, (next_page - 1) * size + 1)) - assert content == expected_content - - def test_page_size_too_big(self): - """ - ask for a page size that's more than the max - """ - page = 3 - size = 20 - request = self.get_test_request({ - self.pagination.page_size_query_param: size, - self.pagination.page_query_param: page - }) - queryset = self.paginate_queryset(request) - assert len(queryset) == self.pagination.max_page_size - assert len(queryset) < size - def test_page_number_deprecation(self): with pytest.warns(DeprecationWarning) as record: - pagination.JsonApiPageNumberPagination() + pagination.PageNumberPagination() assert len(record) == 1 - assert 'JsonApiPageNumberPagination' in str(record[0].message) + assert 'PageNumberPagination' in str(record[0].message) diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index e2a8cf02..00873c99 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -8,24 +8,13 @@ from rest_framework.utils.urls import remove_query_param, replace_query_param from rest_framework.views import Response -from rest_framework_json_api.settings import json_api_settings - -class PageNumberPagination(PageNumberPagination): +class JsonApiPageNumberPagination(PageNumberPagination): """ A json-api compatible pagination format - - An older version of this used `page` and `page_size` so - use a setting to enable the "standard" query param names. """ - if json_api_settings.STANDARD_PAGINATION: - page_query_param = 'page[number]' - page_size_query_param = 'page[size]' - else: - page_query_param = 'page' - page_size_query_param = 'page_size' - warnings.warn("'page' and 'page_size' parameters are deprecated. " - "Set JSON_API_STANDARD_PAGINATION=True", DeprecationWarning) + page_query_param = 'page[number]' + page_size_query_param = 'page[size]' max_page_size = 100 def build_link(self, index): @@ -61,7 +50,7 @@ def get_paginated_response(self, data): }) -class LimitOffsetPagination(LimitOffsetPagination): +class JsonApiLimitOffsetPagination(LimitOffsetPagination): """ A limit/offset based style. For example: http://api.example.org/accounts/?page[limit]=100 @@ -69,11 +58,7 @@ class LimitOffsetPagination(LimitOffsetPagination): """ limit_query_param = 'page[limit]' offset_query_param = 'page[offset]' - if json_api_settings.STANDARD_PAGINATION: - max_limit = 100 - else: - warnings.warn("'max_limit = None' is deprecated. " - "Set JSON_API_STANDARD_PAGINATION=True", DeprecationWarning) + max_limit = 100 def get_last_link(self): if self.count == 0: @@ -115,28 +100,31 @@ def get_paginated_response(self, data): }) -class JsonApiPageNumberPagination(PageNumberPagination): +class PageNumberPagination(JsonApiPageNumberPagination): """ - Changed our minds about the naming scheme. Removed JsonApi prefx. + Deprecated paginator that uses different query parameters """ + page_query_param = 'page' + page_size_query_param = 'page_size' def __init__(self): warnings.warn( - 'JsonApiPageNumberPagination is deprecated. Use PageNumberPagination ' + 'PageNumberPagination is deprecated. Use JsonApiPageNumberPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) super(PageNumberPagination, self).__init__() -class JsonApiLimitOffsetPagination(LimitOffsetPagination): +class LimitOffsetPagination(JsonApiLimitOffsetPagination): """ - Changed our minds about the naming scheme. Removed JsonApi prefx. + Deprecated paginator that uses a different max_limit """ + max_limit = None def __init__(self): warnings.warn( - 'JsonApiLimitOffsetPagination is deprecated. Use LimitOffsetPagination ' + 'LimitOffsetPagination is deprecated. Use JsonApiLimitOffsetPagination ' 'or create custom pagination. See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', DeprecationWarning) diff --git a/rest_framework_json_api/settings.py b/rest_framework_json_api/settings.py index 2c3cd022..6c7eeffe 100644 --- a/rest_framework_json_api/settings.py +++ b/rest_framework_json_api/settings.py @@ -14,7 +14,6 @@ 'FORMAT_TYPES': False, 'PLURALIZE_TYPES': False, 'UNIFORM_EXCEPTIONS': False, - 'STANDARD_PAGINATION': False, # deprecated settings to be removed in the future 'FORMAT_KEYS': None, From b90d98318213e9f93806964216f8b8427d84e60b Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 6 Sep 2018 20:15:58 -0400 Subject: [PATCH 17/20] Reverts JSONAPI prefix to JsonApi prefix on paginators (as is in release 2.5.0). Will replace without the JsonApi prefix in release 3.0. Deprecation warnings added. --- docs/usage.md | 21 +++++++++++---- example/tests/unit/test_pagination.py | 19 +++++++++----- rest_framework_json_api/pagination.py | 38 ++++++++++++++++++++------- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index dc1430f0..31767367 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -50,6 +50,8 @@ DJA pagination is based on [DRF pagination](https://www.django-rest-framework.or When pagination is enabled, the renderer will return a `meta` object with record count and a `links` object with the next, previous, first, and last links. +Optional query parameters can also be provided to customize the page size or offset limit. + #### Configuring the Pagination Style Pagination style can be set on a particular viewset with the `pagination_class` attribute or by default for all viewsets @@ -58,22 +60,29 @@ by setting `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` and by setting `REST_FRA You can configure fixed values for the page size or limit -- or allow the client to choose the size or limit via query parameters. -Two pagination classes are available: +Two pagination classes are available **but their names will change in release 3.0**: - `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 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. + + In release 3.0, `JsonApiPageNumberPagination` will be renamed `PageNumberPagination`. - `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]`). - `limit_query_param` (default `page[limit]`). + - `default_limit` (default `REST_FRAMEWORK['PAGE_SIZE']`) is the default number of items per page unless + overridden by `limit_query_param`. - `max_limit` (default `100`) enforces an upper bound on the limit. Set it to `None` if you don't want to enforce an upper bound. + In release 3.0, `JsonApiLimitOffsetPagination` will be renamed `LimitOffsetPagination`. These examples show how to configure the parameters to use non-standard names and different limits: @@ -82,12 +91,14 @@ from rest_framework_json_api.pagination import JsonApiPageNumberPagination, Json class MyPagePagination(JsonApiPageNumberPagination): page_query_param = 'page_number' - page_size_query_param = 'page_size' + page_size_query_param = 'page_length' + page_size = 3 max_page_size = 1000 class MyLimitPagination(JsonApiLimitOffsetPagination): offset_query_param = 'offset' limit_query_param = 'limit' + default_limit = 3 max_limit = None ``` @@ -146,7 +157,7 @@ If you are also using [`rest_framework.filters.SearchFilter`](https://django-res (which performs single parameter searchs across multiple fields) you'll want to customize the name of the query parameter for searching to make sure it doesn't conflict with a field name defined in the filterset. The recommended value is: `search_param="filter[search]"` but just make sure it's -`filter[_something_]` to comply with the jsonapi spec requirement to use the filter +`filter[_something_]` to comply with the JSON:API spec requirement to use the filter keyword. The default is "search" unless overriden. The filter returns a `400 Bad Request` error for invalid filter query parameters as in this example @@ -446,7 +457,7 @@ class OrderSerializer(serializers.ModelSerializer): ``` -In the [JSON API spec](http://jsonapi.org/format/#document-resource-objects), +In the [JSON:API spec](http://jsonapi.org/format/#document-resource-objects), relationship objects contain links to related objects. To make this work on a serializer we need to tell the `ResourceRelatedField` about the corresponding view. Use the `HyperlinkedModelSerializer` and instantiate @@ -584,7 +595,7 @@ class OrderSerializer(serializers.HyperlinkedModelSerializer): ### RelationshipView `rest_framework_json_api.views.RelationshipView` is used to build relationship views (see the -[JSON API spec](http://jsonapi.org/format/#fetching-relationships)). +[JSON:API spec](http://jsonapi.org/format/#fetching-relationships)). The `self` link on a relationship object should point to the corresponding relationship view. diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index f6e95db0..e09fb848 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -79,23 +79,30 @@ def test_valid_offset_limit(self): assert queryset == list(range(offset + 1, next_offset + 1)) assert content == expected_content + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_limit_offset_deprecation(self): with pytest.warns(DeprecationWarning) as record: pagination.LimitOffsetPagination() - assert len(record) == 1 - assert 'LimitOffsetPagination' in str(record[0].message) + assert len(record) == 2 + assert 'LimitOffsetPagination will change in release 3.0' in str(record[0].message) + assert 'JsonApiLimitOffsetPagination will be replaced by LimitOffsetPagination' \ + in str(record[1].message) # TODO: This test fails under py27 but it's not clear why so just leave it out for now. -@pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), - reason="python2.7 fails for unknown reason") class TestPageNumber: """ Unit tests for `pagination.JsonApiPageNumberPagination`. TODO: add unit tests for changing query parameter names, limits, etc. """ + + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_page_number_deprecation(self): with pytest.warns(DeprecationWarning) as record: pagination.PageNumberPagination() - assert len(record) == 1 - assert 'PageNumberPagination' in str(record[0].message) + assert len(record) == 2 + assert 'PageNumberPagination will change in release 3.0' in str(record[0].message) + assert 'JsonApiPageNumberPagination will be replaced by PageNumberPagination' \ + in str(record[1].message) diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index 00873c99..c43932ef 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -17,6 +17,14 @@ class JsonApiPageNumberPagination(PageNumberPagination): page_size_query_param = 'page[size]' max_page_size = 100 + def __init__(self): + warnings.warn( + 'JsonApiPageNumberPagination will be replaced by PageNumberPagination in release 3.0' + ' See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', + DeprecationWarning) + super(JsonApiPageNumberPagination, self).__init__() + def build_link(self, index): if not index: return None @@ -60,6 +68,14 @@ class JsonApiLimitOffsetPagination(LimitOffsetPagination): offset_query_param = 'page[offset]' max_limit = 100 + def __init__(self): + warnings.warn( + 'JsonApiLimitOffsetPagination will be replaced by LimitOffsetPagination in release 3.0' + ' See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', + DeprecationWarning) + super(JsonApiLimitOffsetPagination, self).__init__() + def get_last_link(self): if self.count == 0: return None @@ -108,11 +124,12 @@ class PageNumberPagination(JsonApiPageNumberPagination): page_size_query_param = 'page_size' def __init__(self): - warnings.warn( - 'PageNumberPagination is deprecated. Use JsonApiPageNumberPagination ' - 'or create custom pagination. See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) + if self.page_query_param == 'page' or self.page_size_query_param == 'page_size': + warnings.warn( + 'PageNumberPagination will change in release 3.0 to default to use page[number] and' + ' page[size] query parameters. See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 + DeprecationWarning) super(PageNumberPagination, self).__init__() @@ -123,9 +140,10 @@ class LimitOffsetPagination(JsonApiLimitOffsetPagination): max_limit = None def __init__(self): - warnings.warn( - 'LimitOffsetPagination is deprecated. Use JsonApiLimitOffsetPagination ' - 'or create custom pagination. See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) + if self.max_limit is None: + warnings.warn( + 'LimitOffsetPagination will change in release 3.0 to default to max_limit=100. ' + 'See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 + DeprecationWarning) super(LimitOffsetPagination, self).__init__() From 5a8d67ecb94556c8807b8ca7ca09a86b4234362f Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Thu, 6 Sep 2018 20:21:07 -0400 Subject: [PATCH 18/20] inadvertenly added a junked file --- .../filters/django_filter.py | 123 ------------------ 1 file changed, 123 deletions(-) delete mode 100644 rest_framework_json_api/filters/django_filter.py diff --git a/rest_framework_json_api/filters/django_filter.py b/rest_framework_json_api/filters/django_filter.py deleted file mode 100644 index 4681c2f2..00000000 --- a/rest_framework_json_api/filters/django_filter.py +++ /dev/null @@ -1,123 +0,0 @@ -import re - -from rest_framework.exceptions import ValidationError -from rest_framework.settings import api_settings - -from django_filters import VERSION -from django_filters.rest_framework import DjangoFilterBackend -from rest_framework_json_api.utils import format_value - - -class DjangoFilterBackend(DjangoFilterBackend): - """ - A Django-style ORM filter implementation, using `django-filter`. - - This is not part of the jsonapi standard per-se, other than the requirement - to use the `filter` keyword: This is an optional implementation of style of - filtering in which each filter is an ORM expression as implemented by - DjangoFilterBackend and seems to be in alignment with an interpretation of - http://jsonapi.org/recommendations/#filtering, including relationship - chaining. It also returns a 400 error for invalid filters. - - Filters can be: - - A resource field equality test: - `?filter[qty]=123` - - Apply other [field lookup](https://docs.djangoproject.com/en/stable/ref/models/querysets/#field-lookups) # noqa: E501 - operators: - `?filter[name.icontains]=bar` or `?filter[name.isnull]=true...` - - 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[...]` - - A related resource path can be used: - `?filter[inventory.item.partNum]=123456 (where `inventory.item` is the relationship path)` - - If you are also using rest_framework.filters.SearchFilter you'll want to customize - the name of the query parameter for searching to make sure it doesn't conflict - with a field name defined in the filterset. - The recommended value is: `search_param="filter[search]"` but just make sure it's - `filter[]` to comply with the jsonapi spec requirement to use the filter - keyword. The default is "search" unless overriden but it's used here just to make sure - we don't complain about it being an invalid filter. - """ - # TODO: find a better way to deal with search_param. - search_param = api_settings.SEARCH_PARAM - - # Make this regex check for 'filter' as well as 'filter[...]' - # Leave other incorrect usages of 'filter' to JSONAPIQueryValidationFilter. - # See http://jsonapi.org/format/#document-member-names for allowed characters - # and http://jsonapi.org/format/#document-member-names-reserved-characters for reserved - # characters (for use in paths, lists or as delimiters). - # regex `\w` matches [a-zA-Z0-9_]. - # TODO: U+0080 and above allowed but not recommended. Leave them out for now.e - # Also, ' ' (space) is allowed within a member name but not recommended. - filter_regex = re.compile(r'^filter(?P\[?)(?P[\w\.\-]*)(?P\]?$)') - - def _validate_filter(self, keys, filterset_class): - for k in keys: - if ((not filterset_class) or (k not in filterset_class.base_filters)): - raise ValidationError("invalid filter[{}]".format(k)) - - def get_filterset(self, request, queryset, view): - """ - Sometimes there's no filterset_class defined yet the client still - requests a filter. Make sure they see an error too. This means - we have to get_filterset_kwargs() even if there's no filterset_class. - - TODO: .base_filters vs. .filters attr (not always present) - """ - filterset_class = self.get_filterset_class(view, queryset) - kwargs = self.get_filterset_kwargs(request, queryset, view) - self._validate_filter(kwargs.pop('filter_keys'), filterset_class) - if filterset_class is None: - return None - return filterset_class(**kwargs) - - def get_filterset_kwargs(self, request, queryset, view): - """ - Turns filter[]= into = which is what - DjangoFilterBackend expects - """ - filter_keys = [] - # rewrite filter[field] query params to make DjangoFilterBackend work. - data = request.query_params.copy() - for qp, val in data.items(): - m = self.filter_regex.match(qp) - if m and (not m.groupdict()['assoc'] or - m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'): - raise ValidationError("invalid filter: {}".format(qp)) - if m and qp != self.search_param: - if not val: - 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 - filter_keys.append(key) - del data[qp] - return { - 'data': data, - 'queryset': queryset, - 'request': request, - 'filter_keys': filter_keys, - } - - def filter_queryset(self, request, queryset, view): - """ - Backwards compatibility to 1.1 (required for Python 2.7) - In 1.1 filter_queryset does not call get_filterset or get_filterset_kwargs. - """ - # TODO: remove when Python 2.7 support is deprecated - if VERSION >= (2, 0, 0): - return super(DjangoFilterBackend, self).filter_queryset(request, queryset, view) - - filter_class = self.get_filter_class(view, queryset) - - kwargs = self.get_filterset_kwargs(request, queryset, view) - self._validate_filter(kwargs.pop('filter_keys'), filter_class) - - if filter_class: - return filter_class(kwargs['data'], queryset=queryset, request=request).qs - - return queryset From 7c8b044bc27419e8f81ec23c631aec26f8392dc3 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Sat, 8 Sep 2018 10:25:11 -0400 Subject: [PATCH 19/20] PendingDeprecationWarnings - for renaming paginators without JsonApi prefix - for changes to attribute defaults --- docs/usage.md | 21 ++++++- example/settings/dev.py | 1 - example/settings/test.py | 2 - example/tests/unit/test_pagination.py | 88 +++++++++++++++++++++++--- rest_framework_json_api/pagination.py | 90 ++++++++++++++++++--------- 5 files changed, 157 insertions(+), 45 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 31767367..58c36ab6 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -60,7 +60,8 @@ by setting `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` and by setting `REST_FRA You can configure fixed values for the page size or limit -- or allow the client to choose the size or limit via query parameters. -Two pagination classes are available **but their names will change in release 3.0**: +Two pagination classes are available *but their names will change in release 3.0 in order to implement a more +consistent class naming style*: - `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]`) @@ -71,7 +72,6 @@ Two pagination classes are available **but their names will change in release 3. - `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. - In release 3.0, `JsonApiPageNumberPagination` will be renamed `PageNumberPagination`. - `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: @@ -82,8 +82,23 @@ Two pagination classes are available **but their names will change in release 3. - `max_limit` (default `100`) enforces an upper bound on the limit. Set it to `None` if you don't want to enforce an upper bound. - In release 3.0, `JsonApiLimitOffsetPagination` will be renamed `LimitOffsetPagination`. +##### Preparing for future renaming of paginators with default attributes +In release 3.0, the `JsonApi` prefix will be removed from the paginator names. If you currently use those (deprecated) +names (`PageNumberPagination` or `LimitOffsetPagination`) or would like to prepare now for this change, you will want to +explicitly override several of the class attributes. + +To retain deprecated values for `PageNumberPagination` set `page_query_param = "page"` and +`page_size_query_param = "page_size"`. + +To prepare for the renaming of `JsonApiPageNumberPagination` use `PageNumberPagination` now with +`page_query_param = "page[number]"` and `page_size_query_param = "page[size]"`. + +To retain deprecated vales for `LimitOffsetPagination` set `max_limit = None`. + +To prepare for the renaming of `JsonApiLimitOffsetPagination` to `LimitOffsetPagination` set `max_limit = 100`. + +##### Examples These examples show how to configure the parameters to use non-standard names and different limits: ```python diff --git a/example/settings/dev.py b/example/settings/dev.py index dfe1ae7b..5356146f 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -68,7 +68,6 @@ JSON_API_FORMAT_FIELD_NAMES = 'camelize' JSON_API_FORMAT_TYPES = 'camelize' -JSON_API_STANDARD_PAGINATION = True REST_FRAMEWORK = { 'PAGE_SIZE': 5, 'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler', diff --git a/example/settings/test.py b/example/settings/test.py index 0cbb6b6b..c165e187 100644 --- a/example/settings/test.py +++ b/example/settings/test.py @@ -12,8 +12,6 @@ JSON_API_FIELD_NAMES = 'camelize' JSON_API_FORMAT_TYPES = 'camelize' JSON_API_PLURALIZE_TYPES = True -# TODO: 13 tests fail when this is True because they use `page` and `page_size`. Fix them. -JSON_API_STANDARD_PAGINATION = False REST_FRAMEWORK.update({ 'PAGE_SIZE': 1, diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index e09fb848..88070f8b 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -82,27 +82,95 @@ def test_valid_offset_limit(self): @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_limit_offset_deprecation(self): - with pytest.warns(DeprecationWarning) as record: + with pytest.warns(PendingDeprecationWarning) as record: pagination.LimitOffsetPagination() - assert len(record) == 2 + assert len(record) == 1 assert 'LimitOffsetPagination will change in release 3.0' in str(record[0].message) - assert 'JsonApiLimitOffsetPagination will be replaced by LimitOffsetPagination' \ - in str(record[1].message) + + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") + def test_jsonapi_limit_offset_deprecation(self): + with pytest.warns(PendingDeprecationWarning) as record: + pagination.JsonApiLimitOffsetPagination() + assert len(record) == 1 + assert 'JsonApiLimitOffsetPagination will be renamed to LimitOffsetPagination'\ + in str(record[0].message) + + class MyInheritedLimitOffsetPagination(pagination.LimitOffsetPagination): + """ + Inherit the default values + """ + pass + + class MyOverridenLimitOffsetPagination(pagination.LimitOffsetPagination): + """ + Explicitly set max_limit to the "old" values. + """ + max_limit = None + + def test_my_limit_offset_deprecation(self): + with pytest.warns(PendingDeprecationWarning) as record: + self.MyInheritedLimitOffsetPagination() + assert len(record) == 1 + assert 'LimitOffsetPagination will change in release 3.0' in str(record[0].message) + + with pytest.warns(None) as record: + self.MyOverridenLimitOffsetPagination() + assert len(record) == 0 -# TODO: This test fails under py27 but it's not clear why so just leave it out for now. class TestPageNumber: """ Unit tests for `pagination.JsonApiPageNumberPagination`. - TODO: add unit tests for changing query parameter names, limits, etc. """ @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_page_number_deprecation(self): - with pytest.warns(DeprecationWarning) as record: + with pytest.warns(PendingDeprecationWarning) as record: pagination.PageNumberPagination() - assert len(record) == 2 + assert len(record) == 1 assert 'PageNumberPagination will change in release 3.0' in str(record[0].message) - assert 'JsonApiPageNumberPagination will be replaced by PageNumberPagination' \ - in str(record[1].message) + + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") + def test_jsonapi_page_number_deprecation(self): + with pytest.warns(PendingDeprecationWarning) as record: + pagination.JsonApiPageNumberPagination() + assert len(record) == 1 + assert 'JsonApiPageNumberPagination will be renamed to PageNumberPagination' \ + in str(record[0].message) + + class MyInheritedPageNumberPagination(pagination.PageNumberPagination): + """ + Inherit the default values + """ + pass + + class MyOverridenPageNumberPagination(pagination.PageNumberPagination): + """ + Explicitly set page_query_param and page_size_query_param to the "old" values. + """ + page_query_param = "page" + page_size_query_param = "page_size" + + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") + def test_my_page_number_deprecation(self): + with pytest.warns(PendingDeprecationWarning) as record: + self.MyInheritedPageNumberPagination() + assert len(record) == 1 + assert 'PageNumberPagination will change in release 3.0' in str(record[0].message) + + with pytest.warns(None) as record: + self.MyOverridenPageNumberPagination() + assert len(record) == 0 + + @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 fails to generate DeprecationWarrning for unknown reason") + def test_my_jsonapi_page_number_deprecation(self): + with pytest.warns(PendingDeprecationWarning) as record: + pagination.JsonApiPageNumberPagination() + assert len(record) == 1 + assert 'JsonApiPageNumberPagination will be renamed to PageNumberPagination' \ + in str(record[0].message) diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index c43932ef..99bdb406 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -9,22 +9,15 @@ from rest_framework.views import Response -class JsonApiPageNumberPagination(PageNumberPagination): +class _JsonApiPageNumberPagination(PageNumberPagination): """ - A json-api compatible pagination format + A json-api compatible pagination format. + Use a private name for the implementation because the public name is pending deprecation. """ page_query_param = 'page[number]' page_size_query_param = 'page[size]' max_page_size = 100 - def __init__(self): - warnings.warn( - 'JsonApiPageNumberPagination will be replaced by PageNumberPagination in release 3.0' - ' See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) - super(JsonApiPageNumberPagination, self).__init__() - def build_link(self, index): if not index: return None @@ -58,24 +51,32 @@ def get_paginated_response(self, data): }) -class JsonApiLimitOffsetPagination(LimitOffsetPagination): +class JsonApiPageNumberPagination(_JsonApiPageNumberPagination): + """ + current public name to be deprecated soon. + """ + def __init__(self): + if type(self) == JsonApiPageNumberPagination: + warnings.warn( + 'JsonApiPageNumberPagination will be renamed to PageNumberPagination in' + ' release 3.0. See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 + PendingDeprecationWarning) + super(_JsonApiPageNumberPagination, self).__init__() + + +class _JsonApiLimitOffsetPagination(LimitOffsetPagination): """ A limit/offset based style. For example: http://api.example.org/accounts/?page[limit]=100 http://api.example.org/accounts/?page[offset]=400&page[limit]=100 + + Use a private name for the implementation because the public name is pending deprecation. """ limit_query_param = 'page[limit]' offset_query_param = 'page[offset]' max_limit = 100 - def __init__(self): - warnings.warn( - 'JsonApiLimitOffsetPagination will be replaced by LimitOffsetPagination in release 3.0' - ' See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - DeprecationWarning) - super(JsonApiLimitOffsetPagination, self).__init__() - def get_last_link(self): if self.count == 0: return None @@ -116,34 +117,65 @@ def get_paginated_response(self, data): }) -class PageNumberPagination(JsonApiPageNumberPagination): +class JsonApiLimitOffsetPagination(_JsonApiLimitOffsetPagination): + """ + current public name to be deprecated soon. + """ + + def __init__(self): + warnings.warn( + 'JsonApiLimitOffsetPagination will be renamed to LimitOffsetPagination in release 3.0' + ' See ' + 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', + PendingDeprecationWarning) + super(JsonApiLimitOffsetPagination, self).__init__() + + + +class PageNumberPagination(_JsonApiPageNumberPagination): """ - Deprecated paginator that uses different query parameters + A soon-to-be-changed paginator that uses non-JSON:API query parameters (default: + 'page' and 'page_size' instead of 'page[number]' and 'page[size]'). """ page_query_param = 'page' page_size_query_param = 'page_size' def __init__(self): - if self.page_query_param == 'page' or self.page_size_query_param == 'page_size': + if type(self) == PageNumberPagination: + warn = self.page_query_param == 'page' or self.page_size_query_param == 'page_size' + else: # inherited class doesn't override the attributes? + warn = ('page_query_param' not in type(self).__dict__ or + 'page_size_query_param' not in type(self).__dict__) + if warn: warnings.warn( - 'PageNumberPagination will change in release 3.0 to default to use page[number] and' - ' page[size] query parameters. See ' + 'PageNumberPagination will change in release 3.0 to change default values to: ' + '`page_query_param = "page[number]"` and `page_size_query_param = "page[size]"`. ' + 'If you want to retain the current defaults you will need to explicitly set ' + '`page_query_param = "page"` and `page_size_query_param = "page_size"`. ' + 'See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 - DeprecationWarning) + PendingDeprecationWarning) + super(PageNumberPagination, self).__init__() -class LimitOffsetPagination(JsonApiLimitOffsetPagination): +class LimitOffsetPagination(_JsonApiLimitOffsetPagination): """ Deprecated paginator that uses a different max_limit """ max_limit = None def __init__(self): - if self.max_limit is None: + if type(self) == LimitOffsetPagination: + warn = self.max_limit is None + else: + warn = 'max_limit' not in type(self).__dict__ + if warn: warnings.warn( - 'LimitOffsetPagination will change in release 3.0 to default to max_limit=100. ' + 'LimitOffsetPagination will change in release 3.0 to default to `max_limit=100`. ' + 'If you want to retain the current default you will need to explicitly set ' + '`max_limit = None`.' 'See ' 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 - DeprecationWarning) + PendingDeprecationWarning) super(LimitOffsetPagination, self).__init__() From 1c810c04ad5ea36c460cf05aca9d7a314c000cf2 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 12 Sep 2018 09:28:56 +0200 Subject: [PATCH 20/20] Clarify deprecation warning of pagination classes --- CHANGELOG.md | 13 +++++- docs/usage.md | 19 +-------- example/tests/unit/test_pagination.py | 43 ++++--------------- rest_framework_json_api/pagination.py | 61 +++++++-------------------- 4 files changed, 36 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54aec896..0230d7e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,6 @@ any parts of the framework not mentioned in the documentation should generally b * Add `HyperlinkedRelatedField` and `SerializerMethodHyperlinkedRelatedField`. See [usage docs](docs/usage.md#related-fields) * Add related urls support. See [usage docs](docs/usage.md#related-urls) * Add optional [jsonapi-style](http://jsonapi.org/format/) filter backends. See [usage docs](docs/usage.md#filter-backends) -* Performance improvement when rendering relationships with `ModelSerializer` ### Changed @@ -25,6 +24,8 @@ any parts of the framework not mentioned in the documentation should generally b ### Fixed * Performance improvement when rendering relationships with `ModelSerializer` +* Do not show deprecation warning when user has implemented custom pagination class overwriting default values. + ## [2.5.0] - 2018-07-11 @@ -41,6 +42,16 @@ any parts of the framework not mentioned in the documentation should generally b ### Deprecated * Deprecate `PageNumberPagination` and `LimitOffsetPagination`. Use `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` instead. + * To retain deprecated values for `PageNumberPagination` and `LimitOffsetPagination` create new custom class like the following in your code base: + ```python + class CustomPageNumberPagination(PageNumberPagination): + page_query_param = "page" + page_size_query_param = "page_size" + + class CustomLimitOffsetPagination(LimitOffsetPagination): + max_limit = None + ``` + and adjust `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` setting accordingly. * Deprecate `JSON_API_FORMAT_KEYS`, use `JSON_API_FORMAT_FIELD_NAMES`. ### Fixed diff --git a/docs/usage.md b/docs/usage.md index 58c36ab6..7d329a3c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -60,8 +60,7 @@ by setting `REST_FRAMEWORK['DEFAULT_PAGINATION_CLASS']` and by setting `REST_FRA You can configure fixed values for the page size or limit -- or allow the client to choose the size or limit via query parameters. -Two pagination classes are available *but their names will change in release 3.0 in order to implement a more -consistent class naming style*: +Two pagination classes are available: - `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]`) @@ -82,22 +81,6 @@ consistent class naming style*: - `max_limit` (default `100`) enforces an upper bound on the limit. Set it to `None` if you don't want to enforce an upper bound. -##### Preparing for future renaming of paginators with default attributes - -In release 3.0, the `JsonApi` prefix will be removed from the paginator names. If you currently use those (deprecated) -names (`PageNumberPagination` or `LimitOffsetPagination`) or would like to prepare now for this change, you will want to -explicitly override several of the class attributes. - -To retain deprecated values for `PageNumberPagination` set `page_query_param = "page"` and -`page_size_query_param = "page_size"`. - -To prepare for the renaming of `JsonApiPageNumberPagination` use `PageNumberPagination` now with -`page_query_param = "page[number]"` and `page_size_query_param = "page[size]"`. - -To retain deprecated vales for `LimitOffsetPagination` set `max_limit = None`. - -To prepare for the renaming of `JsonApiLimitOffsetPagination` to `LimitOffsetPagination` set `max_limit = 100`. - ##### Examples These examples show how to configure the parameters to use non-standard names and different limits: diff --git a/example/tests/unit/test_pagination.py b/example/tests/unit/test_pagination.py index 88070f8b..9da8675f 100644 --- a/example/tests/unit/test_pagination.py +++ b/example/tests/unit/test_pagination.py @@ -82,19 +82,10 @@ def test_valid_offset_limit(self): @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_limit_offset_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: + with pytest.warns(DeprecationWarning) as record: pagination.LimitOffsetPagination() assert len(record) == 1 - assert 'LimitOffsetPagination will change in release 3.0' in str(record[0].message) - - @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), - reason="python2.7 fails to generate DeprecationWarrning for unknown reason") - def test_jsonapi_limit_offset_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: - pagination.JsonApiLimitOffsetPagination() - assert len(record) == 1 - assert 'JsonApiLimitOffsetPagination will be renamed to LimitOffsetPagination'\ - in str(record[0].message) + assert 'LimitOffsetPagination is deprecated' in str(record[0].message) class MyInheritedLimitOffsetPagination(pagination.LimitOffsetPagination): """ @@ -109,10 +100,10 @@ class MyOverridenLimitOffsetPagination(pagination.LimitOffsetPagination): max_limit = None def test_my_limit_offset_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: + with pytest.warns(DeprecationWarning) as record: self.MyInheritedLimitOffsetPagination() assert len(record) == 1 - assert 'LimitOffsetPagination will change in release 3.0' in str(record[0].message) + assert 'LimitOffsetPagination is deprecated' in str(record[0].message) with pytest.warns(None) as record: self.MyOverridenLimitOffsetPagination() @@ -127,19 +118,10 @@ class TestPageNumber: @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_page_number_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: + with pytest.warns(DeprecationWarning) as record: pagination.PageNumberPagination() assert len(record) == 1 - assert 'PageNumberPagination will change in release 3.0' in str(record[0].message) - - @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), - reason="python2.7 fails to generate DeprecationWarrning for unknown reason") - def test_jsonapi_page_number_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: - pagination.JsonApiPageNumberPagination() - assert len(record) == 1 - assert 'JsonApiPageNumberPagination will be renamed to PageNumberPagination' \ - in str(record[0].message) + assert 'PageNumberPagination is deprecated' in str(record[0].message) class MyInheritedPageNumberPagination(pagination.PageNumberPagination): """ @@ -157,20 +139,11 @@ class MyOverridenPageNumberPagination(pagination.PageNumberPagination): @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), reason="python2.7 fails to generate DeprecationWarrning for unknown reason") def test_my_page_number_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: + with pytest.warns(DeprecationWarning) as record: self.MyInheritedPageNumberPagination() assert len(record) == 1 - assert 'PageNumberPagination will change in release 3.0' in str(record[0].message) + assert 'PageNumberPagination is deprecated' in str(record[0].message) with pytest.warns(None) as record: self.MyOverridenPageNumberPagination() assert len(record) == 0 - - @pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), - reason="python2.7 fails to generate DeprecationWarrning for unknown reason") - def test_my_jsonapi_page_number_deprecation(self): - with pytest.warns(PendingDeprecationWarning) as record: - pagination.JsonApiPageNumberPagination() - assert len(record) == 1 - assert 'JsonApiPageNumberPagination will be renamed to PageNumberPagination' \ - in str(record[0].message) diff --git a/rest_framework_json_api/pagination.py b/rest_framework_json_api/pagination.py index 99bdb406..281c78f2 100644 --- a/rest_framework_json_api/pagination.py +++ b/rest_framework_json_api/pagination.py @@ -9,7 +9,7 @@ from rest_framework.views import Response -class _JsonApiPageNumberPagination(PageNumberPagination): +class JsonApiPageNumberPagination(PageNumberPagination): """ A json-api compatible pagination format. Use a private name for the implementation because the public name is pending deprecation. @@ -51,21 +51,7 @@ def get_paginated_response(self, data): }) -class JsonApiPageNumberPagination(_JsonApiPageNumberPagination): - """ - current public name to be deprecated soon. - """ - def __init__(self): - if type(self) == JsonApiPageNumberPagination: - warnings.warn( - 'JsonApiPageNumberPagination will be renamed to PageNumberPagination in' - ' release 3.0. See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 - PendingDeprecationWarning) - super(_JsonApiPageNumberPagination, self).__init__() - - -class _JsonApiLimitOffsetPagination(LimitOffsetPagination): +class JsonApiLimitOffsetPagination(LimitOffsetPagination): """ A limit/offset based style. For example: http://api.example.org/accounts/?page[limit]=100 @@ -117,22 +103,7 @@ def get_paginated_response(self, data): }) -class JsonApiLimitOffsetPagination(_JsonApiLimitOffsetPagination): - """ - current public name to be deprecated soon. - """ - - def __init__(self): - warnings.warn( - 'JsonApiLimitOffsetPagination will be renamed to LimitOffsetPagination in release 3.0' - ' See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', - PendingDeprecationWarning) - super(JsonApiLimitOffsetPagination, self).__init__() - - - -class PageNumberPagination(_JsonApiPageNumberPagination): +class PageNumberPagination(JsonApiPageNumberPagination): """ A soon-to-be-changed paginator that uses non-JSON:API query parameters (default: 'page' and 'page_size' instead of 'page[number]' and 'page[size]'). @@ -148,18 +119,17 @@ def __init__(self): 'page_size_query_param' not in type(self).__dict__) if warn: warnings.warn( - 'PageNumberPagination will change in release 3.0 to change default values to: ' - '`page_query_param = "page[number]"` and `page_size_query_param = "page[size]"`. ' - 'If you want to retain the current defaults you will need to explicitly set ' - '`page_query_param = "page"` and `page_size_query_param = "page_size"`. ' - 'See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 - PendingDeprecationWarning) + 'PageNumberPagination is deprecated use JsonApiPageNumberPagination instead. ' + 'If you want to retain current defaults you will need to implement custom ' + 'pagination class explicitly setting `page_query_param = "page"` and ' + '`page_size_query_param = "page_size"`. ' + 'See changelog for more details.', + DeprecationWarning) super(PageNumberPagination, self).__init__() -class LimitOffsetPagination(_JsonApiLimitOffsetPagination): +class LimitOffsetPagination(JsonApiLimitOffsetPagination): """ Deprecated paginator that uses a different max_limit """ @@ -172,10 +142,9 @@ def __init__(self): warn = 'max_limit' not in type(self).__dict__ if warn: warnings.warn( - 'LimitOffsetPagination will change in release 3.0 to default to `max_limit=100`. ' - 'If you want to retain the current default you will need to explicitly set ' - '`max_limit = None`.' - 'See ' - 'https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#pagination', # noqa: E501 - PendingDeprecationWarning) + 'LimitOffsetPagination is deprecated use JsonApiLimitOffsetPagination instead. ' + 'If you want to retain current defaults you will need to implement custom ' + 'pagination class explicitly setting `max_limit = None`. ' + 'See changelog for more details.', + DeprecationWarning) super(LimitOffsetPagination, self).__init__()