diff --git a/README.rst b/README.rst index a595c007..837163a6 100644 --- a/README.rst +++ b/README.rst @@ -208,9 +208,9 @@ module builds are displayed. These parameters are: value defaults to 1. - ``per_page`` - Specifies how many items per page should be displayed (e.g. ``per_page=20``). This value defaults to 10. -- ``order_by`` - a database column to order the API by in ascending order. This defaults to +- ``order_by`` - a database column to order the API by in ascending order. +- ``order_desc_by`` - a database column to order the API by in descending order. This defaults to ``id``. -- ``order_desc_by`` - a database column to order the API by in descending order. An example of querying the "module-builds" resource with the "per_page" and the "page" parameters:: @@ -226,7 +226,7 @@ parameters:: { "items": [ { - "id": 123, + "id": 124, "koji_tag": "module-de66baf89b40367c", "name": "testmodule", "owner": "mprahl", @@ -257,7 +257,7 @@ parameters:: "version": "20171005183359" }, { - "id": 124, + "id": 123, "koji_tag": "module-4620ad476f3d2b5c", "name": "testmodule", "owner": "mprahl", @@ -293,10 +293,10 @@ parameters:: "last": "http://mbs.fedoraproject.org/module-build-service/1/module-builds/?per_page=2&page=340", "next": "http://mbs.fedoraproject.org/module-build-service/1/module-builds/?per_page=2&page=2", "page": 1, - "pages": 340, + "pages": 60, "per_page": 2, "prev": null, - "total": 1020 + "total": 120 } } @@ -319,7 +319,7 @@ parameters:: 57047, 57048 ], - "id": 123, + "id": 124, "koji_tag": "module-de66baf89b40367c", "modulemd": "...." "name": "testmodule", @@ -365,7 +365,7 @@ parameters:: 57045, 57046 ], - "id": 124, + "id": 123, "koji_tag": "module-4620ad476f3d2b5c", "modulemd": "...." "name": "testmodule", @@ -412,10 +412,10 @@ parameters:: "last": "http://mbs.fedoraproject.org/module-build-service/1/module-builds/?verbose=true&per_page=2&page=340", "next": "http://mbs.fedoraproject.org/module-build-service/1/module-builds/?verbose=true&per_page=2&page=2", "page": 1, - "pages": 340, + "pages": 120, "per_page": 2, "prev": null, - "total": 1020 + "total": 60 } } @@ -460,7 +460,7 @@ and the "submitted_before" parameters:: { "items": [ { - "id": 1, + "id": 3, "state": 3, ... }, @@ -470,7 +470,7 @@ and the "submitted_before" parameters:: ... }, { - "id": 3, + "id": 1, "state": 3, ... } diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 89fc1be5..2c899eef 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -404,6 +404,34 @@ def pagination_metadata(p_query, request_args): return pagination_data +def _add_order_by_clause(flask_request, query, column_source): + """ + Orders the given SQLAlchemy query based on the GET arguments provided + :param flask_request: a Flask request object + :param query: a SQLAlchemy query object + :param column_source: a SQLAlchemy database model + :return: a SQLAlchemy query object + """ + colname = "id" + descending = True + order_desc_by = flask_request.args.get("order_desc_by", None) + if order_desc_by: + colname = order_desc_by + else: + order_by = flask_request.args.get("order_by", None) + if order_by: + colname = order_by + descending = False + + column = getattr(column_source, colname, None) + if not column: + raise ValidationError('An invalid order_by or order_desc_by key ' + 'was supplied') + if descending: + column = column.desc() + return query.order_by(column) + + def filter_component_builds(flask_request): """ Returns a flask_sqlalchemy.Pagination object based on the request parameters @@ -433,18 +461,7 @@ def filter_component_builds(flask_request): if search_query: query = query.filter_by(**search_query) - # Order the results by any column in the ModuleBuild table but default to id. - order_by = flask_request.args.get('order_by', 'id') - order_desc_by = flask_request.args.get('order_desc_by', None) - if order_by or order_desc_by: - column = getattr(models.ComponentBuild, order_desc_by or order_by, None) - if column: - if order_desc_by: - column = column.desc() - query = query.order_by(column) - else: - raise ValidationError('An invalid order_by or order_desc_by key ' - 'was supplied') + query = _add_order_by_clause(flask_request, query, models.ComponentBuild) page = flask_request.args.get('page', 1, type=int) per_page = flask_request.args.get('per_page', 10, type=int) @@ -507,18 +524,7 @@ def filter_module_builds(flask_request): elif context == 'before': query = query.filter(column <= item_datetime) - # Order the results by any column in the ModuleBuild table. - order_by = flask_request.args.get("order_by", None) - order_desc_by = flask_request.args.get("order_desc_by", None) - if order_by or order_desc_by: - column = getattr(models.ModuleBuild, order_desc_by or order_by, None) - if column: - if order_desc_by: - column = column.desc() - query = query.order_by(column) - else: - raise ValidationError('An invalid order_by or order_desc_by key ' - 'was supplied') + query = _add_order_by_clause(flask_request, query, models.ModuleBuild) page = flask_request.args.get('page', 1, type=int) per_page = flask_request.args.get('per_page', 10, type=int) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 4032ca1d..3edc79f9 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -224,75 +224,48 @@ class TestViews(unittest.TestCase): self.assertEquals(meta_data['page'], 2) def test_query_builds(self): - rv = self.client.get('/module-build-service/1/module-builds/?per_page=2') - item = json.loads(rv.data)['items'][0] - self.assertEquals(item['id'], 1) - self.assertEquals(item['name'], 'nginx') - self.assertEquals(item['owner'], 'Moe Szyslak') - self.assertEquals(item['state'], 3) - self.assertDictEqual(item['tasks'], { - 'rpms': { - 'module-build-macros': { - 'task_id': 12312321, - 'state': 1, - 'state_reason': None, - 'nvr': 'module-build-macros-01-1.module_nginx_1_2', - }, - 'nginx': { - 'task_id': 12312345, - 'state': 1, - 'state_reason': None, - 'nvr': 'nginx-1.10.1-2.module_nginx_1_2', - }, - }, - }) - self.assertEquals(item['time_completed'], '2016-09-03T11:25:32Z') - self.assertEquals(item['time_modified'], '2016-09-03T11:25:32Z') - self.assertEquals(item['time_submitted'], '2016-09-03T11:23:20Z') - - def test_query_builds_not_verbose(self): rv = self.client.get('/module-build-service/1/module-builds/?per_page=2') items = json.loads(rv.data)['items'] expected = [ { - 'id': 1, - 'koji_tag': 'module-nginx-1.2', - 'name': 'nginx', - 'owner': 'Moe Szyslak', - 'scmurl': ('git://pkgs.domain.local/modules/nginx?#ba95886c7a443b36a9ce31abda1f9b' - 'ef22f2f8c9'), - 'state': 3, - 'state_name': 'done', + 'id': 30, + 'koji_tag': None, + 'name': 'testmodule', + 'owner': 'some_other_user', + 'scmurl': ('git://pkgs.domain.local/modules/testmodule?' + '#ca95886c7a443b36a9ce31abda1f9bef22f2f8c9'), + 'state': 1, + 'state_name': 'wait', 'state_reason': None, - 'stream': '1', + 'stream': '4.3.43', 'tasks': { 'rpms': { 'module-build-macros': { - 'nvr': 'module-build-macros-01-1.module_nginx_1_2', + 'nvr': 'module-build-macros-01-1.module_postgresql_1_2', 'state': 1, 'state_reason': None, - 'task_id': 12312321 + 'task_id': 47384002 }, - 'nginx': { - 'nvr': 'nginx-1.10.1-2.module_nginx_1_2', - 'state': 1, + 'rubygem-rails': { + 'nvr': 'postgresql-9.5.3-4.module_postgresql_1_2', + 'state': 3, 'state_reason': None, - 'task_id': 12312345 + 'task_id': 2433442 } } }, - 'time_completed': '2016-09-03T11:25:32Z', - 'time_modified': '2016-09-03T11:25:32Z', - 'time_submitted': '2016-09-03T11:23:20Z', - 'version': '2' + 'time_completed': None, + 'time_modified': '2016-09-03T13:58:40Z', + 'time_submitted': '2016-09-03T13:58:33Z', + 'version': '6' }, - { - 'id': 2, + { + 'id': 29, 'koji_tag': 'module-postgressql-1.2', 'name': 'postgressql', 'owner': 'some_user', - 'scmurl': ('git://pkgs.domain.local/modules/postgressql?#aa95886c7a443b36a' - '9ce31abda1f9bef22f2f8c9'), + 'scmurl': ('git://pkgs.domain.local/modules/postgressql?' + '#aa95886c7a443b36a9ce31abda1f9bef22f2f8c9'), 'state': 3, 'state_name': 'done', 'state_reason': None, @@ -303,19 +276,19 @@ class TestViews(unittest.TestCase): 'nvr': 'module-build-macros-01-1.module_postgresql_1_2', 'state': 1, 'state_reason': None, - 'task_id': 47383993 + 'task_id': 47384002 }, 'postgresql': { 'nvr': 'postgresql-9.5.3-4.module_postgresql_1_2', 'state': 1, 'state_reason': None, - 'task_id': 2433433 + 'task_id': 2433442 } } }, - 'time_completed': '2016-09-03T11:27:19Z', - 'time_modified': '2016-09-03T12:27:19Z', - 'time_submitted': '2016-09-03T12:25:33Z', + 'time_completed': '2016-09-03T12:57:19Z', + 'time_modified': '2016-09-03T13:57:19Z', + 'time_submitted': '2016-09-03T13:55:33Z', 'version': '2' } ]