✨ Add ordering options to askar scan and fetch_all methods (openwalle…

…t-foundation#3173) * ✨ add `order_by` and `descending` options to query / scan and fetch_all methods Signed-off-by: ff137 <[email protected]> * ✨ add `order_by` and `descending` options to PaginatedQuerySchema Signed-off-by: ff137 <[email protected]> * ✨ modify `get_limit_offset` to `get_paginated_query_params` Signed-off-by: ff137 <[email protected]> * 🎨 fix ruff warning Signed-off-by: ff137 <[email protected]> * ✅ fix assertions Signed-off-by: ff137 <[email protected]> * 🎨 Update order_by description and default value. Include in schema Signed-off-by: ff137 <[email protected]> * 🐛 fix deserialisation of descending: bool parameter Signed-off-by: ff137 <[email protected]> * 🔥 remove unnecessary record sorts (now that askar sorts by id == sorting by created_at) Signed-off-by: ff137 <[email protected]> * ✅ fix test assertions -- wallet_list and connections_list no longer does additional sorting Signed-off-by: ff137 <[email protected]> --------- Signed-off-by: ff137 <[email protected]> Co-authored-by: jamshale <[email protected]>
didx-xyz · Jan 29, 2025 · 0b69442 · 0b69442
1 parent 481d3c2
commit 0b69442
Show file tree

Hide file tree

Showing 13 changed files with 130 additions and 38 deletions.
diff --git a/acapy_agent/connections/routes.py b/acapy_agent/connections/routes.py
@@ -16,7 +16,10 @@
 from ..connections.models.conn_record import ConnRecord, ConnRecordSchema
 from ..messaging.models.base import BaseModelError
 from ..messaging.models.openapi import OpenAPISchema
-from ..messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ..messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ..messaging.valid import (
     ENDPOINT_EXAMPLE,
     ENDPOINT_VALIDATE,
@@ -247,20 +250,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",
@@ -305,7 +294,7 @@ async def connections_list(request: web.BaseRequest):
     if request.query.get("connection_protocol"):
         post_filter["connection_protocol"] = request.query["connection_protocol"]
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     profile = context.profile
     try:
@@ -315,11 +304,12 @@ async def connections_list(request: web.BaseRequest):
                 tag_filter,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
                 post_filter_positive=post_filter,
                 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/connections/tests/test_routes.py b/acapy_agent/connections/tests/test_routes.py
@@ -86,7 +86,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)
@@ -100,6 +100,8 @@ async def test_connections_list(self):
                     },
                     limit=100,
                     offset=0,
