From 28b79dcf1a2899604fb5277fb85af8e315e1a2bb Mon Sep 17 00:00:00 2001 From: Mourits de Beer <31511766+ff137@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:48:51 +0200 Subject: [PATCH] Rebase the pagination with ordering feature on latest release (#2) * :sparkles: add `order_by` and `descending` options to query / scan and fetch_all methods Signed-off-by: ff137 * :sparkles: add `order_by` and `descending` options to PaginatedQuerySchema Signed-off-by: ff137 * :sparkles: modify `get_limit_offset` to `get_paginated_query_params` Signed-off-by: ff137 * :sparkles: add ordering to InMemoryStorage scan and fetch_all methods Signed-off-by: ff137 * :construction: test in-progress aries-askar PR: https://github.com/hyperledger/aries-askar/pull/291 Signed-off-by: ff137 * :arrow_up: Update lock file Signed-off-by: ff137 * :art: fix ruff warning Signed-off-by: ff137 * :white_check_mark: fix assertions Signed-off-by: ff137 * :construction: test aries-askar with TestPyPI package Signed-off-by: ff137 * :construction: test latest askar testpypi package Signed-off-by: ff137 * :art: Update order_by description and default value. Include in schema Signed-off-by: ff137 * :bug: fix deserialisation of descending: bool parameter Signed-off-by: ff137 * :rewind: revert to testing 0.3.3b0 askar test package Signed-off-by: ff137 * :fire: remove unnecessary record sorts (now that askar sorts by id == sorting by created_at) Signed-off-by: ff137 * :white_check_mark: fix test assertions -- wallet_list and connections_list no longer does additional sorting Signed-off-by: ff137 * :arrow_up: Update lock file Signed-off-by: ff137 --------- Signed-off-by: ff137 --- acapy_agent/messaging/models/paginated_query.py | 9 ++++++++- .../messaging/models/tests/test_base_record.py | 2 +- acapy_agent/multitenant/admin/routes.py | 1 - .../multitenant/admin/tests/test_routes.py | 2 +- acapy_agent/protocols/connections/v1_0/routes.py | 15 --------------- .../connections/v1_0/tests/test_routes.py | 2 +- 6 files changed, 11 insertions(+), 20 deletions(-) diff --git a/acapy_agent/messaging/models/paginated_query.py b/acapy_agent/messaging/models/paginated_query.py index b938af772d..670c3cda2b 100644 --- a/acapy_agent/messaging/models/paginated_query.py +++ b/acapy_agent/messaging/models/paginated_query.py @@ -46,7 +46,10 @@ class PaginatedQuerySchema(OpenAPISchema): descending = fields.Bool( required=False, load_default=False, + truthy={"true", "1", "yes"}, + falsy={"false", "0", "no"}, metadata={"description": "Order results in descending order if true"}, + error_messages={"invalid": "Not a valid boolean."}, ) @@ -67,5 +70,9 @@ def get_paginated_query_params(request: BaseRequest) -> Tuple[int, int, str, boo limit = int(request.query.get("limit", DEFAULT_PAGE_SIZE)) offset = int(request.query.get("offset", 0)) order_by = request.query.get("order_by", "id") - descending = bool(request.query.get("descending", False)) + + # Convert the 'descending' parameter to a boolean + descending_str = request.query.get("descending", "False").lower() + descending = descending_str in {"true", "1", "yes"} + return limit, offset, order_by, descending diff --git a/acapy_agent/messaging/models/tests/test_base_record.py b/acapy_agent/messaging/models/tests/test_base_record.py index 36bf7fa7db..672705dfd7 100644 --- a/acapy_agent/messaging/models/tests/test_base_record.py +++ b/acapy_agent/messaging/models/tests/test_base_record.py @@ -164,9 +164,9 @@ async def test_query(self): result = await BaseRecordImpl.query(session, tag_filter) mock_storage.find_all_records.assert_awaited_once_with( type_filter=BaseRecordImpl.RECORD_TYPE, - tag_query=tag_filter, order_by=None, descending=False, + tag_query=tag_filter, ) assert result and isinstance(result[0], BaseRecordImpl) assert result[0]._id == record_id diff --git a/acapy_agent/multitenant/admin/routes.py b/acapy_agent/multitenant/admin/routes.py index a569902ad4..470e4e059b 100644 --- a/acapy_agent/multitenant/admin/routes.py +++ b/acapy_agent/multitenant/admin/routes.py @@ -399,7 +399,6 @@ async def wallets_list(request: web.BaseRequest): descending=descending, ) results = [format_wallet_record(record) for record in records] - results.sort(key=lambda w: w["created_at"]) except (StorageError, BaseModelError) as err: raise web.HTTPBadRequest(reason=err.roll_up) from err diff --git a/acapy_agent/multitenant/admin/tests/test_routes.py b/acapy_agent/multitenant/admin/tests/test_routes.py index 87dcdde88d..de989bf2b4 100644 --- a/acapy_agent/multitenant/admin/tests/test_routes.py +++ b/acapy_agent/multitenant/admin/tests/test_routes.py @@ -87,7 +87,7 @@ async def test_wallets_list(self): ), ] mock_wallet_record.query = mock.CoroutineMock() - mock_wallet_record.query.return_value = [wallets[2], wallets[0], wallets[1]] + mock_wallet_record.query.return_value = wallets await test_module.wallets_list(self.request) mock_response.assert_called_once_with( diff --git a/acapy_agent/protocols/connections/v1_0/routes.py b/acapy_agent/protocols/connections/v1_0/routes.py index bafe178ede..8277484e40 100644 --- a/acapy_agent/protocols/connections/v1_0/routes.py +++ b/acapy_agent/protocols/connections/v1_0/routes.py @@ -414,20 +414,6 @@ class EndpointsResultSchema(OpenAPISchema): ) -def connection_sort_key(conn): - """Get the sorting key for a particular connection.""" - - conn_rec_state = ConnRecord.State.get(conn["state"]) - if conn_rec_state is ConnRecord.State.ABANDONED: - pfx = "2" - elif conn_rec_state is ConnRecord.State.INVITATION: - pfx = "1" - else: - pfx = "0" - - return pfx + conn["created_at"] - - @docs( tags=["connection"], summary="Query agent-to-agent connections", @@ -488,7 +474,6 @@ async def connections_list(request: web.BaseRequest): alt=True, ) results = [record.serialize() for record in records] - results.sort(key=connection_sort_key) except (StorageError, BaseModelError) as err: raise web.HTTPBadRequest(reason=err.roll_up) from err diff --git a/acapy_agent/protocols/connections/v1_0/tests/test_routes.py b/acapy_agent/protocols/connections/v1_0/tests/test_routes.py index 7ae1129a19..05e279e75b 100644 --- a/acapy_agent/protocols/connections/v1_0/tests/test_routes.py +++ b/acapy_agent/protocols/connections/v1_0/tests/test_routes.py @@ -87,7 +87,7 @@ async def test_connections_list(self): ) ), ] - mock_conn_rec.query.return_value = [conns[2], conns[0], conns[1]] # jumbled + mock_conn_rec.query.return_value = conns with mock.patch.object(test_module.web, "json_response") as mock_response: await test_module.connections_list(self.request)