Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions acapy_agent/anoncreds/holder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand All @@ -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:
Expand All @@ -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

"""
Expand Down Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions acapy_agent/anoncreds/tests/test_holder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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(
Expand All @@ -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())
Expand Down
53 changes: 42 additions & 11 deletions acapy_agent/holder/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down
23 changes: 12 additions & 11 deletions acapy_agent/indy/credx/holder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand All @@ -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:
Expand All @@ -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

"""
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions acapy_agent/indy/credx/tests/test_cred_issuance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 == [
Expand Down Expand Up @@ -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

Expand All @@ -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 == [
Expand Down
8 changes: 4 additions & 4 deletions acapy_agent/indy/models/xform.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Loading
Loading