feat: access-api forwards store/ and upload/ invocations to upload-api #334
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,267 @@ | ||
| import * as Client from '@ucanto/client' | ||
| import * as CAR from '@ucanto/transport/car' | ||
| import * as CBOR from '@ucanto/transport/cbor' | ||
| // eslint-disable-next-line no-unused-vars | ||
| import * as dagUcan from '@ipld/dag-ucan' | ||
| import * as dagUcanDid from '@ipld/dag-ucan/did' | ||
Gozala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import * as HTTP from '@ucanto/transport/http' | ||
| // eslint-disable-next-line no-unused-vars | ||
| import * as Store from '@web3-storage/capabilities/store' | ||
| // eslint-disable-next-line no-unused-vars | ||
| import * as Upload from '@web3-storage/capabilities/upload' | ||
| // eslint-disable-next-line no-unused-vars | ||
| import * as iucanto from '@ucanto/interface' | ||
Gozala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // eslint-disable-next-line no-unused-vars | ||
| import * as ed25519 from '@ucanto/principal/ed25519' | ||
|
|
||
| /** | ||
| * @template {iucanto.Capability} C | ||
| * @template [Success=unknown] | ||
| * @template {{ error: true }} [Failure={error:true}] | ||
| * @callback InvocationResponder | ||
| * @param {iucanto.Invocation<C>} invocationIn | ||
| * @param {iucanto.InvocationContext} context | ||
| * @returns {Promise<iucanto.Result<Success, Failure>>} | ||
| */ | ||
|
|
||
| /** | ||
| * @typedef StoreService | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Store.add>>} add | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Store.list>>} list | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Store.remove>>} remove | ||
| */ | ||
|
|
||
| /** | ||
| * @typedef UploadService | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Upload.add>>} add | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Upload.list>>} list | ||
| * @property {InvocationResponder<iucanto.InferInvokedCapability<typeof Upload.remove>>} remove | ||
| */ | ||
|
|
||
| /** | ||
| * @template {iucanto.Capability} C | ||
| * @template [Success=unknown] | ||
| * @template {{ error: true }} [Failure={error:true}] | ||
| * @param {object} options | ||
| * @param {Pick<Map<dagUcan.DID, iucanto.ConnectionView<any>>, 'get'>} options.connections | ||
| * @param {iucanto.Signer} [options.signer] | ||
| * @returns {InvocationResponder<C, Success, Failure>} | ||
| */ | ||
| function createInvocationResponder(options) { | ||
| /** | ||
| * @template {import('@ucanto/interface').Capability} Capability | ||
| * @param {iucanto.Invocation<Capability>} invocationIn | ||
| * @param {iucanto.InvocationContext} context | ||
| * @returns {Promise<iucanto.Result<any, { error: true }>>} | ||
|
There was a problem hiding this comment. Nit: Do we need the type annotation for This kind of stuff especially becomes useful when updating to new TS with breaking changes. In my experience avoiding There was a problem hiding this comment. great find. I agree on avoiding I can't remove this I will update it so this inner if you prefer to remove the (now generic) There was a problem hiding this comment. |
||
| */ | ||
| return async function handleInvocation(invocationIn, context) { | ||
| const connection = options.connections.get(invocationIn.audience.did()) | ||
| if (!connection) { | ||
| throw new Error( | ||
| `unable to get connection for audience ${invocationIn.audience.did()}}` | ||
| ) | ||
| } | ||
| // eslint-disable-next-line unicorn/prefer-logical-operator-over-ternary | ||
| const proxyInvocationIssuer = options.signer | ||
| ? // this results in a forwarded invocation, but the upstream will reject the signature | ||
| // created using options.signer unless options.signer signs w/ the same private key as the original issuer | ||
| // and it'd be nice to not even have to pass around `options.signer` | ||
| options.signer | ||
| : // this works, but involves lying about the issuer type (it wants a Signer but context.id is only a Verifier) | ||
| // @Gozala can we make it so `iucanto.InvocationOptions['issuer']` can be a Verifier and not just Signer? | ||
Gozala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** @type {ed25519.Signer.Signer} */ (context.id) | ||
|
There was a problem hiding this comment. Why do we need to cast this and specifically why do we need to be specific about it been There was a problem hiding this comment. you're right. |
||
|
|
||
| const [result] = await Client.execute( | ||
| [ | ||
| Client.invoke({ | ||
| issuer: proxyInvocationIssuer, | ||
| capability: invocationIn.capabilities[0], | ||
| audience: invocationIn.audience, | ||
| proofs: [invocationIn], | ||
| }), | ||
| ], | ||
| /** @type {Client.ConnectionView<any>} */ (connection) | ||
| ) | ||
| return result | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @template {Record<string, any>} T | ||
| * @param {object} options | ||
| * @param {iucanto.Signer} [options.signer] | ||
| * @param {Pick<Map<dagUcan.DID, iucanto.ConnectionView<T>>, 'get'>} options.connections | ||
| */ | ||
| function createProxyStoreService(options) { | ||
| const handleInvocation = createInvocationResponder(options) | ||
| /** | ||
| * @type {StoreService} | ||
| */ | ||
| const store = { | ||
| add: handleInvocation, | ||
| list: handleInvocation, | ||
| remove: handleInvocation, | ||
| } | ||
| return store | ||
| } | ||
|
|
||
| /** | ||
| * @template {Record<string, any>} T | ||
| * @param {object} options | ||
| * @param {iucanto.Signer} [options.signer] | ||
| * @param {Pick<Map<dagUcan.DID, iucanto.ConnectionView<T>>, 'get'>} options.connections | ||
| */ | ||
| function createProxyUploadService(options) { | ||
| const handleInvocation = createInvocationResponder(options) | ||
| /** | ||
| * @type {UploadService} | ||
| */ | ||
| const store = { | ||
| add: handleInvocation, | ||
| list: handleInvocation, | ||
| remove: handleInvocation, | ||
| } | ||
| return store | ||
| } | ||
|
|
||
| /** | ||
| * @typedef UcantoHttpConnectionOptions | ||
| * @property {dagUcan.DID} audience | ||
| * @property {typeof globalThis.fetch} options.fetch | ||
| * @property {URL} options.url | ||
| */ | ||
|
|
||
| /** | ||
| * @implements {Pick<Map<dagUcan.DID, iucanto.Connection<any>>, 'get'>} | ||
| */ | ||
| class AudienceConnections { | ||
|
There was a problem hiding this comment. Nit: This whole class seems like incidental complexity, things would be a whole lot simpler with just P.S.: I know that's not a popular opinion, yet I like to share it in a hope to bring peers to a true side :P There was a problem hiding this comment. Is this like what you had in mind? b46dd85
Eh, I don't disagree necessarily. good suggestion. There was a problem hiding this comment. Not exactly, sorry for not been very clear. In linked version you still have Separate data part which is basically a state from the behavior which are all the methods That way it does not matter how you constructed state, you can just pass it into a function and it will compute whatever. Separating data and behavior de-tangles two allowing you to store data in e.g. indexDB and then apply function after reading it from indexDB without having to built a class first. Not the case here but complex class heirerchies also often entangle initialization order. I think there are only two valid reasons to resort to a classes with methods:
I would argue use of classes with methods for any other reason just introduces incidental complexity and gets between you and data nothing else. There was a problem hiding this comment. @Gozala ah I understand what you mean now.
def have run into that. That helped me understand. |
||
| /** @type {Record<dagUcan.DID, URL>} */ | ||
| #audienceToUrl | ||
| /** @type {undefined|iucanto.ConnectionView<any>} */ | ||
| #defaultConnection | ||
| /** @type {typeof globalThis.fetch} */ | ||
| #fetch | ||
|
|
||
| /** | ||
| * @param {UcantoHttpConnectionOptions} options | ||
| * @returns {iucanto.ConnectionView<any>} | ||
| */ | ||
| static createConnection(options) { | ||
| return Client.connect({ | ||
| id: dagUcanDid.parse(options.audience), | ||
| encoder: CAR, | ||
| decoder: CBOR, | ||
| channel: HTTP.open({ | ||
| fetch: options.fetch, | ||
| url: options.url, | ||
| }), | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * @param {object} options | ||
| * @param {Record<dagUcan.DID, URL>} options.audienceToUrl | ||
| * @param {UcantoHttpConnectionOptions} [options.defaultConnection] | ||
| * @param {typeof globalThis.fetch} options.fetch | ||
| */ | ||
| constructor(options) { | ||
| this.#fetch = options.fetch | ||
| this.#audienceToUrl = options.audienceToUrl | ||
| this.#defaultConnection = options.defaultConnection | ||
| ? AudienceConnections.createConnection(options.defaultConnection) | ||
| : undefined | ||
| } | ||
|
|
||
| /** | ||
| * @param {dagUcan.DID} audience | ||
| */ | ||
| get(audience) { | ||
| if (audience in this.#audienceToUrl) { | ||
| return AudienceConnections.createConnection({ | ||
| audience, | ||
| fetch: this.#fetch, | ||
| url: this.#audienceToUrl[audience], | ||
| }) | ||
| } | ||
| return this.#defaultConnection | ||
| } | ||
| } | ||
|
|
||
| export class UploadApiProxyService { | ||
| /** @type {StoreService} */ | ||
| store | ||
| /** @type {UploadService} */ | ||
| upload | ||
|
|
||
| /** | ||
| * @type {Record<string, { | ||
| * audience: dagUcan.DID, | ||
| * url: URL, | ||
| * }>} | ||
| */ | ||
| static #environments = { | ||
| production: { | ||
| audience: 'did:web:web3.storage', | ||
| url: new URL('https://up.web3.storage'), | ||
| }, | ||
| staging: { | ||
| audience: 'did:web:staging.web3.storage', | ||
| url: new URL('https://staging.up.web3.storage'), | ||
| }, | ||
| } | ||
|
|
||
| /** | ||
| * @template {StoreService} T | ||
| * @param {object} options | ||
| * @param {iucanto.Signer} options.signer | ||
| * @param {typeof globalThis.fetch} options.fetch | ||
| */ | ||
| static forSigner(options) { | ||
| // eslint-disable-next-line unicorn/no-array-reduce | ||
| const audienceToUrl = Object.values(this.#environments).reduce( | ||
|
There was a problem hiding this comment. Do we ever need to have multiple connections in practice ? I'd expect we use one in one staging and the other in production, if so it seems like there's bunch of unnecessary code to be maintained with added complexity costs. As a side note we should probably add code coverage which would decentivize adding code which isn't used. There was a problem hiding this comment.
No. Though we could also ask "Do we ever need a unified access-api + upload-api single environment service in practice?". Not yet, but I think we both believe it will make working with the total API more intuitive (i.e. api user won't have to worry about which service to invoke). When we talked I like your vision of 'If you send an invocation to any API, it should try to route it to a place that can handle it'. i.e. unifying api across services. Doing it this way also unifies across environments. I don't think it's strictly necessary, but I do believe it will make working with the overall platform slightly more intuitive. (i.e. api user won't have to worry about which environment's 'unified' api to invoke). I definitely agree code coverage could be useful. #339 (comment) |
||
| (acc, { audience, url }) => { | ||
| acc[audience] = url | ||
| return acc | ||
| }, | ||
| /** @type {Record<dagUcan.DID, URL>} */ ({}) | ||
| ) | ||
| const connections = new AudienceConnections({ | ||
| audienceToUrl, | ||
| defaultConnection: { | ||
| fetch: options.fetch, | ||
| ...this.#environments.production, | ||
| }, | ||
| fetch: options.fetch, | ||
| }) | ||
| const proxyOptions = { | ||
| signer: options.signer, | ||
| connections, | ||
| } | ||
| return new this( | ||
| createProxyStoreService(proxyOptions), | ||
| createProxyUploadService(proxyOptions) | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * @protected | ||
| * @param {StoreService} store | ||
| * @param {UploadService} upload | ||
| */ | ||
| constructor(store, upload) { | ||
| this.store = store | ||
| this.upload = upload | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Return whether the provided stack trace string appears to be generated | ||
| * by a deployed upload-api. | ||
| * Heuristics: | ||
| * * stack trace files paths will start with `file:///var/task/upload-api` because of how the lambda environment is working | ||
| * | ||
| * @param {string} stack | ||
| */ | ||
| export function isUploadApiStack(stack) { | ||
Gozala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return stack.includes('file:///var/task/upload-api') | ||
| } | ||
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.
Nit: We re-export all of the types from the dag-ucan in the ucanto interface & it would be better to use those instead, so we don't have a chance of having two diff versions of dag-ucan in the tree.
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.
Thanks, I updated to that, and will prefer importing from ucanto libs in the future instead of dag-ucan.
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.
fcc80ea
9559d0e