From 45b18cdbd416cf55e74b7a2a8a7c0f140fd737da Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 4 Dec 2023 15:36:51 +0000 Subject: [PATCH 1/9] feat: return allocated bytes in store/add receipt --- packages/capabilities/src/types.ts | 32 ++++++--- packages/upload-api/src/store/add.js | 6 +- packages/upload-api/test/handlers/store.js | 71 +++++++++++++++++-- packages/upload-client/test/index.test.js | 31 ++++---- packages/upload-client/test/store.test.js | 4 ++ .../w3up-client/test/capability/store.test.js | 3 +- packages/w3up-client/test/client.test.js | 24 +++---- 7 files changed, 129 insertions(+), 42 deletions(-) diff --git a/packages/capabilities/src/types.ts b/packages/capabilities/src/types.ts index 2c17f5d11..7bfe14c23 100644 --- a/packages/capabilities/src/types.ts +++ b/packages/capabilities/src/types.ts @@ -438,18 +438,34 @@ export type StoreRemove = InferInvokedCapability export type StoreList = InferInvokedCapability export type StoreAddSuccess = StoreAddSuccessDone | StoreAddSuccessUpload -export interface StoreAddSuccessDone { - status: 'done' + +export type StoreAddSuccessStatusUpload = 'upload' +export type StoreAddSuccessStatusDone = 'done' + +export interface StoreAddSuccessResult { + /** + * Status of the item to store. A "done" status incidactes that it is not + * necessary to upload the item. An "upload" status indicates that the item + * should be uploaded to the provided URL. + */ + status: StoreAddSuccessStatusUpload|StoreAddSuccessStatusDone + /** + * Total bytes allocated in the space to accommodate this stored item. + * May be zero if the item is _already_ stored in _this_ space. + */ + allocated: number + /** DID of the space this item will be stored in. */ with: DID + /** CID of the item. */ link: UnknownLink - url?: undefined - headers?: undefined } -export interface StoreAddSuccessUpload { - status: 'upload' - with: DID - link: UnknownLink +export interface StoreAddSuccessDone extends StoreAddSuccessResult { + status: StoreAddSuccessStatusDone +} + +export interface StoreAddSuccessUpload extends StoreAddSuccessResult { + status: StoreAddSuccessStatusUpload url: ToString headers: Record } diff --git a/packages/upload-api/src/store/add.js b/packages/upload-api/src/store/add.js index e424a2f81..7f0868bf5 100644 --- a/packages/upload-api/src/store/add.js +++ b/packages/upload-api/src/store/add.js @@ -24,7 +24,7 @@ export function storeAddProvider(context) { Server.DID.parse(capability.with).did() ) const issuer = invocation.issuer.did() - const [allocated, carIsLinkedToAccount, carExists] = await Promise.all([ + const [allocated, carIsLinkedToSpace, carExists] = await Promise.all([ allocate( { capability: { @@ -42,7 +42,7 @@ export function storeAddProvider(context) { return allocated } - if (!carIsLinkedToAccount) { + if (!carIsLinkedToSpace) { await storeTable.insert({ space, link, @@ -57,6 +57,7 @@ export function storeAddProvider(context) { return { ok: { status: 'done', + allocated: carIsLinkedToSpace ? 0 : size, with: space, link, }, @@ -67,6 +68,7 @@ export function storeAddProvider(context) { return { ok: { status: 'upload', + allocated: size, with: space, link, url: url.toString(), diff --git a/packages/upload-api/test/handlers/store.js b/packages/upload-api/test/handlers/store.js index 0d51ef246..6fc7e2c8c 100644 --- a/packages/upload-api/test/handlers/store.js +++ b/packages/upload-api/test/handlers/store.js @@ -36,8 +36,10 @@ export const test = { if (!storeAdd.out.ok) { throw new Error('invocation failed', { cause: storeAdd }) } + if (storeAdd.out.ok.status !== 'upload') { + throw new Error(`unexpected status: ${storeAdd.out.ok.status}`) + } - assert.equal(storeAdd.out.ok.status, 'upload') assert.equal(storeAdd.out.ok.with, spaceDid) assert.deepEqual(storeAdd.out.ok.link.toString(), link.toString()) @@ -177,9 +179,11 @@ export const test = { if (!storeAdd.out.ok) { throw new Error('invocation failed', { cause: storeAdd }) } + if (storeAdd.out.ok.status !== 'upload') { + throw new Error(`unexpected status: ${storeAdd.out.ok.status}`) + } - const url = - storeAdd.out.ok.status === 'upload' && new URL(storeAdd.out.ok.url) + const url = new URL(storeAdd.out.ok.url) if (!url) { throw new Error('Expected presigned url in response') } @@ -227,9 +231,11 @@ export const test = { if (!storeAdd.out.ok) { throw new Error('invocation failed', { cause: storeAdd }) } + if (storeAdd.out.ok.status !== 'upload') { + throw new Error(`unexpected status: ${storeAdd.out.ok.status}`) + } - const url = - storeAdd.out.ok.status === 'upload' && new URL(storeAdd.out.ok.url) + const url = new URL(storeAdd.out.ok.url) if (!url) { throw new Error('Expected presigned url in response') } @@ -287,8 +293,10 @@ export const test = { } assert.equal(storeAdd.out.ok.status, 'done') + assert.equal(storeAdd.out.ok.allocated, 5) assert.equal(storeAdd.out.ok.with, spaceDid) assert.deepEqual(storeAdd.out.ok.link.toString(), link.toString()) + // @ts-expect-error making sure it's not an upload status assert.equal(storeAdd.out.ok.url == null, true) const item = await context.storeTable.get(spaceDid, link) @@ -313,6 +321,59 @@ export const test = { ) }, + 'store/add returns allocated: 0 if already added to space': async (assert, context) => { + const { proof, spaceDid } = await registerSpace(alice, context) + const connection = connect({ + id: context.id, + channel: createServer(context), + }) + + const data = new Uint8Array([11, 22, 34, 44, 55]) + const link = await CAR.codec.link(data) + + const { url, headers } = await context.carStoreBucket.createUploadUrl( + link, + data.length + ) + + // simulate an already stored CAR + const put = await fetch(url, { + method: 'PUT', + mode: 'cors', + body: data, + headers, + }) + assert.equal(put.ok, true, 'should be able to upload to presigned url') + + const inv0 = StoreCapabilities.add.invoke({ + issuer: alice, + audience: connection.id, + with: spaceDid, + nb: { link, size: data.byteLength }, + proofs: [proof], + }) + + const r0 = await inv0.execute(connection) + + assert.equal(r0.out.ok?.status, 'done') + assert.equal(r0.out.ok?.allocated, 5) + assert.equal(r0.out.ok?.with, spaceDid) + + const inv1 = StoreCapabilities.add.invoke({ + issuer: alice, + audience: connection.id, + with: spaceDid, + nb: { link, size: data.byteLength }, + proofs: [proof], + }) + + const r1 = await inv1.execute(connection) + + assert.equal(r1.out.ok?.status, 'done') + assert.equal(r1.out.ok?.allocated, 0) + assert.equal(r1.out.ok?.with, spaceDid) + }, + 'store/add disallowed if invocation fails access verification': async ( assert, context diff --git a/packages/upload-client/test/index.test.js b/packages/upload-client/test/index.test.js index 11627871c..ede990904 100644 --- a/packages/upload-client/test/index.test.js +++ b/packages/upload-client/test/index.test.js @@ -51,7 +51,7 @@ describe('uploadFile', () => { }), ]) - /** @type {import('../src/types.js').StoreAddSuccessUpload} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -62,13 +62,12 @@ describe('uploadFile', () => { const service = mockService({ store: { - add: provide(StoreCapabilities.add, ({ invocation }) => { + add: provide(StoreCapabilities.add, ({ invocation, capability }) => { assert.equal(invocation.issuer.did(), agent.did()) assert.equal(invocation.capabilities.length, 1) - const invCap = invocation.capabilities[0] - assert.equal(invCap.can, StoreCapabilities.add.can) - assert.equal(invCap.with, space.did()) - return { ok: res } + assert.equal(capability.can, StoreCapabilities.add.can) + assert.equal(capability.with, space.did()) + return { ok: { ...res, allocated: capability.nb.size } } }), }, upload: { @@ -143,7 +142,7 @@ describe('uploadFile', () => { }), ]) - /** @type {Omit} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -159,6 +158,7 @@ describe('uploadFile', () => { link: /** @type {import('../src/types.js').CARLink} */ ( capability.nb.link ), + allocated: capability.nb.size, }, })), }, @@ -229,7 +229,7 @@ describe('uploadDirectory', () => { }), ]) - /** @type {Omit} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -251,6 +251,7 @@ describe('uploadDirectory', () => { link: /** @type {import('../src/types.js').CARLink} */ ( capability.nb.link ), + allocated: capability.nb.size, }, } }), @@ -323,7 +324,7 @@ describe('uploadDirectory', () => { }), ]) - /** @type {Omit} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -339,6 +340,7 @@ describe('uploadDirectory', () => { link: /** @type {import('../src/types.js').CARLink} */ ( capability.nb.link ), + allocated: capability.nb.size, }, })), }, @@ -545,7 +547,7 @@ describe('uploadCAR', () => { }), ]) - /** @type {Omit} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -567,6 +569,7 @@ describe('uploadCAR', () => { link: /** @type {import('../src/types.js').CARLink} */ ( capability.nb.link ), + allocated: capability.nb.size, }, } }), @@ -646,7 +649,7 @@ describe('uploadCAR', () => { }), ]) - /** @type {Omit} */ + /** @type {Omit} */ const res = { status: 'upload', headers: { 'x-test': 'true' }, @@ -659,15 +662,15 @@ describe('uploadCAR', () => { add: provide(StoreCapabilities.add, ({ capability, invocation }) => { assert.equal(invocation.issuer.did(), agent.did()) assert.equal(invocation.capabilities.length, 1) - const invCap = invocation.capabilities[0] - assert.equal(invCap.can, StoreCapabilities.add.can) - assert.equal(invCap.with, space.did()) + assert.equal(capability.can, StoreCapabilities.add.can) + assert.equal(capability.with, space.did()) return { ok: { ...res, link: /** @type {import('../src/types.js').CARLink} */ ( capability.nb.link ), + allocated: capability.nb.size, }, } }), diff --git a/packages/upload-client/test/store.test.js b/packages/upload-client/test/store.test.js index 2f53d5f8c..02deae8a5 100644 --- a/packages/upload-client/test/store.test.js +++ b/packages/upload-client/test/store.test.js @@ -33,6 +33,7 @@ describe('Store.add', () => { url: 'http://localhost:9200', link: car.cid, with: space.did(), + allocated: car.size, } const service = mockService({ @@ -103,6 +104,7 @@ describe('Store.add', () => { url: 'http://localhost:9400', // this bucket always returns a 400 link: car.cid, with: space.did(), + allocated: car.size, } const service = mockService({ @@ -156,6 +158,7 @@ describe('Store.add', () => { url: 'http://localhost:9500', // this bucket always returns a 500 link: car.cid, with: space.did(), + allocated: car.size, } const service = mockService({ @@ -254,6 +257,7 @@ describe('Store.add', () => { url: 'http://localhost:9200', link: car.cid, with: space.did(), + allocated: car.size, } const service = mockService({ diff --git a/packages/w3up-client/test/capability/store.test.js b/packages/w3up-client/test/capability/store.test.js index 9de3da339..800de7b26 100644 --- a/packages/w3up-client/test/capability/store.test.js +++ b/packages/w3up-client/test/capability/store.test.js @@ -14,7 +14,7 @@ describe('StoreClient', () => { it('should store a CAR file', async () => { const service = mockService({ store: { - add: provide(StoreCapabilities.add, ({ invocation }) => { + add: provide(StoreCapabilities.add, ({ invocation, capability }) => { assert.equal(invocation.issuer.did(), alice.agent.did()) assert.equal(invocation.capabilities.length, 1) const invCap = invocation.capabilities[0] @@ -27,6 +27,7 @@ describe('StoreClient', () => { url: 'http://localhost:9200', link: car.cid, with: space.did(), + allocated: capability.nb.size, }, } }), diff --git a/packages/w3up-client/test/client.test.js b/packages/w3up-client/test/client.test.js index 9316afcb4..3d92d3748 100644 --- a/packages/w3up-client/test/client.test.js +++ b/packages/w3up-client/test/client.test.js @@ -30,12 +30,11 @@ describe('Client', () => { const service = mockService({ store: { - add: provide(StoreCapabilities.add, ({ invocation }) => { + add: provide(StoreCapabilities.add, ({ invocation, capability }) => { assert.equal(invocation.issuer.did(), alice.agent.did()) assert.equal(invocation.capabilities.length, 1) - const invCap = invocation.capabilities[0] - assert.equal(invCap.can, StoreCapabilities.add.can) - assert.equal(invCap.with, alice.currentSpace()?.did()) + assert.equal(capability.can, StoreCapabilities.add.can) + assert.equal(capability.with, alice.currentSpace()?.did()) return { ok: { @@ -46,6 +45,7 @@ describe('Client', () => { invocation.capabilities[0].nb?.link ), with: space.did(), + allocated: capability.nb.size, }, } }), @@ -126,12 +126,11 @@ describe('Client', () => { const service = mockService({ store: { - add: provide(StoreCapabilities.add, ({ invocation }) => { + add: provide(StoreCapabilities.add, ({ invocation, capability }) => { assert.equal(invocation.issuer.did(), alice.agent.did()) assert.equal(invocation.capabilities.length, 1) - const invCap = invocation.capabilities[0] - assert.equal(invCap.can, StoreCapabilities.add.can) - assert.equal(invCap.with, alice.currentSpace()?.did()) + assert.equal(capability.can, StoreCapabilities.add.can) + assert.equal(capability.with, alice.currentSpace()?.did()) return { ok: { status: 'upload', @@ -141,6 +140,7 @@ describe('Client', () => { invocation.capabilities[0].nb?.link ), with: space.did(), + allocated: capability.nb.size, }, } }), @@ -204,12 +204,11 @@ describe('Client', () => { const service = mockService({ store: { - add: provide(StoreCapabilities.add, ({ invocation }) => { + add: provide(StoreCapabilities.add, ({ invocation, capability }) => { assert.equal(invocation.issuer.did(), alice.agent.did()) assert.equal(invocation.capabilities.length, 1) - const invCap = invocation.capabilities[0] - assert.equal(invCap.can, StoreCapabilities.add.can) - assert.equal(invCap.with, space.did()) + assert.equal(capability.can, StoreCapabilities.add.can) + assert.equal(capability.with, space.did()) return { ok: { status: 'upload', @@ -219,6 +218,7 @@ describe('Client', () => { invocation.capabilities[0].nb?.link ), with: space.did(), + allocated: capability.nb.size, }, } }), From b351ab95775d40baaff58e224597ce61442e8cf1 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 4 Dec 2023 15:38:12 +0000 Subject: [PATCH 2/9] fix: typo --- packages/capabilities/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/capabilities/src/types.ts b/packages/capabilities/src/types.ts index 7bfe14c23..2e0d35f8c 100644 --- a/packages/capabilities/src/types.ts +++ b/packages/capabilities/src/types.ts @@ -444,7 +444,7 @@ export type StoreAddSuccessStatusDone = 'done' export interface StoreAddSuccessResult { /** - * Status of the item to store. A "done" status incidactes that it is not + * Status of the item to store. A "done" status indicates that it is not * necessary to upload the item. An "upload" status indicates that the item * should be uploaded to the provided URL. */ From f8d24cdafd0f0ff813cdf5011b5618df92812d75 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 4 Dec 2023 15:49:49 +0000 Subject: [PATCH 3/9] chore: appease linter --- packages/capabilities/src/types.ts | 2 +- packages/eslint-config-w3up/index.js | 2 +- packages/filecoin-api/test/events/aggregator.js | 2 +- packages/upload-api/test/handlers/store.js | 5 ++++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/capabilities/src/types.ts b/packages/capabilities/src/types.ts index 2e0d35f8c..292b090e2 100644 --- a/packages/capabilities/src/types.ts +++ b/packages/capabilities/src/types.ts @@ -448,7 +448,7 @@ export interface StoreAddSuccessResult { * necessary to upload the item. An "upload" status indicates that the item * should be uploaded to the provided URL. */ - status: StoreAddSuccessStatusUpload|StoreAddSuccessStatusDone + status: StoreAddSuccessStatusUpload | StoreAddSuccessStatusDone /** * Total bytes allocated in the space to accommodate this stored item. * May be zero if the item is _already_ stored in _this_ space. diff --git a/packages/eslint-config-w3up/index.js b/packages/eslint-config-w3up/index.js index f0cc00880..8d7f02ee8 100644 --- a/packages/eslint-config-w3up/index.js +++ b/packages/eslint-config-w3up/index.js @@ -10,7 +10,7 @@ module.exports = { EXPERIMENTAL_useProjectService: true, }, rules: { - "@typescript-eslint/no-floating-promises": "error", + '@typescript-eslint/no-floating-promises': 'error', '@typescript-eslint/no-unused-vars': 'off', '@typescript-eslint/ban-ts-comment': 'off', diff --git a/packages/filecoin-api/test/events/aggregator.js b/packages/filecoin-api/test/events/aggregator.js index 4e092cb03..c15844c59 100644 --- a/packages/filecoin-api/test/events/aggregator.js +++ b/packages/filecoin-api/test/events/aggregator.js @@ -738,7 +738,7 @@ export const test = { minPieceInsertedAt: new Date().toISOString(), } const putAggregateRes = await context.aggregateStore.put( - aggregateRecord, + aggregateRecord ) assert.ok(putAggregateRes.ok) diff --git a/packages/upload-api/test/handlers/store.js b/packages/upload-api/test/handlers/store.js index 6fc7e2c8c..b52bd62ed 100644 --- a/packages/upload-api/test/handlers/store.js +++ b/packages/upload-api/test/handlers/store.js @@ -321,7 +321,10 @@ export const test = { ) }, - 'store/add returns allocated: 0 if already added to space': async (assert, context) => { + 'store/add returns allocated: 0 if already added to space': async ( + assert, + context + ) => { const { proof, spaceDid } = await registerSpace(alice, context) const connection = connect({ id: context.id, From 07e222557a758e987e0b1ca2d978d80bcb645466 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 4 Dec 2023 15:51:56 +0000 Subject: [PATCH 4/9] fix: tests --- packages/upload-client/test/index.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/upload-client/test/index.test.js b/packages/upload-client/test/index.test.js index ede990904..97b3eee28 100644 --- a/packages/upload-client/test/index.test.js +++ b/packages/upload-client/test/index.test.js @@ -411,6 +411,7 @@ describe('uploadDirectory', () => { link: /** @type {import('../src/types.js').CARLink} */ ( invocation.capability.nb.link ), + allocated: invocation.capability.nb.size, }, } }), From 880f0013049862658e07038016b647c323920f16 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 4 Dec 2023 16:14:05 +0000 Subject: [PATCH 5/9] refactor: apply suggestions from code review Co-authored-by: Vasco Santos --- packages/upload-api/test/handlers/store.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/upload-api/test/handlers/store.js b/packages/upload-api/test/handlers/store.js index b52bd62ed..a352a2c37 100644 --- a/packages/upload-api/test/handlers/store.js +++ b/packages/upload-api/test/handlers/store.js @@ -293,7 +293,7 @@ export const test = { } assert.equal(storeAdd.out.ok.status, 'done') - assert.equal(storeAdd.out.ok.allocated, 5) + assert.equal(storeAdd.out.ok.allocated, data.byteLength) assert.equal(storeAdd.out.ok.with, spaceDid) assert.deepEqual(storeAdd.out.ok.link.toString(), link.toString()) // @ts-expect-error making sure it's not an upload status @@ -359,7 +359,7 @@ export const test = { const r0 = await inv0.execute(connection) assert.equal(r0.out.ok?.status, 'done') - assert.equal(r0.out.ok?.allocated, 5) + assert.equal(r0.out.ok?.allocated, data.byteLength) assert.equal(r0.out.ok?.with, spaceDid) const inv1 = StoreCapabilities.add.invoke({ From 313fe503ec21e705e5f084a4f78190dc270ca618 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 6 Dec 2023 14:20:57 +0000 Subject: [PATCH 6/9] feat: result return types for upload and store table --- packages/capabilities/src/types.ts | 9 +-- .../upload-api/src/admin/store/inspect.js | 4 +- .../upload-api/src/admin/upload/inspect.js | 4 +- packages/upload-api/src/store/add.js | 33 +++++---- packages/upload-api/src/store/get.js | 14 ++-- packages/upload-api/src/store/lib.js | 29 ++++++++ packages/upload-api/src/store/list.js | 9 +-- packages/upload-api/src/store/remove.js | 41 ++--------- packages/upload-api/src/types.ts | 46 ++++++++---- packages/upload-api/src/upload/add.js | 4 +- packages/upload-api/src/upload/get.js | 14 ++-- packages/upload-api/src/upload/lib.js | 29 ++++++++ packages/upload-api/src/upload/list.js | 9 +-- packages/upload-api/src/upload/remove.js | 12 ++-- packages/upload-api/test/handlers/store.js | 22 +++--- packages/upload-api/test/handlers/upload.js | 29 ++++---- packages/upload-api/test/helpers/result.js | 22 ++++++ .../upload-api/test/storage/store-table.js | 72 ++++++++++--------- .../upload-api/test/storage/upload-table.js | 72 ++++++++++--------- .../test/capability/upload.test.js | 13 +++- 20 files changed, 272 insertions(+), 215 deletions(-) create mode 100644 packages/upload-api/src/store/lib.js create mode 100644 packages/upload-api/src/upload/lib.js create mode 100644 packages/upload-api/test/helpers/result.js diff --git a/packages/capabilities/src/types.ts b/packages/capabilities/src/types.ts index 292b090e2..7eb8bf111 100644 --- a/packages/capabilities/src/types.ts +++ b/packages/capabilities/src/types.ts @@ -513,14 +513,7 @@ export type UploadAddSuccess = Omit export type UploadGetSuccess = UploadListItem -export type UploadRemoveSuccess = UploadDidRemove | UploadDidNotRemove - -export interface UploadDidRemove extends UploadAddSuccess {} - -export interface UploadDidNotRemove { - root?: undefined - shards?: undefined -} +export type UploadRemoveSuccess = UploadAddSuccess export interface UploadListSuccess extends ListResponse {} diff --git a/packages/upload-api/src/admin/store/inspect.js b/packages/upload-api/src/admin/store/inspect.js index e107be728..1ab7fa757 100644 --- a/packages/upload-api/src/admin/store/inspect.js +++ b/packages/upload-api/src/admin/store/inspect.js @@ -24,9 +24,7 @@ const inspect = async ({ capability }, context) => { return { error: new UnknownProvider(capability.with) } } - return { - ok: await context.storeTable.inspect(capability.nb.link), - } + return await context.storeTable.inspect(capability.nb.link) } class UnknownProvider extends Provider.Failure { diff --git a/packages/upload-api/src/admin/upload/inspect.js b/packages/upload-api/src/admin/upload/inspect.js index 56a1cf599..e7c49d1e2 100644 --- a/packages/upload-api/src/admin/upload/inspect.js +++ b/packages/upload-api/src/admin/upload/inspect.js @@ -24,9 +24,7 @@ const inspect = async ({ capability }, context) => { return { error: new UnknownProvider(capability.with) } } - return { - ok: await context.uploadTable.inspect(capability.nb.root), - } + return await context.uploadTable.inspect(capability.nb.root) } class UnknownProvider extends Provider.Failure { diff --git a/packages/upload-api/src/store/add.js b/packages/upload-api/src/store/add.js index 7f0868bf5..07e482dda 100644 --- a/packages/upload-api/src/store/add.js +++ b/packages/upload-api/src/store/add.js @@ -24,7 +24,7 @@ export function storeAddProvider(context) { Server.DID.parse(capability.with).did() ) const issuer = invocation.issuer.did() - const [allocated, carIsLinkedToSpace, carExists] = await Promise.all([ + const [allocated, carExists] = await Promise.all([ allocate( { capability: { @@ -33,7 +33,6 @@ export function storeAddProvider(context) { }, context ), - storeTable.exists(space, link), carStoreBucket.has(link), ]) @@ -42,22 +41,30 @@ export function storeAddProvider(context) { return allocated } - if (!carIsLinkedToSpace) { - await storeTable.insert({ - space, - link, - size, - origin, - issuer, - invocation: invocation.cid, - }) + let allocatedSize = size + const res = await storeTable.insert({ + space, + link, + size, + origin, + issuer, + invocation: invocation.cid, + }) + if (res.error) { + // if the insert failed with conflict then this item has already been + // added to the space and there is no allocation change. + if (res.error.name === 'RecordKeyConflict') { + allocatedSize = 0 + } else { + return res + } } if (carExists) { return { ok: { status: 'done', - allocated: carIsLinkedToSpace ? 0 : size, + allocated: allocatedSize, with: space, link, }, @@ -68,7 +75,7 @@ export function storeAddProvider(context) { return { ok: { status: 'upload', - allocated: size, + allocated: allocatedSize, with: space, link, url: url.toString(), diff --git a/packages/upload-api/src/store/get.js b/packages/upload-api/src/store/get.js index e8a3c1983..a9e5862fb 100644 --- a/packages/upload-api/src/store/get.js +++ b/packages/upload-api/src/store/get.js @@ -1,6 +1,7 @@ import * as Server from '@ucanto/server' import * as Store from '@web3-storage/capabilities/store' import * as API from '../types.js' +import { StoreItemNotFound } from './lib.js' /** * @param {API.StoreServiceContext} context @@ -14,16 +15,9 @@ export function storeGetProvider(context) { } const space = Server.DID.parse(capability.with).did() const res = await context.storeTable.get(space, link) - if (!res) { - return { - error: { - name: 'StoreItemNotFound', - message: 'Store item not found', - }, - } - } - return { - ok: res, + if (res.error && res.error.name === 'RecordNotFound') { + return Server.error(new StoreItemNotFound(space, link)) } + return res }) } diff --git a/packages/upload-api/src/store/lib.js b/packages/upload-api/src/store/lib.js new file mode 100644 index 000000000..e66063c49 --- /dev/null +++ b/packages/upload-api/src/store/lib.js @@ -0,0 +1,29 @@ +import { Failure } from '@ucanto/server' + +export class StoreItemNotFound extends Failure { + /** + * @param {import('@ucanto/interface').DID} space + * @param {import('@ucanto/interface').UnknownLink} link + */ + constructor(space, link) { + super() + this.space = space + this.link = link + } + + get name() { + return 'StoreItemNotFound' + } + + describe() { + return `${this.link} not found in ${this.space}` + } + + toJSON() { + return { + ...super.toJSON(), + space: this.space, + link: { '/': this.link.toString() }, + } + } +} diff --git a/packages/upload-api/src/store/list.js b/packages/upload-api/src/store/list.js index 58cf057a9..16466b893 100644 --- a/packages/upload-api/src/store/list.js +++ b/packages/upload-api/src/store/list.js @@ -10,13 +10,6 @@ export function storeListProvider(context) { return Server.provide(Store.list, async ({ capability }) => { const { cursor, size, pre } = capability.nb const space = Server.DID.parse(capability.with).did() - - return { - ok: await context.storeTable.list(space, { - size, - cursor, - pre, - }), - } + return await context.storeTable.list(space, { size, cursor, pre }) }) } diff --git a/packages/upload-api/src/store/remove.js b/packages/upload-api/src/store/remove.js index 52e4ced80..ea893a647 100644 --- a/packages/upload-api/src/store/remove.js +++ b/packages/upload-api/src/store/remove.js @@ -1,6 +1,7 @@ import * as Server from '@ucanto/server' import * as Store from '@web3-storage/capabilities/store' import * as API from '../types.js' +import { StoreItemNotFound } from './lib.js' /** * @param {API.StoreServiceContext} context @@ -11,41 +12,13 @@ export function storeRemoveProvider(context) { const { link } = capability.nb const space = Server.DID.parse(capability.with).did() - const item = await context.storeTable.get(space, link) - if (!item) { - return Server.error(new StoreItemNotFound(space, link)) + const res = await context.storeTable.remove(space, link) + if (res.error && res.error.name === 'RecordNotFound') { + if (res.error.name === 'RecordNotFound') { + return Server.error(new StoreItemNotFound(space, link)) + } } - await context.storeTable.remove(space, link) - - return Server.ok({ size: item.size }) + return res }) } - -class StoreItemNotFound extends Server.Failure { - /** - * @param {import('@ucanto/interface').DID} space - * @param {import('@ucanto/interface').UnknownLink} link - */ - constructor(space, link) { - super() - this.space = space - this.link = link - } - - get name() { - return 'StoreItemNotFound' - } - - describe() { - return `${this.link} not found in ${this.space}` - } - - toJSON() { - return { - ...super.toJSON(), - space: this.space, - link: { '/': this.link.toString() }, - } - } -} diff --git a/packages/upload-api/src/types.ts b/packages/upload-api/src/types.ts index 38b8bb582..de9f9152d 100644 --- a/packages/upload-api/src/types.ts +++ b/packages/upload-api/src/types.ts @@ -422,28 +422,50 @@ export interface DudewhereBucket { put: (dataCid: string, carCid: string) => Promise } +/** + * Indicates the requested record was not present in the table. + */ +export interface RecordNotFound extends Failure { + name: 'RecordNotFound' +} + +/** + * Indicates the inserted record key conflicts with an existing key of a record + * that already exists in the table. + */ +export interface RecordKeyConflict extends Failure { + name: 'RecordKeyConflict' +} + export interface StoreTable { - inspect: (link: UnknownLink) => Promise - exists: (space: DID, link: UnknownLink) => Promise - get: (space: DID, link: UnknownLink) => Promise - insert: (item: StoreAddInput) => Promise - remove: (space: DID, link: UnknownLink) => Promise + inspect: (link: UnknownLink) => Promise> + exists: (space: DID, link: UnknownLink) => Promise> + get: (space: DID, link: UnknownLink) => Promise> + /** Inserts an item in the table if it does not already exist. */ + insert: (item: StoreAddInput) => Promise> + /** Removes an item from the table but fails if the item does not exist. */ + remove: (space: DID, link: UnknownLink) => Promise> list: ( space: DID, options?: ListOptions - ) => Promise> + ) => Promise, Failure>> } export interface UploadTable { - inspect: (link: UnknownLink) => Promise - exists: (space: DID, root: UnknownLink) => Promise - get: (space: DID, link: UnknownLink) => Promise - insert: (item: UploadAddInput) => Promise - remove: (space: DID, root: UnknownLink) => Promise + inspect: (link: UnknownLink) => Promise> + exists: (space: DID, root: UnknownLink) => Promise> + get: (space: DID, link: UnknownLink) => Promise> + /** + * Inserts an item in the table if it does not already exist or updates an + * existing item if it does exist. + */ + upsert: (item: UploadAddInput) => Promise> + /** Removes an item from the table but fails if the item does not exist. */ + remove: (space: DID, root: UnknownLink) => Promise> list: ( space: DID, options?: ListOptions - ) => Promise> + ) => Promise, Failure>> } export type SpaceInfoSuccess = { diff --git a/packages/upload-api/src/upload/add.js b/packages/upload-api/src/upload/add.js index c57312ce4..9c861570a 100644 --- a/packages/upload-api/src/upload/add.js +++ b/packages/upload-api/src/upload/add.js @@ -30,7 +30,7 @@ export function uploadAddProvider(context) { const [res] = await Promise.all([ // Store in Database - uploadTable.insert({ + uploadTable.upsert({ space, root, shards, @@ -40,7 +40,7 @@ export function uploadAddProvider(context) { writeDataCidToCarCidsMapping(dudewhereBucket, root, shards), ]) - return { ok: res } + return res }) } diff --git a/packages/upload-api/src/upload/get.js b/packages/upload-api/src/upload/get.js index d15547c51..e76a3fdcc 100644 --- a/packages/upload-api/src/upload/get.js +++ b/packages/upload-api/src/upload/get.js @@ -1,6 +1,7 @@ import * as Server from '@ucanto/server' import * as Upload from '@web3-storage/capabilities/upload' import * as API from '../types.js' +import { UploadNotFound } from './lib.js' /** * @param {API.UploadServiceContext} context @@ -14,16 +15,9 @@ export function uploadGetProvider(context) { } const space = Server.DID.parse(capability.with).did() const res = await context.uploadTable.get(space, root) - if (!res) { - return { - error: { - name: 'UploadNotFound', - message: 'Upload not found', - }, - } - } - return { - ok: res, + if (res.error && res.error.name === 'RecordNotFound') { + return Server.error(new UploadNotFound(space, root)) } + return res }) } diff --git a/packages/upload-api/src/upload/lib.js b/packages/upload-api/src/upload/lib.js new file mode 100644 index 000000000..611e642d8 --- /dev/null +++ b/packages/upload-api/src/upload/lib.js @@ -0,0 +1,29 @@ +import { Failure } from '@ucanto/server' + +export class UploadNotFound extends Failure { + /** + * @param {import('@ucanto/interface').DID} space + * @param {import('@ucanto/interface').UnknownLink} root + */ + constructor(space, root) { + super() + this.space = space + this.root = root + } + + get name() { + return 'UploadNotFound' + } + + describe() { + return `${this.root} not found in ${this.space}` + } + + toJSON() { + return { + ...super.toJSON(), + space: this.space, + root: { '/': this.root.toString() }, + } + } +} diff --git a/packages/upload-api/src/upload/list.js b/packages/upload-api/src/upload/list.js index afcc36c4f..375eacc78 100644 --- a/packages/upload-api/src/upload/list.js +++ b/packages/upload-api/src/upload/list.js @@ -10,13 +10,6 @@ export function uploadListProvider(context) { return Server.provide(Upload.list, async ({ capability }) => { const { cursor, size, pre } = capability.nb const space = Server.DID.parse(capability.with).did() - - return { - ok: await context.uploadTable.list(space, { - size, - cursor, - pre, - }), - } + return await context.uploadTable.list(space, { size, cursor, pre }) }) } diff --git a/packages/upload-api/src/upload/remove.js b/packages/upload-api/src/upload/remove.js index 16ea85a68..97cc604b5 100644 --- a/packages/upload-api/src/upload/remove.js +++ b/packages/upload-api/src/upload/remove.js @@ -1,6 +1,7 @@ import * as Server from '@ucanto/server' import * as Upload from '@web3-storage/capabilities/upload' import * as API from '../types.js' +import { UploadNotFound } from './lib.js' /** * @param {API.UploadServiceContext} context @@ -11,10 +12,13 @@ export function uploadRemoveProvider(context) { const { root } = capability.nb const space = Server.DID.parse(capability.with).did() - const result = await context.uploadTable.remove(space, root) - - return { - ok: result || {}, + const res = await context.uploadTable.remove(space, root) + if (res.error) { + if (res.error.name === 'RecordNotFound') { + return Server.error(new UploadNotFound(space, root)) + } } + + return res }) } diff --git a/packages/upload-api/test/handlers/store.js b/packages/upload-api/test/handlers/store.js index b52bd62ed..6bcbf74b6 100644 --- a/packages/upload-api/test/handlers/store.js +++ b/packages/upload-api/test/handlers/store.js @@ -6,6 +6,7 @@ import * as StoreCapabilities from '@web3-storage/capabilities/store' import { alice, bob, createSpace, registerSpace } from '../util.js' import { Absentee } from '@ucanto/principal' import { provisionProvider } from '../helpers/utils.js' +import * as Result from '../helpers/result.js' /** * @type {API.Tests} @@ -73,11 +74,7 @@ export const test = { assert.equal(goodPut.status, 200, await goodPut.text()) - const item = await context.storeTable.get(spaceDid, link) - - if (!item) { - return assert.equal(item != null, true) - } + const item = Result.unwrap(await context.storeTable.get(spaceDid, link)) assert.deepEqual( { @@ -95,9 +92,9 @@ export const test = { true ) - const { spaces } = await context.storeTable.inspect(link) - assert.equal(spaces.length, 1) - assert.equal(spaces[0].did, spaceDid) + const { ok: info } = await context.storeTable.inspect(link) + assert.equal(info?.spaces.length, 1) + assert.equal(info?.spaces[0].did, spaceDid) }, 'store/add should allow add the same content to be stored in multiple spaces': @@ -146,9 +143,9 @@ export const test = { assert.ok(bobStoreAdd.out.ok, `Bob failed to store ${link.toString()}`) - const { spaces } = await context.storeTable.inspect(link) + const { spaces } = Result.unwrap(await context.storeTable.inspect(link)) assert.equal(spaces.length, 2) - const spaceDids = spaces.map((space) => space.did) + const spaceDids = (spaces ?? []).map((space) => space.did) assert.ok(spaceDids.includes(aliceSpaceDid)) assert.ok(spaceDids.includes(bobSpaceDid)) }, @@ -299,10 +296,7 @@ export const test = { // @ts-expect-error making sure it's not an upload status assert.equal(storeAdd.out.ok.url == null, true) - const item = await context.storeTable.get(spaceDid, link) - if (!item) { - throw assert.equal(item != null, true, 'should have stored item') - } + const item = Result.unwrap(await context.storeTable.get(spaceDid, link)) assert.deepEqual( { diff --git a/packages/upload-api/test/handlers/upload.js b/packages/upload-api/test/handlers/upload.js index 23b3950ef..0304c57f7 100644 --- a/packages/upload-api/test/handlers/upload.js +++ b/packages/upload-api/test/handlers/upload.js @@ -9,6 +9,7 @@ import { } from '../util.js' import { createServer, connect } from '../../src/lib.js' import { Upload } from '@web3-storage/capabilities' +import * as Result from '../helpers/result.js' // https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-dynamodb/classes/batchwriteitemcommand.html const BATCH_MAX_SAFE_LIMIT = 25 @@ -54,7 +55,7 @@ export const test = { shards.map(String).sort() ) - const { results } = await context.uploadTable.list(spaceDid) + const { results } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.deepEqual(results.length, 1) const [item] = results @@ -65,7 +66,7 @@ export const test = { assert.equal(msAgo < 60_000, true) assert.equal(msAgo >= 0, true) - const { spaces } = await context.uploadTable.inspect(root) + const { spaces } = Result.unwrap(await context.uploadTable.inspect(root)) assert.equal(spaces.length, 1) assert.equal(spaces[0].did, spaceDid) }, @@ -117,7 +118,7 @@ export const test = { .execute(connection) assert.ok(bobUploadAdd.out.ok, `Bob failed to upload ${root.toString()}`) - const { spaces } = await context.uploadTable.inspect(root) + const { spaces } = Result.unwrap(await context.uploadTable.inspect(root)) assert.equal(spaces.length, 2) const spaceDids = spaces.map((space) => space.did) assert.ok(spaceDids.includes(aliceSpaceDid)) @@ -159,7 +160,7 @@ export const test = { 'Should have an empty shards array' ) - const { results } = await context.uploadTable.list(spaceDid) + const { results } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.equal(results.length, 1) const [upload] = results assert.deepEqual(upload.shards, []) @@ -216,7 +217,7 @@ export const test = { shards.map(String).sort() ) - const { results } = await context.uploadTable.list(spaceDid) + const { results } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.equal(results.length, 1) const [upload] = results assert.equal(upload.root.toString(), root.toString()) @@ -282,7 +283,7 @@ export const test = { [cars[0].cid, cars[1].cid, cars[2].cid].map(String).sort() ) - const { results } = await context.uploadTable.list(spaceDid) + const { results } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.equal(results.length, 1) const [upload] = results assert.deepEqual( @@ -388,7 +389,7 @@ export const test = { ) }, - 'upload/remove does not fail for non existent upload': async ( + 'upload/remove fails for non existent upload': async ( assert, context ) => { @@ -413,11 +414,7 @@ export const test = { }) .execute(connection) - assert.deepEqual( - uploadRemove.out.ok, - {}, - 'expect falsy response when removing an upload you do not have' - ) + assert.equal(uploadRemove.out.error?.name, 'UploadNotFound') }, 'upload/remove only removes an upload for the given space': async ( @@ -495,7 +492,7 @@ export const test = { }) .execute(connection) - const { results: spaceAItems } = await context.uploadTable.list(spaceDidA) + const { results: spaceAItems } = Result.unwrap(await context.uploadTable.list(spaceDidA)) assert.equal( spaceAItems.some((x) => x.root.toString() === carA.roots[0].toString()), false, @@ -508,7 +505,7 @@ export const test = { 'SpaceA should have upload for carB.root' ) - const { results: spaceBItems } = await context.uploadTable.list(spaceDidB) + const { results: spaceBItems } = Result.unwrap(await context.uploadTable.list(spaceDidB)) assert.equal( spaceBItems.some((x) => x.root.toString() === carB.roots[0].toString()), false, @@ -556,7 +553,7 @@ export const test = { assert.equal(uploadAdd.out.ok.shards?.length, shards.length) // Validate DB before remove - const { results } = await context.uploadTable.list(spaceDid) + const { results } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.equal(results.length, 1) // Remove Car from Space @@ -570,7 +567,7 @@ export const test = { }) .execute(connection) - const { results: resultsAfter } = await context.uploadTable.list(spaceDid) + const { results: resultsAfter } = Result.unwrap(await context.uploadTable.list(spaceDid)) assert.equal(resultsAfter.length, 0) }, diff --git a/packages/upload-api/test/helpers/result.js b/packages/upload-api/test/helpers/result.js new file mode 100644 index 000000000..1df360b81 --- /dev/null +++ b/packages/upload-api/test/helpers/result.js @@ -0,0 +1,22 @@ +export * from '@ucanto/core/result' +import * as API from '@ucanto/interface' + +/** + * Returns contained `ok` if result is and throws `error` if result is not ok. + * + * @template T + * @param {API.Result} result + * @returns {T} + */ +export const unwrap = ({ ok, error }) => { + if (error) { + throw error + } else { + return /** @type {T} */ (ok) + } +} + +/** + * Also expose as `Result.try` which is arguably more clear. + */ +export { unwrap as try } diff --git a/packages/upload-api/test/storage/store-table.js b/packages/upload-api/test/storage/store-table.js index 3092aaa0e..8cf1d4682 100644 --- a/packages/upload-api/test/storage/store-table.js +++ b/packages/upload-api/test/storage/store-table.js @@ -11,9 +11,14 @@ export class StoreTable { /** * @param {API.StoreAddInput} input - * @returns {Promise} + * @returns {ReturnType} */ async insert({ space, issuer, invocation, ...output }) { + if (this.items.some(i => i.space === space && i.link.equals(output.link))) { + return { + error: { name: 'RecordKeyConflict', message: 'record key conflict' } + } + } this.items.unshift({ space, issuer, @@ -21,62 +26,66 @@ export class StoreTable { ...output, insertedAt: new Date().toISOString(), }) - return output + return { ok: output } } /** - * * @param {API.UnknownLink} link - * @returns {Promise} + * @returns {ReturnType} */ async inspect(link) { - const items = - this.items?.filter((item) => item.link.toString() === link.toString()) || - [] + const items = this.items.filter((item) => item.link.equals(link)) return { - spaces: items.map((item) => ({ - did: item.space, - insertedAt: item.insertedAt, - })), + ok: { + spaces: items.map((item) => ({ + did: item.space, + insertedAt: item.insertedAt, + })), + } } } /** - * Get info for a single shard or undefined if it doesn't exist - * * @param {API.DID} space * @param {API.UnknownLink} link - * @returns {Promise<(API.StoreAddInput & API.StoreListItem) | undefined>} + * @returns {ReturnType} */ async get(space, link) { - return this.items.find( - (item) => item.space === space && item.link.toString() === link.toString() - ) + const item = this.items.find(i => i.space === space && i.link.equals(link)) + if (!item) { + return { error: { name: 'RecordNotFound', message: 'record not found' } } + } + return { ok: item } } /** * @param {API.DID} space * @param {API.UnknownLink} link - * @returns + * @returns {ReturnType} */ async exists(space, link) { - // eslint-disable-next-line yoda - return null != (await this.get(space, link)) + const item = this.items.find(i => i.space === space && i.link.equals(link)) + return { ok: !!item } } /** * @param {API.DID} space * @param {API.UnknownLink} link + * @returns {ReturnType} */ async remove(space, link) { - this.items = this.items.filter( - (item) => item.space !== space && item.link.toString() !== link.toString() - ) + const item = this.items.find(i => i.space === space && i.link.equals(link)) + if (!item) { + return { error: { name: 'RecordNotFound', message: 'record not found' } } + } + this.items = this.items.filter(i => i !== item) + return { ok: item } } /** * @param {API.DID} space * @param {API.ListOptions} options + * @returns {ReturnType} */ async list( space, @@ -90,10 +99,7 @@ export class StoreTable { .slice(0, size) if (matches.length === 0) { - return { - size: 0, - results: [], - } + return { ok: { size: 0, results: [] } } } const first = matches[0] @@ -108,11 +114,13 @@ export class StoreTable { : [`${start + offset}`, `${end + 1 + offset}`, values] return { - size: values.length, - before, - after, - cursor: after, - results, + ok: { + size: values.length, + before, + after, + cursor: after, + results, + } } } } diff --git a/packages/upload-api/test/storage/upload-table.js b/packages/upload-api/test/storage/upload-table.js index d6dcf597c..057c944f5 100644 --- a/packages/upload-api/test/storage/upload-table.js +++ b/packages/upload-api/test/storage/upload-table.js @@ -6,31 +6,31 @@ import { parseLink } from '@ucanto/core' */ export class UploadTable { constructor() { - /** @type {(API.UploadListItem & API.UploadAddInput & { insertedAt: string, updatedAt: string })[]} */ + /** @type {(API.UploadListItem & API.UploadAddInput)[]} */ this.items = [] } /** - * * @param {API.UnknownLink} link - * @returns {Promise} + * @returns {ReturnType} */ async inspect(link) { - const items = - this.items?.filter((item) => item.root.toString() === link.toString()) || - [] + const items = this.items.filter((item) => item.root.equals(link)) return { - spaces: items.map((item) => ({ - did: item.space, - insertedAt: item.insertedAt, - })), + ok: { + spaces: items.map((item) => ({ + did: item.space, + insertedAt: item.insertedAt, + })), + }, } } /** * @param {API.UploadAddInput} input + * @returns {ReturnType} */ - async insert({ space, issuer, invocation, root, shards = [] }) { + async upsert({ space, issuer, invocation, root, shards = [] }) { const time = new Date().toISOString() const item = this.items.find( (item) => item.space === space && item.root.toString() === root.toString() @@ -47,7 +47,7 @@ export class UploadTable { updatedAt: time, }) - return { root, shards: item.shards } + return { ok: { root, shards: item.shards } } } else { this.items.unshift({ space, @@ -59,48 +59,51 @@ export class UploadTable { updatedAt: time, }) - return { root, shards } + return { ok: { root, shards } } } } /** * @param {API.DID} space * @param {API.UnknownLink} root + * @returns {ReturnType} */ async remove(space, root) { - const item = this.items.find( - (item) => item.space === space && item.root.toString() === root.toString() - ) - - if (item) { - this.items.splice(this.items.indexOf(item), 1) + const item = this.items.find(i => i.space === space && i.root.equals(root)) + if (!item) { + return { error: { name: 'RecordNotFound', message: 'record not found' } } } - - return item || null + this.items = this.items.filter(i => i !== item) + return { ok: item } } /** * @param {API.DID} space * @param {API.UnknownLink} root + * @returns {ReturnType} */ async get(space, root) { - return this.items.find( - (item) => item.space === space && item.root.toString() === root.toString() - ) + const item = this.items.find(i => i.space === space && i.root.equals(root)) + if (!item) { + return { error: { name: 'RecordNotFound', message: 'record not found' } } + } + return { ok: item } } /** * @param {API.DID} space * @param {API.UnknownLink} link - * @returns + * @returns {ReturnType} */ async exists(space, link) { - return null != (await this.get(space, link)) + const item = this.items.find(i => i.space === space && i.root.equals(link)) + return { ok: !!item } } /** * @param {API.DID} space * @param {API.ListOptions} options + * @returns {ReturnType} */ async list( space, @@ -114,10 +117,7 @@ export class UploadTable { .slice(0, size) if (matches.length === 0) { - return { - size: 0, - results: [], - } + return { ok: { size: 0, results: [] } } } const first = matches[0] @@ -132,11 +132,13 @@ export class UploadTable { : [`${start + offset}`, `${end + 1 + offset}`, values] return { - size: values.length, - before, - after, - cursor: after, - results, + ok: { + size: values.length, + before, + after, + cursor: after, + results, + } } } } diff --git a/packages/w3up-client/test/capability/upload.test.js b/packages/w3up-client/test/capability/upload.test.js index 393265cc7..9f1c32c46 100644 --- a/packages/w3up-client/test/capability/upload.test.js +++ b/packages/w3up-client/test/capability/upload.test.js @@ -9,7 +9,7 @@ import { mockService, mockServiceConf } from '../helpers/mocks.js' import { Client } from '../../src/client.js' import { validateAuthorization } from '../helpers/utils.js' -describe('StoreClient', () => { +describe('UploadClient', () => { describe('add', () => { it('should register an upload', async () => { const car = await randomCAR(128) @@ -121,6 +121,8 @@ describe('StoreClient', () => { describe('remove', () => { it('should remove an upload', async () => { + const car = await randomCAR(128) + const service = mockService({ upload: { remove: provide(UploadCapabilities.remove, ({ invocation }) => { @@ -129,7 +131,12 @@ describe('StoreClient', () => { const invCap = invocation.capabilities[0] assert.equal(invCap.can, UploadCapabilities.remove.can) assert.equal(invCap.with, alice.currentSpace()?.did()) - return { ok: {} } + return { + ok: { + root: car.roots[0], + shards: [car.cid], + } + } }), }, }) @@ -151,7 +158,7 @@ describe('StoreClient', () => { await alice.addSpace(auth) await alice.setCurrentSpace(space.did()) - await alice.capability.upload.remove((await randomCAR(128)).roots[0]) + await alice.capability.upload.remove(car.roots[0]) assert(service.upload.remove.called) assert.equal(service.upload.remove.callCount, 1) From db568cea75754220d472b911e780ae5ace2fb702 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 6 Dec 2023 14:23:10 +0000 Subject: [PATCH 7/9] chore: appease linter --- packages/upload-api/src/types.ts | 24 +++++++++++++++---- packages/upload-api/test/handlers/upload.js | 17 +++++++------ .../upload-api/test/storage/store-table.js | 24 ++++++++++++------- .../upload-api/test/storage/upload-table.js | 16 +++++++++---- .../test/capability/upload.test.js | 2 +- 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/packages/upload-api/src/types.ts b/packages/upload-api/src/types.ts index de9f9152d..369229eed 100644 --- a/packages/upload-api/src/types.ts +++ b/packages/upload-api/src/types.ts @@ -440,11 +440,19 @@ export interface RecordKeyConflict extends Failure { export interface StoreTable { inspect: (link: UnknownLink) => Promise> exists: (space: DID, link: UnknownLink) => Promise> - get: (space: DID, link: UnknownLink) => Promise> + get: ( + space: DID, + link: UnknownLink + ) => Promise> /** Inserts an item in the table if it does not already exist. */ - insert: (item: StoreAddInput) => Promise> + insert: ( + item: StoreAddInput + ) => Promise> /** Removes an item from the table but fails if the item does not exist. */ - remove: (space: DID, link: UnknownLink) => Promise> + remove: ( + space: DID, + link: UnknownLink + ) => Promise> list: ( space: DID, options?: ListOptions @@ -454,14 +462,20 @@ export interface StoreTable { export interface UploadTable { inspect: (link: UnknownLink) => Promise> exists: (space: DID, root: UnknownLink) => Promise> - get: (space: DID, link: UnknownLink) => Promise> + get: ( + space: DID, + link: UnknownLink + ) => Promise> /** * Inserts an item in the table if it does not already exist or updates an * existing item if it does exist. */ upsert: (item: UploadAddInput) => Promise> /** Removes an item from the table but fails if the item does not exist. */ - remove: (space: DID, root: UnknownLink) => Promise> + remove: ( + space: DID, + root: UnknownLink + ) => Promise> list: ( space: DID, options?: ListOptions diff --git a/packages/upload-api/test/handlers/upload.js b/packages/upload-api/test/handlers/upload.js index 0304c57f7..4f915c0c3 100644 --- a/packages/upload-api/test/handlers/upload.js +++ b/packages/upload-api/test/handlers/upload.js @@ -389,10 +389,7 @@ export const test = { ) }, - 'upload/remove fails for non existent upload': async ( - assert, - context - ) => { + 'upload/remove fails for non existent upload': async (assert, context) => { const { proof, spaceDid } = await registerSpace(alice, context) const connection = connect({ id: context.id, @@ -492,7 +489,9 @@ export const test = { }) .execute(connection) - const { results: spaceAItems } = Result.unwrap(await context.uploadTable.list(spaceDidA)) + const { results: spaceAItems } = Result.unwrap( + await context.uploadTable.list(spaceDidA) + ) assert.equal( spaceAItems.some((x) => x.root.toString() === carA.roots[0].toString()), false, @@ -505,7 +504,9 @@ export const test = { 'SpaceA should have upload for carB.root' ) - const { results: spaceBItems } = Result.unwrap(await context.uploadTable.list(spaceDidB)) + const { results: spaceBItems } = Result.unwrap( + await context.uploadTable.list(spaceDidB) + ) assert.equal( spaceBItems.some((x) => x.root.toString() === carB.roots[0].toString()), false, @@ -567,7 +568,9 @@ export const test = { }) .execute(connection) - const { results: resultsAfter } = Result.unwrap(await context.uploadTable.list(spaceDid)) + const { results: resultsAfter } = Result.unwrap( + await context.uploadTable.list(spaceDid) + ) assert.equal(resultsAfter.length, 0) }, diff --git a/packages/upload-api/test/storage/store-table.js b/packages/upload-api/test/storage/store-table.js index 8cf1d4682..3cb197048 100644 --- a/packages/upload-api/test/storage/store-table.js +++ b/packages/upload-api/test/storage/store-table.js @@ -14,9 +14,11 @@ export class StoreTable { * @returns {ReturnType} */ async insert({ space, issuer, invocation, ...output }) { - if (this.items.some(i => i.space === space && i.link.equals(output.link))) { + if ( + this.items.some((i) => i.space === space && i.link.equals(output.link)) + ) { return { - error: { name: 'RecordKeyConflict', message: 'record key conflict' } + error: { name: 'RecordKeyConflict', message: 'record key conflict' }, } } this.items.unshift({ @@ -41,7 +43,7 @@ export class StoreTable { did: item.space, insertedAt: item.insertedAt, })), - } + }, } } @@ -51,7 +53,9 @@ export class StoreTable { * @returns {ReturnType} */ async get(space, link) { - const item = this.items.find(i => i.space === space && i.link.equals(link)) + const item = this.items.find( + (i) => i.space === space && i.link.equals(link) + ) if (!item) { return { error: { name: 'RecordNotFound', message: 'record not found' } } } @@ -64,7 +68,9 @@ export class StoreTable { * @returns {ReturnType} */ async exists(space, link) { - const item = this.items.find(i => i.space === space && i.link.equals(link)) + const item = this.items.find( + (i) => i.space === space && i.link.equals(link) + ) return { ok: !!item } } @@ -74,11 +80,13 @@ export class StoreTable { * @returns {ReturnType} */ async remove(space, link) { - const item = this.items.find(i => i.space === space && i.link.equals(link)) + const item = this.items.find( + (i) => i.space === space && i.link.equals(link) + ) if (!item) { return { error: { name: 'RecordNotFound', message: 'record not found' } } } - this.items = this.items.filter(i => i !== item) + this.items = this.items.filter((i) => i !== item) return { ok: item } } @@ -120,7 +128,7 @@ export class StoreTable { after, cursor: after, results, - } + }, } } } diff --git a/packages/upload-api/test/storage/upload-table.js b/packages/upload-api/test/storage/upload-table.js index 057c944f5..6fe9afb4f 100644 --- a/packages/upload-api/test/storage/upload-table.js +++ b/packages/upload-api/test/storage/upload-table.js @@ -69,11 +69,13 @@ export class UploadTable { * @returns {ReturnType} */ async remove(space, root) { - const item = this.items.find(i => i.space === space && i.root.equals(root)) + const item = this.items.find( + (i) => i.space === space && i.root.equals(root) + ) if (!item) { return { error: { name: 'RecordNotFound', message: 'record not found' } } } - this.items = this.items.filter(i => i !== item) + this.items = this.items.filter((i) => i !== item) return { ok: item } } @@ -83,7 +85,9 @@ export class UploadTable { * @returns {ReturnType} */ async get(space, root) { - const item = this.items.find(i => i.space === space && i.root.equals(root)) + const item = this.items.find( + (i) => i.space === space && i.root.equals(root) + ) if (!item) { return { error: { name: 'RecordNotFound', message: 'record not found' } } } @@ -96,7 +100,9 @@ export class UploadTable { * @returns {ReturnType} */ async exists(space, link) { - const item = this.items.find(i => i.space === space && i.root.equals(link)) + const item = this.items.find( + (i) => i.space === space && i.root.equals(link) + ) return { ok: !!item } } @@ -138,7 +144,7 @@ export class UploadTable { after, cursor: after, results, - } + }, } } } diff --git a/packages/w3up-client/test/capability/upload.test.js b/packages/w3up-client/test/capability/upload.test.js index 9f1c32c46..15b151beb 100644 --- a/packages/w3up-client/test/capability/upload.test.js +++ b/packages/w3up-client/test/capability/upload.test.js @@ -135,7 +135,7 @@ describe('UploadClient', () => { ok: { root: car.roots[0], shards: [car.cid], - } + }, } }), }, From 7623f6a33b497e829c3bbc034d1b178fa4a1e421 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 6 Dec 2023 15:26:19 +0000 Subject: [PATCH 8/9] fix: remove unneeded codes --- packages/upload-api/src/store/remove.js | 4 +--- packages/upload-api/src/upload/remove.js | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/upload-api/src/store/remove.js b/packages/upload-api/src/store/remove.js index ea893a647..3081c924f 100644 --- a/packages/upload-api/src/store/remove.js +++ b/packages/upload-api/src/store/remove.js @@ -14,9 +14,7 @@ export function storeRemoveProvider(context) { const res = await context.storeTable.remove(space, link) if (res.error && res.error.name === 'RecordNotFound') { - if (res.error.name === 'RecordNotFound') { - return Server.error(new StoreItemNotFound(space, link)) - } + return Server.error(new StoreItemNotFound(space, link)) } return res diff --git a/packages/upload-api/src/upload/remove.js b/packages/upload-api/src/upload/remove.js index 97cc604b5..f7e42e299 100644 --- a/packages/upload-api/src/upload/remove.js +++ b/packages/upload-api/src/upload/remove.js @@ -13,10 +13,8 @@ export function uploadRemoveProvider(context) { const space = Server.DID.parse(capability.with).did() const res = await context.uploadTable.remove(space, root) - if (res.error) { - if (res.error.name === 'RecordNotFound') { - return Server.error(new UploadNotFound(space, root)) - } + if (res.error && res.error.name === 'RecordNotFound') { + return Server.error(new UploadNotFound(space, root)) } return res From 089888ef933008ad5ea181cbffaa3d19ac2974d8 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 6 Dec 2023 16:03:46 +0000 Subject: [PATCH 9/9] fix: less churn --- packages/upload-api/test/handlers/store.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/upload-api/test/handlers/store.js b/packages/upload-api/test/handlers/store.js index 5104e8b52..e2619fe63 100644 --- a/packages/upload-api/test/handlers/store.js +++ b/packages/upload-api/test/handlers/store.js @@ -92,9 +92,9 @@ export const test = { true ) - const { ok: info } = await context.storeTable.inspect(link) - assert.equal(info?.spaces.length, 1) - assert.equal(info?.spaces[0].did, spaceDid) + const { spaces } = Result.unwrap(await context.storeTable.inspect(link)) + assert.equal(spaces.length, 1) + assert.equal(spaces[0].did, spaceDid) }, 'store/add should allow add the same content to be stored in multiple spaces': @@ -145,7 +145,7 @@ export const test = { const { spaces } = Result.unwrap(await context.storeTable.inspect(link)) assert.equal(spaces.length, 2) - const spaceDids = (spaces ?? []).map((space) => space.did) + const spaceDids = spaces.map((space) => space.did) assert.ok(spaceDids.includes(aliceSpaceDid)) assert.ok(spaceDids.includes(bobSpaceDid)) }, @@ -290,7 +290,7 @@ export const test = { } assert.equal(storeAdd.out.ok.status, 'done') - assert.equal(storeAdd.out.ok.allocated, data.byteLength) + assert.equal(storeAdd.out.ok.allocated, 5) assert.equal(storeAdd.out.ok.with, spaceDid) assert.deepEqual(storeAdd.out.ok.link.toString(), link.toString()) // @ts-expect-error making sure it's not an upload status @@ -353,7 +353,7 @@ export const test = { const r0 = await inv0.execute(connection) assert.equal(r0.out.ok?.status, 'done') - assert.equal(r0.out.ok?.allocated, data.byteLength) + assert.equal(r0.out.ok?.allocated, 5) assert.equal(r0.out.ok?.with, spaceDid) const inv1 = StoreCapabilities.add.invoke({