+                    order_by="id",
+                    descending=False,
                     post_filter_positive={
                         "their_role": list(ConnRecord.Role.REQUESTER.value),
                         "connection_protocol": "connections/1.0",

diff --git a/acapy_agent/messaging/models/base_record.py b/acapy_agent/messaging/models/base_record.py
@@ -293,6 +293,8 @@ async def query(
         *,
         limit: Optional[int] = None,
         offset: Optional[int] = None,
+        order_by: Optional[str] = None,
+        descending: bool = False,
         post_filter_positive: Optional[dict] = None,
         post_filter_negative: Optional[dict] = None,
         alt: bool = False,
@@ -304,6 +306,8 @@ async def query(
             tag_filter: An optional dictionary of tag filter clauses
             limit: The maximum number of records to retrieve
             offset: The offset to start retrieving records from
+            order_by: An optional field by which to order the records.
+            descending: Whether to order the records in descending order.
             post_filter_positive: Additional value filters to apply matching positively
             post_filter_negative: Additional value filters to apply matching negatively
             alt: set to match any (positive=True) value or miss all (positive=False)
@@ -327,11 +331,15 @@ async def query(
                 tag_query=tag_query,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
             )
         else:
             rows = await storage.find_all_records(
                 type_filter=cls.RECORD_TYPE,
                 tag_query=tag_query,
+                order_by=order_by,
+                descending=descending,
             )
 
         num_results_post_filter = 0  # used if applying pagination post-filter

diff --git a/acapy_agent/messaging/models/paginated_query.py b/acapy_agent/messaging/models/paginated_query.py
@@ -4,7 +4,7 @@
 
 from aiohttp.web import BaseRequest
 from marshmallow import fields
-from marshmallow.validate import Range
+from marshmallow.validate import OneOf, Range
 
 from ...messaging.models.openapi import OpenAPISchema
 from ...storage.base import DEFAULT_PAGE_SIZE, MAXIMUM_PAGE_SIZE
@@ -25,18 +25,47 @@ class PaginatedQuerySchema(OpenAPISchema):
         validate=Range(min=0),
         metadata={"description": "Offset for pagination", "example": 0},
     )
+    order_by = fields.Str(
+        required=False,
+        load_default="id",
+        validate=OneOf(["id"]),  # only one possible column supported in askar
+        metadata={
+            "description": (
+                'The column to order results by. Only "id" is currently supported.'
+            )
+        },
+        error_messages={"validator_failed": '`order_by` only supports column "id"'},
+    )
+    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."},
+    )
 
 
-def get_limit_offset(request: BaseRequest) -> Tuple[int, int]:
-    """Read the limit and offset query parameters from a request as ints, with defaults.
+def get_paginated_query_params(request: BaseRequest) -> Tuple[int, int, str, bool]:
+    """Read the limit, offset, order_by, and descending query parameters from a request.
 
     Args:
-        request: aiohttp request object
+        request: aiohttp request object.
 
     Returns:
-        A tuple of the limit and offset values
+        A tuple containing:
+        - limit (int): The number of results to return, defaulting to DEFAULT_PAGE_SIZE.
+        - offset (int): The offset for pagination, defaulting to 0.
+        - order_by (str): The field by which to order results, defaulting to "id".
+        - descending (bool): Order results in descending order; defaults to False.
     """
 
     limit = int(request.query.get("limit", DEFAULT_PAGE_SIZE))
     offset = int(request.query.get("offset", 0))
-    return limit, offset
+    order_by = request.query.get("order_by", "id")
+
+    # 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
@@ -164,6 +164,8 @@ 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,
+                order_by=None,
+                descending=False,
                 tag_query=tag_filter,
             )
             assert result and isinstance(result[0], BaseRecordImpl)
@@ -215,6 +217,8 @@ async def test_query_post_filter(self):
             )
             mock_storage.find_all_records.assert_awaited_once_with(
                 type_filter=ARecordImpl.RECORD_TYPE,
+                order_by=None,
+                descending=False,
                 tag_query=tag_filter,
             )
             assert result and isinstance(result[0], ARecordImpl)
@@ -339,6 +343,8 @@ async def test_query_with_limit(self):
                 tag_query=tag_filter,
                 limit=10,
                 offset=0,
+                order_by=None,
+                descending=False,
             )
             assert result and isinstance(result[0], ARecordImpl)
             assert result[0]._id == record_id
@@ -369,6 +375,8 @@ async def test_query_with_offset(self):
                 tag_query=tag_filter,
                 limit=DEFAULT_PAGE_SIZE,
                 offset=10,
+                order_by=None,
+                descending=False,
             )
             assert result and isinstance(result[0], ARecordImpl)
             assert result[0]._id == record_id
@@ -399,6 +407,8 @@ async def test_query_with_limit_and_offset(self):
                 tag_query=tag_filter,
                 limit=10,
                 offset=5,
+                order_by=None,
+                descending=False,
             )
             assert result and isinstance(result[0], ARecordImpl)
             assert result[0]._id == record_id
@@ -433,7 +443,10 @@ async def test_query_with_limit_and_offset_and_post_filter(self):
                 post_filter_positive={"a": "one"},
             )
             mock_storage.find_all_records.assert_awaited_once_with(
-                type_filter=ARecordImpl.RECORD_TYPE, tag_query=tag_filter
+                type_filter=ARecordImpl.RECORD_TYPE,
+                order_by=None,
+                descending=False,
+                tag_query=tag_filter,
             )
             assert len(result) == 10
             assert result and isinstance(result[0], ARecordImpl)

diff --git a/acapy_agent/multitenant/admin/routes.py b/acapy_agent/multitenant/admin/routes.py
@@ -16,7 +16,10 @@
 from ...core.profile import ProfileManagerProvider
 from ...messaging.models.base import BaseModelError
 from ...messaging.models.openapi import OpenAPISchema
-from ...messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ...messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ...messaging.valid import UUID4_EXAMPLE, JSONWebToken
 from ...multitenant.base import BaseMultitenantManager
 from ...storage.error import StorageError, StorageNotFoundError
@@ -381,7 +384,7 @@ async def wallets_list(request: web.BaseRequest):
     if wallet_name:
         query["wallet_name"] = wallet_name
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     try:
         async with profile.session() as session:
@@ -390,9 +393,10 @@ async def wallets_list(request: web.BaseRequest):
                 tag_filter=query,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                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
@@ -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/issue_credential/v1_0/routes.py b/acapy_agent/protocols/issue_credential/v1_0/routes.py
@@ -23,7 +23,10 @@
 from ....messaging.credential_definitions.util import CRED_DEF_TAGS
 from ....messaging.models.base import BaseModelError
 from ....messaging.models.openapi import OpenAPISchema
-from ....messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ....messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ....messaging.valid import (
     INDY_CRED_DEF_ID_EXAMPLE,
     INDY_CRED_DEF_ID_VALIDATE,
@@ -411,7 +414,7 @@ async def credential_exchange_list(request: web.BaseRequest):
         if request.query.get(k, "") != ""
     }
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     try:
         async with context.profile.session() as session:
@@ -420,6 +423,8 @@ async def credential_exchange_list(request: web.BaseRequest):
                 tag_filter=tag_filter,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
                 post_filter_positive=post_filter,
             )
         results = [record.serialize() for record in records]

diff --git a/acapy_agent/protocols/issue_credential/v2_0/routes.py b/acapy_agent/protocols/issue_credential/v2_0/routes.py
@@ -26,7 +26,10 @@
 from ....messaging.decorators.attach_decorator import AttachDecorator
 from ....messaging.models.base import BaseModelError
 from ....messaging.models.openapi import OpenAPISchema
-from ....messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ....messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ....messaging.valid import (
     ANONCREDS_CRED_DEF_ID_EXAMPLE,
     ANONCREDS_DID_EXAMPLE,
@@ -626,7 +629,7 @@ async def credential_exchange_list(request: web.BaseRequest):
         if request.query.get(k, "") != ""
     }
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     try:
         async with profile.session() as session:
@@ -635,6 +638,8 @@ async def credential_exchange_list(request: web.BaseRequest):
                 tag_filter=tag_filter,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
                 post_filter_positive=post_filter,
             )
 

diff --git a/acapy_agent/protocols/present_proof/v1_0/routes.py b/acapy_agent/protocols/present_proof/v1_0/routes.py
@@ -25,7 +25,10 @@
 from ....messaging.decorators.attach_decorator import AttachDecorator
 from ....messaging.models.base import BaseModelError
 from ....messaging.models.openapi import OpenAPISchema
-from ....messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ....messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ....messaging.valid import (
     INDY_EXTRA_WQL_EXAMPLE,
     INDY_EXTRA_WQL_VALIDATE,
@@ -309,7 +312,7 @@ async def presentation_exchange_list(request: web.BaseRequest):
         if request.query.get(k, "") != ""
     }
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     try:
         async with context.profile.session() as session:
@@ -318,6 +321,8 @@ async def presentation_exchange_list(request: web.BaseRequest):
                 tag_filter=tag_filter,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
                 post_filter_positive=post_filter,
             )
         results = [record.serialize() for record in records]

diff --git a/acapy_agent/protocols/present_proof/v2_0/routes.py b/acapy_agent/protocols/present_proof/v2_0/routes.py
@@ -27,7 +27,10 @@
 from ....messaging.decorators.attach_decorator import AttachDecorator
 from ....messaging.models.base import BaseModelError
 from ....messaging.models.openapi import OpenAPISchema
-from ....messaging.models.paginated_query import PaginatedQuerySchema, get_limit_offset
+from ....messaging.models.paginated_query import (
+    PaginatedQuerySchema,
+    get_paginated_query_params,
+)
 from ....messaging.valid import (
     INDY_EXTRA_WQL_EXAMPLE,
     INDY_EXTRA_WQL_VALIDATE,
@@ -468,7 +471,7 @@ async def present_proof_list(request: web.BaseRequest):
         if request.query.get(k, "") != ""
     }
 
-    limit, offset = get_limit_offset(request)
+    limit, offset, order_by, descending = get_paginated_query_params(request)
 
     try:
         async with profile.session() as session:
@@ -477,6 +480,8 @@ async def present_proof_list(request: web.BaseRequest):
                 tag_filter=tag_filter,
                 limit=limit,
                 offset=offset,
+                order_by=order_by,
+                descending=descending,
                 post_filter_positive=post_filter,
             )
         results = [record.serialize() for record in records]