From a5d5f1bf4c1dd37cc213ebe01127cd6cd8e29932 Mon Sep 17 00:00:00 2001 From: Karsten Hopp Date: Aug 08 2018 13:01:35 +0000 Subject: paginate /api/0/projects by default On pagure instances with a huge number of projects querying /api/0/projects could lead to out of memory errors. This change forces paginating the output. Signed-off-by: Karsten Hopp --- diff --git a/pagure/api/project.py b/pagure/api/project.py index 88b4c32..59afe5a 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -344,11 +344,9 @@ def api_projects(): | | | | entrie project JSON | | | | | or just a sub-set | +---------------+----------+---------------+--------------------------+ - | ``page`` | int | Optional | | Specifies that | - | | | | pagination should be | - | | | | turned on and that | - | | | | this specific page | - | | | | should be displayed | + | ``page`` | int | Optional | | Specifies which | + | | | | page to return | + | | | | (defaults to: 1) | +---------------+----------+---------------+--------------------------+ | ``per_page`` | int | Optional | | The number of projects | | | | | to return per page. | @@ -361,93 +359,6 @@ def api_projects(): :: { - "total_projects": 2, - "projects": [ - { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [], - "commit": [ - "some_user" - ], - "owner": [ - "pingou" - ], - "ticket": [] - }, - "close_status": [], - "custom_keys": [], - "date_created": "1427441537", - "date_modified": "1427441537", - "description": "A web-based calendar for Fedora", - "milestones": {}, - "namespace": null, - "id": 7, - "name": "fedocal", - "fullname": "fedocal", - "parent": null, - "priorities": {}, - "tags": [], - "user": { - "fullname": "Pierre-Yves C", - "name": "pingou" - } - }, - { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [], - "commit": [], - "owner": [ - "pingou" - ], - "ticket": [] - }, - "close_status": [], - "custom_keys": [], - "date_created": "1431666007", - "description": "An awesome messaging servicefor everyone", - "id": 12, - "milestones": {}, - "name": "fedmsg", - "namespace": null, - "fullname": "forks/pingou/fedmsg", - "parent": { - "date_created": "1433423298", - "description": "An awesome messaging servicefor everyone", - "id": 11, - "name": "fedmsg", - "fullname": "fedmsg", - "parent": null, - "user": { - "fullname": "Ralph B", - "name": "ralph" - } - }, - "priorities": {}, - "tags": [], - "user": { - "fullname": "Pierre-Yves C", - "name": "pingou" - } - } - ] - } - - Sample Response With Pagination - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - - :: - - { "args": { "fork": null, "namespace": null, @@ -562,7 +473,9 @@ def api_projects(): pagination_metadata = None query_start = None query_limit = None - if page: + if not page: + page = 1 + else: try: page = int(page) except (TypeError, ValueError): @@ -573,23 +486,23 @@ def api_projects(): raise pagure.exceptions.APIError( 400, error_code=APIERROR.EINVALIDREQ) - if per_page: - try: - per_page = int(per_page) - except (TypeError, ValueError): - raise pagure.exceptions.APIError( - 400, error_code=APIERROR.EINVALIDREQ) + if per_page: + try: + per_page = int(per_page) + except (TypeError, ValueError): + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ) - if per_page < 1 or per_page > 100: - raise pagure.exceptions.APIError( - 400, error_code=APIERROR.EINVALIDPERPAGEVALUE) - else: - per_page = 20 + if per_page < 1 or per_page > 100: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDPERPAGEVALUE) + else: + per_page = 20 - pagination_metadata = pagure.lib.get_pagination_metadata( - flask.request, page, per_page, project_count) - query_start = (page - 1) * per_page - query_limit = per_page + pagination_metadata = pagure.lib.get_pagination_metadata( + flask.request, page, per_page, project_count) + query_start = (page - 1) * per_page + query_limit = per_page projects = pagure.lib.search_projects( flask.g.session, username=username, fork=fork, tags=tags, diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index d5de9e5..9d38f1d 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -263,12 +263,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": "test", + "per_page": 20, "short": False, "tags": [], "username": None @@ -324,12 +327,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.get('/api/0/projects?pattern=te*&short=1') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": "te*", + "per_page": 20, "short": True, "tags": [], "username": None @@ -356,6 +362,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): ], "total_projects": 3 } + self.maxDiff = None self.assertDictEqual(data, expected_data) def test_api_projects(self): @@ -381,6 +388,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.get('/api/0/projects?tags=inf') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + null = None + del data['pagination'] self.assertDictEqual( data, { @@ -390,11 +399,13 @@ class PagureFlaskApiProjecttests(tests.Modeltests): "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": ["inf"], "username": None - } + }, } ) output = self.app.get('/api/0/projects?tags=infra') @@ -402,12 +413,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": ["infra"], "username": None @@ -459,12 +473,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data['projects'][1]['date_modified'] = "1436527638" data['projects'][2]['date_created'] = "1436527638" data['projects'][2]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": "pingou", + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": [], "username": None @@ -592,12 +609,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data['projects'][1]['date_modified'] = "1436527638" data['projects'][2]['date_created'] = "1436527638" data['projects'][2]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": [], "username": "pingou" @@ -718,12 +738,15 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": ["infra"], "username": "pingou", @@ -771,11 +794,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] expected_data = { "args": { "fork": None, "owner": None, + "page": 1, "namespace": "somenamespace", + "per_page": 20, "pattern": None, "short": False, "tags": [], @@ -3545,7 +3571,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): """ Test the api_modify_acls method of the flask api when no ACL are specified, so the user tries to remove their own access but the user is the project owner. """ - # Add the user `foo` to the project + # Add the user `foo` to the project self.test_api_modify_acls_user() # Ensure `foo` was properly added: diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 5fdd2eb..f950cfc 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -1097,6 +1097,7 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.get('/api/0/projects?tags=inf') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + del data['pagination'] self.assertDictEqual( data, { @@ -1104,7 +1105,9 @@ class PagurePrivateRepotest(tests.Modeltests): 'fork': None, 'namespace': None, 'owner': None, + 'page': 1, 'pattern': None, + 'per_page': 20, 'short': False, 'tags': ['inf'], 'username': None @@ -1118,6 +1121,7 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.get('/api/0/projects?tags=infra') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + del data['pagination'] self.assertDictEqual( data, { @@ -1125,7 +1129,9 @@ class PagurePrivateRepotest(tests.Modeltests): 'fork': None, 'namespace': None, 'owner': None, + 'page': 1, 'pattern': None, + 'per_page': 20, 'short': False, 'tags': ['infra'], 'username': None @@ -1141,6 +1147,7 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.get('/api/0/projects?tags=infra') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + del data['pagination'] self.assertDictEqual( data, { @@ -1148,7 +1155,9 @@ class PagurePrivateRepotest(tests.Modeltests): 'fork': None, 'namespace': None, 'owner': None, + 'page': 1, 'pattern': None, + 'per_page': 20, 'short': False, 'tags': ['infra'], 'username': None @@ -1164,6 +1173,7 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.get('/api/0/projects?tags=infra') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) + del data['pagination'] self.assertDictEqual( data, { @@ -1171,7 +1181,9 @@ class PagurePrivateRepotest(tests.Modeltests): 'fork': None, 'namespace': None, 'owner': None, + 'page': 1, 'pattern': None, + 'per_page': 20, 'short': False, 'tags': ['infra'], 'username': None @@ -1186,6 +1198,7 @@ class PagurePrivateRepotest(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] self.assertDictEqual( data, { @@ -1193,12 +1206,13 @@ class PagurePrivateRepotest(tests.Modeltests): "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": [], "username": "pingou" }, - "total_projects": 1, "projects": [ { @@ -1243,6 +1257,7 @@ class PagurePrivateRepotest(tests.Modeltests): data = json.loads(output.get_data(as_text=True)) data['projects'][0]['date_created'] = "1436527638" data['projects'][0]['date_modified'] = "1436527638" + del data['pagination'] self.assertDictEqual( data, { @@ -1250,7 +1265,9 @@ class PagurePrivateRepotest(tests.Modeltests): "fork": None, "namespace": None, "owner": None, + "page": 1, "pattern": None, + "per_page": 20, "short": False, "tags": ["infra"], "username": "pingou"