Merge pull request AdCombo#55 from igieon/pagination_improvement

Pagination improvement use dict of ints and expect it everywhere

Author: mahenzon

docs/configuration.rst

@@ -3,7 +3,7 @@
 Configuration
 =============
 
-You have access to 5 configration keys:
+You have access to 5 configuration keys:
 
 * PAGE_SIZE: the number of items in a page (default is 30)
 * MAX_PAGE_SIZE: the maximum page size. If you specify a page size greater than this value you will receive a 400 Bad Request response.

flask_combo_jsonapi/data_layers/alchemy.py

@@ -12,7 +12,6 @@
 from marshmallow import class_registry
 from marshmallow.base import SchemaABC
 
-from flask import current_app
 from flask_combo_jsonapi.data_layers.base import BaseDataLayer
 from flask_combo_jsonapi.data_layers.sorting.alchemy import create_sorts
 from flask_combo_jsonapi.exceptions import (
@@ -649,13 +648,13 @@ def paginate_query(self, query, paginate_info):
         :param dict paginate_info: pagination information
         :return Query: the paginated query
         """
-        if int(paginate_info.get("size", 1)) == 0:
+        if paginate_info.get("size") == 0:
             return query
 
-        page_size = int(paginate_info.get("size", 0)) or current_app.config["PAGE_SIZE"]
+        page_size = paginate_info.get("size")
         query = query.limit(page_size)
         if paginate_info.get("number"):
-            query = query.offset((int(paginate_info["number"]) - 1) * page_size)
+            query = query.offset((paginate_info["number"] - 1) * page_size)
 
         return query
 

flask_combo_jsonapi/pagination.py

@@ -1,11 +1,10 @@
 """Helper to create pagination links according to jsonapi specification"""
 
 from __future__ import division
-from urllib.parse import urlencode
-from math import ceil
-from copy import copy
 
-from flask import current_app
+from copy import copy
+from math import ceil
+from urllib.parse import urlencode
 
 
 def add_pagination_links(data, object_count, querystring, base_url):
@@ -25,9 +24,9 @@ def add_pagination_links(data, object_count, querystring, base_url):
     if all_qs_args:
         links['self'] += '?' + urlencode(all_qs_args)
 
-    if querystring.pagination.get('size') != '0' and object_count > 1:
+    if querystring.pagination.get('size') != 0 and object_count > 1:
         # compute last link
-        page_size = int(querystring.pagination.get('size', 1)) or current_app.config['PAGE_SIZE']
+        page_size = querystring.pagination.get('size')
         last_page = int(ceil(object_count / page_size))
 
         if last_page > 1:
@@ -43,7 +42,7 @@ def add_pagination_links(data, object_count, querystring, base_url):
             links['last'] += '?' + urlencode(all_qs_args)
 
             # compute previous and next link
-            current_page = int(querystring.pagination.get('number', 0)) or 1
+            current_page = querystring.pagination.get('number') or 1
             if current_page > 1:
                 all_qs_args.update({'page[number]': current_page - 1})
                 links['prev'] = '?'.join((base_url, urlencode(all_qs_args)))

flask_combo_jsonapi/querystring.py

@@ -91,34 +91,30 @@ def filters(self):
 
     @property
     def pagination(self):
-        """Return all page parameters as a dict.
+        """Return parameters page[size] and page[number) as a dict.
+        If missing parmeter `size` then default parameter PAGE_SIZE is used.
 
         :return dict: a dict of pagination information
 
-        To allow multiples strategies, all parameters starting with `page` will be included. e.g::
-
-            {
-                "number": '25',
-                "size": '150',
-            }
-
         Example with number strategy::
 
             >>> query_string = {'page[number]': '25', 'page[size]': '10'}
             >>> parsed_query.pagination
-            {'number': '25', 'size': '10'}
+            {'number': 25, 'size': 10}
         """
         # check values type
         result = self._get_key_values('page')
         for key, value in result.items():
             if key not in ('number', 'size'):
                 raise BadRequest("{} is not a valid parameter of pagination".format(key), source={'parameter': 'page'})
             try:
-                int(value)
+                result[key] = int(value)
             except ValueError:
                 raise BadRequest("Parse error", source={'parameter': 'page[{}]'.format(key)})
 
-        if current_app.config.get('ALLOW_DISABLE_PAGINATION', True) is False and int(result.get('size', 1)) == 0:
+        result.setdefault('size', current_app.config.get('PAGE_SIZE', 30))
+
+        if current_app.config.get('ALLOW_DISABLE_PAGINATION', True) is False and result.get('size') == 0:
             raise BadRequest("You are not allowed to disable pagination", source={'parameter': 'page[size]'})
 
         if current_app.config.get('MAX_PAGE_SIZE') is not None and 'size' in result:

tests/test_sqlalchemy_data_layer.py

@@ -198,6 +198,21 @@ def person_2(session, person_model):
     session_.commit()
 
 
+@pytest.fixture()
+def persons(session, person_model):
+    persons = []
+    session_ = session
+    for i in range(100):
+        person_ = person_model(name=f"test{i}")
+        session_.add(person_)
+        persons.append(person_)
+    session_.commit()
+    yield persons
+    for person_ in persons:
+        session_.delete(person_)
+    session_.commit()
+
+
 @pytest.fixture()
 def computer(session, computer_model):
     computer_ = computer_model(serial="1")
@@ -621,6 +636,12 @@ def api_blueprint(client):
     yield bp
 
 
+@pytest.fixture()
+def app_disabled_pagination(app):
+    app.config['PAGE_SIZE'] = 0
+    yield app
+    app.config['PAGE_SIZE'] = 30
+
 @pytest.fixture(scope="module")
 def register_routes_custom_qs(
         client,
@@ -633,7 +654,6 @@ def register_routes_custom_qs(
     api.route(person_list_2, "person_list_qs", "/qs/persons")
     api.init_app(app)
 
-
 @pytest.fixture(scope="module")
 def register_routes(
         client,
@@ -838,6 +858,27 @@ def test_get_list(client, register_routes, person, person_2):
         assert response.status_code == 200
 
 
+def test_get_list_default_pagination(client, register_routes, persons):
+    with client:
+        response = client.get("/persons", content_type="application/vnd.api+json")
+        assert response.status_code == 200
+        assert len(response.json['data']) == 30
+        assert response.json['meta']['count'] == len(persons)
+        assert 'last' in response.json['links']
+        assert 'first' in response.json['links']
+        assert 'next' in response.json['links']
+
+
+def test_get_list_default_pagination_default_disabled(client, app_disabled_pagination, register_routes, persons):
+    with client:
+        response = client.get("/persons", content_type="application/vnd.api+json")
+        assert response.status_code == 200
+        assert len(response.json['data']) == len(persons)
+        assert 'last' not in response.json['links']
+        assert 'first' not in response.json['links']
+        assert 'next' not in response.json['links']
+
+
 def test_get_list_with_simple_filter(client, register_routes, person, person_2):
     with client:
         querystring = urlencode(