From a3252f88f3eee87b5b77253d6777f8911643fe79 Mon Sep 17 00:00:00 2001 From: Kristaps Fabians Geikins Date: Wed, 9 Oct 2024 12:54:10 +0300 Subject: [PATCH] chore(server): IoC 34 - updateStreamAndNotifyFactory --- .../comments/tests/comments.graph.spec.js | 15 ++-- .../modules/core/domain/streams/operations.ts | 20 ++++- .../modules/core/graph/resolvers/projects.ts | 14 ++- .../modules/core/graph/resolvers/streams.ts | 14 ++- .../modules/core/repositories/streams.ts | 87 +++++++++---------- .../server/modules/core/services/streams.js | 10 --- .../core/services/streams/management.ts | 85 ++++++++++-------- .../core/services/streams/onboarding.ts | 3 +- .../server/modules/core/tests/streams.spec.ts | 15 ++-- 9 files changed, 153 insertions(+), 110 deletions(-) diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js index cd175eb0a..3c0c38191 100644 --- a/packages/server/modules/comments/tests/comments.graph.spec.js +++ b/packages/server/modules/comments/tests/comments.graph.spec.js @@ -4,10 +4,7 @@ const crs = require('crypto-random-string') const { buildApolloServer } = require('@/app') const { beforeEachContext } = require('@/test/hooks') const { Roles } = require('@/modules/core/helpers/mainConstants') -const { - grantPermissionsStream, - updateStream -} = require('@/modules/core/services/streams') +const { grantPermissionsStream } = require('@/modules/core/services/streams') const { createUser } = require('@/modules/core/services/users') const { gql } = require('graphql-tag') const { createObject } = require('@/modules/core/services/objects') @@ -54,7 +51,8 @@ const { const { markCommitStreamUpdated, getStreamFactory, - createStreamFactory + createStreamFactory, + updateStreamFactory } = require('@/modules/core/repositories/streams') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') const { @@ -63,7 +61,8 @@ const { const { getObjectFactory } = require('@/modules/core/repositories/objects') const { legacyCreateStreamFactory, - createStreamReturnRecordFactory + createStreamReturnRecordFactory, + legacyUpdateStreamFactory } = require('@/modules/core/services/streams/management') const { inviteUsersToProjectFactory @@ -157,6 +156,10 @@ const createStream = legacyCreateStreamFactory({ }) }) +const updateStream = legacyUpdateStreamFactory({ + updateStream: updateStreamFactory({ db }) +}) + function buildCommentInputFromString(textString) { return convertBasicStringToDocument(textString) } diff --git a/packages/server/modules/core/domain/streams/operations.ts b/packages/server/modules/core/domain/streams/operations.ts index b699f81b4..b875c2c00 100644 --- a/packages/server/modules/core/domain/streams/operations.ts +++ b/packages/server/modules/core/domain/streams/operations.ts @@ -7,10 +7,12 @@ import { import { TokenResourceIdentifier } from '@/modules/core/domain/tokens/types' import { ProjectCreateInput, - StreamCreateInput + ProjectUpdateInput, + StreamCreateInput, + StreamUpdateInput } from '@/modules/core/graph/generated/graphql' import { ContextResourceAccessRules } from '@/modules/core/helpers/token' -import { MaybeNullOrUndefined, Optional, StreamRoles } from '@speckle/shared' +import { MaybeNullOrUndefined, Nullable, Optional, StreamRoles } from '@speckle/shared' import { Knex } from 'knex' export type GetStreams = ( @@ -56,6 +58,10 @@ export type StoreStream = ( export type DeleteStreamRecords = (streamId: string) => Promise +export type UpdateStreamRecord = ( + update: StreamUpdateInput | ProjectUpdateInput +) => Promise> + export type CreateStream = ( params: (StreamCreateInput | ProjectCreateInput) & { ownerId: string @@ -78,3 +84,13 @@ export type DeleteStream = ( skipAccessChecks?: boolean } ) => Promise + +export type UpdateStream = ( + update: StreamUpdateInput | ProjectUpdateInput, + updaterId: string, + updaterResourceAccessRules: ContextResourceAccessRules +) => Promise + +export type LegacyUpdateStream = ( + update: StreamUpdateInput +) => Promise> diff --git a/packages/server/modules/core/graph/resolvers/projects.ts b/packages/server/modules/core/graph/resolvers/projects.ts index 942878a2b..3e93bd0ab 100644 --- a/packages/server/modules/core/graph/resolvers/projects.ts +++ b/packages/server/modules/core/graph/resolvers/projects.ts @@ -2,7 +2,8 @@ import { db } from '@/db/knex' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { addStreamCreatedActivityFactory, - addStreamDeletedActivity + addStreamDeletedActivity, + addStreamUpdatedActivity } from '@/modules/activitystream/services/streamActivity' import { RateLimitError } from '@/modules/core/errors/ratelimit' import { StreamNotFoundError } from '@/modules/core/errors/stream' @@ -23,7 +24,8 @@ import { getStreamFactory, getStreamCollaboratorsFactory, createStreamFactory, - deleteStreamFactory + deleteStreamFactory, + updateStreamFactory } from '@/modules/core/repositories/streams' import { getUsers } from '@/modules/core/repositories/users' import { @@ -33,7 +35,7 @@ import { import { createStreamReturnRecordFactory, deleteStreamAndNotifyFactory, - updateStreamAndNotify, + updateStreamAndNotifyFactory, updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' import { createOnboardingStream } from '@/modules/core/services/streams/onboarding' @@ -93,6 +95,12 @@ const deleteStreamAndNotify = deleteStreamAndNotifyFactory({ addStreamDeletedActivity, deleteAllResourceInvites: deleteAllResourceInvitesFactory({ db }) }) +const updateStreamAndNotify = updateStreamAndNotifyFactory({ + authorizeResolver, + getStream, + updateStream: updateStreamFactory({ db }), + addStreamUpdatedActivity +}) export = { Query: { diff --git a/packages/server/modules/core/graph/resolvers/streams.ts b/packages/server/modules/core/graph/resolvers/streams.ts index c39c2fc16..1a6a1bef3 100644 --- a/packages/server/modules/core/graph/resolvers/streams.ts +++ b/packages/server/modules/core/graph/resolvers/streams.ts @@ -30,12 +30,13 @@ import { getUserStreams, getStreamFactory, createStreamFactory, - deleteStreamFactory + deleteStreamFactory, + updateStreamFactory } from '@/modules/core/repositories/streams' import { createStreamReturnRecordFactory, deleteStreamAndNotifyFactory, - updateStreamAndNotify, + updateStreamAndNotifyFactory, updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' import { adminOverrideEnabled } from '@/modules/shared/helpers/envHelper' @@ -67,7 +68,8 @@ import { getEventBus } from '@/modules/shared/services/eventBus' import { createBranchFactory } from '@/modules/core/repositories/branches' import { addStreamCreatedActivityFactory, - addStreamDeletedActivity + addStreamDeletedActivity, + addStreamUpdatedActivity } from '@/modules/activitystream/services/streamActivity' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' @@ -106,6 +108,12 @@ const deleteStreamAndNotify = deleteStreamAndNotifyFactory({ addStreamDeletedActivity, deleteAllResourceInvites: deleteAllResourceInvitesFactory({ db }) }) +const updateStreamAndNotify = updateStreamAndNotifyFactory({ + authorizeResolver, + getStream, + updateStream: updateStreamFactory({ db }), + addStreamUpdatedActivity +}) const getUserStreamsCore = async ( forOtherUser: boolean, diff --git a/packages/server/modules/core/repositories/streams.ts b/packages/server/modules/core/repositories/streams.ts index 48070ffea..0f7aaf96d 100644 --- a/packages/server/modules/core/repositories/streams.ts +++ b/packages/server/modules/core/repositories/streams.ts @@ -78,7 +78,8 @@ import { GetStream, GetStreamCollaborators, GetStreams, - DeleteStreamRecords + DeleteStreamRecords, + UpdateStreamRecord } from '@/modules/core/domain/streams/operations' export type { StreamWithOptionalRole, StreamWithCommitId } @@ -955,58 +956,56 @@ const isProjectUpdateInput = ( i: StreamUpdateInput | ProjectUpdateInput ): i is ProjectUpdateInput => has(i, 'visibility') -/** @deprecated Replace all calls with `updateProjectFacotry` */ -export async function updateStream( - update: StreamUpdateInput | ProjectUpdateInput, - db?: Knex -) { - const { id: streamId } = update +export const updateStreamFactory = + (deps: { db: Knex }): UpdateStreamRecord => + async (update: StreamUpdateInput | ProjectUpdateInput) => { + const { id: streamId } = update - if (!update.name) update.name = null // to prevent saving name '' - const validUpdate: Partial & Record = omitBy( - update, - (v) => isNull(v) || isUndefined(v) - ) + if (!update.name) update.name = null // to prevent saving name '' + const validUpdate: Partial & Record = omitBy( + update, + (v) => isNull(v) || isUndefined(v) + ) - if (isProjectUpdateInput(update)) { - if (has(validUpdate, 'visibility')) { - validUpdate.isPublic = update.visibility !== ProjectVisibility.Private - validUpdate.isDiscoverable = update.visibility === ProjectVisibility.Public - delete validUpdate['visibility'] // cause it's not a real column + if (isProjectUpdateInput(update)) { + if (has(validUpdate, 'visibility')) { + validUpdate.isPublic = update.visibility !== ProjectVisibility.Private + validUpdate.isDiscoverable = update.visibility === ProjectVisibility.Public + delete validUpdate['visibility'] // cause it's not a real column + } + } else { + if (has(validUpdate, 'isPublic') && !validUpdate.isPublic) { + validUpdate.isDiscoverable = false + } } - } else { + if (has(validUpdate, 'isPublic') && !validUpdate.isPublic) { - validUpdate.isDiscoverable = false + validUpdate.allowPublicComments = false + } else if ( + has(validUpdate, 'allowPublicComments') && + validUpdate.allowPublicComments + ) { + validUpdate.isPublic = true } + + if (!Object.keys(validUpdate).length) return null + + const [updatedStream] = await tables + .streams(deps.db) + .returning('*') + .where({ id: streamId }) + .update({ + ...validUpdate, + updatedAt: knex.fn.now() + }) + + return updatedStream } - if (has(validUpdate, 'isPublic') && !validUpdate.isPublic) { - validUpdate.allowPublicComments = false - } else if ( - has(validUpdate, 'allowPublicComments') && - validUpdate.allowPublicComments - ) { - validUpdate.isPublic = true - } - - if (!Object.keys(validUpdate).length) return null - - const [updatedStream] = await tables - .streams(db ?? knex) - .returning('*') - .where({ id: streamId }) - .update({ - ...validUpdate, - updatedAt: knex.fn.now() - }) - - return updatedStream -} - export const updateProjectFactory = ({ db }: { db: Knex }): UpdateProject => async ({ projectUpdate }) => { - const updatedStream = await updateStream(projectUpdate, db) + const updatedStream = await updateStreamFactory({ db })(projectUpdate) if (!updatedStream) { throw new StreamUpdateError() @@ -1209,7 +1208,7 @@ export async function markOnboardingBaseStream(streamId: string, version: string if (!stream) { throw new Error(`Stream ${streamId} not found`) } - await updateStream({ + await updateStreamFactory({ db })({ id: streamId, name: 'Onboarding Stream Local Source - Do Not Delete' }) diff --git a/packages/server/modules/core/services/streams.js b/packages/server/modules/core/services/streams.js index 6617c83be..aba3798d1 100644 --- a/packages/server/modules/core/services/streams.js +++ b/packages/server/modules/core/services/streams.js @@ -6,7 +6,6 @@ const { getFavoritedStreamsCount, setStreamFavorited, canUserFavoriteStream, - updateStream: updateStreamInDb, revokeStreamPermissions, grantStreamPermissions, getStreamFactory @@ -25,15 +24,6 @@ const { */ module.exports = { - /** - * @deprecated Use updateStreamAndNotify or use the repository function directly - * @param {import('@/modules/core/graph/generated/graphql').StreamUpdateInput} update - */ - async updateStream(update) { - const updatedStream = await updateStreamInDb(update) - return updatedStream?.id || null - }, - setStreamFavorited, /** diff --git a/packages/server/modules/core/services/streams/management.ts b/packages/server/modules/core/services/streams/management.ts index ea791bf42..2f1c723ee 100644 --- a/packages/server/modules/core/services/streams/management.ts +++ b/packages/server/modules/core/services/streams/management.ts @@ -14,7 +14,6 @@ import { StreamUpdatePermissionInput } from '@/modules/core/graph/generated/graphql' import { StreamRecord } from '@/modules/core/helpers/types' -import { getStreamFactory, updateStream } from '@/modules/core/repositories/streams' import { StreamInvalidAccessError, StreamUpdateError @@ -30,8 +29,6 @@ import { ContextResourceAccessRules, isNewResourceAllowed } from '@/modules/core/helpers/token' -import { authorizeResolver } from '@/modules/shared' -import db from '@/db/knex' import { TokenResourceIdentifier, TokenResourceIdentifierType @@ -46,8 +43,12 @@ import { CreateStream, DeleteStream, DeleteStreamRecords, + GetStream, LegacyCreateStream, - StoreStream + LegacyUpdateStream, + StoreStream, + UpdateStream, + UpdateStreamRecord } from '@/modules/core/domain/streams/operations' import { StoreBranch } from '@/modules/core/domain/branches/operations' import { AuthorizeResolver } from '@/modules/shared/domain/operations' @@ -183,42 +184,58 @@ export const deleteStreamAndNotifyFactory = /** * Update stream metadata & notify users (emit events & save activity) */ -export async function updateStreamAndNotify( - update: StreamUpdateInput | ProjectUpdateInput, - updaterId: string, - updaterResourceAccessRules: ContextResourceAccessRules -) { - await authorizeResolver( - updaterId, - update.id, - Roles.Stream.Owner, - updaterResourceAccessRules - ) +export const updateStreamAndNotifyFactory = + (deps: { + authorizeResolver: AuthorizeResolver + getStream: GetStream + updateStream: UpdateStreamRecord + addStreamUpdatedActivity: typeof addStreamUpdatedActivity + }): UpdateStream => + async ( + update: StreamUpdateInput | ProjectUpdateInput, + updaterId: string, + updaterResourceAccessRules: ContextResourceAccessRules + ) => { + await deps.authorizeResolver( + updaterId, + update.id, + Roles.Stream.Owner, + updaterResourceAccessRules + ) - const getStream = getStreamFactory({ db }) - const oldStream = await getStream({ streamId: update.id, userId: updaterId }) - if (!oldStream) { - throw new StreamUpdateError('Stream not found', { - info: { updaterId, streamId: update.id } + const oldStream = await deps.getStream({ streamId: update.id, userId: updaterId }) + if (!oldStream) { + throw new StreamUpdateError('Stream not found', { + info: { updaterId, streamId: update.id } + }) + } + + const newStream = await deps.updateStream(update) + if (!newStream) { + return oldStream + } + + await deps.addStreamUpdatedActivity({ + streamId: update.id, + updaterId, + oldStream, + newStream, + update }) + + return newStream } - const newStream = await updateStream(update) - if (!newStream) { - return oldStream +/** + * @deprecated Use updateStreamAndNotifyFactory() or the repo fn directly + */ +export const legacyUpdateStreamFactory = + (deps: { updateStream: UpdateStreamRecord }): LegacyUpdateStream => + async (update) => { + const updatedStream = await deps.updateStream(update) + return updatedStream?.id || null } - await addStreamUpdatedActivity({ - streamId: update.id, - updaterId, - oldStream, - newStream, - update - }) - - return newStream -} - type PermissionUpdateInput = | StreamUpdatePermissionInput | StreamRevokePermissionInput diff --git a/packages/server/modules/core/services/streams/onboarding.ts b/packages/server/modules/core/services/streams/onboarding.ts index cc36386fb..af385539e 100644 --- a/packages/server/modules/core/services/streams/onboarding.ts +++ b/packages/server/modules/core/services/streams/onboarding.ts @@ -11,7 +11,7 @@ import { createStreamFactory, getOnboardingBaseStream, getStreamFactory, - updateStream + updateStreamFactory } from '@/modules/core/repositories/streams' import { getUser } from '@/modules/core/services/users' import { @@ -51,6 +51,7 @@ export async function createOnboardingStream( ) } + const updateStream = updateStreamFactory({ db }) const addStreamCreatedActivity = addStreamCreatedActivityFactory({ saveActivity: saveActivityFactory({ db }), publish diff --git a/packages/server/modules/core/tests/streams.spec.ts b/packages/server/modules/core/tests/streams.spec.ts index 5a99ff6f1..efe3e0394 100644 --- a/packages/server/modules/core/tests/streams.spec.ts +++ b/packages/server/modules/core/tests/streams.spec.ts @@ -1,9 +1,5 @@ import { expect } from 'chai' -import { - updateStream, - getStreamUsers, - grantPermissionsStream -} from '@/modules/core/services/streams' +import { getStreamUsers, grantPermissionsStream } from '@/modules/core/services/streams' import { createObject } from '@/modules/core/services/objects' @@ -31,7 +27,8 @@ import { getStreamFactory, markBranchStreamUpdated, markCommitStreamUpdated, - revokeStreamPermissions + revokeStreamPermissions, + updateStreamFactory } from '@/modules/core/repositories/streams' import { has, times } from 'lodash' import { Streams } from '@/modules/core/dbSchema' @@ -75,7 +72,8 @@ import { addCommitCreatedActivity } from '@/modules/activitystream/services/comm import { getObjectFactory } from '@/modules/core/repositories/objects' import { createStreamReturnRecordFactory, - legacyCreateStreamFactory + legacyCreateStreamFactory, + legacyUpdateStreamFactory } from '@/modules/core/services/streams/management' import { inviteUsersToProjectFactory } from '@/modules/serverinvites/services/projectInviteManagement' import { createAndSendInviteFactory } from '@/modules/serverinvites/services/creation' @@ -154,6 +152,9 @@ const createStream = legacyCreateStreamFactory({ }) }) const deleteStream = deleteStreamFactory({ db }) +const updateStream = legacyUpdateStreamFactory({ + updateStream: updateStreamFactory({ db }) +}) describe('Streams @core-streams', () => { const userOne: BasicTestUser = {