diff --git a/packages/server/modules/shared/command.ts b/packages/server/modules/shared/command.ts index e69eb5a05..e492b8c64 100644 --- a/packages/server/modules/shared/command.ts +++ b/packages/server/modules/shared/command.ts @@ -1,12 +1,20 @@ -import { EmitArg, EventBus, EventBusEmit } from '@/modules/shared/services/eventBus' +import { mainDb } from '@/db/knex' +import { withTransaction } from '@/modules/shared/helpers/dbHelper' +import { + EmitArg, + EventBus, + EventBusEmit, + getEventBus +} from '@/modules/shared/services/eventBus' +import { withOperationLogging } from '@/observability/domain/businessLogging' +import { MaybeAsync } from '@speckle/shared' import { Knex } from 'knex' +import { isBoolean } from 'lodash' +import { Logger } from 'pino' /** - * TODO: Fix api - make operationFactory db arg actually return the trx. Currently many usages of this - * are not working correctly cause they just use the db, skipping the transaction - * - * Also: withOperationLogging and withOperationTransaction could all be merged into this, with - * this having a better name like `operationFactory` + * @deprecated asOperation does this and more. Also many usages of commandFactory are broken + * in the sense that they're not actually using the transaction correctly */ export const commandFactory = ) => ReturnType>({ @@ -40,3 +48,75 @@ export const commandFactory = throw err } } + +/** + * Adds logging & transaction support to an operation + */ +export const asOperation = async ( + operation: (args: { db: Knex; emit: EventBusEmit }) => MaybeAsync, + params: { + name: string + logger: Logger + description?: string + /** + * Defaults to main DB + */ + db?: Knex + /** + * Defaults to main event bus + */ + eventBus?: EventBus + /** + * Whether to treat the operation as a transaction. That makes the injected DB a knex transaction + * and also collects eventBus events to be emitted at the end of the operation. + * + * Can be a bool or an obj describing how the trx should be set up + */ + transaction?: + | boolean + | { + db: true // db trx can't be turned off, only the eventBus trx can + eventBus: boolean + } + } +): Promise => { + const { + db = mainDb, + eventBus = getEventBus(), + logger, + name, + description, + transaction + } = params + + return await withOperationLogging( + async () => { + if (!transaction) { + return await operation({ db, emit: eventBus.emit }) + } + + const events: EmitArg[] = [] + const emit: EventBusEmit = async ({ eventName, payload }) => { + events.push({ eventName, payload }) + } + const trxRet = await withTransaction( + async ({ trx }) => { + const useEmitTrx = isBoolean(transaction) ? transaction : transaction.eventBus + + return await operation({ db: trx, emit: useEmitTrx ? emit : eventBus.emit }) + }, + { db } + ) + for (const event of events) { + await eventBus.emit(event) + } + + return trxRet + }, + { + logger, + operationName: name, + operationDescription: description + } + ) +} diff --git a/packages/server/modules/shared/helpers/dbHelper.ts b/packages/server/modules/shared/helpers/dbHelper.ts index 942b72e17..dbdbf5bdc 100644 --- a/packages/server/modules/shared/helpers/dbHelper.ts +++ b/packages/server/modules/shared/helpers/dbHelper.ts @@ -107,7 +107,7 @@ export const numberOfFreeConnections = (knex: Knex) => { } export const withTransaction = async ( - operation: (args: { db: Knex }) => MaybeAsync, + operation: (args: { db: Knex; trx: Knex }) => MaybeAsync, params: { db: Knex } @@ -116,7 +116,8 @@ export const withTransaction = async ( const trx = await db.transaction() try { - const result = await operation({ db: trx }) + // db and trx are just aliases, you can use whichever is more convenient + const result = await operation({ db: trx, trx }) await trx.commit() return result } catch (e) { diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 2f5946d72..c8e229035 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -147,7 +147,7 @@ import { import { updateStreamRoleAndNotifyFactory } from '@/modules/core/services/streams/management' import { getUserFactory, getUsersFactory } from '@/modules/core/repositories/users' import { getServerInfoFactory } from '@/modules/core/repositories/server' -import { commandFactory } from '@/modules/shared/command' +import { asOperation, commandFactory } from '@/modules/shared/command' import { withTransaction } from '@/modules/shared/helpers/dbHelper' import { getRateLimitResult, @@ -506,86 +506,82 @@ export = FF_WORKSPACES_MODULE_ENABLED const logger = context.log - const createWorkspace = commandFactory({ - db, - eventBus, - operationFactory: ({ trx, emit }) => { + return await asOperation( + async ({ db, emit }) => { const createWorkspace = createWorkspaceFactory({ validateSlug: validateSlugFactory({ - getWorkspaceBySlug: getWorkspaceBySlugFactory({ db: trx }) + getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), generateValidSlug: generateValidSlugFactory({ - getWorkspaceBySlug: getWorkspaceBySlugFactory({ db: trx }) + getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), - upsertWorkspace: upsertWorkspaceFactory({ db: trx }), - upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db: trx }), + upsertWorkspace: upsertWorkspaceFactory({ db }), + upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db }), emitWorkspaceEvent: emit, ensureValidWorkspaceRoleSeat: ensureValidWorkspaceRoleSeatFactory({ - createWorkspaceSeat: createWorkspaceSeatFactory({ db: trx }), - getWorkspaceUserSeat: getWorkspaceUserSeatFactory({ db: trx }), + createWorkspaceSeat: createWorkspaceSeatFactory({ db }), + getWorkspaceUserSeat: getWorkspaceUserSeatFactory({ db }), eventEmit: emit }) }) const updateWorkspace = updateWorkspaceFactory({ validateSlug: validateSlugFactory({ - getWorkspaceBySlug: getWorkspaceBySlugFactory({ db: trx }) + getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), - getWorkspace: getWorkspaceWithDomainsFactory({ db: trx }), + getWorkspace: getWorkspaceWithDomainsFactory({ db }), getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderFactory({ - db: trx, + db, decrypt: getDecryptor() }), - upsertWorkspace: upsertWorkspaceFactory({ db: trx }), + upsertWorkspace: upsertWorkspaceFactory({ db }), emitWorkspaceEvent: emit }) const addDomain = addDomainToWorkspaceFactory({ - getWorkspace: getWorkspaceFactory({ db: trx }), - findEmailsByUserId: findEmailsByUserIdFactory({ db: trx }), - storeWorkspaceDomain: storeWorkspaceDomainFactory({ db: trx }), - getDomains: getWorkspaceDomainsFactory({ db: trx }), + getWorkspace: getWorkspaceFactory({ db }), + findEmailsByUserId: findEmailsByUserIdFactory({ db }), + storeWorkspaceDomain: storeWorkspaceDomainFactory({ db }), + getDomains: getWorkspaceDomainsFactory({ db }), emitWorkspaceEvent: emit }) - return async () => { - let workspace = await createWorkspace({ + let workspace = await createWorkspace({ + userId: context.userId!, + workspaceInput: { + name, + slug, + description: description ?? null, + logo: logo ?? null + }, + userResourceAccessLimits: context.resourceAccessRules + }) + + if (enableDomainDiscoverabilityForDomain) { + // Add domain & enable discoverability + await addDomain({ + workspaceId: workspace.id, userId: context.userId!, - workspaceInput: { - name, - slug, - description: description ?? null, - logo: logo ?? null - }, - userResourceAccessLimits: context.resourceAccessRules + domain: enableDomainDiscoverabilityForDomain }) - if (enableDomainDiscoverabilityForDomain) { - // Add domain & enable discoverability - await addDomain({ - workspaceId: workspace.id, - userId: context.userId!, - domain: enableDomainDiscoverabilityForDomain - }) - - workspace = await updateWorkspace({ - workspaceId: workspace.id, - workspaceInput: { - discoverabilityEnabled: true - } - }) - } - - return workspace + workspace = await updateWorkspace({ + workspaceId: workspace.id, + workspaceInput: { + discoverabilityEnabled: true + } + }) } - } - }) - return await withOperationLogging(async () => await createWorkspace(), { - logger, - operationName: 'createWorkspace', - operationDescription: 'Create workspace' - }) + return workspace + }, + { + logger, + name: 'createWorkspace', + description: 'Create workspace', + transaction: true + } + ) }, delete: async (_parent, args, context) => { const { workspaceId } = args