diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index 3fd209b9d..f47e8dfad 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -5,13 +5,15 @@ import { Commit, CommitBranch, CommitWithStreamId, - LegacyUserCommit + LegacyUserCommit, + LegacyStreamCommit } from '@/modules/core/domain/commits/types' import { CommitsMoveInput, CommitUpdateInput, ModelVersionsFilter, MoveVersionsInput, + StreamCommitsArgs, UpdateVersionInput } from '@/modules/core/graph/generated/graphql' import { BranchCommitRecord, StreamCommitRecord } from '@/modules/core/helpers/types' @@ -254,3 +256,22 @@ export type LegacyGetPaginatedUserCommitsTotalCount = ({ publicOnly?: MaybeNullOrUndefined streamIdWhitelist?: MaybeNullOrUndefined }) => Promise + +export type LegacyGetPaginatedStreamCommitsPage = (params: { + streamId: string + limit?: MaybeNullOrUndefined + cursor?: MaybeNullOrUndefined + ignoreGlobalsBranch?: MaybeNullOrUndefined +}) => Promise<{ + commits: LegacyStreamCommit[] + cursor: Nullable +}> + +export type LegacyGetPaginatedStreamCommits = ( + streamId: string, + params: StreamCommitsArgs +) => Promise<{ + items: LegacyStreamCommit[] + cursor: Nullable + totalCount: number +}> diff --git a/packages/server/modules/core/graph/resolvers/commitsNew.ts b/packages/server/modules/core/graph/resolvers/commitsNew.ts index ddfc1a1b5..3174fece7 100644 --- a/packages/server/modules/core/graph/resolvers/commitsNew.ts +++ b/packages/server/modules/core/graph/resolvers/commitsNew.ts @@ -2,10 +2,9 @@ import { CommitNotFoundError } from '@/modules/core/errors/commit' import { publish } from '@/modules/shared/utils/subscriptions' import { authorizeResolver } from '@/modules/shared' -import { getCommitsByStreamId } from '@/modules/core/services/commits' import { - getPaginatedStreamCommits, - getPaginatedBranchCommitsFactory + getPaginatedBranchCommitsFactory, + legacyGetPaginatedStreamCommits } from '@/modules/core/services/commit/retrieval' import { markCommitReceivedAndNotify, @@ -43,7 +42,9 @@ import { getCommitsFactory, moveCommitsToBranchFactory, legacyGetPaginatedUserCommitsPage, - legacyGetPaginatedUserCommitsTotalCount + legacyGetPaginatedUserCommitsTotalCount, + legacyGetPaginatedStreamCommitsPageFactory, + getStreamCommitCountFactory } from '@/modules/core/repositories/commits' import { db } from '@/db/knex' import { @@ -144,6 +145,11 @@ const batchMoveCommits = batchMoveCommitsFactory({ const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) const getCommitsByUserId = legacyGetPaginatedUserCommitsPage({ db }) const getCommitsTotalCountByUserId = legacyGetPaginatedUserCommitsTotalCount({ db }) +const getCommitsByStreamId = legacyGetPaginatedStreamCommitsPageFactory({ db }) +const getPaginatedStreamCommits = legacyGetPaginatedStreamCommits({ + legacyGetPaginatedStreamCommitsPage: getCommitsByStreamId, + getStreamCommitCount: getStreamCommitCountFactory({ db }) +}) const getAuthorId = (commit: CommitGraphQLReturn) => { if ('author' in commit) return commit.author @@ -247,9 +253,7 @@ export = { if (!args.id) { const { commits } = await getCommitsByStreamId({ streamId: parent.id, - limit: 1, - cursor: undefined, - ignoreGlobalsBranch: undefined + limit: 1 }) if (commits.length !== 0) return commits[0] throw new CommitNotFoundError( diff --git a/packages/server/modules/core/graph/resolvers/models.ts b/packages/server/modules/core/graph/resolvers/models.ts index fe7bea5ee..b52556a8a 100644 --- a/packages/server/modules/core/graph/resolvers/models.ts +++ b/packages/server/modules/core/graph/resolvers/models.ts @@ -16,7 +16,7 @@ import { last } from 'lodash' import { getViewerResourceGroupsFactory } from '@/modules/core/services/commit/viewerResources' import { getPaginatedBranchCommitsFactory, - getPaginatedStreamCommits + legacyGetPaginatedStreamCommits } from '@/modules/core/services/commit/retrieval' import { filteredSubscribe, @@ -44,7 +44,9 @@ import { getAllBranchCommitsFactory, getBranchCommitsTotalCountFactory, getPaginatedBranchCommitsItemsFactory, - getSpecificBranchCommitsFactory + getSpecificBranchCommitsFactory, + getStreamCommitCountFactory, + legacyGetPaginatedStreamCommitsPageFactory } from '@/modules/core/repositories/commits' import { db } from '@/db/knex' import { @@ -108,6 +110,12 @@ const getPaginatedBranchCommits = getPaginatedBranchCommitsFactory({ getPaginatedBranchCommitsItems: getPaginatedBranchCommitsItemsFactory({ db }), getBranchCommitsTotalCount: getBranchCommitsTotalCountFactory({ db }) }) +const getPaginatedStreamCommits = legacyGetPaginatedStreamCommits({ + legacyGetPaginatedStreamCommitsPage: legacyGetPaginatedStreamCommitsPageFactory({ + db + }), + getStreamCommitCount: getStreamCommitCountFactory({ db }) +}) export = { User: { diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index fc1b8d75f..771a9d925 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -24,6 +24,7 @@ import { Knex } from 'knex' import { MaybeNullOrUndefined, Optional } from '@speckle/shared' import { CommitWithStreamBranchMetadata, + LegacyStreamCommit, LegacyUserCommit } from '@/modules/core/domain/commits/types' import { @@ -55,7 +56,8 @@ import { GetBranchCommitsTotalCount, MoveCommitsToBranch, LegacyGetPaginatedUserCommitsPage, - LegacyGetPaginatedUserCommitsTotalCount + LegacyGetPaginatedUserCommitsTotalCount, + LegacyGetPaginatedStreamCommitsPage } from '@/modules/core/domain/commits/operations' const tables = { @@ -538,6 +540,10 @@ export const getUserAuthoredCommitCountsFactory = return mapValues(keyBy(res, 'author'), (r) => parseInt(r.count)) } +/** + * @deprecated Deprecated because of the weird/messy commit structure. It should return CommitRecords + * without any joins, and let those be handled by GQL dataloaders + */ const getCommitsByUserIdBaseFactory = (deps: { db: Knex }) => ({ @@ -582,6 +588,10 @@ const getCommitsByUserIdBaseFactory = return query } +/** + * @deprecated Deprecated because of the weird/messy commit structure. It should return CommitRecords + * without any joins, and let those be handled by GQL dataloaders + */ export const legacyGetPaginatedUserCommitsPage = (deps: { db: Knex }): LegacyGetPaginatedUserCommitsPage => async ({ userId, limit, cursor, publicOnly, streamIdWhitelist }) => { @@ -605,6 +615,10 @@ export const legacyGetPaginatedUserCommitsPage = } } +/** + * @deprecated Deprecated because of the weird/messy commit structure. It should return CommitRecords + * without any joins, and let those be handled by GQL dataloaders + */ export const legacyGetPaginatedUserCommitsTotalCount = (deps: { db: Knex }): LegacyGetPaginatedUserCommitsTotalCount => async ({ userId, publicOnly, streamIdWhitelist }) => { @@ -619,3 +633,49 @@ export const legacyGetPaginatedUserCommitsTotalCount = const [res] = (await query) as Array<{ count: string }> return parseInt(res.count) } + +/** + * @deprecated Deprecated because of the weird/messy commit structure. It should return CommitRecords + * without any joins, and let those be handled by GQL dataloaders + */ +export const legacyGetPaginatedStreamCommitsPageFactory = + (deps: { db: Knex }): LegacyGetPaginatedStreamCommitsPage => + async ({ streamId, limit, cursor, ignoreGlobalsBranch }) => { + limit = clamp(limit || 25, 0, 100) + if (!limit) return { commits: [], cursor: null } + + const query = tables + .streamCommits(deps.db) + .columns([ + { id: 'commits.id' }, + 'message', + 'referencedObject', + 'sourceApplication', + 'totalChildrenCount', + 'parents', + 'commits.createdAt', + { branchName: 'branches.name' }, + { authorName: 'users.name' }, + { authorId: 'users.id' }, + { authorAvatar: 'users.avatar' }, + knex.raw(`?? as "author"`, ['users.id']) + ]) + .select() + .join('commits', 'commits.id', 'stream_commits.commitId') + .join('branch_commits', 'commits.id', 'branch_commits.commitId') + .join('branches', 'branches.id', 'branch_commits.branchId') + .leftJoin('users', 'commits.author', 'users.id') + .where('stream_commits.streamId', streamId) + + if (ignoreGlobalsBranch) query.andWhere('branches.name', '!=', 'globals') + + if (cursor) query.andWhere('commits.createdAt', '<', cursor) + + query.orderBy('commits.createdAt', 'desc').limit(limit) + + const rows = (await query) as LegacyStreamCommit[] + return { + commits: rows, + cursor: rows.length > 0 ? rows[rows.length - 1].createdAt.toISOString() : null + } + } diff --git a/packages/server/modules/core/services/commit/retrieval.ts b/packages/server/modules/core/services/commit/retrieval.ts index 2f364bcda..3da9623e2 100644 --- a/packages/server/modules/core/services/commit/retrieval.ts +++ b/packages/server/modules/core/services/commit/retrieval.ts @@ -3,38 +3,41 @@ import { ModelVersionsFilter, StreamCommitsArgs } from '@/modules/core/graph/generated/graphql' -import { getStreamCommitCountFactory } from '@/modules/core/repositories/commits' -import { getCommitsByStreamId } from '@/modules/core/services/commits' import { BadRequestError } from '@/modules/shared/errors' -import { db } from '@/db/knex' import { GetBranchCommitsTotalCount, GetPaginatedBranchCommits, GetPaginatedBranchCommitsItems, GetSpecificBranchCommits, + GetStreamCommitCount, + LegacyGetPaginatedStreamCommits, + LegacyGetPaginatedStreamCommitsPage, PaginatedBranchCommitsParams } from '@/modules/core/domain/commits/operations' -export async function getPaginatedStreamCommits( - streamId: string, - params: StreamCommitsArgs -) { - if (params.limit && params.limit > 100) - throw new BadRequestError( - 'Cannot return more than 100 items, please use pagination.' - ) - const { commits: items, cursor } = await getCommitsByStreamId({ - streamId, - limit: params.limit, - cursor: params.cursor, - ignoreGlobalsBranch: true - }) - const totalCount = await getStreamCommitCountFactory({ db })(streamId, { - ignoreGlobalsBranch: true - }) +export const legacyGetPaginatedStreamCommits = + (deps: { + legacyGetPaginatedStreamCommitsPage: LegacyGetPaginatedStreamCommitsPage + getStreamCommitCount: GetStreamCommitCount + }): LegacyGetPaginatedStreamCommits => + async (streamId: string, params: StreamCommitsArgs) => { + if (params.limit && params.limit > 100) + throw new BadRequestError( + 'Cannot return more than 100 items, please use pagination.' + ) - return { items, cursor, totalCount } -} + const { commits: items, cursor } = await deps.legacyGetPaginatedStreamCommitsPage({ + streamId, + limit: params.limit, + cursor: params.cursor, + ignoreGlobalsBranch: true + }) + const totalCount = await deps.getStreamCommitCount(streamId, { + ignoreGlobalsBranch: true + }) + + return { items, cursor, totalCount } + } export const getPaginatedBranchCommitsFactory = (deps: { diff --git a/packages/server/modules/core/services/commits.js b/packages/server/modules/core/services/commits.js index a95c3fced..66c53fe4d 100644 --- a/packages/server/modules/core/services/commits.js +++ b/packages/server/modules/core/services/commits.js @@ -6,9 +6,6 @@ const { getPaginatedBranchCommitsItemsFactory } = require('@/modules/core/repositories/commits') -const StreamCommits = () => knex('stream_commits') -const { clamp } = require('lodash') - module.exports = { async getCommitsTotalCountByBranchName({ streamId, branchName }) { branchName = branchName.toLowerCase() @@ -33,50 +30,5 @@ module.exports = { db: knex }) return getPaginatedBranchCommits({ branchId: myBranch.id, limit, cursor }) - }, - - /** - * @returns {Promise<{ - * commits: import('@/modules/core/helpers/types').CommitRecord[], - * cursor: string | null - * }>} - */ - async getCommitsByStreamId({ streamId, limit, cursor, ignoreGlobalsBranch }) { - limit = clamp(limit || 25, 0, 100) - if (!limit) return { commits: [], cursor: null } - - const query = StreamCommits() - .columns([ - { id: 'commits.id' }, - 'message', - 'referencedObject', - 'sourceApplication', - 'totalChildrenCount', - 'parents', - 'commits.createdAt', - { branchName: 'branches.name' }, - { authorName: 'users.name' }, - { authorId: 'users.id' }, - { authorAvatar: 'users.avatar' }, - knex.raw(`?? as "author"`, ['users.id']) - ]) - .select() - .join('commits', 'commits.id', 'stream_commits.commitId') - .join('branch_commits', 'commits.id', 'branch_commits.commitId') - .join('branches', 'branches.id', 'branch_commits.branchId') - .leftJoin('users', 'commits.author', 'users.id') - .where('stream_commits.streamId', streamId) - - if (ignoreGlobalsBranch) query.andWhere('branches.name', '!=', 'globals') - - if (cursor) query.andWhere('commits.createdAt', '<', cursor) - - query.orderBy('commits.createdAt', 'desc').limit(limit) - - const rows = await query - return { - commits: rows, - cursor: rows.length > 0 ? rows[rows.length - 1].createdAt.toISOString() : null - } } } diff --git a/packages/server/modules/core/tests/commits.spec.js b/packages/server/modules/core/tests/commits.spec.js index b9d943575..66075b701 100644 --- a/packages/server/modules/core/tests/commits.spec.js +++ b/packages/server/modules/core/tests/commits.spec.js @@ -7,8 +7,7 @@ const { createObject } = require('../services/objects') const { getCommitsTotalCountByBranchName, - getCommitsByBranchName, - getCommitsByStreamId + getCommitsByBranchName } = require('../services/commits') const { createBranchAndNotifyFactory @@ -34,7 +33,8 @@ const { switchCommitBranchFactory, updateCommitFactory, getStreamCommitCountFactory, - legacyGetPaginatedUserCommitsPage + legacyGetPaginatedUserCommitsPage, + legacyGetPaginatedStreamCommitsPageFactory } = require('@/modules/core/repositories/commits') const { deleteCommitAndNotifyFactory, @@ -237,6 +237,7 @@ const createUser = createUserFactory({ usersEventsEmitter: UsersEmitter.emit }) const getCommitsByUserId = legacyGetPaginatedUserCommitsPage({ db }) +const getCommitsByStreamId = legacyGetPaginatedStreamCommitsPageFactory({ db }) describe('Commits @core-commits', () => { const user = { diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index 6f41e6644..81df1589f 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/packages/server/modules/core/tests/users.spec.js @@ -9,7 +9,7 @@ const { const { getBranchesByStreamId } = require('../services/branches') -const { getCommitsByBranchName, getCommitsByStreamId } = require('../services/commits') +const { getCommitsByBranchName } = require('../services/commits') const { createObject } = require('../services/objects') const { beforeEachContext } = require('@/test/hooks') @@ -26,7 +26,8 @@ const { getCommitFactory, createCommitFactory, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + legacyGetPaginatedStreamCommitsPageFactory } = require('@/modules/core/repositories/commits') const { createCommitByBranchIdFactory, @@ -285,6 +286,7 @@ const validateToken = validateTokenFactory({ }), updateApiToken: updateApiTokenFactory({ db }) }) +const getCommitsByStreamId = legacyGetPaginatedStreamCommitsPageFactory({ db }) describe('Actors & Tokens @user-services', () => { const myTestActor = { diff --git a/packages/server/modules/previews/index.ts b/packages/server/modules/previews/index.ts index 78b06476a..194461280 100644 --- a/packages/server/modules/previews/index.ts +++ b/packages/server/modules/previews/index.ts @@ -1,10 +1,7 @@ /* istanbul ignore file */ 'use strict' import { validateScopes, authorizeResolver } from '@/modules/shared' -import { - getCommitsByStreamId, - getCommitsByBranchName -} from '@/modules/core/services/commits' +import { getCommitsByBranchName } from '@/modules/core/services/commits' import { makeOgImage } from '@/modules/previews/ogImage' import { moduleLogger } from '@/logging/logging' @@ -26,7 +23,8 @@ import { import { publish } from '@/modules/shared/utils/subscriptions' import { getCommitFactory, - getObjectCommitsWithStreamIdsFactory + getObjectCommitsWithStreamIdsFactory, + legacyGetPaginatedStreamCommitsPageFactory } from '@/modules/core/repositories/commits' import { SpeckleModule } from '@/modules/shared/helpers/typeHelper' import { getStreamFactory } from '@/modules/core/repositories/streams' @@ -43,6 +41,7 @@ export const init: SpeckleModule['init'] = (app, isInitial) => { moduleLogger.info('📸 Init object preview module') } + const getCommitsByStreamId = legacyGetPaginatedStreamCommitsPageFactory({ db }) const getStream = getStreamFactory({ db }) const getObjectPreviewBufferOrFilepath = getObjectPreviewBufferOrFilepathFactory({ getObject,