🎨 Deprecate count/start query params and implement limit/offset (#3208)

* 🎨 Modify count/start query params to be Integers, not Strings Signed-off-by: ff137 <[email protected]> * 📝 Update openapi spec Signed-off-by: ff137 <[email protected]> * ⏪ Revert start/count field to remain String type; mark as deprecated Signed-off-by: ff137 <[email protected]> * ✨ Implement limit/offset alongside deprecated count/start Signed-off-by: ff137 <[email protected]> * 🎨 Standardise naming convention from count/start to limit/offset Signed-off-by: ff137 <[email protected]> * ⏪ Revert start/count field to remain String type; mark as deprecated Signed-off-by: ff137 <[email protected]> * ✨ Implement limit/offset alongside deprecated count/start Signed-off-by: ff137 <[email protected]> * 🎨 Remove unused query string schema from `w3c_creds_list` Signed-off-by: ff137 <[email protected]> * 🎨 Rename args Signed-off-by: ff137 <[email protected]> * 📝 Update openapi specs Signed-off-by: ff137 <[email protected]> * ✅ Signed-off-by: ff137 <[email protected]> * 🎨 Update start/count keywords to offset/limit Signed-off-by: ff137 <[email protected]> * 🎨 Replace validate lambda with Range Signed-off-by: ff137 <[email protected]> --------- Signed-off-by: ff137 <[email protected]> Co-authored-by: Stephen Curran <[email protected]> Co-authored-by: jamshale <[email protected]>
openwallet-foundation · Jan 29, 2025 · 558be60 · 558be60
1 parent 0b69442
commit 558be60
Show file tree

Hide file tree

Showing 11 changed files with 248 additions and 138 deletions.
diff --git a/acapy_agent/anoncreds/holder.py b/acapy_agent/anoncreds/holder.py
@@ -372,12 +372,12 @@ async def store_credential_w3c(
 
         return credential_id
 
-    async def get_credentials(self, start: int, count: int, wql: dict):
+    async def get_credentials(self, *, offset: int, limit: int, wql: dict):
         """Get credentials stored in the wallet.
 
         Args:
-            start: Starting index
-            count: Number of records to return
+            offset: Starting index
+            limit: Number of records to return
             wql: wql query dict
 
         """
@@ -388,8 +388,8 @@ async def get_credentials(self, start: int, count: int, wql: dict):
             rows = self.profile.store.scan(
                 category=CATEGORY_CREDENTIAL,
                 tag_filter=wql,
-                offset=start,
-                limit=count,
+                offset=offset,
+                limit=limit,
                 profile=self.profile.settings.get("wallet.askar_profile"),
             )
             async for row in rows:
@@ -406,17 +406,18 @@ async def get_credentials_for_presentation_request_by_referent(
         self,
         presentation_request: dict,
         referents: Sequence[str],
-        start: int,
-        count: int,
+        *,
+        offset: int,
+        limit: int,
         extra_query: Optional[dict] = None,
     ):
         """Get credentials stored in the wallet.
 
         Args:
             presentation_request: Valid presentation request from issuer
             referents: Presentation request referents to use to search for creds
-            start: Starting index
-            count: Maximum number of records to return
+            offset: Starting index
+            limit: Maximum number of records to return
             extra_query: wql query dict
 
         """
@@ -459,8 +460,8 @@ async def get_credentials_for_presentation_request_by_referent(
             rows = self.profile.store.scan(
                 category=CATEGORY_CREDENTIAL,
                 tag_filter=tag_filter,
-                offset=start,
-                limit=count,
+                offset=offset,
+                limit=limit,
                 profile=self.profile.settings.get("wallet.askar_profile"),
             )
             async for row in rows:

diff --git a/acapy_agent/anoncreds/models/utils.py b/acapy_agent/anoncreds/models/utils.py
@@ -39,8 +39,8 @@ async def get_requested_creds_from_proof_request_preview(
         credentials = await holder.get_credentials_for_presentation_request_by_referent(
             presentation_request=proof_request,
             referents=(referent,),
-            start=0,
-            count=100,
+            offset=0,
+            limit=100,
         )
         if not credentials:
             raise ValueError(_get_value_error_msg(proof_request, referent))
@@ -61,8 +61,8 @@ async def get_requested_creds_from_proof_request_preview(
         credentials = await holder.get_credentials_for_presentation_request_by_referent(
             presentation_request=proof_request,
             referents=(referent,),
-            start=0,
-            count=100,
+            offset=0,
+            limit=100,
         )
         if not credentials:
             raise ValueError(_get_value_error_msg(proof_request, referent))

diff --git a/acapy_agent/anoncreds/tests/test_holder.py b/acapy_agent/anoncreds/tests/test_holder.py
@@ -373,7 +373,7 @@ async def test_store_credential_failed_trx(self, *_):
     async def test_get_credentials(self, _):
         async with self.profile.session() as session:
             await session.handle.insert(CATEGORY_CREDENTIAL, json.dumps(MOCK_CRED))
-        result = await self.holder.get_credentials(0, 10, {})
+        result = await self.holder.get_credentials(offset=0, limit=10, wql={})
         assert isinstance(result, list)
         assert len(result) == 1
 
@@ -386,9 +386,9 @@ async def test_get_credentials_errors(self):
         )
 
         with self.assertRaises(AnonCredsHolderError):
-            await self.holder.get_credentials(0, 10, {})
+            await self.holder.get_credentials(offset=0, limit=10, wql={})
         with self.assertRaises(AnonCredsHolderError):
-            await self.holder.get_credentials(0, 10, {})
+            await self.holder.get_credentials(offset=0, limit=10, wql={})
 
     async def test_get_credentials_for_presentation_request_by_referent(self):
         self.profile.store.scan = mock.Mock(
@@ -408,13 +408,13 @@ async def test_get_credentials_for_presentation_request_by_referent(self):
             "restrictions": [{"schema_name": "MYCO Biomarker"}],
         }
         await self.holder.get_credentials_for_presentation_request_by_referent(
-            mock_pres_req, None, start=0, count=10
+            mock_pres_req, None, offset=0, limit=10
         )
 
         # non-existent referent
         with self.assertRaises(AnonCredsHolderError):
             await self.holder.get_credentials_for_presentation_request_by_referent(
-                mock_pres_req, "not-found-ref", start=0, count=10
+                mock_pres_req, "not-found-ref", offset=0, limit=10
             )
 
     @mock.patch.object(Credential, "load", return_value=MockCredential())

diff --git a/acapy_agent/holder/routes.py b/acapy_agent/holder/routes.py
@@ -13,6 +13,7 @@
 )
 from aries_askar import AskarErrorCode
 from marshmallow import fields
+from marshmallow.validate import Range
 
 from ..admin.decorators.auth import tenant_authentication
 from ..admin.request_context import AdminRequestContext
@@ -33,6 +34,7 @@
     NUM_STR_WHOLE_VALIDATE,
     UUID4_EXAMPLE,
 )
+from ..storage.base import DEFAULT_PAGE_SIZE, MAXIMUM_PAGE_SIZE
 from ..storage.error import StorageError, StorageNotFoundError
 from ..storage.vc_holder.base import VCHolder
 from ..storage.vc_holder.vc_record import VCRecordSchema
@@ -66,17 +68,34 @@ class CredentialsListQueryStringSchema(OpenAPISchema):
 
     start = fields.Str(
         required=False,
+        load_default=0,
         validate=NUM_STR_WHOLE_VALIDATE,
-        metadata={"description": "Start index", "example": NUM_STR_WHOLE_EXAMPLE},
+        metadata={
+            "description": "Start index (DEPRECATED - use offset instead)",
+            "example": NUM_STR_WHOLE_EXAMPLE,
+            "deprecated": True,
+        },
     )
     count = fields.Str(
         required=False,
+        load_default=10,
         validate=NUM_STR_NATURAL_VALIDATE,
         metadata={
-            "description": "Maximum number to retrieve",
+            "description": "Maximum number to retrieve (DEPRECATED - use limit instead)",
             "example": NUM_STR_NATURAL_EXAMPLE,
+            "deprecated": True,
         },
     )
+    limit = fields.Int(
+        required=False,
+        validate=Range(min=1, max=MAXIMUM_PAGE_SIZE),
+        metadata={"description": "Number of results to return", "example": 50},
+    )
+    offset = fields.Int(
+        required=False,
+        validate=Range(min=0),
+        metadata={"description": "Offset for pagination", "example": 0},
+    )
     wql = fields.Str(
         required=False,
         validate=INDY_WQL_VALIDATE,
@@ -379,25 +398,32 @@ async def credentials_list(request: web.BaseRequest):
 
     """
     context: AdminRequestContext = request["context"]
-    start = request.query.get("start")
-    count = request.query.get("count")
+
+    # Handle both old style start/count and new limit/offset
+    # TODO: Remove start/count and swap to PaginatedQuerySchema and get_limit_offset
+    if "limit" in request.query or "offset" in request.query:
+        # New style - use limit/offset
+        limit = int(request.query.get("limit", DEFAULT_PAGE_SIZE))
+        offset = int(request.query.get("offset", 0))
+    else:
+        # Old style - use start/count
+        limit = int(request.query.get("count", "10"))
+        offset = int(request.query.get("start", "0"))
 
     # url encoded json wql
     encoded_wql = request.query.get("wql") or "{}"
     wql = json.loads(encoded_wql)
 
-    # defaults
-    start = int(start) if isinstance(start, str) else 0
-    count = int(count) if isinstance(count, str) else 10
-
     if context.settings.get(wallet_type_config) == "askar-anoncreds":
         holder = AnonCredsHolder(context.profile)
-        credentials = await holder.get_credentials(start, count, wql)
+        credentials = await holder.get_credentials(limit=limit, offset=offset, wql=wql)
     else:
         async with context.profile.session() as session:
             holder = session.inject(IndyHolder)
             try:
-                credentials = await holder.get_credentials(start, count, wql)
+                credentials = await holder.get_credentials(
+                    limit=limit, offset=offset, wql=wql
+                )
             except IndyHolderError as err:
                 raise web.HTTPBadRequest(reason=err.roll_up) from err
 
@@ -476,7 +502,6 @@ async def w3c_cred_remove(request: web.BaseRequest):
     summary="Fetch W3C credentials from wallet",
 )
 @request_schema(W3CCredentialsListRequestSchema())
-@querystring_schema(CredentialsListQueryStringSchema())
 @response_schema(VCRecordListSchema(), 200, description="")
 @tenant_authentication
 async def w3c_creds_list(request: web.BaseRequest):

diff --git a/acapy_agent/indy/credx/holder.py b/acapy_agent/indy/credx/holder.py
@@ -236,12 +236,12 @@ async def store_credential(
 
         return credential_id
 
-    async def get_credentials(self, start: int, count: int, wql: dict):
+    async def get_credentials(self, *, offset: int, limit: int, wql: dict):
         """Get credentials stored in the wallet.
 
         Args:
-            start: Starting index
-            count: Number of records to return
+            offset: Starting index
+            limit: Number of records to return
             wql: wql query dict
 
         """
@@ -252,8 +252,8 @@ async def get_credentials(self, start: int, count: int, wql: dict):
             rows = self._profile.store.scan(
                 category=CATEGORY_CREDENTIAL,
                 tag_filter=wql,
-                offset=start,
-                limit=count,
+                offset=offset,
+                limit=limit,
                 profile=self._profile.settings.get("wallet.askar_profile"),
             )
             async for row in rows:
@@ -270,17 +270,18 @@ async def get_credentials_for_presentation_request_by_referent(
         self,
         presentation_request: dict,
         referents: Sequence[str],
-        start: int,
-        count: int,
+        *,
+        offset: int,
+        limit: int,
         extra_query: Optional[dict] = None,
     ):
         """Get credentials stored in the wallet.
 
         Args:
             presentation_request: Valid presentation request from issuer
             referents: Presentation request referents to use to search for creds
-            start: Starting index
-            count: Maximum number of records to return
+            offset: Starting index
+            limit: Maximum number of records to return
             extra_query: wql query dict
 
         """
@@ -323,8 +324,8 @@ async def get_credentials_for_presentation_request_by_referent(
             rows = self._profile.store.scan(
                 category=CATEGORY_CREDENTIAL,
                 tag_filter=tag_filter,
-                offset=start,
-                limit=count,
+                offset=offset,
+                limit=limit,
                 profile=self._profile.settings.get("wallet.askar_profile"),
             )
             async for row in rows:

diff --git a/acapy_agent/indy/credx/tests/test_cred_issuance.py b/acapy_agent/indy/credx/tests/test_cred_issuance.py
@@ -132,7 +132,7 @@ async def test_issue_store_non_rev(self):
 
         assert not await self.holder.get_mime_type(cred_id, "name")
 
-        creds = await self.holder.get_credentials(None, None, None)
+        creds = await self.holder.get_credentials(offset=None, limit=None, wql=None)
         assert len(creds) == 1
         assert creds[0] == stored_cred
 
@@ -142,9 +142,9 @@ async def test_issue_store_non_rev(self):
             await self.holder.get_credentials_for_presentation_request_by_referent(
                 PRES_REQ_NON_REV,
                 None,
-                0,
-                10,
-                {},
+                offset=0,
+                limit=10,
+                extra_query={},
             )
         )
         assert pres_creds == [
@@ -247,7 +247,7 @@ async def test_issue_store_rev(self):
             assert found
             stored_cred = json.loads(found)
 
-            creds = await self.holder.get_credentials(None, None, None)
+            creds = await self.holder.get_credentials(offset=None, limit=None, wql=None)
             assert len(creds) == 1
             assert creds[0] == stored_cred
 
@@ -257,9 +257,9 @@ async def test_issue_store_rev(self):
                 await self.holder.get_credentials_for_presentation_request_by_referent(
                     PRES_REQ_REV,
                     None,
-                    0,
-                    10,
-                    {},
+                    offset=0,
+                    limit=10,
+                    extra_query={},
                 )
             )
             assert pres_creds == [

diff --git a/acapy_agent/indy/models/xform.py b/acapy_agent/indy/models/xform.py
@@ -34,8 +34,8 @@ async def indy_proof_req_preview2indy_requested_creds(
         credentials = await holder.get_credentials_for_presentation_request_by_referent(
             presentation_request=indy_proof_req,
             referents=(referent,),
-            start=0,
-            count=100,
+            offset=0,
+            limit=100,
         )
         if not credentials:
             raise ValueError(
@@ -87,8 +87,8 @@ async def indy_proof_req_preview2indy_requested_creds(
         credentials = await holder.get_credentials_for_presentation_request_by_referent(
             presentation_request=indy_proof_req,
             referents=(referent,),
-            start=0,
-            count=100,
+            offset=0,
+            limit=100,
         )
         if not credentials:
             raise ValueError(