-
Notifications
You must be signed in to change notification settings - Fork 22
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: access-api handles provider/add invocations #462
Changes from 13 commits
1c5d196
679e3c2
b8849a4
d15d0db
6da1479
25c4e0e
da6adb6
2f64d27
624b355
bd05cbd
fbb0ce4
27acf17
0c62dbc
e537211
d9768ad
6c1d1be
49004e8
e403bb3
237d188
e62faaa
2e90305
cd7dc57
184c55a
754463d
4a48928
a199c11
6903891
40a4f16
c1e37f9
9b9a14d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/** | ||
* @param {Array<import("../types/provisions").StorageProvisionCreation>} storage | ||
* @returns {import("../types/provisions").StorageProvisions} | ||
*/ | ||
export function createStorageProvisions(storage = []) { | ||
/** @type {import("../types/provisions").StorageProvisions['hasStorageProvider']} */ | ||
const hasStorageProvider = (consumerId) => { | ||
return Boolean(storage.some(({ space }) => space === consumerId)) | ||
} | ||
/** @type {import("../types/provisions").StorageProvisions['create']} */ | ||
const create = async (storageProvision) => { | ||
storage.push(storageProvision) | ||
} | ||
return { | ||
create, | ||
hasStorageProvider, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import * as Ucanto from '@ucanto/interface' | ||
import * as Server from '@ucanto/server' | ||
import { Provider } from '@web3-storage/capabilities' | ||
import * as validator from '@ucanto/validator' | ||
|
||
/** | ||
* @typedef {import('@web3-storage/capabilities/types').ProviderAdd} ProviderAdd | ||
* @typedef {import('@web3-storage/capabilities/types').ProviderAddSuccess} ProviderAddSuccess | ||
* @typedef {import('@web3-storage/capabilities/types').ProviderAddFailure} ProviderAddFailure | ||
*/ | ||
|
||
/** | ||
* @callback ProviderAddHandler | ||
* @param {Ucanto.Invocation<import('@web3-storage/capabilities/types').ProviderAdd>} invocation | ||
* @returns {Promise<Ucanto.Result<ProviderAddSuccess, ProviderAddFailure>>} | ||
*/ | ||
|
||
/** | ||
* @param {object} options | ||
* @param {import('../types/provisions').StorageProvisions} options.storageProvisions | ||
* @returns {ProviderAddHandler} | ||
*/ | ||
export function createProviderAddHandler(options) { | ||
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. Nit: This obviously follows the style of code that is in the repo already, but I do find amount of indirection adds so much unnecessary complexity e.g. I would much rather have something like: /**
* @param {object} input
* @param {Ucanto.Invocation<import('@web3-storage/capabilities/types').ProviderAdd>} input.invocation
* @param {{ storageProvisions: import('../types/provisions').StorageProvisions }} input.context
*/
export const add ({ invocation, context: { storageProvisions } }) => {
// ...
} And in the service defs something like Server.provide(Provider.add, ({ invocation }) => add({ invocation, context }) Than all this layers and closures all over. Also for what it's worth I'm going to add 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. this is a good idea and I'll try it out across all the relevant things I wrote in that style to use the less-closures style you suggest. #481 |
||
/** @type {ProviderAddHandler} */ | ||
return async (invocation) => { | ||
const [providerAddCap] = invocation.capabilities | ||
const { | ||
nb: { consumer, provider }, | ||
with: accountDID, | ||
} = providerAddCap | ||
if (!validator.DID.match({ method: 'mailto' }).is(accountDID)) { | ||
return { | ||
error: true, | ||
name: 'Unauthorized', | ||
message: 'Issuer must be a mailto DID', | ||
} | ||
} | ||
await options.storageProvisions.create({ | ||
space: consumer, | ||
provider, | ||
account: accountDID, | ||
}) | ||
return {} | ||
} | ||
} | ||
|
||
/** | ||
* @param {object} ctx | ||
* @param {Pick<import('../bindings').RouteContext['models'], 'storageProvisions'>} ctx.models | ||
*/ | ||
export function providerAddProvider(ctx) { | ||
return Server.provide(Provider.add, async ({ invocation }) => { | ||
const handler = createProviderAddHandler({ | ||
storageProvisions: ctx.models.storageProvisions, | ||
}) | ||
return handler(/** @type {Ucanto.Invocation<ProviderAdd>} */ (invocation)) | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,17 @@ | ||||||||||
import * as Ucanto from '@ucanto/interface' | ||||||||||
|
||||||||||
export type AlphaStorageProvider = 'did:web:web3.storage:providers:w3up-alpha' | ||||||||||
|
||||||||||
export interface StorageProvisionCreation { | ||||||||||
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. Nit: I would prefer if we did not had 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. At this layer of the types I wanted to model this particular use case really explicitly (e.g. with
Because the underlying table schema accomodates this, I don't want to change this particular action type now. I'm not against revising it or adding other events in subsequent PR. or e.g. renaming the model to not have storage prefix but still maybe having this particular event be specific to adding a storage provider. 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. I removed 'Storage' prefix from the model object itself e62faaa 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. Nit: I find naming to be very confusing, can we simply call this
Suggested change
Or better yet
Suggested change
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. |
||||||||||
space: Ucanto.DID<'key'> | ||||||||||
account: Ucanto.DID<'mailto'> | ||||||||||
provider: AlphaStorageProvider | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* stores instances of a storage provider being consumed by a consumer | ||||||||||
*/ | ||||||||||
export interface StorageProvisions { | ||||||||||
hasStorageProvider: (consumer: Ucanto.DID<'key'>) => boolean | ||||||||||
create: (provision: StorageProvisionCreation) => Promise<void> | ||||||||||
} |
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.
Can we just amend
space/info
and addproviders: DID[]
to it ? We would need that forstore/*
capabilities anyway.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.
yeah. I thought about it but didn't want to extend the public api without discussion.
In this case I am very supportive because that will also be helpful to upload-api and things like storacha/w3infra#134 (comment)
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.
can I do it as a fast follow once I have verified that the space registration via
provider/add
works on staging at all?#480