From 4c50787590df72c726b9fb241ae3d2825ba23564 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 15:00:32 -0600 Subject: [PATCH 01/11] Adding "total_count" in "meta" for data collections. --- pinax/api/jsonapi.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index ed0c626..0a66fd1 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -77,16 +77,16 @@ def get_serializable_data(self, request=None): included=self.included, request=request, )) - return ret + return ret, dict(total_count=len(self.data)) elif isinstance(self.data, Resource): return self.data.serializable( links=self.links, linkage=self.linkage, included=self.included, request=request, - ) + ), None else: - return self.data + return self.data, None def build_links(self, request=None): links = {} @@ -123,7 +123,13 @@ def build_links(self, request=None): def serializable(self, request=None): res = {"jsonapi": {"version": "1.0"}} if self.data is not None: - res.update(dict(data=self.get_serializable_data(request=request))) + data, meta = self.get_serializable_data(request=request) + res.update(dict(data=data)) + if meta: + if self.meta: + self.meta.update(meta) + else: + self.meta = meta if self.errors is not None: res.update(dict(errors=self.errors)) if self.included: From a4209138153c7d6fb8221507d4967da847aa64c9 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 15:15:10 -0600 Subject: [PATCH 02/11] Setting self.meta in get_serializable_data --- pinax/api/jsonapi.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 0a66fd1..6a93e73 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -77,16 +77,22 @@ def get_serializable_data(self, request=None): included=self.included, request=request, )) - return ret, dict(total_count=len(self.data)) + total_count = dict(total_count=len(self.data)) + if self.meta: + self.meta.update(total_count) + else: + self.meta = total_count + return ret + elif isinstance(self.data, Resource): return self.data.serializable( links=self.links, linkage=self.linkage, included=self.included, request=request, - ), None + ) else: - return self.data, None + return self.data def build_links(self, request=None): links = {} @@ -123,13 +129,7 @@ def build_links(self, request=None): def serializable(self, request=None): res = {"jsonapi": {"version": "1.0"}} if self.data is not None: - data, meta = self.get_serializable_data(request=request) - res.update(dict(data=data)) - if meta: - if self.meta: - self.meta.update(meta) - else: - self.meta = meta + res.update(dict(data=self.get_serializable_data(request=request))) if self.errors is not None: res.update(dict(errors=self.errors)) if self.included: From 0e24795f907a56cf3dd3d67f20230180e9d09e90 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 15:16:21 -0600 Subject: [PATCH 03/11] Removing extra line --- pinax/api/jsonapi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 6a93e73..6eecbc7 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -83,7 +83,6 @@ def get_serializable_data(self, request=None): else: self.meta = total_count return ret - elif isinstance(self.data, Resource): return self.data.serializable( links=self.links, From 07017ed67839a1c45692c33dfeca610330622148 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 15:43:29 -0600 Subject: [PATCH 04/11] Enhancing iterable collection meta-data --- pinax/api/jsonapi.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 6eecbc7..83cef9c 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -77,11 +77,16 @@ def get_serializable_data(self, request=None): included=self.included, request=request, )) - total_count = dict(total_count=len(self.data)) + + # Obtain meta-data for iterable pagination + paginator = dict(paginator=dict( + count=paginator.count, + num_pages=paginator.num_pages + )) if self.meta: - self.meta.update(total_count) + self.meta.update(paginator) else: - self.meta = total_count + self.meta = paginator return ret elif isinstance(self.data, Resource): return self.data.serializable( From 14517a0200e76a88652869c282dddc941da170a4 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 16:27:15 -0600 Subject: [PATCH 05/11] Referencing request.GET["page[size]"] for pagination --- pinax/api/jsonapi.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 83cef9c..e56073d 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -13,6 +13,9 @@ from .resource import Resource +PAGINATOR_PER_PAGE = "100" # default number of items shown per page + + class Included(set): def __init__(self, paths): @@ -59,7 +62,14 @@ def get_serializable_data(self, request=None): ret = [] data = self.data if request is not None: - paginator = Paginator(data, 100) + if "page[size]" in request.GET: + try: + per_page = int(request.GET.get("page[size]", PAGINATOR_PER_PAGE)) + except ValueError: + per_page = int(PAGINATOR_PER_PAGE) + else: + per_page = int(PAGINATOR_PER_PAGE) + paginator = Paginator(data, per_page) if "page[number]" in request.GET: try: page_number = int(request.GET.get("page[number]", "1")) From 312e6b9dae51251eb35cbec08f2edb011b6663a4 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 16:29:58 -0600 Subject: [PATCH 06/11] Improving efficiency --- pinax/api/jsonapi.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index e56073d..397a147 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -13,7 +13,7 @@ from .resource import Resource -PAGINATOR_PER_PAGE = "100" # default number of items shown per page +PAGINATOR_PER_PAGE = 100 # default number of items shown per page class Included(set): @@ -64,11 +64,11 @@ def get_serializable_data(self, request=None): if request is not None: if "page[size]" in request.GET: try: - per_page = int(request.GET.get("page[size]", PAGINATOR_PER_PAGE)) + per_page = int(request.GET.get("page[size]", str(PAGINATOR_PER_PAGE))) except ValueError: - per_page = int(PAGINATOR_PER_PAGE) + per_page = PAGINATOR_PER_PAGE else: - per_page = int(PAGINATOR_PER_PAGE) + per_page = PAGINATOR_PER_PAGE paginator = Paginator(data, per_page) if "page[number]" in request.GET: try: From 1e1acc8207373e9af2edf1a135aaa4038cedb02e Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Wed, 6 Apr 2016 16:59:14 -0600 Subject: [PATCH 07/11] Making "meta" an empty dictionary by default. Adding test stubs. --- pinax/api/jsonapi.py | 9 ++-- pinax/api/tests/tests.py | 88 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 397a147..22d2071 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -46,7 +46,7 @@ def from_validation_error(cls, exc, resource_class): errs.append(err) return cls(errors=errs) - def __init__(self, data=None, errors=None, links=False, included=None, meta=None, linkage=False): + def __init__(self, data=None, errors=None, links=False, included=None, meta={}, linkage=False): self.data = data self.errors = errors self.links = links @@ -93,10 +93,7 @@ def get_serializable_data(self, request=None): count=paginator.count, num_pages=paginator.num_pages )) - if self.meta: - self.meta.update(paginator) - else: - self.meta = paginator + self.meta.update(paginator) return ret elif isinstance(self.data, Resource): return self.data.serializable( @@ -148,7 +145,7 @@ def serializable(self, request=None): res.update(dict(errors=self.errors)) if self.included: res.update(dict(included=[r.serializable(links=self.links, request=request) for r in self.included])) - if self.meta is not None: + if self.meta is not {}: res.update(dict(meta=self.meta)) if self.links: res.update(dict(links=self.build_links(request=request))) diff --git a/pinax/api/tests/tests.py b/pinax/api/tests/tests.py index 8ee73a7..52b83dc 100644 --- a/pinax/api/tests/tests.py +++ b/pinax/api/tests/tests.py @@ -115,3 +115,91 @@ def test_different_items(self): b = [self.first_item, self.fourth_item] with self.assertRaises(AssertionError): self.assertResourceGraphEqual(a, b) + + +class TestPagination(TestCase): + """ + Check the meta["paginator"] response values. + """ + + def setUp(self): + pass + + def test_empty_collection(self): + pass + + def test_single_item(self): + """ + Ensure correct response for a "collection" of one item. + """ + pass + + def test_multiple_items(self): + """ + Ensure correct response for a collection of several items. + """ + pass + + +class TestPaginationPageSize(TestCase): + """ + Verify proper operation of "page[size]" pagination in request.GET. + """ + + def setUp(self): + pass + + def test_one_item_size_zero(self): + """ + No idea what will happen here! + """ + pass + + def test_two_items_size_one(self): + """ + Ensure we see just first item in response. + """ + + def test_two_items_size_two(self): + """ + Ensure we see both items in response. + """ + + +class TestPaginationPageNumber(TestCase): + """ + Verify proper operation of "page[number]" pagination in request.GET. + """ + + def setUp(self): + pass + + def test_page_zero(self): + """ + No idea what will happen here! + """ + pass + + def test_page_negative(self): + """ + No idea what will happen here! + """ + pass + + def test_first_page(self): + """ + Ensure correct items are returned + """ + pass + + def test_last_page(self): + """ + Ensure correct items are returned + """ + pass + + def test_beyond_page(self): + """ + Ensure error is correct for page exceeding the number of pages. + """ + pass From ed301ec2c46d8a1b989d40aaf3cf61d5962bd8e7 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Thu, 7 Apr 2016 13:40:27 -0600 Subject: [PATCH 08/11] Fixing and adding pagination tests. --- pinax/api/__init__.py | 1 - pinax/api/jsonapi.py | 19 +- pinax/api/tests/models.py | 4 + pinax/api/tests/resources.py | 17 + pinax/api/{ => tests}/test.py | 0 pinax/api/tests/{tests.py => test_assert.py} | 90 +---- pinax/api/tests/test_pagination.py | 397 +++++++++++++++++++ 7 files changed, 431 insertions(+), 97 deletions(-) create mode 100644 pinax/api/tests/models.py create mode 100644 pinax/api/tests/resources.py rename pinax/api/{ => tests}/test.py (100%) rename pinax/api/tests/{tests.py => test_assert.py} (66%) create mode 100644 pinax/api/tests/test_pagination.py diff --git a/pinax/api/__init__.py b/pinax/api/__init__.py index 88ba93c..c193fbb 100644 --- a/pinax/api/__init__.py +++ b/pinax/api/__init__.py @@ -10,7 +10,6 @@ from .registry import register, bind, registry # noqa from .relationships import Relationship # noqa from .resource import Resource, Attribute # noqa -from .test import TestCase # noqa from .urls import URL as url # noqa from .views import handler404 # noqa from .viewsets import ResourceEndpointSet, RelationshipEndpointSet # noqa diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 22d2071..77aef68 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -69,6 +69,10 @@ def get_serializable_data(self, request=None): per_page = PAGINATOR_PER_PAGE else: per_page = PAGINATOR_PER_PAGE + if per_page == 0: + # Zero is invalid number of items per page. + # Protect against Django division by zero error. + per_page = PAGINATOR_PER_PAGE paginator = Paginator(data, per_page) if "page[number]" in request.GET: try: @@ -80,6 +84,14 @@ def get_serializable_data(self, request=None): else: page = paginator.page(1) self._current_page = data = page + + # Obtain pagination meta-data + paginator = dict(paginator=dict( + count=paginator.count, + num_pages=paginator.num_pages + )) + self.meta.update(paginator) + for x in data: ret.append(x.serializable( links=self.links, @@ -87,13 +99,6 @@ def get_serializable_data(self, request=None): included=self.included, request=request, )) - - # Obtain meta-data for iterable pagination - paginator = dict(paginator=dict( - count=paginator.count, - num_pages=paginator.num_pages - )) - self.meta.update(paginator) return ret elif isinstance(self.data, Resource): return self.data.serializable( diff --git a/pinax/api/tests/models.py b/pinax/api/tests/models.py new file mode 100644 index 0000000..13ceb2e --- /dev/null +++ b/pinax/api/tests/models.py @@ -0,0 +1,4 @@ +from django.db import models + +class TestItem(models.Model): + title = models.CharField(max_length=100) diff --git a/pinax/api/tests/resources.py b/pinax/api/tests/resources.py new file mode 100644 index 0000000..0cfc5cc --- /dev/null +++ b/pinax/api/tests/resources.py @@ -0,0 +1,17 @@ +from pinax import api + +from .models import TestItem + + +@api.register +class TestItemResource(api.Resource): + + api_type = "testitem" + model = TestItem + attributes = [ + "title", + ] + + @property + def id(self): + return self.obj.pk diff --git a/pinax/api/test.py b/pinax/api/tests/test.py similarity index 100% rename from pinax/api/test.py rename to pinax/api/tests/test.py diff --git a/pinax/api/tests/tests.py b/pinax/api/tests/test_assert.py similarity index 66% rename from pinax/api/tests/tests.py rename to pinax/api/tests/test_assert.py index 52b83dc..9daa971 100644 --- a/pinax/api/tests/tests.py +++ b/pinax/api/tests/test_assert.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from ..test import TestCase +from .test import TestCase class TestAssertResourceGraph(TestCase): @@ -115,91 +115,3 @@ def test_different_items(self): b = [self.first_item, self.fourth_item] with self.assertRaises(AssertionError): self.assertResourceGraphEqual(a, b) - - -class TestPagination(TestCase): - """ - Check the meta["paginator"] response values. - """ - - def setUp(self): - pass - - def test_empty_collection(self): - pass - - def test_single_item(self): - """ - Ensure correct response for a "collection" of one item. - """ - pass - - def test_multiple_items(self): - """ - Ensure correct response for a collection of several items. - """ - pass - - -class TestPaginationPageSize(TestCase): - """ - Verify proper operation of "page[size]" pagination in request.GET. - """ - - def setUp(self): - pass - - def test_one_item_size_zero(self): - """ - No idea what will happen here! - """ - pass - - def test_two_items_size_one(self): - """ - Ensure we see just first item in response. - """ - - def test_two_items_size_two(self): - """ - Ensure we see both items in response. - """ - - -class TestPaginationPageNumber(TestCase): - """ - Verify proper operation of "page[number]" pagination in request.GET. - """ - - def setUp(self): - pass - - def test_page_zero(self): - """ - No idea what will happen here! - """ - pass - - def test_page_negative(self): - """ - No idea what will happen here! - """ - pass - - def test_first_page(self): - """ - Ensure correct items are returned - """ - pass - - def test_last_page(self): - """ - Ensure correct items are returned - """ - pass - - def test_beyond_page(self): - """ - Ensure error is correct for page exceeding the number of pages. - """ - pass diff --git a/pinax/api/tests/test_pagination.py b/pinax/api/tests/test_pagination.py new file mode 100644 index 0000000..64e80ce --- /dev/null +++ b/pinax/api/tests/test_pagination.py @@ -0,0 +1,397 @@ +from __future__ import unicode_literals + +from django.core.paginator import EmptyPage +from django.test import RequestFactory + +from ..jsonapi import TopLevel +from .models import TestItem +from .resources import TestItemResource +from .test import TestCase + + +class TestPagination(TestCase): + """ + Check the meta["paginator"] response values. + """ + def setUp(self): + self.request = RequestFactory() + self.request.GET = {} + + def test_empty_collection(self): + """ + Verify result with no items in collection. + """ + resources = TestItemResource.from_queryset(TestItem.objects.none()) + data = { + "data": resources, + "linkage": False + } + top_level = TopLevel(**data) + payload = top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 0, + "num_pages": 1 + } + }, + "data": [ + ] + } + ) + + def test_single_item(self): + """ + Ensure correct response for a "collection" of one item. + """ + item1 = TestItem.objects.create(title="test 1") + resources = TestItemResource.from_queryset(TestItem.objects.all()) + data = { + "data": resources, + "linkage": False + } + top_level = TopLevel(**data) + payload = top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 1, + "num_pages": 1 + } + }, + "data": [ + { + "type": "testitem", + "id": str(item1.pk), + "attributes": { + "title": item1.title, + }, + }, + ] + } + ) + + def test_multiple_items(self): + """ + Ensure correct response for a collection of several items. + """ + item1 = TestItem.objects.create(title="test 1") + item2 = TestItem.objects.create(title="test 2") + item3 = TestItem.objects.create(title="test 3") + resources = TestItemResource.from_queryset(TestItem.objects.all()) + data = { + "data": resources, + "linkage": False + } + top_level = TopLevel(**data) + payload = top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 1 + } + }, + "data": [ + { + "type": "testitem", + "id": str(item1.pk), + "attributes": { + "title": item1.title, + }, + }, + { + "type": "testitem", + "id": str(item2.pk), + "attributes": { + "title": item2.title, + }, + }, + { + "type": "testitem", + "id": str(item3.pk), + "attributes": { + "title": item3.title, + }, + }, + ] + } + ) + + +class TestPaginationPageSize(TestCase): + """ + Verify proper operation of "page[size]" pagination in request.GET. + """ + def setUp(self): + self.request = RequestFactory() + self.request.GET = {} + self.item1 = TestItem.objects.create(title="test 1") + self.item2 = TestItem.objects.create(title="test 2") + self.item3 = TestItem.objects.create(title="test 3") + resources = TestItemResource.from_queryset(TestItem.objects.all()) + data = { + "data": resources, + "linkage": False + } + self.top_level = TopLevel(**data) + + def test_page_size_zero(self): + """ + Verify if page[size] == 0, all items are returned anyway. + """ + self.request.GET["page[size]"] = 0 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 1 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item1.pk), + "attributes": { + "title": self.item1.title, + } + }, + { + "type": "testitem", + "id": str(self.item2.pk), + "attributes": { + "title": self.item2.title, + } + }, + { + "type": "testitem", + "id": str(self.item3.pk), + "attributes": { + "title": self.item3.title, + } + } + ] + } + ) + + def test_page_size_one(self): + """ + Ensure we see just first item in response. + """ + self.request.GET["page[size]"] = 1 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 3 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item1.pk), + "attributes": { + "title": self.item1.title, + } + } + ] + } + ) + + def test_page_size_two(self): + """ + Ensure we see two items in response. + """ + self.request.GET["page[size]"] = 2 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 2 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item1.pk), + "attributes": { + "title": self.item1.title, + } + }, + { + "type": "testitem", + "id": str(self.item2.pk), + "attributes": { + "title": self.item2.title, + } + } + ] + } + ) + + def test_page_size_five(self): + """ + Ensure we see two items in response. + """ + self.request.GET["page[size]"] = 5 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 1 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item1.pk), + "attributes": { + "title": self.item1.title, + } + }, + { + "type": "testitem", + "id": str(self.item2.pk), + "attributes": { + "title": self.item2.title, + } + }, + { + "type": "testitem", + "id": str(self.item3.pk), + "attributes": { + "title": self.item3.title, + } + } + ] + } + ) + + +class TestPaginationPageNumber(TestCase): + """ + Verify proper operation of "page[number]" pagination in request.GET. + """ + def setUp(self): + self.request = RequestFactory() + self.request.GET = {} + self.item1 = TestItem.objects.create(title="test 1") + self.item2 = TestItem.objects.create(title="test 2") + self.item3 = TestItem.objects.create(title="test 3") + resources = TestItemResource.from_queryset(TestItem.objects.all()) + data = { + "data": resources, + "linkage": False + } + self.top_level = TopLevel(**data) + + def test_page_zero(self): + """ + Ensure expected exception for page number == 0. + """ + self.request.GET["page[size]"] = 1 + self.request.GET["page[number]"] = 0 + with self.assertRaises(EmptyPage): + self.top_level.serializable(request=self.request) + + def test_page_negative(self): + """ + Ensure expected exception for page number == -1. + """ + self.request.GET["page[size]"] = 1 + self.request.GET["page[number]"] = -1 + with self.assertRaises(EmptyPage): + self.top_level.serializable(request=self.request) + + def test_first_page(self): + """ + Ensure correct items are returned + """ + self.request.GET["page[size]"] = 1 + self.request.GET["page[number]"] = 1 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 3 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item1.pk), + "attributes": { + "title": self.item1.title, + } + } + ] + } + ) + + def test_last_page(self): + """ + Ensure correct items are returned + """ + self.request.GET["page[size]"] = 1 + self.request.GET["page[number]"] = 3 + payload = self.top_level.serializable(request=self.request) + self.assertEqual( + payload, + { + "jsonapi": {"version": "1.0"}, + "meta": { + "paginator": { + "count": 3, + "num_pages": 3 + } + }, + "data": [ + { + "type": "testitem", + "id": str(self.item3.pk), + "attributes": { + "title": self.item3.title, + } + } + ] + } + ) + + def test_beyond_page(self): + """ + Ensure expected exception for page exceeding the number of pages. + """ + self.request.GET["page[size]"] = 1 + self.request.GET["page[number]"] = 5 + with self.assertRaises(EmptyPage): + self.top_level.serializable(request=self.request) + From 1f318a240af00b53f3a8ceeb8134a5d244ee61f8 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Thu, 7 Apr 2016 13:41:18 -0600 Subject: [PATCH 09/11] Refactoring "meta" parameter in methods. Removing "meta" keyword argument in methods so that TopLevel can use it's default "meta" value instead. --- pinax/api/viewsets.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pinax/api/viewsets.py b/pinax/api/viewsets.py index 1dcdb07..5bfeaad 100644 --- a/pinax/api/viewsets.py +++ b/pinax/api/viewsets.py @@ -139,17 +139,17 @@ def validate(self, resource_class, obj=None): status=400, ) - def render(self, resource, meta=None): + def render(self, resource, **kwargs): try: - payload = self.create_top_level(resource, meta=meta).serializable(request=self.request) + payload = self.create_top_level(resource, **kwargs).serializable(request=self.request) except SerializationError as exc: return self.render_error(str(exc), status=400) else: return Response(payload, status=200) - def render_create(self, resource, meta=None): + def render_create(self, resource, **kwargs): try: - payload = self.create_top_level(resource, meta=meta).serializable(request=self.request) + payload = self.create_top_level(resource, **kwargs).serializable(request=self.request) except SerializationError as exc: return self.render_error(str(exc), status=400) else: @@ -184,13 +184,14 @@ def get_object_or_404(self, qs, **kwargs): except ObjectDoesNotExist: raise Http404("{} does not exist.".format(qs.model._meta.verbose_name.capitalize())) - def create_top_level(self, resource, linkage=False, meta=None): - kwargs = { - "data": resource, - "meta": meta, - "links": True, - "linkage": linkage, - } + def create_top_level(self, resource, linkage=False, **kwargs): + kwargs.update( + { + "data": resource, + "links": True, + "linkage": linkage, + } + ) if "include" in self.request.GET: kwargs["included"] = Included(self.request.GET["include"].split(",")) return TopLevel(**kwargs) From 80dc1f6983433761dc7b41374378aa325f84f60c Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Thu, 7 Apr 2016 13:51:38 -0600 Subject: [PATCH 10/11] Fixing flake8 errors. --- pinax/api/jsonapi.py | 45 ++++++++++++++++-------------- pinax/api/tests/models.py | 1 + pinax/api/tests/test_pagination.py | 1 - 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 77aef68..66917e4 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -62,28 +62,9 @@ def get_serializable_data(self, request=None): ret = [] data = self.data if request is not None: - if "page[size]" in request.GET: - try: - per_page = int(request.GET.get("page[size]", str(PAGINATOR_PER_PAGE))) - except ValueError: - per_page = PAGINATOR_PER_PAGE - else: - per_page = PAGINATOR_PER_PAGE - if per_page == 0: - # Zero is invalid number of items per page. - # Protect against Django division by zero error. - per_page = PAGINATOR_PER_PAGE + per_page, page_number = self.get_pagination_values(request) paginator = Paginator(data, per_page) - if "page[number]" in request.GET: - try: - page_number = int(request.GET.get("page[number]", "1")) - except ValueError: - page = paginator.page(1) - else: - page = paginator.page(page_number) - else: - page = paginator.page(1) - self._current_page = data = page + self._current_page = data = paginator.page(page_number) # Obtain pagination meta-data paginator = dict(paginator=dict( @@ -110,6 +91,28 @@ def get_serializable_data(self, request=None): else: return self.data + def get_pagination_values(self, request): + if "page[size]" in request.GET: + try: + per_page = int(request.GET.get("page[size]", str(PAGINATOR_PER_PAGE))) + except ValueError: + per_page = PAGINATOR_PER_PAGE + else: + per_page = PAGINATOR_PER_PAGE + if per_page == 0: + # Zero is invalid number of items per page. + # Protect against Django division by zero error. + per_page = PAGINATOR_PER_PAGE + + if "page[number]" in request.GET: + try: + page_number = int(request.GET.get("page[number]", "1")) + except ValueError: + page_number = 1 + else: + page_number = 1 + return per_page, page_number + def build_links(self, request=None): links = {} if request is not None: diff --git a/pinax/api/tests/models.py b/pinax/api/tests/models.py index 13ceb2e..d70f044 100644 --- a/pinax/api/tests/models.py +++ b/pinax/api/tests/models.py @@ -1,4 +1,5 @@ from django.db import models + class TestItem(models.Model): title = models.CharField(max_length=100) diff --git a/pinax/api/tests/test_pagination.py b/pinax/api/tests/test_pagination.py index 64e80ce..26f701d 100644 --- a/pinax/api/tests/test_pagination.py +++ b/pinax/api/tests/test_pagination.py @@ -394,4 +394,3 @@ def test_beyond_page(self): self.request.GET["page[number]"] = 5 with self.assertRaises(EmptyPage): self.top_level.serializable(request=self.request) - From fa0d8dd3dd4c67a4e75b230a0e8057005c30af30 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Thu, 7 Apr 2016 14:57:59 -0600 Subject: [PATCH 11/11] Fixing parameter initialization mistake. --- pinax/api/jsonapi.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pinax/api/jsonapi.py b/pinax/api/jsonapi.py index 66917e4..75c199e 100644 --- a/pinax/api/jsonapi.py +++ b/pinax/api/jsonapi.py @@ -46,12 +46,12 @@ def from_validation_error(cls, exc, resource_class): errs.append(err) return cls(errors=errs) - def __init__(self, data=None, errors=None, links=False, included=None, meta={}, linkage=False): + def __init__(self, data=None, errors=None, links=False, included=None, meta=None, linkage=False): self.data = data self.errors = errors self.links = links self.included = included - self.meta = meta + self.meta = meta if meta else {} self.linkage = linkage # internal state @@ -153,7 +153,7 @@ def serializable(self, request=None): res.update(dict(errors=self.errors)) if self.included: res.update(dict(included=[r.serializable(links=self.links, request=request) for r in self.included])) - if self.meta is not {}: + if self.meta: res.update(dict(meta=self.meta)) if self.links: res.update(dict(links=self.build_links(request=request)))