From 057ac26fcc0cbf7386ccfd468984c595f0ef4324 Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Thu, 24 Oct 2024 14:09:45 +0900 Subject: [PATCH 1/3] refactor: rename and update CloudflareKv to KvStore Abstracted CloudflareKv to KvStore interface and added EmptyStore class. --- src/github/github-event-handler.ts | 6 ++-- src/github/utils/cloudflare-kv.ts | 15 -------- src/github/utils/kv-store.ts | 55 ++++++++++++++++++++++++++++++ src/worker.ts | 4 +-- tests/dispatch.test.ts | 6 +++- 5 files changed, 65 insertions(+), 21 deletions(-) delete mode 100644 src/github/utils/cloudflare-kv.ts create mode 100644 src/github/utils/kv-store.ts diff --git a/src/github/github-event-handler.ts b/src/github/github-event-handler.ts index 0b00a96..b807b55 100644 --- a/src/github/github-event-handler.ts +++ b/src/github/github-event-handler.ts @@ -2,7 +2,7 @@ import { EmitterWebhookEvent, Webhooks } from "@octokit/webhooks"; import { customOctokit } from "./github-client"; import { GitHubContext, SimplifiedContext } from "./github-context"; import { createAppAuth } from "@octokit/auth-app"; -import { CloudflareKv } from "./utils/cloudflare-kv"; +import { KvStore } from "./utils/kv-store"; import { PluginChainState } from "./types/plugin"; export type Options = { @@ -10,7 +10,7 @@ export type Options = { webhookSecret: string; appId: string | number; privateKey: string; - pluginChainState: CloudflareKv; + pluginChainState: KvStore; }; export class GitHubEventHandler { @@ -18,7 +18,7 @@ export class GitHubEventHandler { public on: Webhooks["on"]; public onAny: Webhooks["onAny"]; public onError: Webhooks["onError"]; - public pluginChainState: CloudflareKv; + public pluginChainState: KvStore; readonly environment: "production" | "development"; private readonly _webhookSecret: string; diff --git a/src/github/utils/cloudflare-kv.ts b/src/github/utils/cloudflare-kv.ts deleted file mode 100644 index f5407a3..0000000 --- a/src/github/utils/cloudflare-kv.ts +++ /dev/null @@ -1,15 +0,0 @@ -export class CloudflareKv { - private _kv: KVNamespace; - - constructor(kv: KVNamespace) { - this._kv = kv; - } - - get(id: string): Promise { - return this._kv.get(id, "json"); - } - - put(id: string, state: T): Promise { - return this._kv.put(id, JSON.stringify(state)); - } -} diff --git a/src/github/utils/kv-store.ts b/src/github/utils/kv-store.ts new file mode 100644 index 0000000..7b0a6cb --- /dev/null +++ b/src/github/utils/kv-store.ts @@ -0,0 +1,55 @@ +/** + * KvStore is an interface representing a simple key-value store. + * + * @template T - The type of the value to be stored and retrieved. + */ +export interface KvStore { + get(id: string): Promise; + put(id: string, state: T): Promise; +} + +/** + * CloudflareKv is a class that provides an interface to interact with + * Cloudflare KV (Key-Value) storage. + * + * It implements the KvStore interface to handle generic types. + * + * @template T - The type of the values being stored. + */ +export class CloudflareKv implements KvStore { + private _kv: KVNamespace; + + constructor(kv: KVNamespace) { + this._kv = kv; + } + + get(id: string): Promise { + return this._kv.get(id, "json"); + } + + put(id: string, state: T): Promise { + return this._kv.put(id, JSON.stringify(state)); + } +} + +/** + * A class that implements the KvStore interface, representing an empty key-value store. + * All get operations return null and put operations do nothing, but log the action. + * + * @template T - The type of values to be stored. + */ +export class EmptyStore implements KvStore { + constructor(kv: KVNamespace) { + console.log(`Creating empty kv`, kv); + } + + get(id: string): Promise { + console.log(`get KV ${id}`); + return Promise.resolve(null); + } + + put(id: string, state: T): Promise { + console.log(`put KV ${id} ${state}`); + return Promise.resolve(); + } +} diff --git a/src/worker.ts b/src/worker.ts index 7314600..f186bcc 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -3,7 +3,7 @@ import { Value } from "@sinclair/typebox/value"; import { GitHubEventHandler } from "./github/github-event-handler"; import { bindHandlers } from "./github/handlers"; import { Env, envSchema } from "./github/types/env"; -import { CloudflareKv } from "./github/utils/cloudflare-kv"; +import { EmptyStore } from "./github/utils/kv-store"; import { WebhookEventName } from "@octokit/webhooks-types"; export default { @@ -18,7 +18,7 @@ export default { webhookSecret: env.APP_WEBHOOK_SECRET, appId: env.APP_ID, privateKey: env.APP_PRIVATE_KEY, - pluginChainState: new CloudflareKv(env.PLUGIN_CHAIN_STATE), + pluginChainState: new EmptyStore(env.PLUGIN_CHAIN_STATE), }); bindHandlers(eventHandler); await eventHandler.webhooks.verifyAndReceive({ id, name: eventName, payload: await request.text(), signature: signatureSha256 }); diff --git a/tests/dispatch.test.ts b/tests/dispatch.test.ts index afb2b9c..38de8fc 100644 --- a/tests/dispatch.test.ts +++ b/tests/dispatch.test.ts @@ -12,11 +12,15 @@ jest.mock("@octokit/auth-app", () => ({ createAppAuth: jest.fn(() => () => jest.fn(() => "1234")), })); -jest.mock("../src/github/utils/cloudflare-kv", () => ({ +jest.mock("../src/github/utils/kv-store", () => ({ CloudflareKv: jest.fn().mockImplementation(() => ({ get: jest.fn(), put: jest.fn(), })), + EmptyStore: jest.fn().mockImplementation(() => ({ + get: jest.fn(), + put: jest.fn(), + })), })); jest.mock("../src/github/types/plugin", () => { From 4ba813817bcb5cd69e3edc631145ea41f089e96a Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Thu, 24 Oct 2024 14:13:05 +0900 Subject: [PATCH 2/3] refactor: commented unused export --- src/github/utils/kv-store.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/github/utils/kv-store.ts b/src/github/utils/kv-store.ts index 7b0a6cb..acd115d 100644 --- a/src/github/utils/kv-store.ts +++ b/src/github/utils/kv-store.ts @@ -16,21 +16,21 @@ export interface KvStore { * * @template T - The type of the values being stored. */ -export class CloudflareKv implements KvStore { - private _kv: KVNamespace; - - constructor(kv: KVNamespace) { - this._kv = kv; - } - - get(id: string): Promise { - return this._kv.get(id, "json"); - } - - put(id: string, state: T): Promise { - return this._kv.put(id, JSON.stringify(state)); - } -} +// export class CloudflareKv implements KvStore { +// private _kv: KVNamespace; +// +// constructor(kv: KVNamespace) { +// this._kv = kv; +// } +// +// get(id: string): Promise { +// return this._kv.get(id, "json"); +// } +// +// put(id: string, state: T): Promise { +// return this._kv.put(id, JSON.stringify(state)); +// } +// } /** * A class that implements the KvStore interface, representing an empty key-value store. From 6037f76c1ec2bad7abf34b6971b477b1109439c9 Mon Sep 17 00:00:00 2001 From: Mentlegen <9807008+gentlementlegen@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:29:20 +0900 Subject: [PATCH 3/3] refactor: optimize plugin chain handling with Promise.all Utilize Promise.all to concurrently handle plugin chains and improve efficiency. --- src/github/handlers/index.ts | 94 +++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/src/github/handlers/index.ts b/src/github/handlers/index.ts index 3905d0e..21876a0 100644 --- a/src/github/handlers/index.ts +++ b/src/github/handlers/index.ts @@ -69,50 +69,56 @@ async function handleEvent(event: EmitterWebhookEvent, eventHandler: InstanceTyp return; } - for (const pluginChain of pluginChains) { - if (await shouldSkipPlugin(context, pluginChain)) { - continue; - } - - // invoke the first plugin in the chain - const { plugin, with: settings } = pluginChain.uses[0]; - const isGithubPluginObject = isGithubPlugin(plugin); - console.log(`Calling handler ${JSON.stringify(plugin)} for event ${event.name}`); - - const stateId = crypto.randomUUID(); - - const state = { - eventId: context.id, - eventName: context.key, - eventPayload: event.payload, - currentPlugin: 0, - pluginChain: pluginChain.uses, - outputs: new Array(pluginChain.uses.length), - inputs: new Array(pluginChain.uses.length), - }; - - const ref = isGithubPluginObject ? (plugin.ref ?? (await getDefaultBranch(context, plugin.owner, plugin.repo))) : plugin; - const token = await eventHandler.getToken(event.payload.installation.id); - const inputs = new PluginInput(context.eventHandler, stateId, context.key, event.payload, settings, token, ref); - - state.inputs[0] = inputs; - await eventHandler.pluginChainState.put(stateId, state); + await Promise.all( + pluginChains.map(async (pluginChain) => { + if (await shouldSkipPlugin(context, pluginChain)) { + return; + } + if (!("installation" in event.payload) || event.payload.installation?.id === undefined) { + console.log(`No installation found, cannot invoke plugin`, pluginChain); + return; + } - // We wrap the dispatch so a failing plugin doesn't break the whole execution - try { - if (!isGithubPluginObject) { - await dispatchWorker(plugin, await inputs.getWorkerInputs()); - } else { - await dispatchWorkflow(context, { - owner: plugin.owner, - repository: plugin.repo, - workflowId: plugin.workflowId, - ref: plugin.ref, - inputs: await inputs.getWorkflowInputs(), - }); + // invoke the first plugin in the chain + const { plugin, with: settings } = pluginChain.uses[0]; + const isGithubPluginObject = isGithubPlugin(plugin); + console.log(`Calling handler ${JSON.stringify(plugin)} for event ${event.name}`); + + const stateId = crypto.randomUUID(); + + const state = { + eventId: context.id, + eventName: context.key, + eventPayload: event.payload, + currentPlugin: 0, + pluginChain: pluginChain.uses, + outputs: new Array(pluginChain.uses.length), + inputs: new Array(pluginChain.uses.length), + }; + + const ref = isGithubPluginObject ? (plugin.ref ?? (await getDefaultBranch(context, plugin.owner, plugin.repo))) : plugin; + const token = await eventHandler.getToken(event.payload.installation.id); + const inputs = new PluginInput(context.eventHandler, stateId, context.key, event.payload, settings, token, ref); + + state.inputs[0] = inputs; + await eventHandler.pluginChainState.put(stateId, state); + + // We wrap the dispatch so a failing plugin doesn't break the whole execution + try { + if (!isGithubPluginObject) { + await dispatchWorker(plugin, await inputs.getWorkerInputs()); + } else { + await dispatchWorkflow(context, { + owner: plugin.owner, + repository: plugin.repo, + workflowId: plugin.workflowId, + ref: plugin.ref, + inputs: await inputs.getWorkflowInputs(), + }); + } + } catch (e) { + console.error(`An error occurred while processing the plugin chain, will skip plugin ${JSON.stringify(plugin)}`, e); } - } catch (e) { - console.error(`An error occurred while processing the plugin chain, will skip plugin ${JSON.stringify(plugin)}`, e); - } - } + }) + ); }