From 00c7aa266577e700392a051d18c554c26f8e5a60 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 09:52:39 -0400 Subject: [PATCH 01/17] Ignoring null values and added get_pagination_values method --- jsonapi_collections/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/jsonapi_collections/__init__.py b/jsonapi_collections/__init__.py index cc41b43..42a9992 100644 --- a/jsonapi_collections/__init__.py +++ b/jsonapi_collections/__init__.py @@ -100,6 +100,11 @@ def paginate_query(self, query): page = self.parameters['page'] return query.limit(page['limit']).offset(page['offset']) + def get_pagination_values(self): + """Return the limit and offset values.""" + page = self.parameters['page'] + return page['limit'], page['offset'] + def compound_response(self, models): """Compound a response object. @@ -180,6 +185,8 @@ def _get_pagination_parameters(self, parameters): if key not in limits and key not in offsets: continue try: + if not value: + continue pagination_parameters[key] = int(value) except ValueError: errors.append({ From b6fea9172c7e1f25d252240729d843dae51a043e Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 10:01:27 -0400 Subject: [PATCH 02/17] Added default pagination values coverage --- tests/unit/page_tests.py | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/unit/page_tests.py b/tests/unit/page_tests.py index 11e73cf..39e55e8 100644 --- a/tests/unit/page_tests.py +++ b/tests/unit/page_tests.py @@ -59,6 +59,20 @@ def test_limit(self): self.assertEqual(len(result), 1) self.assertTrue(result[0].name == 'First') + def test_blank_limit_values(self): + """Test defaulting blank limit values.""" + parameters = {'page[limit]': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + + parameters = {'page[size]': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + + parameters = {'limit': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + def test_page_offset(self): """Test offsetting a page by the page[offset] parameter.""" PersonModel.mock(name='First') @@ -95,6 +109,20 @@ def test_offset(self): self.assertEqual(len(result), 1) self.assertTrue(result[0].name == 'Second') + def test_blank_offset_values(self): + """Test defaulting blank offset values.""" + parameters = {'page[offset]': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) + + parameters = {'page[number]': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) + + parameters = {'offset': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) + class MarshmallowPaginationTestCase(PaginationTestCase): """Marshmallow driver pagination tests.""" @@ -141,6 +169,20 @@ def test_limit(self): self.assertEqual(len(result), 1) self.assertTrue(result[0].name == 'First') + def test_blank_limit_values(self): + """Test defaulting blank limit values.""" + parameters = {'page[limit]': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + + parameters = {'page[size]': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + + parameters = {'limit': ''} + limit, _ = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(limit == 100) + def test_page_offset(self): """Test offsetting a page by the page[offset] parameter.""" PersonModel.mock(name='First') @@ -182,3 +224,17 @@ def test_offset(self): result = query.all() self.assertEqual(len(result), 1) self.assertTrue(result[0].name == 'Second') + + def test_blank_offset_values(self): + """Test defaulting blank offset values.""" + parameters = {'page[offset]': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) + + parameters = {'page[number]': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) + + parameters = {'offset': ''} + _, offset = Resource(self.model, parameters).get_pagination_values() + self.assertTrue(offset == 0) From b552e869e51a0107097147e75e7240c324e9e4c5 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 15:14:14 -0400 Subject: [PATCH 03/17] Added pagination management module --- jsonapi_collections/page.py | 176 ++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 jsonapi_collections/page.py diff --git a/jsonapi_collections/page.py b/jsonapi_collections/page.py new file mode 100644 index 0000000..7173f39 --- /dev/null +++ b/jsonapi_collections/page.py @@ -0,0 +1,176 @@ +# -*- coding: utf-8 -*- +"""JSONAPI pagination implementation. + +This module validates, exposes, and acts upon pagination related data. +""" +import urllib + + +class Pagination(object): + _limit = 100 + _offset = 0 + + def __init__(self, parameters): + self.parameters = parameters + + @property + def limit(self): + """The amount to limit a query by.""" + return self._limit + + @limit.setter + def limit(self, value): + self._limit = value + + @property + def offset(self): + """The amount to offset a query by.""" + return self._offset + + @offset.setter + def offset(self, value): + self._offset = value + + @property + def strategy_limit(self): + """Return the valid keys for a limit and offset based strategy.""" + return ['page[limit]', 'page[offset]'] + + @property + def strategy_page(self): + """Return the valid keys for a page based strategy.""" + return ['page[size]', 'page[number]'] + + @property + def strategy_cursor(self): + """Return the valid keys for a cusor based strategy. + + Cursor based strategies have not been implemented. + """ + return ['page[cursor]'] + + def get_links_object(self, base_url, total): + """Return a pagination links object.""" + def get_page(offset): + if offset == 0: + return 1 + return (offset / self.limit) + 1 + + last_offset = total - self.limit + next_offset = min(self.offset + self.limit, total - self.limit) + prev_offset = max(self.offset - self.limit, 0) + + self_params = urllib.urlencode(self.parameters) + + first_obj = self.parameters + first_obj = self._update_if_exists('page[offset]', 0, first_obj) + first_obj = self._update_if_exists('page[number]', 1, first_obj) + first_params = urllib.urlencode(first_obj) + + last_obj = self.parameters + last_obj = self._update_if_exists( + 'page[offset]', last_offset, last_obj) + last_obj = self._update_if_exists( + 'page[number]', get_page(last_offset), last_obj) + last_params = urllib.urlencode(last_obj) + + next_obj = self.parameters + next_obj = self._update_if_exists( + 'page[offset]', next_offset, next_obj) + next_obj = self._update_if_exists( + 'page[number]', get_page(next_offset), next_obj) + next_params = urllib.urlencode(next_obj) + + prev_obj = self.parameters + prev_obj = self._update_if_exists( + 'page[offset]', prev_offset, prev_obj) + prev_obj = self._update_if_exists( + 'page[number]', get_page(prev_offset), prev_obj) + prev_params = urllib.urlencode(prev_obj) + + return { + 'self': '{}?{}'.format(base_url, self_params), + 'first': '{}?{}'.format(base_url, first_params), + 'last': '{}?{}'.format(base_url, last_params), + 'next': '{}?{}'.format(base_url, next_params), + 'prev': '{}?{}'.format(base_url, prev_params) + } + + def get_pagination_values(self): + """Return the limit and offset values.""" + return self.limit, self.offset + + def set_pagination_values(self): + """Set the limit and offset values.""" + pagination_keys = self._extract_pagination_keys() + for key in pagination_keys: + value = self.parameters.get(key) + if not value: + continue + if key in ['page[limit]', 'page[size]']: + self.limit = int(value) + else: + self.offset = int(value) + if 'page[number]' in pagination_keys: + self.offset = max(self.offset * self.limit - self.limit, 1) + return self + + def paginate_query(self, query): + """Return a paginated query object.""" + return query.offset(self.offset).limit(self.limit) + + def validate_parameters(self): + """Validate the provided pagination strategy.""" + errors = [] + pagination_keys = self._extract_pagination_keys() + errors.extend(self._validate_pagination_keys(pagination_keys)) + errors.extend(self._validate_pagination_values(pagination_keys)) + return errors + + def _extract_pagination_keys(self): + pagination_keys = [] + for parameter in self.parameters: + if parameter in self.strategy_limit: + pagination_keys.append(parameter) + elif parameter in self.strategy_page: + pagination_keys.append(parameter) + return pagination_keys + + def _update_if_exists(self, key, value, obj): + if key in obj: + obj[key] = value + return obj + + def _validate_pagination_keys(self, pagination_keys): + errors = [] + if len(pagination_keys) > 2: + for key in pagination_keys: + errors.append({ + 'detail': 'More than one pagination strategy specified.', + 'source': {'parameter': key} + }) + elif (len(pagination_keys) == 2 and + set(pagination_keys) != set(self.strategy_page) and + set(pagination_keys) != set(self.strategy_limit)): + for key in pagination_keys: + errors.append({ + 'detail': 'Mismatched pagination strategies specified.', + 'source': {'parameter': key} + }) + return errors + + def _validate_pagination_values(self, pagination_keys): + errors = [] + for key, value in self.parameters.iteritems(): + if key not in pagination_keys: + continue + try: + if not value: + continue + int(value) + except ValueError: + errors.append({ + 'detail': 'Value must be type number.', + 'source': {'parameter': key} + }) + return errors From 47a775515f12c661a4247d350438790aba1d7120 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 15:15:07 -0400 Subject: [PATCH 04/17] Added get_pagination_links method --- jsonapi_collections/__init__.py | 42 ++++++++++----------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/jsonapi_collections/__init__.py b/jsonapi_collections/__init__.py index 42a9992..78a5bee 100644 --- a/jsonapi_collections/__init__.py +++ b/jsonapi_collections/__init__.py @@ -3,6 +3,7 @@ from jsonapi_collections.errors import JSONAPIError from jsonapi_collections.filter import FilterParameter from jsonapi_collections.include import IncludeValue +from jsonapi_collections.page import Pagination from jsonapi_collections.sort import SortValue @@ -98,12 +99,17 @@ def sort_query(self, query): def paginate_query(self, query): """Paginate and retrieve a list of models.""" page = self.parameters['page'] - return query.limit(page['limit']).offset(page['offset']) + return page.paginate_query(query) + + def get_pagination_links(self, base_url, total): + """Return a pagination links object.""" + page = self.parameters['page'] + return page.get_links_object(base_url, total) def get_pagination_values(self): """Return the limit and offset values.""" page = self.parameters['page'] - return page['limit'], page['offset'] + return page.get_pagination_values() def compound_response(self, models): """Compound a response object. @@ -176,33 +182,11 @@ def _get_filtered_fields(self, parameters): def _get_pagination_parameters(self, parameters): """Return a dictionary of parameter, value pairs to paginate by.""" - errors = [] - pagination_parameters = {} - - limits = ['limit', 'page[size]', 'page[limit]'] - offsets = ['offset', 'page[number]', 'page[offset]'] - for key, value in parameters.iteritems(): - if key not in limits and key not in offsets: - continue - try: - if not value: - continue - pagination_parameters[key] = int(value) - except ValueError: - errors.append({ - 'detail': self.ERRORS['value'].format(key), - 'source': {'parameter': key} - }) - - page = {'limit': 100, 'offset': 0} - for key, value in pagination_parameters.iteritems(): - if key == 'page[limit]' or key == 'limit' or key == 'page[size]': - page['limit'] = value - elif key == 'page[offset]' or key == 'offset': - page['offset'] = value - elif key == 'page[number]': - page['offset'] = value * parameters.get('page[size]', 0) - return page, errors + page = Pagination(parameters) + errors = page.validate_parameters() + if errors: + return None, errors + return page.set_pagination_values(), errors def _get_included_parameters(self, parameters): """Return a list of field names. From bdbe37d88005559aaee0097c1f1da485a821dd3a Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 15:16:42 -0400 Subject: [PATCH 05/17] Refactored pagination coverage --- tests/unit/page_tests.py | 246 ++++++++++++++------------------------- 1 file changed, 88 insertions(+), 158 deletions(-) diff --git a/tests/unit/page_tests.py b/tests/unit/page_tests.py index 39e55e8..f23d2a1 100644 --- a/tests/unit/page_tests.py +++ b/tests/unit/page_tests.py @@ -4,14 +4,15 @@ This module is dedicated to testing against the various pagination strategies described in the JSONAPI 1.0 specification. """ -from jsonapi_collections import Resource -from jsonapi_collections.drivers.marshmallow import MarshmallowDriver +from urlparse import parse_qs, urlparse + +from jsonapi_collections import Resource, JSONAPIError from tests import UnitTestCase -from tests.mock import PersonModel, PersonSchema +from tests.mock import PersonModel class PaginationTestCase(UnitTestCase): - """Base pagination test case.""" + """Pagination test case.""" def setUp(self): """Establish some helpful model and query shortcuts.""" @@ -19,10 +20,6 @@ def setUp(self): self.model = PersonModel self.query = PersonModel.query - -class SQLAlchemyPaginationTestCase(PaginationTestCase): - """SQLAlchemy driver pagination tests.""" - def test_page_limit(self): """Test limiting a page by the page[limit] parameter.""" PersonModel.mock(name='First') @@ -47,18 +44,6 @@ def test_page_size(self): self.assertEqual(len(result), 1) self.assertTrue(result[0].name == 'First') - def test_limit(self): - """Test limiting a page by the limit parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'limit': '1'} - query = Resource(self.model, parameters).paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'First') - def test_blank_limit_values(self): """Test defaulting blank limit values.""" parameters = {'page[limit]': ''} @@ -69,10 +54,6 @@ def test_blank_limit_values(self): limit, _ = Resource(self.model, parameters).get_pagination_values() self.assertTrue(limit == 100) - parameters = {'limit': ''} - limit, _ = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(limit == 100) - def test_page_offset(self): """Test offsetting a page by the page[offset] parameter.""" PersonModel.mock(name='First') @@ -90,19 +71,7 @@ def test_page_number(self): PersonModel.mock(name='First') PersonModel.mock(name='Second') - parameters = {'page[number]': '1', 'page[size]': '1'} - query = Resource(self.model, parameters).paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'Second') - - def test_offset(self): - """Test offsetting a page by the offset parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'offset': '1'} + parameters = {'page[number]': '2', 'page[size]': '1'} query = Resource(self.model, parameters).paginate_query(self.query) result = query.all() @@ -117,124 +86,85 @@ def test_blank_offset_values(self): parameters = {'page[number]': ''} _, offset = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(offset == 0) - - parameters = {'offset': ''} - _, offset = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(offset == 0) - - -class MarshmallowPaginationTestCase(PaginationTestCase): - """Marshmallow driver pagination tests.""" - - def test_page_limit(self): - """Test limiting a page by the page[limit] parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'page[limit]': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'First') - - def test_page_size(self): - """Test limiting a page by the page[size] parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'page[size]': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'First') - - def test_limit(self): - """Test limiting a page by the limit parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'limit': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'First') - - def test_blank_limit_values(self): - """Test defaulting blank limit values.""" - parameters = {'page[limit]': ''} - limit, _ = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(limit == 100) - - parameters = {'page[size]': ''} - limit, _ = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(limit == 100) - - parameters = {'limit': ''} - limit, _ = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(limit == 100) - - def test_page_offset(self): - """Test offsetting a page by the page[offset] parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'page[offset]': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'Second') - - def test_page_number(self): - """Test offsetting a page by the page[number] parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'page[number]': '1', 'page[size]': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'Second') - - def test_offset(self): - """Test offsetting a page by the offset parameter.""" - PersonModel.mock(name='First') - PersonModel.mock(name='Second') - - parameters = {'offset': '1'} - query = Resource( - self.model, parameters, MarshmallowDriver, PersonSchema).\ - paginate_query(self.query) - - result = query.all() - self.assertEqual(len(result), 1) - self.assertTrue(result[0].name == 'Second') - - def test_blank_offset_values(self): - """Test defaulting blank offset values.""" - parameters = {'page[offset]': ''} - _, offset = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(offset == 0) - - parameters = {'page[number]': ''} - _, offset = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(offset == 0) - - parameters = {'offset': ''} - _, offset = Resource(self.model, parameters).get_pagination_values() - self.assertTrue(offset == 0) + self.assertTrue(offset == 1) + + def test_mismatched_strategies(self): + """Test erroring when mismatched strategies are provided.""" + try: + parameters = {'page[offset]': '1', 'page[size]': '1'} + Resource(self.model, parameters).paginate_query(self.query) + except JSONAPIError as exc: + self.assertIn('detail', exc.message['errors'][0]) + self.assertIn('source', exc.message['errors'][0]) + self.assertIn('parameter', exc.message['errors'][0]['source']) + + self.assertIn('detail', exc.message['errors'][1]) + self.assertIn('source', exc.message['errors'][1]) + self.assertIn('parameter', exc.message['errors'][1]['source']) + + try: + parameters = {'page[number]': '1', 'page[limit]': '1'} + Resource(self.model, parameters).paginate_query(self.query) + except JSONAPIError as exc: + self.assertIn('detail', exc.message['errors'][0]) + self.assertIn('source', exc.message['errors'][0]) + self.assertIn('parameter', exc.message['errors'][0]['source']) + + self.assertIn('detail', exc.message['errors'][1]) + self.assertIn('source', exc.message['errors'][1]) + self.assertIn('parameter', exc.message['errors'][1]['source']) + + def test_invalid_value(self): + """Test paginating with invalid parameter values.""" + try: + parameters = {'page[offset]': 'x'} + Resource(self.model, parameters).paginate_query(self.query) + except JSONAPIError as exc: + self.assertIn('detail', exc.message['errors'][0]) + self.assertIn('source', exc.message['errors'][0]) + self.assertIn('parameter', exc.message['errors'][0]['source']) + self.assertTrue( + exc.message['errors'][0]['source']['parameter'] == + 'page[offset]') + + def test_get_links_object_paged(self): + """Test retrieving the pagination links object for page strategies.""" + url = 'google.com' + + parameters = {'page[number]': '10', 'page[size]': '5'} + links = Resource(self.model, parameters).get_pagination_links(url, 100) + for key, value in links.iteritems(): + value = parse_qs(urlparse(value).query) + number = value['page[number]'][0] + if key == 'self': + self.assertTrue(number == '10') + elif key == 'first': + self.assertTrue(number == '1') + elif key == 'last': + self.assertTrue(number == '20') + elif key == 'next': + self.assertTrue(number == '11') + elif key == 'prev': + self.assertTrue(number == '9') + self.assertTrue(value['page[size]'][0] == '5') + + def test_get_links_object_limited(self): + """Test retrieving the pagination links object for limit strategies.""" + url = 'google.com' + + parameters = {'page[offset]': '50', 'page[limit]': '5'} + links = Resource(self.model, parameters).get_pagination_links(url, 100) + for key, value in links.iteritems(): + value = parse_qs(urlparse(value).query) + offset = value['page[offset]'][0] + if key == 'self': + self.assertTrue(offset == '50') + elif key == 'first': + self.assertTrue(offset == '0') + elif key == 'last': + self.assertTrue(offset == '95') + elif key == 'next': + self.assertTrue(offset == '55') + elif key == 'prev': + self.assertTrue(offset == '45') + self.assertTrue(value['page[limit]'][0] == '5') From c7ac896542dc01547daea683a257a61e557d4e2a Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 16:52:18 -0400 Subject: [PATCH 06/17] Using default values in pagination links response --- jsonapi_collections/page.py | 17 ++++++++++++----- tests/unit/page_tests.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/jsonapi_collections/page.py b/jsonapi_collections/page.py index 7173f39..e434189 100644 --- a/jsonapi_collections/page.py +++ b/jsonapi_collections/page.py @@ -56,32 +56,39 @@ def get_page(offset): return 1 return (offset / self.limit) + 1 + parameters = self.parameters + if ('page[offset]' not in parameters and + 'page[number]' not in parameters): + parameters['page[offset]'] = self.offset + if 'page[limit]' not in parameters and 'page[size]' not in parameters: + parameters['page[limit]'] = self.limit + last_offset = total - self.limit next_offset = min(self.offset + self.limit, total - self.limit) prev_offset = max(self.offset - self.limit, 0) - self_params = urllib.urlencode(self.parameters) + self_params = urllib.urlencode(parameters) - first_obj = self.parameters + first_obj = parameters first_obj = self._update_if_exists('page[offset]', 0, first_obj) first_obj = self._update_if_exists('page[number]', 1, first_obj) first_params = urllib.urlencode(first_obj) - last_obj = self.parameters + last_obj = parameters last_obj = self._update_if_exists( 'page[offset]', last_offset, last_obj) last_obj = self._update_if_exists( 'page[number]', get_page(last_offset), last_obj) last_params = urllib.urlencode(last_obj) - next_obj = self.parameters + next_obj = parameters next_obj = self._update_if_exists( 'page[offset]', next_offset, next_obj) next_obj = self._update_if_exists( 'page[number]', get_page(next_offset), next_obj) next_params = urllib.urlencode(next_obj) - prev_obj = self.parameters + prev_obj = parameters prev_obj = self._update_if_exists( 'page[offset]', prev_offset, prev_obj) prev_obj = self._update_if_exists( diff --git a/tests/unit/page_tests.py b/tests/unit/page_tests.py index f23d2a1..9f9dc23 100644 --- a/tests/unit/page_tests.py +++ b/tests/unit/page_tests.py @@ -168,3 +168,21 @@ def test_get_links_object_limited(self): elif key == 'prev': self.assertTrue(offset == '45') self.assertTrue(value['page[limit]'][0] == '5') + + def test_get_blank_pagination_links(self): + """Test retrieving the pagination links object with no query.""" + links = Resource(self.model, {}).get_pagination_links('google.co', 500) + for key, value in links.iteritems(): + value = parse_qs(urlparse(value).query) + offset = value['page[offset]'][0] + if key == 'self': + self.assertTrue(offset == '0') + elif key == 'first': + self.assertTrue(offset == '0') + elif key == 'last': + self.assertTrue(offset == '400') + elif key == 'next': + self.assertTrue(offset == '100') + elif key == 'prev': + self.assertTrue(offset == '0') + self.assertTrue(value['page[limit]'][0] == '100') From 0c5e85c7a212c8bbcdb74e2cb2181f56e323140a Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 16:55:55 -0400 Subject: [PATCH 07/17] Updated the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e7dbd0..b5527d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,3 +7,4 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Initial base version built to support JSON API v1.0 filtering, sorting, paging and compound documents. - Added driver support for SQLAlchemy and Marshmallow. - Project setup including CI, static analysis, code coverage and pull request template. +- Added pagination support. \ No newline at end of file From 91417dd42acb8517cb15d7f79394de00949212d8 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 21:57:55 -0400 Subject: [PATCH 08/17] Corrected pathing validation --- jsonapi_collections/drivers/marshmallow.py | 3 +++ jsonapi_collections/drivers/sqlalchemy.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/jsonapi_collections/drivers/marshmallow.py b/jsonapi_collections/drivers/marshmallow.py index 0e68e3c..0330445 100644 --- a/jsonapi_collections/drivers/marshmallow.py +++ b/jsonapi_collections/drivers/marshmallow.py @@ -78,5 +78,8 @@ def validate_relationship_path(self, path): if not self.is_relationship(column): return False + model = self.get_column_model(column) + field = self.get_field(field, schema) + schema = self.get_related_schema(field) return True diff --git a/jsonapi_collections/drivers/sqlalchemy.py b/jsonapi_collections/drivers/sqlalchemy.py index 1e6072e..5d44ee0 100644 --- a/jsonapi_collections/drivers/sqlalchemy.py +++ b/jsonapi_collections/drivers/sqlalchemy.py @@ -8,7 +8,6 @@ import json - class UnsafeEncoder(json.JSONEncoder): """Do not use an encoder like this in production. You need to have your own specialized security concious encoder. From 8bb9b0d9cb11454f3a920f3cdd20cdcb33b39c2d Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 21:58:16 -0400 Subject: [PATCH 09/17] Returning intermediary relationships --- jsonapi_collections/include.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/jsonapi_collections/include.py b/jsonapi_collections/include.py index 6e9463a..59c068b 100644 --- a/jsonapi_collections/include.py +++ b/jsonapi_collections/include.py @@ -22,9 +22,9 @@ def __init__(self, driver, field_name): def __call__(self, model): """Return the serailized output of a related field.""" - values, schema = self._get_included_values( + data = self._get_included_values( self.field_name.split('.'), self.driver.collection.schema, [model]) - return self.driver.serialize(schema, values) + return data def _get_included_values(self, path, schema, values): """Return a set of model instances and a related serializer. @@ -33,12 +33,14 @@ def _get_included_values(self, path, schema, values): :param schema: Serializer class object. :param values: List of `SQLAlchemy` model instances. """ + data = [] for field_name in path: column_name = self.driver.get_column_name(field_name, schema) values = self._get_nested_values(values, column_name) field = self.driver.get_field(field_name, schema) schema = self.driver.get_related_schema(field) - return values, schema + data.extend(self.driver.serialize(schema, values)) + return data def _get_nested_values(self, values, name): """Return a set of `SQLAlchemy` model instances. From d36a3ebded876226be61e17669cce4ee1a4165fb Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 21:58:26 -0400 Subject: [PATCH 10/17] Extended nested include coverage --- tests/mock.py | 13 +++++-- tests/unit/include_tests.py | 76 ++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/tests/mock.py b/tests/mock.py index 671c553..aeb9783 100644 --- a/tests/mock.py +++ b/tests/mock.py @@ -126,8 +126,9 @@ class EmployeeSchema(Schema): is_manager = fields.Boolean() created_at = fields.DateTime() - person = SchemaRelationship(related_schema='PersonSchema') - person_id = SchemaRelationship() + person = SchemaRelationship( + include_data=True, type_='people', related_schema='PersonSchema') + person_id = SchemaRelationship(include_data=True, type_='people') class Meta: model = EmployeeModel @@ -158,8 +159,12 @@ class PersonSchema(Schema): employed_integer = fields.Integer() created_at = fields.DateTime(format='%Y-%m-%d') - companies = SchemaRelationship(many=True, related_schema=CompanySchema) - employee = SchemaRelationship(many=False, related_schema=EmployeeSchema) + companies = SchemaRelationship( + include_data=True, type_='companies', many=True, + related_schema=CompanySchema) + employee = SchemaRelationship( + include_data=True, type_='employees', many=False, + related_schema=EmployeeSchema) employee_id = SchemaRelationship(many=False, related_schema=EmployeeSchema) class Meta: diff --git a/tests/unit/include_tests.py b/tests/unit/include_tests.py index 2d66170..68f35e8 100644 --- a/tests/unit/include_tests.py +++ b/tests/unit/include_tests.py @@ -20,6 +20,66 @@ def setUp(self): class MarshmallowIncludeTestCase(IncludeTestCase): + def test_include_nested(self): + """Test including a nested relationship.""" + company = CompanyModel.mock() + model = PersonModel.mock(companies=[company]) + EmployeeModel.mock(person_id=model.id) + + parameters = {'include': 'employee.person.companies'} + included = Resource(self.model, parameters, self.driver, self.view).\ + compound_response(model) + self.assertTrue(len(included) == 3) + + for pos, item in enumerate(included): + if item['type'] == 'companies': + companies = pos + elif item['type'] == 'people': + person = pos + elif item['type'] == 'employees': + employee = pos + + data = included[employee] + self.assertIn('id', data) + self.assertIn('type', data) + + attributes = data['attributes'] + self.assertIn('name', attributes) + self.assertIn('months_of_service', attributes) + self.assertIn('is_manager', attributes) + self.assertIn('created_at', attributes) + + relationships = data['relationships'] + self.assertIn('person', relationships) + self.assertIn('person_id', relationships) + + data = included[person] + self.assertIn('id', data) + self.assertIn('type', data) + + attributes = data['attributes'] + self.assertIn('name', attributes) + self.assertIn('age', attributes) + self.assertIn('is_employed', attributes) + self.assertIn('gender', attributes) + self.assertIn('rate', attributes) + self.assertIn('employed_integer', attributes) + self.assertIn('created_at', attributes) + + relationships = data['relationships'] + self.assertIn('companies', relationships) + self.assertIn('employee', relationships) + + data = included[companies] + self.assertIn('id', data) + self.assertIn('type', data) + + attributes = data['attributes'] + self.assertIn('name', attributes) + self.assertIn('year_established', attributes) + self.assertIn('is_profitable', attributes) + self.assertIn('created_at', attributes) + def test_include_one_to_one(self): """Test including a one-to-one relationship.""" model = PersonModel.mock() @@ -31,6 +91,20 @@ def test_include_one_to_one(self): compound_response(model) self.assertTrue(len(included) == 1) + data = included[0] + self.assertIn('id', data) + self.assertIn('type', data) + + attributes = data['attributes'] + self.assertIn('name', attributes) + self.assertIn('months_of_service', attributes) + self.assertIn('is_manager', attributes) + self.assertIn('created_at', attributes) + + relationships = data['relationships'] + self.assertIn('person', relationships) + self.assertIn('person_id', relationships) + def test_include_many_to_many(self): """Test including a many-to-many relationship.""" company = CompanyModel.mock() @@ -85,7 +159,7 @@ def test_include_nested(self): parameters = {'include': 'employee.person.companies'} included = Resource(self.model, parameters).compound_response(model) - self.assertTrue(len(included) == 1) + self.assertTrue(len(included) == 3) def test_include_one_to_one(self): """Test including a one-to-one relationship.""" From 22969afd351a6b4e52e87cdad9d051326520f16e Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 22:15:31 -0400 Subject: [PATCH 11/17] Using the driver to extract the relationship --- jsonapi_collections/include.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonapi_collections/include.py b/jsonapi_collections/include.py index 59c068b..18885a4 100644 --- a/jsonapi_collections/include.py +++ b/jsonapi_collections/include.py @@ -59,7 +59,7 @@ def _get_nested_values(self, values, name): """ new_values = [] for value in values: - relationship = getattr(value, name) + relationship = self.driver.get_column(name, value) if relationship is None: continue elif not isinstance(relationship, list): From 1cca4da79b92f096b7790d456ab870c7ade7d675 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 22:31:47 -0400 Subject: [PATCH 12/17] Catching attribute errors --- jsonapi_collections/drivers/sqlalchemy.py | 6 +++--- tests/unit/include_tests.py | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/jsonapi_collections/drivers/sqlalchemy.py b/jsonapi_collections/drivers/sqlalchemy.py index 5d44ee0..b88817b 100644 --- a/jsonapi_collections/drivers/sqlalchemy.py +++ b/jsonapi_collections/drivers/sqlalchemy.py @@ -44,10 +44,10 @@ def get_field(self, field_name, schema=None): :param field_name: A string reference to a field's name. """ - field = getattr(schema or self.collection.model, field_name, None) - if field is None: + try: + return getattr(schema or self.collection.model, field_name) + except AttributeError: raise FieldError('Invalid field specified: {}.'.format(field_name)) - return field def get_related_schema(self, field): """Return a related schema reference.""" diff --git a/tests/unit/include_tests.py b/tests/unit/include_tests.py index 68f35e8..ce0fe9d 100644 --- a/tests/unit/include_tests.py +++ b/tests/unit/include_tests.py @@ -3,9 +3,7 @@ from jsonapi_collections.drivers import marshmallow from jsonapi_collections.errors import JSONAPIError from tests import UnitTestCase -from tests.mock import ( - CompanyModel, EmployeeModel, EmployeeSchema, PersonModel, PersonSchema -) +from tests.mock import CompanyModel, EmployeeModel, PersonModel, PersonSchema class IncludeTestCase(UnitTestCase): From 16c6e1b4d7f0f1218fe426dad27973d0f23c0ee3 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 25 May 2016 22:33:43 -0400 Subject: [PATCH 13/17] Updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5527d3..5493366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,4 +7,5 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Initial base version built to support JSON API v1.0 filtering, sorting, paging and compound documents. - Added driver support for SQLAlchemy and Marshmallow. - Project setup including CI, static analysis, code coverage and pull request template. -- Added pagination support. \ No newline at end of file +- Added pagination support. +- Serialized intermediary include data. \ No newline at end of file From 76e48549705d7366b8e6f514f21a0b03751a05f2 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 26 May 2016 16:49:23 -0400 Subject: [PATCH 14/17] Setting schema during iteration --- jsonapi_collections/drivers/marshmallow.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jsonapi_collections/drivers/marshmallow.py b/jsonapi_collections/drivers/marshmallow.py index 0330445..ddb6e47 100644 --- a/jsonapi_collections/drivers/marshmallow.py +++ b/jsonapi_collections/drivers/marshmallow.py @@ -60,7 +60,9 @@ def validate_attribute_path(self, path): if pos != length: if not self.is_relationship(column): return False - model = column.property.mapper.class_ + model = self.get_column_model(column) + field = self.get_field(field, schema) + schema = self.get_related_schema(field) if pos == length and self.is_relationship(column): return False return True From 1b3841a633d19cc3601bca91aa5e684e9d162951 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 26 May 2016 16:56:51 -0400 Subject: [PATCH 15/17] Updated path validation --- jsonapi_collections/__init__.py | 4 ++-- jsonapi_collections/drivers/marshmallow.py | 4 +--- jsonapi_collections/drivers/sqlalchemy.py | 4 +--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/jsonapi_collections/__init__.py b/jsonapi_collections/__init__.py index 78a5bee..25f035b 100644 --- a/jsonapi_collections/__init__.py +++ b/jsonapi_collections/__init__.py @@ -166,7 +166,7 @@ def _get_filtered_fields(self, parameters): if not key.startswith('filter['): continue - if not self.driver.validate_attribute_path(key[7:-1]): + if not self.driver.validate_path(key[7:-1]): errors.append({ 'detail': self.ERRORS['parameter'].format(key), 'source': {'parameter': key} @@ -228,7 +228,7 @@ def _get_sorted_parameters(self, parameters): fields = fields.split(',') for field in fields: validated_field = field[1:] if field.startswith('-') else field - if not self.driver.validate_attribute_path(validated_field): + if not self.driver.validate_path(validated_field): errors.append({ 'detail': self.ERRORS['field'].format(field), 'source': {'parameter': 'sort'} diff --git a/jsonapi_collections/drivers/marshmallow.py b/jsonapi_collections/drivers/marshmallow.py index ddb6e47..008fa17 100644 --- a/jsonapi_collections/drivers/marshmallow.py +++ b/jsonapi_collections/drivers/marshmallow.py @@ -43,7 +43,7 @@ def deserialize(self, column, field_name, values, schema=None): def serialize(self, schema, items): return schema(many=True).dump(items).data.get('data', []) - def validate_attribute_path(self, path): + def validate_path(self, path): """Return `False` if the provided path cannot be found.""" fields = path.split('.') length = len(fields) @@ -63,8 +63,6 @@ def validate_attribute_path(self, path): model = self.get_column_model(column) field = self.get_field(field, schema) schema = self.get_related_schema(field) - if pos == length and self.is_relationship(column): - return False return True def validate_relationship_path(self, path): diff --git a/jsonapi_collections/drivers/sqlalchemy.py b/jsonapi_collections/drivers/sqlalchemy.py index b88817b..eb63803 100644 --- a/jsonapi_collections/drivers/sqlalchemy.py +++ b/jsonapi_collections/drivers/sqlalchemy.py @@ -84,7 +84,7 @@ def serialize(self, schema, items): """Dangerously serialize `SQLAlchemy` model instance.""" return [json.dumps(item, cls=UnsafeEncoder) for item in items] - def validate_attribute_path(self, path): + def validate_path(self, path): """Return `False` if the provided path cannot be found.""" fields = path.split('.') length = len(fields) @@ -98,8 +98,6 @@ def validate_attribute_path(self, path): if not self.is_relationship(field): return False model = field.property.mapper.class_ - if pos == length and self.is_relationship(field): - return False return True def validate_relationship_path(self, path): From e544654f86349e20ae3844be07847f2c5018133e Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 26 May 2016 17:11:24 -0400 Subject: [PATCH 16/17] Revert "Updated path validation" This reverts commit 1b3841a633d19cc3601bca91aa5e684e9d162951. --- jsonapi_collections/__init__.py | 4 ++-- jsonapi_collections/drivers/marshmallow.py | 4 +++- jsonapi_collections/drivers/sqlalchemy.py | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jsonapi_collections/__init__.py b/jsonapi_collections/__init__.py index 25f035b..78a5bee 100644 --- a/jsonapi_collections/__init__.py +++ b/jsonapi_collections/__init__.py @@ -166,7 +166,7 @@ def _get_filtered_fields(self, parameters): if not key.startswith('filter['): continue - if not self.driver.validate_path(key[7:-1]): + if not self.driver.validate_attribute_path(key[7:-1]): errors.append({ 'detail': self.ERRORS['parameter'].format(key), 'source': {'parameter': key} @@ -228,7 +228,7 @@ def _get_sorted_parameters(self, parameters): fields = fields.split(',') for field in fields: validated_field = field[1:] if field.startswith('-') else field - if not self.driver.validate_path(validated_field): + if not self.driver.validate_attribute_path(validated_field): errors.append({ 'detail': self.ERRORS['field'].format(field), 'source': {'parameter': 'sort'} diff --git a/jsonapi_collections/drivers/marshmallow.py b/jsonapi_collections/drivers/marshmallow.py index 008fa17..ddb6e47 100644 --- a/jsonapi_collections/drivers/marshmallow.py +++ b/jsonapi_collections/drivers/marshmallow.py @@ -43,7 +43,7 @@ def deserialize(self, column, field_name, values, schema=None): def serialize(self, schema, items): return schema(many=True).dump(items).data.get('data', []) - def validate_path(self, path): + def validate_attribute_path(self, path): """Return `False` if the provided path cannot be found.""" fields = path.split('.') length = len(fields) @@ -63,6 +63,8 @@ def validate_path(self, path): model = self.get_column_model(column) field = self.get_field(field, schema) schema = self.get_related_schema(field) + if pos == length and self.is_relationship(column): + return False return True def validate_relationship_path(self, path): diff --git a/jsonapi_collections/drivers/sqlalchemy.py b/jsonapi_collections/drivers/sqlalchemy.py index eb63803..b88817b 100644 --- a/jsonapi_collections/drivers/sqlalchemy.py +++ b/jsonapi_collections/drivers/sqlalchemy.py @@ -84,7 +84,7 @@ def serialize(self, schema, items): """Dangerously serialize `SQLAlchemy` model instance.""" return [json.dumps(item, cls=UnsafeEncoder) for item in items] - def validate_path(self, path): + def validate_attribute_path(self, path): """Return `False` if the provided path cannot be found.""" fields = path.split('.') length = len(fields) @@ -98,6 +98,8 @@ def validate_path(self, path): if not self.is_relationship(field): return False model = field.property.mapper.class_ + if pos == length and self.is_relationship(field): + return False return True def validate_relationship_path(self, path): From 227e9346c220b1994fbac5633a83e65f0e60c7d0 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 7 Jul 2016 19:50:55 -0400 Subject: [PATCH 17/17] Removed unnecessary relationship validation checking --- jsonapi_collections/drivers/marshmallow.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/jsonapi_collections/drivers/marshmallow.py b/jsonapi_collections/drivers/marshmallow.py index ddb6e47..00c9c0d 100644 --- a/jsonapi_collections/drivers/marshmallow.py +++ b/jsonapi_collections/drivers/marshmallow.py @@ -69,19 +69,14 @@ def validate_attribute_path(self, path): def validate_relationship_path(self, path): """Return `False` if the path cannot be found.""" - model = None schema = None for field in path.split('.'): try: - column_name = self.get_column_name(field, schema) - column = self.get_column(column_name, model) + field = self.get_field(field, schema) except FieldError: return False - if not self.is_relationship(column): + if not hasattr(field.__class__, 'schema'): return False - - model = self.get_column_model(column) - field = self.get_field(field, schema) schema = self.get_related_schema(field) return True