mirror of
https://pagure.io/fm-orchestrator.git
synced 2026-04-14 07:09:46 +08:00
Refactor the API ordering to accept multiple order keyword arguments of the same direction
This also improves how a column is determined to be valid for ordering.
This commit is contained in:
@@ -280,9 +280,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.
|
||||
- ``order_desc_by`` - a database column to order the API by in descending order. This defaults to
|
||||
``id``.
|
||||
- ``order_by`` - a database column to order the API by in ascending order. Multiple can be provided.
|
||||
- ``order_desc_by`` - a database column to order the API by in descending order. Multiple can be
|
||||
provided. This defaults to ``id``.
|
||||
|
||||
An example of querying the "module-builds" resource with the "per_page" and the "page"
|
||||
parameters::
|
||||
|
||||
@@ -30,6 +30,7 @@ from datetime import datetime
|
||||
from flask import request, url_for, Response
|
||||
from sqlalchemy.sql.sqltypes import Boolean as sqlalchemy_boolean
|
||||
from sqlalchemy.orm import aliased
|
||||
import sqlalchemy
|
||||
|
||||
from module_build_service import models, api_version, conf
|
||||
from module_build_service.errors import ValidationError, NotFound
|
||||
@@ -96,30 +97,44 @@ def pagination_metadata(p_query, api_version, request_args):
|
||||
|
||||
def _add_order_by_clause(flask_request, query, column_source):
|
||||
"""
|
||||
Orders the given SQLAlchemy query based on the GET arguments provided
|
||||
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"
|
||||
order_by = flask_request.args.getlist('order_by')
|
||||
order_desc_by = flask_request.args.getlist('order_desc_by')
|
||||
# Default to ordering by ID in descending order
|
||||
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
|
||||
requested_order = ['id']
|
||||
|
||||
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)
|
||||
if order_by and order_desc_by:
|
||||
raise ValidationError('You may not specify both order_by and order_desc_by')
|
||||
elif order_by:
|
||||
descending = False
|
||||
requested_order = order_by
|
||||
elif order_desc_by:
|
||||
descending = True
|
||||
requested_order = order_desc_by
|
||||
|
||||
column_dict = dict(column_source.__table__.columns)
|
||||
order_args = []
|
||||
for column_name in requested_order:
|
||||
if column_name not in column_dict:
|
||||
raise ValidationError(
|
||||
'An invalid ordering key of "{}" was supplied'.format(column_name))
|
||||
column = column_dict[column_name]
|
||||
# If the version column is provided, cast it as an integer so the sorting is correct
|
||||
if column_name == 'version':
|
||||
column = sqlalchemy.cast(column, sqlalchemy.BigInteger)
|
||||
if descending:
|
||||
column = column.desc()
|
||||
|
||||
order_args.append(column)
|
||||
|
||||
return query.order_by(*order_args)
|
||||
|
||||
|
||||
def str_to_bool(value):
|
||||
|
||||
@@ -663,6 +663,20 @@ class TestViews:
|
||||
assert items[0]['name'] == 'candy'
|
||||
assert items[1]['name'] == 'nginx'
|
||||
|
||||
def test_query_builds_order_by_multiple(self):
|
||||
init_data(data_size=1, multiple_stream_versions=True)
|
||||
platform_f28 = db.session.query(module_build_service.models.ModuleBuild).get(1)
|
||||
platform_f28.version = '150'
|
||||
db.session.add(platform_f28)
|
||||
db.session.commit()
|
||||
rv = self.client.get(
|
||||
'/module-build-service/1/module-builds/?order_desc_by=stream_version'
|
||||
'&order_desc_by=version')
|
||||
items = json.loads(rv.data)['items']
|
||||
expected_ids = [8, 6, 4, 1, 2, 12, 3, 5, 7, 9]
|
||||
actual_ids = [item['id'] for item in items]
|
||||
assert actual_ids == expected_ids
|
||||
|
||||
def test_query_builds_order_desc_by(self):
|
||||
rv = self.client.get('/module-build-service/1/module-builds/?'
|
||||
'per_page=10&order_desc_by=id')
|
||||
@@ -685,23 +699,28 @@ class TestViews:
|
||||
|
||||
def test_query_builds_order_by_order_desc_by(self):
|
||||
"""
|
||||
Test that when both order_by and order_desc_by is set,
|
||||
we prefer order_desc_by.
|
||||
Test that when both order_by and order_desc_by are set, an error is returned.
|
||||
"""
|
||||
rv = self.client.get('/module-build-service/1/module-builds/?'
|
||||
'per_page=10&order_desc_by=id&order_by=name')
|
||||
items = json.loads(rv.data)['items']
|
||||
# Check that the id is items[0]["id"], items[0]["id"] - 1, ...
|
||||
for idx, item in enumerate(items):
|
||||
assert item["id"] == items[0]["id"] - idx
|
||||
error = json.loads(rv.data)
|
||||
expected = {
|
||||
'error': 'Bad Request',
|
||||
'message': 'You may not specify both order_by and order_desc_by',
|
||||
'status': 400
|
||||
}
|
||||
assert error == expected
|
||||
|
||||
def test_query_builds_order_by_wrong_key(self):
|
||||
rv = self.client.get('/module-build-service/1/module-builds/?'
|
||||
'per_page=10&order_by=unknown')
|
||||
data = json.loads(rv.data)
|
||||
assert data['status'] == 400
|
||||
assert data['error'] == 'Bad Request'
|
||||
assert data['message'] == 'An invalid order_by or order_desc_by key was supplied'
|
||||
error = json.loads(rv.data)
|
||||
expected = {
|
||||
'error': 'Bad Request',
|
||||
'message': 'An invalid ordering key of "unknown" was supplied',
|
||||
'status': 400,
|
||||
}
|
||||
assert error == expected
|
||||
|
||||
def test_query_base_module_br_filters(self):
|
||||
reuse_component_init_data()
|
||||
|
||||
Reference in New Issue
Block a user