From 33722f68af808693d42508b4628fc9e1766de3eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Lafr=C3=A9choux?= <jerome@jolimont.fr>
Date: Wed, 26 Sep 2018 22:27:20 +0200
Subject: [PATCH] Return 200 rather than 404 if pagination out of range

---
 flask_rest_api/exceptions.py |  4 ----
 flask_rest_api/pagination.py | 29 ++++++++++-------------------
 tests/test_examples.py       |  2 +-
 tests/test_pagination.py     | 20 +++++++++++++++++---
 4 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/flask_rest_api/exceptions.py b/flask_rest_api/exceptions.py
index 6e412fd3..5597d5f8 100644
--- a/flask_rest_api/exceptions.py
+++ b/flask_rest_api/exceptions.py
@@ -15,10 +15,6 @@ class InvalidLocation(FlaskRestApiError):
     """Parameter location is not a valid location"""
 
 
-class PageOutOfRangeError(FlaskRestApiError):
-    """Requested page number out of page range"""
-
-
 class NotModified(wexc.HTTPException, FlaskRestApiError):
     """Resource was not modified (Etag is unchanged)
 
diff --git a/flask_rest_api/pagination.py b/flask_rest_api/pagination.py
index 3631d67b..e70c1d34 100644
--- a/flask_rest_api/pagination.py
+++ b/flask_rest_api/pagination.py
@@ -15,9 +15,8 @@
 
 import marshmallow as ma
 
-from .args_parser import abort, parser
+from .args_parser import parser
 from .utils import get_appcontext
-from .exceptions import PageOutOfRangeError
 from .compat import MARSHMALLOW_VERSION_MAJOR
 
 
@@ -86,7 +85,6 @@ def make_paginator(self, data):
 class PaginationMetadata:
 
     def __init__(self, page, page_size, item_count):
-        self.page = page
         self.page_size = page_size
         self.item_count = item_count
 
@@ -97,17 +95,13 @@ def __init__(self, page, page_size, item_count):
             self.first_page = 1
             self.page_count = ((self.item_count - 1) // self.page_size) + 1
             self.last_page = self.first_page + self.page_count - 1
-            # Check if requested page number is out of range
-            if (self.page < self.first_page) or (self.page > self.last_page):
-                raise PageOutOfRangeError(
-                    "Page {} out of [{}-{}] range.".format(
-                        self.page, self.first_page, self.last_page)
-                )
-            # Previous / next page
-            if self.page > self.first_page:
-                self.previous_page = self.page-1
-            if self.page < self.last_page:
-                self.next_page = self.page+1
+            # Page, previous / next page
+            if page <= self.last_page:
+                self.page = page
+                if page > self.first_page:
+                    self.previous_page = page - 1
+                if page < self.last_page:
+                    self.next_page = page + 1
 
     def __repr__(self):
         return ("{}(page={!r},page_size={!r},item_count={!r})"
@@ -188,11 +182,8 @@ def _set_pagination_header(page_params):
         current_app.logger.warning(
             'item_count not set in endpoint {}'.format(request.endpoint))
         return
-    try:
-        pagination_metadata = PaginationMetadata(
-            page_params.page, page_params.page_size, item_count)
-    except PageOutOfRangeError as exc:
-        abort(404, messages=str(exc), exc=exc)
+    pagination_metadata = PaginationMetadata(
+        page_params.page, page_params.page_size, item_count)
     page_header = PaginationMetadataSchema().dumps(pagination_metadata)
     if MARSHMALLOW_VERSION_MAJOR < 3:
         page_header = page_header[0]
diff --git a/tests/test_examples.py b/tests/test_examples.py
index 54b0e658..4fdb1b5c 100644
--- a/tests/test_examples.py
+++ b/tests/test_examples.py
@@ -244,7 +244,7 @@ def assert_counters(
             list_etag = response.headers['ETag']
             assert len(response.json) == 0
             assert json.loads(response.headers['X-Pagination']) == {
-                'total': 0, 'total_pages': 0, 'page': 1}
+                'total': 0, 'total_pages': 0}
 
         # GET collection with correct ETag: Not modified
         with assert_counters(0, 1, 0, 1 if bp_schema == 'ETag schema' else 0):
diff --git a/tests/test_pagination.py b/tests/test_pagination.py
index 812b2f2b..39e9f12c 100644
--- a/tests/test_pagination.py
+++ b/tests/test_pagination.py
@@ -199,13 +199,27 @@ def test_pagination_parameters_default_page_page_size(self, app_fixture):
                 'previous_page': 1, 'next_page': 3,
             }
 
+    def test_pagination_empty_collection(self, app_fixture):
+        # empty collection -> 200 with empty list, partial pagination metadata
+        response = app_fixture.client.get('/test/')
+        assert response.status_code == 200
+        assert json.loads(response.headers['X-Pagination']) == {
+            'total': 0, 'total_pages': 0,
+        }
+        assert response.json == []
+
     @pytest.mark.parametrize('collection', [1000, ], indirect=True)
     def test_pagination_page_out_of_range(self, app_fixture):
-        # page = 120, page_size = 10: page out of range -> 404
+        # page = 120, page_size = 10
+        # page out of range -> 200 with empty list, partial pagination metadata
         response = app_fixture.client.get(
             '/test/', query_string={'page': 120, 'page_size': 10})
-        assert response.status_code == 404
-        assert 'errors' in response.json
+        assert response.status_code == 200
+        assert json.loads(response.headers['X-Pagination']) == {
+            'total': 1000, 'total_pages': 100,
+            'first_page': 1, 'last_page': 100,
+        }
+        assert response.json == []
 
     @pytest.mark.parametrize('collection', [1000, ], indirect=True)
     def test_pagination_min_page_page_size(self, app_fixture):