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

* art: Modify count/start query params to be Integers, not Strings Signed-off-by: ff137 <[email protected]> * memo: Update openapi spec Signed-off-by: ff137 <[email protected]> * rewind: Revert start/count field to remain String type; mark as deprecated Signed-off-by: ff137 <[email protected]> * sparkles: Implement limit/offset alongside deprecated count/start Signed-off-by: ff137 <[email protected]> * art: Standardise naming convention from count/start to limit/offset Signed-off-by: ff137 <[email protected]> * rewind: Revert start/count field to remain String type; mark as deprecated Signed-off-by: ff137 <[email protected]> * sparkles: Implement limit/offset alongside deprecated count/start Signed-off-by: ff137 <[email protected]> * art: Remove unused query string schema from `w3c_creds_list` Signed-off-by: ff137 <[email protected]> * art: Rename args Signed-off-by: ff137 <[email protected]> * memo: Update openapi specs Signed-off-by: ff137 <[email protected]> * white_check_mark: Signed-off-by: ff137 <[email protected]> --------- Signed-off-by: ff137 <[email protected]>
didx-xyz · Nov 14, 2024 · 765e281 · 765e281
1 parent 665700d
commit 765e281
Show file tree

Hide file tree

Showing 10 changed files with 294 additions and 171 deletions.
diff --git a/acapy_agent/anoncreds/holder.py b/acapy_agent/anoncreds/holder.py
@@ -379,12 +379,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
 
         """
@@ -395,8 +395,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:
@@ -413,17 +413,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
 
         """
@@ -466,8 +467,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/tests/test_holder.py b/acapy_agent/anoncreds/tests/test_holder.py
@@ -397,7 +397,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
 
@@ -410,9 +410,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(
@@ -432,13 +432,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
@@ -33,6 +33,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 +67,41 @@ 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=lambda x: x > 0 and x <= MAXIMUM_PAGE_SIZE,
+        metadata={"description": "Number of results to return", "example": 50},
+        error_messages={
+            "validator_failed": (
+                "Value must be greater than 0 and "
+                f"less than or equal to {MAXIMUM_PAGE_SIZE}"
+            )
         },
     )
+    offset = fields.Int(
+        required=False,
+        validate=lambda x: x >= 0,
+        metadata={"description": "Offset for pagination", "example": 0},
+        error_messages={"validator_failed": "Value must be 0 or greater"},
+    )
     wql = fields.Str(
         required=False,
         validate=INDY_WQL_VALIDATE,
@@ -379,25 +404,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 +508,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(