From ebd836f4fd200d4d03c2ffea6460cc2f5e121f93 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 05 2018 10:01:28 +0000 Subject: Add 'order_by' GET arg to REST API, so we can order by various keys even in desc order. --- diff --git a/freshmaker/api_utils.py b/freshmaker/api_utils.py index 65ceadf..db2a33e 100644 --- a/freshmaker/api_utils.py +++ b/freshmaker/api_utils.py @@ -52,6 +52,35 @@ def pagination_metadata(p_query): return pagination_data +def _order_by(flask_request, query, base_class, allowed_keys, default_key): + """ + Parses the "order_by" argument from flask_request.args, checks that + it is allowed for ordering in `allowed_keys` list and sets the ordering + in the `query`. + In case "order_by" is not set in flask_request.args, use `default_key` + instead. + + If "order_by" argument starts with minus sign ('-'), the descending order + is used. + """ + order_by = flask_request.args.get('order_by', default_key, type=str) + if order_by and len(order_by) > 1 and order_by[0] == "-": + order_asc = False + order_by = order_by[1:] + else: + order_asc = True + + if order_by not in allowed_keys: + raise ValueError( + 'An invalid order_by key was suplied, allowed keys are: ' + '%r' % allowed_keys) + + order_by_attr = getattr(base_class, order_by) + if not order_asc: + order_by_attr = order_by_attr.desc() + return query.order_by(order_by_attr) + + def filter_artifact_builds(flask_request): """ Returns a flask_sqlalchemy.Pagination object based on the request parameters @@ -107,7 +136,9 @@ def filter_artifact_builds(flask_request): ea = db.aliased(Event) query = query.join(ea).filter(ea.search_key == event_search_key) - query = query.order_by(ArtifactBuild.id) + query = _order_by(flask_request, query, ArtifactBuild, + ["id", "name", "event_id", "dep_on_id", "build_id", + "original_nvr", "rebuilt_nvr"], "-id") page = flask_request.args.get('page', 1, type=int) per_page = flask_request.args.get('per_page', 10, type=int) @@ -134,7 +165,8 @@ def filter_events(flask_request): search_attr = getattr(Event, key) query = query.filter(search_attr.in_(values)) - query = query.order_by(Event.id) + query = _order_by(flask_request, query, Event, + ["id", "message_id"], "-id") 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.py b/tests/test_views.py index b258f74..f08e170 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -148,7 +148,7 @@ class TestViews(helpers.ModelsTestCase): for build_id in [1234, 1235, 1236]: self.assertIn(build_id, [b['build_id'] for b in builds]) - def test_query_builds_order(self): + def test_query_builds_order_by_default(self): event = models.Event.create(db.session, "2017-00000000-0000-0000-0000-000000000003", "RHSA-2018-103", events.TestingEvent) build9 = models.ArtifactBuild.create(db.session, event, "make", "module", 1237) build9.id = 9 @@ -160,9 +160,47 @@ class TestViews(helpers.ModelsTestCase): resp = self.client.get('/api/1/builds/') builds = json.loads(resp.get_data(as_text=True))['items'] self.assertEqual(len(builds), 5) + for id, build in zip([9, 8, 3, 2, 1], builds): + self.assertEqual(id, build['id']) + + def test_query_builds_order_by_id_asc(self): + event = models.Event.create(db.session, "2017-00000000-0000-0000-0000-000000000003", "RHSA-2018-103", events.TestingEvent) + build9 = models.ArtifactBuild.create(db.session, event, "make", "module", 1237) + build9.id = 9 + db.session.commit() + build8 = models.ArtifactBuild.create(db.session, event, "attr", "module", 1238) + build8.id = 8 + db.session.commit() + db.session.expire_all() + resp = self.client.get('/api/1/builds/?order_by=id') + builds = json.loads(resp.get_data(as_text=True))['items'] + self.assertEqual(len(builds), 5) for id, build in zip([1, 2, 3, 8, 9], builds): self.assertEqual(id, build['id']) + def test_query_builds_order_by_build_id_desc(self): + event = models.Event.create(db.session, "2017-00000000-0000-0000-0000-000000000003", "RHSA-2018-103", events.TestingEvent) + build9 = models.ArtifactBuild.create(db.session, event, "make", "module", 1237) + build9.id = 9 + db.session.commit() + build8 = models.ArtifactBuild.create(db.session, event, "attr", "module", 1238) + build8.id = 8 + db.session.commit() + db.session.expire_all() + resp = self.client.get('/api/1/builds/?order_by=-build_id') + builds = json.loads(resp.get_data(as_text=True))['items'] + self.assertEqual(len(builds), 5) + for id, build in zip([8, 9, 3, 2, 1], builds): + self.assertEqual(id, build['id']) + + def test_query_builds_order_by_unknown_key(self): + resp = self.client.get('/api/1/builds/?order_by=-foo') + data = json.loads(resp.get_data(as_text=True)) + self.assertEqual(data['status'], 400) + self.assertEqual(data['error'], 'Bad Request') + self.assertTrue(data['message'].startswith( + "An invalid order_by key was suplied, allowed keys are")) + def test_query_builds_by_name(self): resp = self.client.get('/api/1/builds/?name=ed') builds = json.loads(resp.get_data(as_text=True))['items'] @@ -293,6 +331,24 @@ class TestViews(helpers.ModelsTestCase): self.assertEqual(len(evs), 1) self.assertEqual(evs[0]['search_key'], 'RHSA-2018-101') + def test_query_event_order_by_default(self): + resp = self.client.get('/api/1/events/') + evs = json.loads(resp.get_data(as_text=True))['items'] + for id, build in zip([2, 1], evs): + self.assertEqual(id, build['id']) + + def test_query_event_order_by_id_asc(self): + resp = self.client.get('/api/1/events/?order_by=id') + evs = json.loads(resp.get_data(as_text=True))['items'] + for id, build in zip([1, 2], evs): + self.assertEqual(id, build['id']) + + def test_query_event_order_by_id_message_id_desc(self): + resp = self.client.get('/api/1/events/?order_by=-message_id') + evs = json.loads(resp.get_data(as_text=True))['items'] + for id, build in zip([2, 1], evs): + self.assertEqual(id, build['id']) + def test_query_event_types(self): resp = self.client.get('/api/1/event-types/') event_types = json.loads(resp.get_data(as_text=True))['items']