-
Notifications
You must be signed in to change notification settings - Fork 516
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
ff137
wants to merge
17
commits into
openwallet-foundation:main
Choose a base branch
from
ff137:fix/start-count-types-for-proof-credential-list
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+266
−143
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
3838eb8
:art: Modify count/start query params to be Integers, not Strings
ff137 da28bc4
:memo: Update openapi spec
ff137 299e963
:rewind: Revert start/count field to remain String type; mark as depr…
ff137 56c6cdf
:sparkles: Implement limit/offset alongside deprecated count/start
ff137 37a6e28
:art: Standardise naming convention from count/start to limit/offset
ff137 7730ded
:rewind: Revert start/count field to remain String type; mark as depr…
ff137 1753b0d
:sparkles: Implement limit/offset alongside deprecated count/start
ff137 0e309d6
:art: Remove unused query string schema from `w3c_creds_list`
ff137 9bd5f30
:art: Rename args
ff137 94ab4d0
:memo: Update openapi specs
ff137 56d4010
Merge branch 'main' into fix/start-count-types-for-proof-credential-list
ff137 19cdaff
:white_check_mark:
ff137 5cc8805
Merge branch 'main' into fix/start-count-types-for-proof-credential-list
swcurran 3c16aef
:art: Update start/count keywords to offset/limit
ff137 dbfde34
Merge branch 'main' into fix/start-count-types-for-proof-credential-list
jamshale f19061f
Merge branch 'main' into fix/start-count-types-for-proof-credential-list
jamshale 0627431
Merge branch 'main' into fix/start-count-types-for-proof-credential-list
jamshale File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Range(min=0) Can be changed after the recent upgrades. |
||
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): | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Range(min=1, max=MAXIMUM_PAGE_SIZE)