diff --git a/packages/server/modules/activitystream/tests/activitySummary.spec.ts b/packages/server/modules/activitystream/tests/activitySummary.spec.ts index 95f820d29..559165f10 100644 --- a/packages/server/modules/activitystream/tests/activitySummary.spec.ts +++ b/packages/server/modules/activitystream/tests/activitySummary.spec.ts @@ -6,7 +6,6 @@ import { sendActivityNotificationsFactory } from '@/modules/activitystream/services/summary' import { expect } from 'chai' -import { deleteStream } from '@/modules/core/services/streams' import { ActionTypes, ResourceTypes } from '@/modules/activitystream/helpers/types' import { ActivityDigestMessage, @@ -21,6 +20,7 @@ import { import { db } from '@/db/knex' import { createStreamFactory, + deleteStreamFactory, getStreamFactory } from '@/modules/core/repositories/streams' import { @@ -82,6 +82,7 @@ const createStream = legacyCreateStreamFactory({ projectsEventsEmitter: ProjectsEmitter.emit }) }) +const deleteStream = deleteStreamFactory({ db }) describe('Activity summary @activity', () => { const userA: BasicTestUser = { @@ -163,7 +164,7 @@ describe('Activity summary @activity', () => { info: {}, message: 'foo' }) - await deleteStream({ streamId }) + await deleteStream(streamId) const summary = await createActivitySummary({ userId: userA.id, streamIds: [streamId], diff --git a/packages/server/modules/core/domain/streams/operations.ts b/packages/server/modules/core/domain/streams/operations.ts index 2238a857c..b699f81b4 100644 --- a/packages/server/modules/core/domain/streams/operations.ts +++ b/packages/server/modules/core/domain/streams/operations.ts @@ -9,6 +9,7 @@ import { ProjectCreateInput, StreamCreateInput } from '@/modules/core/graph/generated/graphql' +import { ContextResourceAccessRules } from '@/modules/core/helpers/token' import { MaybeNullOrUndefined, Optional, StreamRoles } from '@speckle/shared' import { Knex } from 'knex' @@ -53,6 +54,8 @@ export type StoreStream = ( }> ) => Promise +export type DeleteStreamRecords = (streamId: string) => Promise + export type CreateStream = ( params: (StreamCreateInput | ProjectCreateInput) & { ownerId: string @@ -66,3 +69,12 @@ export type CreateStream = ( export type LegacyCreateStream = ( params: StreamCreateInput & { ownerId: string } ) => Promise + +export type DeleteStream = ( + streamId: string, + deleterId: string, + deleterResourceAccessRules: ContextResourceAccessRules, + options?: { + skipAccessChecks?: boolean + } +) => Promise diff --git a/packages/server/modules/core/graph/resolvers/projects.ts b/packages/server/modules/core/graph/resolvers/projects.ts index e026dd961..942878a2b 100644 --- a/packages/server/modules/core/graph/resolvers/projects.ts +++ b/packages/server/modules/core/graph/resolvers/projects.ts @@ -1,6 +1,9 @@ import { db } from '@/db/knex' import { saveActivityFactory } from '@/modules/activitystream/repositories' -import { addStreamCreatedActivityFactory } from '@/modules/activitystream/services/streamActivity' +import { + addStreamCreatedActivityFactory, + addStreamDeletedActivity +} from '@/modules/activitystream/services/streamActivity' import { RateLimitError } from '@/modules/core/errors/ratelimit' import { StreamNotFoundError } from '@/modules/core/errors/stream' import { WorkspacesModuleDisabledError } from '@/modules/core/errors/workspaces' @@ -19,7 +22,8 @@ import { getUserStreams, getStreamFactory, getStreamCollaboratorsFactory, - createStreamFactory + createStreamFactory, + deleteStreamFactory } from '@/modules/core/repositories/streams' import { getUsers } from '@/modules/core/repositories/users' import { @@ -28,13 +32,14 @@ import { } from '@/modules/core/services/ratelimiter' import { createStreamReturnRecordFactory, - deleteStreamAndNotify, + deleteStreamAndNotifyFactory, updateStreamAndNotify, updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' import { createOnboardingStream } from '@/modules/core/services/streams/onboarding' import { removeStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { + deleteAllResourceInvitesFactory, findUserByTargetFactory, insertInviteAndDeleteOldFactory } from '@/modules/serverinvites/repositories/serverInvites' @@ -82,6 +87,12 @@ const createStreamReturnRecord = createStreamReturnRecordFactory({ }), projectsEventsEmitter: ProjectsEmitter.emit }) +const deleteStreamAndNotify = deleteStreamAndNotifyFactory({ + deleteStream: deleteStreamFactory({ db }), + authorizeResolver, + addStreamDeletedActivity, + deleteAllResourceInvites: deleteAllResourceInvitesFactory({ db }) +}) export = { Query: { diff --git a/packages/server/modules/core/graph/resolvers/streams.ts b/packages/server/modules/core/graph/resolvers/streams.ts index 1c14b2047..c39c2fc16 100644 --- a/packages/server/modules/core/graph/resolvers/streams.ts +++ b/packages/server/modules/core/graph/resolvers/streams.ts @@ -29,11 +29,12 @@ import { getUserStreamsCount, getUserStreams, getStreamFactory, - createStreamFactory + createStreamFactory, + deleteStreamFactory } from '@/modules/core/repositories/streams' import { createStreamReturnRecordFactory, - deleteStreamAndNotify, + deleteStreamAndNotifyFactory, updateStreamAndNotify, updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' @@ -50,6 +51,7 @@ import { UserStreamsArgs } from '@/modules/core/graph/generated/graphql' import { + deleteAllResourceInvitesFactory, findUserByTargetFactory, insertInviteAndDeleteOldFactory, queryAllResourceInvitesFactory @@ -63,7 +65,10 @@ import { collectAndValidateCoreTargetsFactory } from '@/modules/serverinvites/se import { buildCoreInviteEmailContentsFactory } from '@/modules/serverinvites/services/coreEmailContents' import { getEventBus } from '@/modules/shared/services/eventBus' import { createBranchFactory } from '@/modules/core/repositories/branches' -import { addStreamCreatedActivityFactory } from '@/modules/activitystream/services/streamActivity' +import { + addStreamCreatedActivityFactory, + addStreamDeletedActivity +} from '@/modules/activitystream/services/streamActivity' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' @@ -95,6 +100,12 @@ const createStreamReturnRecord = createStreamReturnRecordFactory({ }), projectsEventsEmitter: ProjectsEmitter.emit }) +const deleteStreamAndNotify = deleteStreamAndNotifyFactory({ + deleteStream: deleteStreamFactory({ db }), + authorizeResolver, + addStreamDeletedActivity, + deleteAllResourceInvites: deleteAllResourceInvitesFactory({ db }) +}) const getUserStreamsCore = async ( forOtherUser: boolean, diff --git a/packages/server/modules/core/repositories/streams.ts b/packages/server/modules/core/repositories/streams.ts index 864415b5c..48070ffea 100644 --- a/packages/server/modules/core/repositories/streams.ts +++ b/packages/server/modules/core/repositories/streams.ts @@ -77,7 +77,8 @@ import { GetCommitStreams, GetStream, GetStreamCollaborators, - GetStreams + GetStreams, + DeleteStreamRecords } from '@/modules/core/domain/streams/operations' export type { StreamWithOptionalRole, StreamWithCommitId } @@ -905,20 +906,22 @@ export async function getUserStreamCounts(params: { return _.mapValues(_.keyBy(results, 'userId'), (r) => parseInt(r.count)) } -export async function deleteStream(streamId: string) { - // Delete stream commits (not automatically cascaded) - await knex.raw( - ` +export const deleteStreamFactory = + (deps: { db: Knex }): DeleteStreamRecords => + async (streamId: string) => { + // Delete stream commits (not automatically cascaded) + await deps.db.raw( + ` DELETE FROM commits WHERE id IN ( SELECT sc."commitId" FROM streams s INNER JOIN stream_commits sc ON s.id = sc."streamId" WHERE s.id = ? ) `, - [streamId] - ) - return await Streams.knex().where(Streams.col.id, streamId).del() -} + [streamId] + ) + return await tables.streams(deps.db).where(Streams.col.id, streamId).del() + } export async function getStreamsSourceApps(streamIds: string[]) { if (!streamIds?.length) return {} diff --git a/packages/server/modules/core/services/streams.js b/packages/server/modules/core/services/streams.js index bd0b145ef..6617c83be 100644 --- a/packages/server/modules/core/services/streams.js +++ b/packages/server/modules/core/services/streams.js @@ -6,14 +6,12 @@ const { getFavoritedStreamsCount, setStreamFavorited, canUserFavoriteStream, - deleteStream: deleteStreamFromDb, updateStream: updateStreamInDb, revokeStreamPermissions, grantStreamPermissions, getStreamFactory } = require('@/modules/core/repositories/streams') const { UnauthorizedError, InvalidArgumentError } = require('@/modules/shared/errors') -const { dbLogger } = require('@/logging/logging') const { isResourceAllowed } = require('@/modules/core/helpers/token') const { TokenResourceIdentifierType @@ -52,14 +50,6 @@ module.exports = { return await revokeStreamPermissions({ streamId, userId }) }, - /** - * @deprecated Use deleteStreamAndNotify or use the repository function directly - */ - async deleteStream({ streamId }) { - dbLogger.info('Deleting stream %s', streamId) - return await deleteStreamFromDb(streamId) - }, - /** * @param {Object} p * @param {string | Date | null} [p.cursor] diff --git a/packages/server/modules/core/services/streams/management.ts b/packages/server/modules/core/services/streams/management.ts index 7448ff6b7..ea791bf42 100644 --- a/packages/server/modules/core/services/streams/management.ts +++ b/packages/server/modules/core/services/streams/management.ts @@ -14,11 +14,7 @@ import { StreamUpdatePermissionInput } from '@/modules/core/graph/generated/graphql' import { StreamRecord } from '@/modules/core/helpers/types' -import { - deleteStream, - getStreamFactory, - updateStream -} from '@/modules/core/repositories/streams' +import { getStreamFactory, updateStream } from '@/modules/core/repositories/streams' import { StreamInvalidAccessError, StreamUpdateError @@ -35,7 +31,6 @@ import { isNewResourceAllowed } from '@/modules/core/helpers/token' import { authorizeResolver } from '@/modules/shared' -import { deleteAllResourceInvitesFactory } from '@/modules/serverinvites/repositories/serverInvites' import db from '@/db/knex' import { TokenResourceIdentifier, @@ -49,10 +44,14 @@ import { inviteUsersToProjectFactory } from '@/modules/serverinvites/services/pr import { ProjectInviteResourceType } from '@/modules/serverinvites/domain/constants' import { CreateStream, + DeleteStream, + DeleteStreamRecords, LegacyCreateStream, StoreStream } from '@/modules/core/domain/streams/operations' import { StoreBranch } from '@/modules/core/domain/branches/operations' +import { AuthorizeResolver } from '@/modules/shared/domain/operations' +import { DeleteAllResourceInvites } from '@/modules/serverinvites/domain/operations' export const createStreamReturnRecordFactory = (deps: { @@ -136,46 +135,50 @@ export const legacyCreateStreamFactory = /** * Delete stream & notify users (emit events & save activity) - * @param {string} streamId - * @param {string} deleterId */ -export async function deleteStreamAndNotify( - streamId: string, - deleterId: string, - deleterResourceAccessRules: ContextResourceAccessRules, - options?: { - skipAccessChecks?: boolean +export const deleteStreamAndNotifyFactory = + (deps: { + deleteStream: DeleteStreamRecords + authorizeResolver: AuthorizeResolver + addStreamDeletedActivity: typeof addStreamDeletedActivity + deleteAllResourceInvites: DeleteAllResourceInvites + }): DeleteStream => + async ( + streamId: string, + deleterId: string, + deleterResourceAccessRules: ContextResourceAccessRules, + options?: { + skipAccessChecks?: boolean + } + ) => { + const { skipAccessChecks = false } = options || {} + + if (!skipAccessChecks) { + await deps.authorizeResolver( + deleterId, + streamId, + Roles.Stream.Owner, + deleterResourceAccessRules + ) + } + + await deps.addStreamDeletedActivity({ streamId, deleterId }) + + // TODO: this has been around since before my time, we should get rid of it... + // delay deletion by a bit so we can do auth checks + await wait(250) + + // Delete after event so we can do authz + const deleteAllStreamInvites = deps.deleteAllResourceInvites + await Promise.all([ + deleteAllStreamInvites({ + resourceId: streamId, + resourceType: ProjectInviteResourceType + }), + deps.deleteStream(streamId) + ]) + return true } -) { - const { skipAccessChecks = false } = options || {} - - if (!skipAccessChecks) { - await authorizeResolver( - deleterId, - streamId, - Roles.Stream.Owner, - deleterResourceAccessRules - ) - } - - await addStreamDeletedActivity({ streamId, deleterId }) - - // TODO: this has been around since before my time, we should get rid of it... - // delay deletion by a bit so we can do auth checks - await wait(250) - - // TODO: use proper injection once we refactor this module - // Delete after event so we can do authz - const deleteAllStreamInvites = deleteAllResourceInvitesFactory({ db }) - await Promise.all([ - deleteAllStreamInvites({ - resourceId: streamId, - resourceType: ProjectInviteResourceType - }), - deleteStream(streamId) - ]) - return true -} /** * Update stream metadata & notify users (emit events & save activity) diff --git a/packages/server/modules/core/services/users.js b/packages/server/modules/core/services/users.js index 8d269d0ca..a6f59e988 100644 --- a/packages/server/modules/core/services/users.js +++ b/packages/server/modules/core/services/users.js @@ -16,7 +16,6 @@ const { const Users = () => UsersSchema.knex() const Acl = () => ServerAclSchema.knex() -const { deleteStream } = require('./streams') const { LIMITED_USER_FIELDS } = require('@/modules/core/helpers/userHelper') const { getUserByEmail, @@ -58,6 +57,7 @@ const { } = require('@/modules/emails/repositories') const { renderEmail } = require('@/modules/emails/services/emailRendering') const { sendEmail } = require('@/modules/emails/services/sending') +const { deleteStreamFactory } = require('@/modules/core/repositories/streams') const _changeUserRole = async ({ userId, role }) => await Acl().where({ userId }).update({ role }) @@ -325,6 +325,7 @@ module.exports = { * @param {{ deleteAllUserInvites: import('@/modules/serverinvites/domain/operations').DeleteAllUserInvites }} param0 */ deleteUser({ deleteAllUserInvites }) { + const deleteStream = deleteStreamFactory({ db }) return async (id) => { //TODO: check for the last admin user to survive dbLogger.info('Deleting user ' + id) @@ -351,7 +352,7 @@ module.exports = { [id] ) for (const i in streams.rows) { - await deleteStream({ streamId: streams.rows[i].id }) + await deleteStream(streams.rows[i].id) } // Delete all invites (they don't have a FK, so we need to do this manually) diff --git a/packages/server/modules/core/tests/streams.spec.ts b/packages/server/modules/core/tests/streams.spec.ts index 34f516f2f..5a99ff6f1 100644 --- a/packages/server/modules/core/tests/streams.spec.ts +++ b/packages/server/modules/core/tests/streams.spec.ts @@ -1,7 +1,6 @@ import { expect } from 'chai' import { updateStream, - deleteStream, getStreamUsers, grantPermissionsStream } from '@/modules/core/services/streams' @@ -28,6 +27,7 @@ import { import { StreamWithOptionalRole, createStreamFactory, + deleteStreamFactory, getStreamFactory, markBranchStreamUpdated, markCommitStreamUpdated, @@ -153,6 +153,7 @@ const createStream = legacyCreateStreamFactory({ projectsEventsEmitter: ProjectsEmitter.emit }) }) +const deleteStream = deleteStreamFactory({ db }) describe('Streams @core-streams', () => { const userOne: BasicTestUser = { @@ -253,7 +254,7 @@ describe('Streams @core-streams', () => { ownerId: userOne.id }) - await deleteStream({ streamId: id }) + await deleteStream(id) const stream = await getStream({ streamId: id }) expect(stream).to.not.be.ok diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 09a830b0e..43f88176d 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -9,7 +9,8 @@ import { updateProjectFactory, upsertProjectRoleFactory, getRolesByUserIdFactory, - getStreamFactory + getStreamFactory, + deleteStreamFactory } from '@/modules/core/repositories/streams' import { getUser, getUsers } from '@/modules/core/repositories/users' import { getStreams } from '@/modules/core/services/streams' @@ -109,7 +110,6 @@ import { } from '@/modules/workspaces/services/retrieval' import { Roles, WorkspaceRoles, removeNullOrUndefinedKeys } from '@speckle/shared' import { chunk } from 'lodash' -import { deleteStream } from '@/modules/core/repositories/streams' import { findEmailsByUserIdFactory, findVerifiedEmailsByUserIdFactory, @@ -184,6 +184,7 @@ const buildCreateAndSendWorkspaceInvite = () => payload }) }) +const deleteStream = deleteStreamFactory({ db }) const { FF_WORKSPACES_MODULE_ENABLED } = getFeatureFlags() diff --git a/packages/server/modules/workspaces/services/management.ts b/packages/server/modules/workspaces/services/management.ts index d45206404..e4273e9c4 100644 --- a/packages/server/modules/workspaces/services/management.ts +++ b/packages/server/modules/workspaces/services/management.ts @@ -26,7 +26,6 @@ import { validateWorkspaceSlug } from '@speckle/shared' import cryptoRandomString from 'crypto-random-string' -import { deleteStream } from '@/modules/core/repositories/streams' import { DeleteWorkspaceRole, GetWorkspaceRoleForUser, @@ -64,6 +63,7 @@ import { chunk, isEmpty, omit } from 'lodash' import { userEmailsCompliantWithWorkspaceDomains } from '@/modules/workspaces/domain/logic' import { workspaceRoles as workspaceRoleDefinitions } from '@/modules/workspaces/roles' import { blockedDomains } from '@speckle/shared' +import { DeleteStreamRecords } from '@/modules/core/domain/streams/operations' type WorkspaceCreateArgs = { userId: string @@ -278,7 +278,7 @@ export const deleteWorkspaceFactory = deleteAllResourceInvites }: { deleteWorkspace: DeleteWorkspace - deleteProject: typeof deleteStream + deleteProject: DeleteStreamRecords queryAllWorkspaceProjects: QueryAllWorkspaceProjects deleteAllResourceInvites: DeleteAllResourceInvites }) =>