From 904d8cb890c8ce4bc125799bb5b702f75f1a59f2 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Fri, 13 Sep 2024 16:46:29 +0200 Subject: [PATCH 1/5] chore(webhooks): refactor webhooks dispatch event function --- .../modules/activitystream/services/index.js | 16 +++- .../modules/webhooks/domain/operations.ts | 5 + .../modules/webhooks/repositories/webhooks.ts | 7 ++ .../modules/webhooks/services/webhooks-new.ts | 94 +++++++++++++++++++ .../modules/webhooks/services/webhooks.js | 61 ------------ .../modules/webhooks/tests/webhooks.spec.js | 23 ++++- 6 files changed, 140 insertions(+), 66 deletions(-) diff --git a/packages/server/modules/activitystream/services/index.js b/packages/server/modules/activitystream/services/index.js index 57108887e..6afb1ca87 100644 --- a/packages/server/modules/activitystream/services/index.js +++ b/packages/server/modules/activitystream/services/index.js @@ -2,7 +2,13 @@ const knex = require('@/db/knex') -const { dispatchStreamEvent } = require('../../webhooks/services/webhooks') +const { dispatchStreamEvent } = require('../../webhooks/services/webhooks-new') +const { getStream } = require('@/modules/core/repositories/streams') +const { + createWebhookEventFactory +} = require('@/modules/webhooks/repositories/webhooks') +const { getUser } = require('@/modules/core/repositories/users') +const { getServerInfo } = require('@/modules/core/services/generic') const StreamActivity = () => knex('stream_activity') const StreamAcl = () => knex('stream_acl') @@ -41,7 +47,13 @@ module.exports = { } } - await dispatchStreamEvent( + await dispatchStreamEvent({ + db: trx ?? knex.db, + getServerInfo, + getStream, + createWebhookEvent: createWebhookEventFactory({ db: knex.db }), + getUser + })( { streamId, event: actionType, diff --git a/packages/server/modules/webhooks/domain/operations.ts b/packages/server/modules/webhooks/domain/operations.ts index 443dd3357..d57891e83 100644 --- a/packages/server/modules/webhooks/domain/operations.ts +++ b/packages/server/modules/webhooks/domain/operations.ts @@ -1,4 +1,5 @@ import { Webhook } from '@/modules/webhooks/domain/types' +import { WebhookEvent } from '@/test/graphql/generated/graphql' export type CreateWebhook = ( webhook: Pick< @@ -32,3 +33,7 @@ export type DeleteWebhook = ({ id }: Pick) => Promise export type GetStreamWebhooks = ({ streamId }: Pick) => Promise + +export type CreateWebhookEvent = ( + event: Pick +) => Promise diff --git a/packages/server/modules/webhooks/repositories/webhooks.ts b/packages/server/modules/webhooks/repositories/webhooks.ts index 887d94719..fcb7f9a8c 100644 --- a/packages/server/modules/webhooks/repositories/webhooks.ts +++ b/packages/server/modules/webhooks/repositories/webhooks.ts @@ -3,6 +3,7 @@ import { Webhook } from '@/modules/webhooks/domain/types' import { CountWebhooksByStreamId, CreateWebhook, + CreateWebhookEvent, DeleteWebhook, GetStreamWebhooks, GetWebhookById, @@ -94,3 +95,9 @@ export const getStreamWebhooksFactory = triggers: toTriggersArray(webhook.triggers) })) } + +export const createWebhookEventFactory = + ({ db }: { db: Knex }): CreateWebhookEvent => + async (event) => { + return await tables(db).webhooksEvents.insert(event).returning('id') + } diff --git a/packages/server/modules/webhooks/services/webhooks-new.ts b/packages/server/modules/webhooks/services/webhooks-new.ts index 7dc19d694..c086a80d1 100644 --- a/packages/server/modules/webhooks/services/webhooks-new.ts +++ b/packages/server/modules/webhooks/services/webhooks-new.ts @@ -1,7 +1,9 @@ +import { getServerInfo as getServerInfoFn } from '@/modules/core/services/generic' import { ForbiddenError } from '@/modules/shared/errors' import { CountWebhooksByStreamId, CreateWebhook, + CreateWebhookEvent, DeleteWebhook, GetWebhookById, UpdateWebhook @@ -9,6 +11,15 @@ import { import { Webhook } from '@/modules/webhooks/domain/types' import { SetValuesNullable } from '@speckle/shared' import crs from 'crypto-random-string' +import { + StreamWithOptionalRole, + getStream as getStreamFn +} from '@/modules/core/repositories/streams' +import { + getUser as getUserFn, + UserWithOptionalRole +} from '@/modules/core/repositories/users' +import { Knex } from 'knex' const MAX_STREAM_WEBHOOKS = 100 @@ -87,3 +98,86 @@ export const deleteWebhook = return id } + +export const dispatchStreamEvent = + ({ + db, + getServerInfo, + getStream, + createWebhookEvent, + getUser + }: { + db: Knex // TODO: this should not be injected here + getServerInfo: typeof getServerInfoFn + getStream: typeof getStreamFn + createWebhookEvent: CreateWebhookEvent + getUser: typeof getUserFn + }) => + async ( + { + streamId, + event, + eventPayload + }: { + streamId: string + event: string + eventPayload: { + server: { id?: number; canonicalUrl?: string } + streamId?: string + stream?: StreamWithOptionalRole + userId?: string + user: Partial | null + webhook: Webhook + } + }, + { trx }: { trx?: Knex.Transaction } = {} + ) => { + // Add server info + eventPayload.server = await getServerInfo() + eventPayload.server.canonicalUrl = process.env.CANONICAL_URL + delete eventPayload.server.id + + // Add stream info + if (eventPayload.streamId) { + eventPayload.stream = await getStream( + { + streamId: eventPayload.streamId, + userId: eventPayload.userId + }, + { trx } + ) + } + + // Add user info (except email and pwd) + if (eventPayload.userId) { + eventPayload.user = await getUser(eventPayload.userId) + if (eventPayload.user) { + delete eventPayload.user.passwordDigest + delete eventPayload.user.email + } + } + + // with this select, we must have the streamid available on the webhook config, + // even when the stream is deleted, to dispatch the stream deleted webhook events + const { rows } = await db.raw( + ` + SELECT * FROM webhooks_config WHERE "streamId" = ? + `, + [streamId] + ) + for (const wh of rows) { + if (!wh.enabled) continue + if (!(event in wh.triggers)) continue + + // Add webhook info (the key `webhook` will be replaced for each webhook configured, before serializing the payload and storing it) + eventPayload.webhook = wh + eventPayload.webhook.triggers = Object.keys(eventPayload.webhook.triggers) + delete eventPayload.webhook.secret + + await createWebhookEvent({ + id: crs({ length: 20 }), + webhookId: wh.id, + payload: JSON.stringify(eventPayload) + }) + } + } diff --git a/packages/server/modules/webhooks/services/webhooks.js b/packages/server/modules/webhooks/services/webhooks.js index e9dd3c1e0..3f830e25e 100644 --- a/packages/server/modules/webhooks/services/webhooks.js +++ b/packages/server/modules/webhooks/services/webhooks.js @@ -1,71 +1,10 @@ 'use strict' const knex = require('@/db/knex') -const { getStream } = require('@/modules/core/repositories/streams') -const crs = require('crypto-random-string') const WebhooksEvents = () => knex('webhooks_events') -const Users = () => knex('users') - -const { getServerInfo } = require('../../core/services/generic') module.exports = { - async dispatchStreamEvent({ streamId, event, eventPayload }, { trx } = {}) { - // Add server info - eventPayload.server = await getServerInfo() - eventPayload.server.canonicalUrl = process.env.CANONICAL_URL - delete eventPayload.server.id - - // Add stream info - if (eventPayload.streamId) { - eventPayload.stream = await getStream( - { - streamId: eventPayload.streamId, - userId: eventPayload.userId - }, - { trx } - ) - } - - // Add user info (except email and pwd) - if (eventPayload.userId) { - eventPayload.user = await Users() - .where({ id: eventPayload.userId }) - .select('*') - .first() - if (eventPayload.user) { - delete eventPayload.user.passwordDigest - delete eventPayload.user.email - } - } - - // with this select, we must have the streamid available on the webhook config, - // even when the stream is deleted, to dispatch the stream deleted webhook events - const { rows } = await knex.raw( - ` - SELECT * FROM webhooks_config WHERE "streamId" = ? - `, - [streamId] - ) - for (const wh of rows) { - if (!wh.enabled) continue - if (!(event in wh.triggers)) continue - - // Add webhook info (the key `webhook` will be replaced for each webhook configured, before serializing the payload and storing it) - eventPayload.webhook = wh - eventPayload.webhook.triggers = Object.keys(eventPayload.webhook.triggers) - delete eventPayload.webhook.secret - - const q = WebhooksEvents().insert({ - id: crs({ length: 20 }), - webhookId: wh.id, - payload: JSON.stringify(eventPayload) - }) - if (trx) q.transacting(trx) - await q - } - }, - async getLastWebhookEvents({ webhookId, limit }) { if (!limit) { limit = 100 diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index 8319acfbf..e55b22b75 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -9,7 +9,7 @@ const { } = require('@/test/hooks') const { noErrors } = require('@/test/helpers') const { createPersonalAccessToken } = require('../../core/services/tokens') -const { getLastWebhookEvents, dispatchStreamEvent } = require('../services/webhooks') +const { getLastWebhookEvents } = require('../services/webhooks') const { createUser } = require('../../core/services/users') const { createStream, grantPermissionsStream } = require('../../core/services/streams') const { Scopes, Roles } = require('@speckle/shared') @@ -19,15 +19,20 @@ const { getWebhookByIdFactory, updateWebhookFactory, deleteWebhookFactory, - getStreamWebhooksFactory + getStreamWebhooksFactory, + createWebhookEventFactory } = require('@/modules/webhooks/repositories/webhooks') const { db } = require('@/db/knex') const { createWebhook, updateWebhook: updateWebhookService, - deleteWebhook + deleteWebhook, + dispatchStreamEvent } = require('@/modules/webhooks/services/webhooks-new') const { Users, Streams } = require('@/modules/core/dbSchema') +const { getServerInfo } = require('@/modules/core/services/generic') +const { getStream } = require('@/modules/core/repositories/streams') +const { getUser } = require('@/modules/core/repositories/users') const updateWebhook = updateWebhookService({ updateWebhookConfig: updateWebhookFactory({ db }) @@ -192,6 +197,12 @@ describe('Webhooks @webhooks', () => { countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) await dispatchStreamEvent({ + db, + getServerInfo, + getStream, + createWebhookEvent: createWebhookEventFactory({ db }), + getUser + })({ streamId, event: 'commit_create', eventPayload: { test: 'payload123' } @@ -260,6 +271,12 @@ describe('Webhooks @webhooks', () => { it('Should get stream webhooks and the previous events', async () => { await dispatchStreamEvent({ + db, + getServerInfo, + getStream, + createWebhookEvent: createWebhookEventFactory({ db }), + getUser + })({ streamId: streamTwo.id, event: 'commit_create', eventPayload: { test: 'payload321' } From 440ac2fa499095e78977132e4e0ed81a9836f95a Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Mon, 16 Sep 2024 15:54:52 +0200 Subject: [PATCH 2/5] chore(webhooks): refactor last functions for multi region --- .../modules/activitystream/services/index.js | 2 +- .../modules/webhooks/domain/operations.ts | 17 +++++++++-- .../server/modules/webhooks/domain/types.ts | 10 +++++++ .../webhooks/graph/resolvers/webhooks.js | 18 ------------ .../{webhooks-new.ts => webhooks.ts} | 17 +++++++++-- .../modules/webhooks/repositories/webhooks.ts | 28 +++++++++++++++++-- .../modules/webhooks/services/webhooks.js | 25 ----------------- .../services/{webhooks-new.ts => webhooks.ts} | 0 .../modules/webhooks/tests/webhooks.spec.js | 8 +++--- 9 files changed, 71 insertions(+), 54 deletions(-) delete mode 100644 packages/server/modules/webhooks/graph/resolvers/webhooks.js rename packages/server/modules/webhooks/graph/resolvers/{webhooks-new.ts => webhooks.ts} (87%) delete mode 100644 packages/server/modules/webhooks/services/webhooks.js rename packages/server/modules/webhooks/services/{webhooks-new.ts => webhooks.ts} (100%) diff --git a/packages/server/modules/activitystream/services/index.js b/packages/server/modules/activitystream/services/index.js index 6afb1ca87..2b7b57077 100644 --- a/packages/server/modules/activitystream/services/index.js +++ b/packages/server/modules/activitystream/services/index.js @@ -2,7 +2,7 @@ const knex = require('@/db/knex') -const { dispatchStreamEvent } = require('../../webhooks/services/webhooks-new') +const { dispatchStreamEvent } = require('../../webhooks/services/webhooks') const { getStream } = require('@/modules/core/repositories/streams') const { createWebhookEventFactory diff --git a/packages/server/modules/webhooks/domain/operations.ts b/packages/server/modules/webhooks/domain/operations.ts index d57891e83..fca991da3 100644 --- a/packages/server/modules/webhooks/domain/operations.ts +++ b/packages/server/modules/webhooks/domain/operations.ts @@ -1,5 +1,4 @@ -import { Webhook } from '@/modules/webhooks/domain/types' -import { WebhookEvent } from '@/test/graphql/generated/graphql' +import { Webhook, WebhookEvent } from '@/modules/webhooks/domain/types' export type CreateWebhook = ( webhook: Pick< @@ -37,3 +36,17 @@ export type GetStreamWebhooks = ({ export type CreateWebhookEvent = ( event: Pick ) => Promise + +export type GetLastWebhookEvents = ({ + webhookId, + limit +}: { + webhookId: string + limit?: number +}) => Promise + +export type GetWebhookEventsCount = ({ + webhookId +}: { + webhookId: string +}) => Promise diff --git a/packages/server/modules/webhooks/domain/types.ts b/packages/server/modules/webhooks/domain/types.ts index 56a9343f0..99644d74e 100644 --- a/packages/server/modules/webhooks/domain/types.ts +++ b/packages/server/modules/webhooks/domain/types.ts @@ -9,3 +9,13 @@ export type Webhook = { createdAt: Date updatedAt: Date } + +export type WebhookEvent = { + id: string + webhookId: string + status: number + statusInfo: string + retryCount: number + lastUpdate: Date + payload: string +} diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks.js b/packages/server/modules/webhooks/graph/resolvers/webhooks.js deleted file mode 100644 index 1ba8346fa..000000000 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks.js +++ /dev/null @@ -1,18 +0,0 @@ -const { - getLastWebhookEvents, - getWebhookEventsCount -} = require('../../services/webhooks') - -module.exports = { - Webhook: { - async history(parent, args) { - const items = await getLastWebhookEvents({ - webhookId: parent.id, - limit: args.limit - }) - const totalCount = await getWebhookEventsCount({ webhookId: parent.id }) - - return { items, totalCount } - } - } -} diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks-new.ts b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts similarity index 87% rename from packages/server/modules/webhooks/graph/resolvers/webhooks-new.ts rename to packages/server/modules/webhooks/graph/resolvers/webhooks.ts index 361095e05..edfb97dc6 100644 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks-new.ts +++ b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts @@ -4,14 +4,16 @@ import { createWebhook, deleteWebhook, updateWebhook -} from '@/modules/webhooks/services/webhooks-new' +} from '@/modules/webhooks/services/webhooks' import { Roles } from '@speckle/shared' import { countWebhooksByStreamIdFactory, createWebhookFactory, deleteWebhookFactory, + getLastWebhookEventsFactory, getStreamWebhooksFactory, getWebhookByIdFactory, + getWebhookEventsCountFactory, updateWebhookFactory } from '@/modules/webhooks/repositories/webhooks' import { db } from '@/db/knex' @@ -43,7 +45,18 @@ const streamWebhooksResolver = async ( export = { Webhook: { projectId: (parent) => parent.streamId, - hasSecret: (parent) => !!parent.secret?.length + hasSecret: (parent) => !!parent.secret?.length, + history: async (parent, args) => { + const items = await getLastWebhookEventsFactory({ db })({ + webhookId: parent.id, + limit: args.limit + }) + const totalCount = await getWebhookEventsCountFactory({ db })({ + webhookId: parent.id + }) + + return { items, totalCount } + } }, Stream: { webhooks: streamWebhooksResolver diff --git a/packages/server/modules/webhooks/repositories/webhooks.ts b/packages/server/modules/webhooks/repositories/webhooks.ts index fcb7f9a8c..6cfadfceb 100644 --- a/packages/server/modules/webhooks/repositories/webhooks.ts +++ b/packages/server/modules/webhooks/repositories/webhooks.ts @@ -1,12 +1,14 @@ import { Knex } from 'knex' -import { Webhook } from '@/modules/webhooks/domain/types' +import { Webhook, WebhookEvent } from '@/modules/webhooks/domain/types' import { CountWebhooksByStreamId, CreateWebhook, CreateWebhookEvent, DeleteWebhook, + GetLastWebhookEvents, GetStreamWebhooks, GetWebhookById, + GetWebhookEventsCount, UpdateWebhook } from '@/modules/webhooks/domain/operations' @@ -14,7 +16,7 @@ type WebhookConfig = Omit & { triggers: Record ({ webhooksConfigs: db('webhooks_config'), - webhooksEvents: db('webhooks_events') + webhooksEvents: db('webhooks_events') }) const toTriggersObj = (triggers: string[]): Record => @@ -101,3 +103,25 @@ export const createWebhookEventFactory = async (event) => { return await tables(db).webhooksEvents.insert(event).returning('id') } + +export const getLastWebhookEventsFactory = + ({ db }: { db: Knex }): GetLastWebhookEvents => + async ({ webhookId, limit }) => { + if (!limit) { + limit = 100 + } + + return await tables(db) + .webhooksEvents.select('*') + .where({ webhookId }) + .orderBy('lastUpdate', 'desc') + .limit(limit) + } + +export const getWebhookEventsCountFactory = + ({ db }: { db: Knex }): GetWebhookEventsCount => + async ({ webhookId }) => { + const [res] = await tables(db).webhooksEvents.count().where({ webhookId }) + + return parseInt(res.count.toString()) + } diff --git a/packages/server/modules/webhooks/services/webhooks.js b/packages/server/modules/webhooks/services/webhooks.js deleted file mode 100644 index 3f830e25e..000000000 --- a/packages/server/modules/webhooks/services/webhooks.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict' - -const knex = require('@/db/knex') - -const WebhooksEvents = () => knex('webhooks_events') - -module.exports = { - async getLastWebhookEvents({ webhookId, limit }) { - if (!limit) { - limit = 100 - } - - return await WebhooksEvents() - .select('*') - .where({ webhookId }) - .orderBy('lastUpdate', 'desc') - .limit(limit) - }, - - async getWebhookEventsCount({ webhookId }) { - const [res] = await WebhooksEvents().count().where({ webhookId }) - - return parseInt(res.count) - } -} diff --git a/packages/server/modules/webhooks/services/webhooks-new.ts b/packages/server/modules/webhooks/services/webhooks.ts similarity index 100% rename from packages/server/modules/webhooks/services/webhooks-new.ts rename to packages/server/modules/webhooks/services/webhooks.ts diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index e55b22b75..3b974c9c9 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -9,7 +9,6 @@ const { } = require('@/test/hooks') const { noErrors } = require('@/test/helpers') const { createPersonalAccessToken } = require('../../core/services/tokens') -const { getLastWebhookEvents } = require('../services/webhooks') const { createUser } = require('../../core/services/users') const { createStream, grantPermissionsStream } = require('../../core/services/streams') const { Scopes, Roles } = require('@speckle/shared') @@ -20,7 +19,8 @@ const { updateWebhookFactory, deleteWebhookFactory, getStreamWebhooksFactory, - createWebhookEventFactory + createWebhookEventFactory, + getLastWebhookEventsFactory } = require('@/modules/webhooks/repositories/webhooks') const { db } = require('@/db/knex') const { @@ -28,7 +28,7 @@ const { updateWebhook: updateWebhookService, deleteWebhook, dispatchStreamEvent -} = require('@/modules/webhooks/services/webhooks-new') +} = require('@/modules/webhooks/services/webhooks') const { Users, Streams } = require('@/modules/core/dbSchema') const { getServerInfo } = require('@/modules/core/services/generic') const { getStream } = require('@/modules/core/repositories/streams') @@ -207,7 +207,7 @@ describe('Webhooks @webhooks', () => { event: 'commit_create', eventPayload: { test: 'payload123' } }) - const lastEvents = await getLastWebhookEvents({ webhookId }) + const lastEvents = await getLastWebhookEventsFactory({ db })({ webhookId }) expect(lastEvents).to.have.lengthOf(1) expect(JSON.parse(lastEvents[0].payload).test).to.equal('payload123') }) From 194d49dc6144cf7ddf8851c1ddbd20e995945bdb Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Tue, 17 Sep 2024 10:25:03 +0200 Subject: [PATCH 3/5] chore(webhooks): rename repository functions --- .../modules/webhooks/domain/operations.ts | 6 +++--- .../webhooks/graph/resolvers/webhooks.ts | 12 ++++++------ .../modules/webhooks/repositories/webhooks.ts | 18 +++++++++--------- .../modules/webhooks/services/webhooks.ts | 12 ++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/server/modules/webhooks/domain/operations.ts b/packages/server/modules/webhooks/domain/operations.ts index fca991da3..370ea3058 100644 --- a/packages/server/modules/webhooks/domain/operations.ts +++ b/packages/server/modules/webhooks/domain/operations.ts @@ -1,6 +1,6 @@ import { Webhook, WebhookEvent } from '@/modules/webhooks/domain/types' -export type CreateWebhook = ( +export type CreateWebhookConfig = ( webhook: Pick< Webhook, 'id' | 'streamId' | 'url' | 'description' | 'secret' | 'enabled' | 'triggers' @@ -13,7 +13,7 @@ export type CountWebhooksByStreamId = ({ export type GetWebhookById = ({ id }: Pick) => Promise -export type UpdateWebhook = ({ +export type UpdateWebhookConfig = ({ webhookId, webhookInput }: { @@ -27,7 +27,7 @@ export type UpdateWebhook = ({ > }) => Promise -export type DeleteWebhook = ({ id }: Pick) => Promise +export type DeleteWebhookConfig = ({ id }: Pick) => Promise export type GetStreamWebhooks = ({ streamId diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks.ts b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts index edfb97dc6..c47804c14 100644 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks.ts +++ b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts @@ -8,13 +8,13 @@ import { import { Roles } from '@speckle/shared' import { countWebhooksByStreamIdFactory, - createWebhookFactory, - deleteWebhookFactory, + createWebhookConfigFactory, + deleteWebhookConfigFactory, getLastWebhookEventsFactory, getStreamWebhooksFactory, getWebhookByIdFactory, getWebhookEventsCountFactory, - updateWebhookFactory + updateWebhookConfigFactory } from '@/modules/webhooks/repositories/webhooks' import { db } from '@/db/knex' import { ForbiddenError } from '@/modules/shared/errors' @@ -74,7 +74,7 @@ export = { ) const id = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })({ streamId: args.webhook.streamId, @@ -102,7 +102,7 @@ export = { ) const updated = await updateWebhook({ - updateWebhookConfig: updateWebhookFactory({ db }) + updateWebhookConfig: updateWebhookConfigFactory({ db }) })({ id: args.webhook.id, url: args.webhook.url, @@ -123,7 +123,7 @@ export = { ) return await deleteWebhook({ - deleteWebhookConfig: deleteWebhookFactory({ db }), + deleteWebhookConfig: deleteWebhookConfigFactory({ db }), getWebhookById: getWebhookByIdFactory({ db }) })(args.webhook) } diff --git a/packages/server/modules/webhooks/repositories/webhooks.ts b/packages/server/modules/webhooks/repositories/webhooks.ts index 6cfadfceb..da03f7986 100644 --- a/packages/server/modules/webhooks/repositories/webhooks.ts +++ b/packages/server/modules/webhooks/repositories/webhooks.ts @@ -2,14 +2,14 @@ import { Knex } from 'knex' import { Webhook, WebhookEvent } from '@/modules/webhooks/domain/types' import { CountWebhooksByStreamId, - CreateWebhook, + CreateWebhookConfig, CreateWebhookEvent, - DeleteWebhook, + DeleteWebhookConfig, GetLastWebhookEvents, GetStreamWebhooks, GetWebhookById, GetWebhookEventsCount, - UpdateWebhook + UpdateWebhookConfig } from '@/modules/webhooks/domain/operations' type WebhookConfig = Omit & { triggers: Record } @@ -25,8 +25,8 @@ const toTriggersObj = (triggers: string[]): Record => const toTriggersArray = (triggers: Record): string[] => Object.keys(triggers) -export const createWebhookFactory = - ({ db }: { db: Knex }): CreateWebhook => +export const createWebhookConfigFactory = + ({ db }: { db: Knex }): CreateWebhookConfig => async ({ id, streamId, url, description, secret, enabled, triggers }) => { const triggersObj = toTriggersObj(triggers) @@ -62,8 +62,8 @@ export const getWebhookByIdFactory = return { ...webhook, triggers: toTriggersArray(webhook.triggers) } } -export const updateWebhookFactory = - ({ db }: { db: Knex }): UpdateWebhook => +export const updateWebhookConfigFactory = + ({ db }: { db: Knex }): UpdateWebhookConfig => async ({ webhookId, webhookInput }) => { const { triggers, ...update } = webhookInput let triggersObj: Record | undefined @@ -78,8 +78,8 @@ export const updateWebhookFactory = return webhookId } -export const deleteWebhookFactory = - ({ db }: { db: Knex }): DeleteWebhook => +export const deleteWebhookConfigFactory = + ({ db }: { db: Knex }): DeleteWebhookConfig => async ({ id }) => { return await tables(db).webhooksConfigs.where({ id }).del() } diff --git a/packages/server/modules/webhooks/services/webhooks.ts b/packages/server/modules/webhooks/services/webhooks.ts index c086a80d1..bc7f98c04 100644 --- a/packages/server/modules/webhooks/services/webhooks.ts +++ b/packages/server/modules/webhooks/services/webhooks.ts @@ -2,11 +2,11 @@ import { getServerInfo as getServerInfoFn } from '@/modules/core/services/generi import { ForbiddenError } from '@/modules/shared/errors' import { CountWebhooksByStreamId, - CreateWebhook, + CreateWebhookConfig, CreateWebhookEvent, - DeleteWebhook, + DeleteWebhookConfig, GetWebhookById, - UpdateWebhook + UpdateWebhookConfig } from '@/modules/webhooks/domain/operations' import { Webhook } from '@/modules/webhooks/domain/types' import { SetValuesNullable } from '@speckle/shared' @@ -28,7 +28,7 @@ export const createWebhook = createWebhookConfig, countWebhooksByStreamId }: { - createWebhookConfig: CreateWebhook + createWebhookConfig: CreateWebhookConfig countWebhooksByStreamId: CountWebhooksByStreamId }) => async ({ @@ -59,7 +59,7 @@ export const createWebhook = } export const updateWebhook = - ({ updateWebhookConfig }: { updateWebhookConfig: UpdateWebhook }) => + ({ updateWebhookConfig }: { updateWebhookConfig: UpdateWebhookConfig }) => async ( webhook: Pick & Partial>> @@ -84,7 +84,7 @@ export const deleteWebhook = deleteWebhookConfig, getWebhookById }: { - deleteWebhookConfig: DeleteWebhook + deleteWebhookConfig: DeleteWebhookConfig getWebhookById: GetWebhookById }) => async ({ id, streamId }: { id: string; streamId: string }) => { From 06052244776c7c28991a48baee951b170908d072 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Tue, 17 Sep 2024 11:41:14 +0200 Subject: [PATCH 4/5] chore(webhooks): fix tests --- .../modules/webhooks/tests/webhooks.spec.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index 3b974c9c9..2381a6a87 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -13,11 +13,11 @@ const { createUser } = require('../../core/services/users') const { createStream, grantPermissionsStream } = require('../../core/services/streams') const { Scopes, Roles } = require('@speckle/shared') const { - createWebhookFactory, + createWebhookConfigFactory, countWebhooksByStreamIdFactory, getWebhookByIdFactory, - updateWebhookFactory, - deleteWebhookFactory, + updateWebhookConfigFactory, + deleteWebhookConfigFactory, getStreamWebhooksFactory, createWebhookEventFactory, getLastWebhookEventsFactory @@ -35,7 +35,7 @@ const { getStream } = require('@/modules/core/repositories/streams') const { getUser } = require('@/modules/core/repositories/users') const updateWebhook = updateWebhookService({ - updateWebhookConfig: updateWebhookFactory({ db }) + updateWebhookConfig: updateWebhookConfigFactory({ db }) }) const getStreamWebhooks = getStreamWebhooksFactory({ db }) @@ -88,7 +88,7 @@ describe('Webhooks @webhooks', () => { describe('Create, Read, Update, Delete Webhooks', () => { it('Should create a webhook', async () => { webhookOne.id = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhookOne) expect(webhookOne).to.have.property('id') @@ -104,7 +104,7 @@ describe('Webhooks @webhooks', () => { it('Should update a webhook', async () => { const webhookId = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhookOne) @@ -133,11 +133,11 @@ describe('Webhooks @webhooks', () => { triggers: ['commit_create', 'commit_update'] } webhook.id = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) await deleteWebhook({ - deleteWebhookConfig: deleteWebhookFactory({ db }), + deleteWebhookConfig: deleteWebhookConfigFactory({ db }), getWebhookById: getWebhookByIdFactory({ db }) })(webhook) const webhookDeleted = await getWebhookByIdFactory({ db })({ id: webhook.id }) @@ -164,7 +164,7 @@ describe('Webhooks @webhooks', () => { triggers: ['commit_create', 'commit_update'] } await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) streamWebhooks = await getStreamWebhooks({ streamId }) @@ -193,7 +193,7 @@ describe('Webhooks @webhooks', () => { triggers: ['commit_create', 'commit_update'] } const webhookId = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) await dispatchStreamEvent({ @@ -352,7 +352,7 @@ describe('Webhooks @webhooks', () => { triggers: ['commit_create', 'commit_update'] } webhook.id = await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) const res = await sendRequest(userOne.token, { @@ -402,14 +402,14 @@ describe('Webhooks @webhooks', () => { } for (let i = 0; i < limit; i++) { await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) } try { await createWebhook({ - createWebhookConfig: createWebhookFactory({ db }), + createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) } catch (err) { From b03a8c38bdc8408251475a691b55cf7d35bba819 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Tue, 17 Sep 2024 15:58:27 +0200 Subject: [PATCH 5/5] chore(webhooks): rename functions to factory --- .../modules/activitystream/services/index.js | 4 +-- .../webhooks/graph/resolvers/webhooks.ts | 12 +++---- .../modules/webhooks/services/webhooks.ts | 8 ++--- .../modules/webhooks/tests/webhooks.spec.js | 32 +++++++++---------- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/server/modules/activitystream/services/index.js b/packages/server/modules/activitystream/services/index.js index 2b7b57077..35f9168b9 100644 --- a/packages/server/modules/activitystream/services/index.js +++ b/packages/server/modules/activitystream/services/index.js @@ -2,7 +2,7 @@ const knex = require('@/db/knex') -const { dispatchStreamEvent } = require('../../webhooks/services/webhooks') +const { dispatchStreamEventFactory } = require('@/modules/webhooks/services/webhooks') const { getStream } = require('@/modules/core/repositories/streams') const { createWebhookEventFactory @@ -47,7 +47,7 @@ module.exports = { } } - await dispatchStreamEvent({ + await dispatchStreamEventFactory({ db: trx ?? knex.db, getServerInfo, getStream, diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks.ts b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts index c47804c14..3401ad7aa 100644 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks.ts +++ b/packages/server/modules/webhooks/graph/resolvers/webhooks.ts @@ -1,9 +1,9 @@ import { Resolvers } from '@/modules/core/graph/generated/graphql' import { authorizeResolver } from '@/modules/shared' import { - createWebhook, - deleteWebhook, - updateWebhook + createWebhookFactory, + deleteWebhookFactory, + updateWebhookFactory } from '@/modules/webhooks/services/webhooks' import { Roles } from '@speckle/shared' import { @@ -73,7 +73,7 @@ export = { context.resourceAccessRules ) - const id = await createWebhook({ + const id = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })({ @@ -101,7 +101,7 @@ export = { 'The webhook id and stream id do not match. Please check your inputs.' ) - const updated = await updateWebhook({ + const updated = await updateWebhookFactory({ updateWebhookConfig: updateWebhookConfigFactory({ db }) })({ id: args.webhook.id, @@ -122,7 +122,7 @@ export = { context.resourceAccessRules ) - return await deleteWebhook({ + return await deleteWebhookFactory({ deleteWebhookConfig: deleteWebhookConfigFactory({ db }), getWebhookById: getWebhookByIdFactory({ db }) })(args.webhook) diff --git a/packages/server/modules/webhooks/services/webhooks.ts b/packages/server/modules/webhooks/services/webhooks.ts index bc7f98c04..84207aaee 100644 --- a/packages/server/modules/webhooks/services/webhooks.ts +++ b/packages/server/modules/webhooks/services/webhooks.ts @@ -23,7 +23,7 @@ import { Knex } from 'knex' const MAX_STREAM_WEBHOOKS = 100 -export const createWebhook = +export const createWebhookFactory = ({ createWebhookConfig, countWebhooksByStreamId @@ -58,7 +58,7 @@ export const createWebhook = }) } -export const updateWebhook = +export const updateWebhookFactory = ({ updateWebhookConfig }: { updateWebhookConfig: UpdateWebhookConfig }) => async ( webhook: Pick & @@ -79,7 +79,7 @@ export const updateWebhook = }) } -export const deleteWebhook = +export const deleteWebhookFactory = ({ deleteWebhookConfig, getWebhookById @@ -99,7 +99,7 @@ export const deleteWebhook = return id } -export const dispatchStreamEvent = +export const dispatchStreamEventFactory = ({ db, getServerInfo, diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index 2381a6a87..ee4fb75e7 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -24,17 +24,17 @@ const { } = require('@/modules/webhooks/repositories/webhooks') const { db } = require('@/db/knex') const { - createWebhook, - updateWebhook: updateWebhookService, - deleteWebhook, - dispatchStreamEvent + createWebhookFactory, + updateWebhookFactory, + deleteWebhookFactory, + dispatchStreamEventFactory } = require('@/modules/webhooks/services/webhooks') const { Users, Streams } = require('@/modules/core/dbSchema') const { getServerInfo } = require('@/modules/core/services/generic') const { getStream } = require('@/modules/core/repositories/streams') const { getUser } = require('@/modules/core/repositories/users') -const updateWebhook = updateWebhookService({ +const updateWebhook = updateWebhookFactory({ updateWebhookConfig: updateWebhookConfigFactory({ db }) }) const getStreamWebhooks = getStreamWebhooksFactory({ db }) @@ -87,7 +87,7 @@ describe('Webhooks @webhooks', () => { describe('Create, Read, Update, Delete Webhooks', () => { it('Should create a webhook', async () => { - webhookOne.id = await createWebhook({ + webhookOne.id = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhookOne) @@ -103,7 +103,7 @@ describe('Webhooks @webhooks', () => { }) it('Should update a webhook', async () => { - const webhookId = await createWebhook({ + const webhookId = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhookOne) @@ -132,11 +132,11 @@ describe('Webhooks @webhooks', () => { enabled: true, triggers: ['commit_create', 'commit_update'] } - webhook.id = await createWebhook({ + webhook.id = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) - await deleteWebhook({ + await deleteWebhookFactory({ deleteWebhookConfig: deleteWebhookConfigFactory({ db }), getWebhookById: getWebhookByIdFactory({ db }) })(webhook) @@ -163,7 +163,7 @@ describe('Webhooks @webhooks', () => { enabled: true, triggers: ['commit_create', 'commit_update'] } - await createWebhook({ + await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) @@ -192,11 +192,11 @@ describe('Webhooks @webhooks', () => { enabled: true, triggers: ['commit_create', 'commit_update'] } - const webhookId = await createWebhook({ + const webhookId = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) - await dispatchStreamEvent({ + await dispatchStreamEventFactory({ db, getServerInfo, getStream, @@ -270,7 +270,7 @@ describe('Webhooks @webhooks', () => { }) it('Should get stream webhooks and the previous events', async () => { - await dispatchStreamEvent({ + await dispatchStreamEventFactory({ db, getServerInfo, getStream, @@ -351,7 +351,7 @@ describe('Webhooks @webhooks', () => { enabled: true, triggers: ['commit_create', 'commit_update'] } - webhook.id = await createWebhook({ + webhook.id = await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) @@ -401,14 +401,14 @@ describe('Webhooks @webhooks', () => { triggers: ['commit_create', 'commit_update'] } for (let i = 0; i < limit; i++) { - await createWebhook({ + await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook) } try { - await createWebhook({ + await createWebhookFactory({ createWebhookConfig: createWebhookConfigFactory({ db }), countWebhooksByStreamId: countWebhooksByStreamIdFactory({ db }) })(webhook)