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

feat: add support for list pagination in list capability invocations #184

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Nov 18, 2022

Adds support for list pagination in store/list and upload/list.

Relevant notes on decisions made:

  • unfortunately, Databases like DynamoDB does not support pagination by just giving pageNumber and size. Aiming to have something generic (that works in DynamoDB, but also other DBs), chosen approach is to rely on cursor and size .
  • at first, attempted to make cursor a Link property of the capability. And it worked well for store/list, but then I got to use cases (upload/list for instance) where it cannot be a link. This is because Upload Table primary key contains ${dataCid}#${carCid} to be unique. So, in this case that is the cursor we need to have. There might be other kind of use cases, so having this as string is really the most flexible approach to support pagination.

Note:

BREAKING CHANGE: store/list and upload/list types now require nb object with optional properties

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 18, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2b20085
Status: ✅  Deploy successful!
Preview URL: https://2475a6b0.ucan-protocol.pages.dev
Branch Preview URL: https://feat-add-support-for-list-pa.ucan-protocol.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the feat/add-support-for-list-pagination-in-list-capability-invocation branch from 8291f4c to 6fa28b8 Compare November 18, 2022 10:21
@@ -328,6 +328,7 @@ describe('upload capabilities', function () {
audience: w3,
with: account.did(),
proofs: [await any],
nb: {},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Types require empty object when all nb properties are optional. need to look

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not happen theres a lot of infer types in ucanto to mitigate this. Did you open an issue in ucanto ?

packages/access-client/src/capabilities/upload.js Outdated Show resolved Hide resolved
packages/access-client/src/capabilities/store.js Outdated Show resolved Hide resolved
packages/access-client/src/capabilities/store.js Outdated Show resolved Hide resolved
packages/access-client/src/capabilities/upload.js Outdated Show resolved Hide resolved
@@ -130,6 +130,18 @@ export const list = base.derive({
* be stored.
*/
with: URI.match({ protocol: 'did:' }),
nb: {
/**
* Item where a previous list operation stopped, inclusive of the previous
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds specific to the particular implementation backing this. Maybe this could be more generic?

@vasco-santos vasco-santos marked this pull request as ready for review November 21, 2022 10:15
@vasco-santos vasco-santos force-pushed the feat/add-support-for-list-pagination-in-list-capability-invocation branch 2 times, most recently from 3b953fe to df2c7a9 Compare November 21, 2022 10:53
vasco-santos and others added 2 commits November 22, 2022 15:22
BREAKING CHANGE: store/list and upload/list types now require nb object with optional properties
@vasco-santos vasco-santos force-pushed the feat/add-support-for-list-pagination-in-list-capability-invocation branch from 9615cde to 28e6761 Compare November 22, 2022 14:22
@vasco-santos vasco-santos force-pushed the feat/add-support-for-list-pagination-in-list-capability-invocation branch from 28e6761 to 2b20085 Compare November 22, 2022 14:25
@vasco-santos vasco-santos merged commit ced23db into main Nov 22, 2022
@vasco-santos vasco-santos deleted the feat/add-support-for-list-pagination-in-list-capability-invocation branch November 22, 2022 14:47
hugomrdias added a commit that referenced this pull request Nov 22, 2022
* 'main' of https://github.com/web3-storage/w3-protocol:
  feat: add support for list pagination in list capability invocations (#184)
vasco-santos pushed a commit that referenced this pull request Nov 22, 2022
🤖 I have created a release *beep* *boop*
---


##
[6.0.0](access-v5.0.2...access-v6.0.0)
(2022-11-22)


### ⚠ BREAKING CHANGES

* **access-client:** bump to major
* store/list and upload/list types now require nb object with optional
properties

### Features

* [#153](#153)
([#177](#177))
([d6d448c](d6d448c))
* **access-client:** bump to major
([4d5899f](4d5899f))
* account recover with email
([#149](#149))
([6c659ba](6c659ba))
* add support for list pagination in list capability invocations
([#184](#184))
([ced23db](ced23db))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
gobengo pushed a commit that referenced this pull request Apr 11, 2023
…184)

Adds support for list pagination in `store/list` and `upload/list`.

Relevant notes on decisions made:
- unfortunately, Databases like DynamoDB does not support pagination by
just giving `pageNumber` and `size`. Aiming to have something generic
(that works in DynamoDB, but also other DBs), chosen approach is to rely
on `cursor` and `size` .
- at first, attempted to make `cursor` a Link property of the
capability. And it worked well for `store/list`, but then I got to use
cases (`upload/list` for instance) where it cannot be a link. This is
because Upload Table primary key contains `${dataCid}#${carCid}` to be
unique. So, in this case that is the cursor we need to have. There might
be other kind of use cases, so having this as string is really the most
flexible approach to support pagination.

BREAKING CHANGE: store/list and upload/list types now require nb object
with optional properties
gobengo pushed a commit that referenced this pull request Apr 11, 2023
* 'main' of https://github.com/web3-storage/w3-protocol:
  feat: add support for list pagination in list capability invocations (#184)
gobengo pushed a commit that referenced this pull request Apr 11, 2023
🤖 I have created a release *beep* *boop*
---


##
[6.0.0](access-v5.0.2...access-v6.0.0)
(2022-11-22)


### ⚠ BREAKING CHANGES

* **access-client:** bump to major
* store/list and upload/list types now require nb object with optional
properties

### Features

* [#153](#153)
([#177](#177))
([7fbf2a1](7fbf2a1))
* **access-client:** bump to major
([7be598d](7be598d))
* account recover with email
([#149](#149))
([91ad47d](91ad47d))
* add support for list pagination in list capability invocations
([#184](#184))
([23206ad](23206ad))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Peeja pushed a commit to storacha/upload-service that referenced this pull request Jan 17, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.0.1](storacha/w3ui@vue-uploads-list-v2.0.0...vue-uploads-list-v2.0.1)
(2022-12-15)


### Bug Fixes

* update dependencies
([996871f](storacha/w3ui@996871f))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Peeja pushed a commit to storacha/upload-service that referenced this pull request Jan 29, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.0.1](storacha/w3ui@vue-uploads-list-v2.0.0...vue-uploads-list-v2.0.1)
(2022-12-15)


### Bug Fixes

* update dependencies
([c4752b0](storacha/w3ui@c4752b0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants