diff --git a/packages/server/modules/automate/tests/automations.spec.ts b/packages/server/modules/automate/tests/automations.spec.ts index 6c980e9a3..f826fe360 100644 --- a/packages/server/modules/automate/tests/automations.spec.ts +++ b/packages/server/modules/automate/tests/automations.spec.ts @@ -14,7 +14,7 @@ import { import { getGenericRedis } from '@/modules/core' import { ProjectAutomationRevisionCreateInput } from '@/modules/core/graph/generated/graphql' import { BranchRecord } from '@/modules/core/helpers/types' -import { getLatestStreamBranch } from '@/modules/core/repositories/branches' +import { getLatestStreamBranchFactory } from '@/modules/core/repositories/branches' import { addOrUpdateStreamCollaborator, validateStreamAccess @@ -309,7 +309,7 @@ const buildAutomationUpdate = () => { projectId: myStream.id, userId: me.id }) - projectModel = await getLatestStreamBranch(myStream.id) + projectModel = await getLatestStreamBranchFactory({ db })(myStream.id) }) it('works successfully', async () => { diff --git a/packages/server/modules/automate/tests/trigger.spec.ts b/packages/server/modules/automate/tests/trigger.spec.ts index cdc665d53..5973951b8 100644 --- a/packages/server/modules/automate/tests/trigger.spec.ts +++ b/packages/server/modules/automate/tests/trigger.spec.ts @@ -53,7 +53,7 @@ import { Automate } from '@speckle/shared' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getBranchLatestCommitsFactory, - getLatestStreamBranch + getLatestStreamBranchFactory } from '@/modules/core/repositories/branches' import { buildAutomationCreate, @@ -148,7 +148,7 @@ const getBranchLatestCommits = getBranchLatestCommitsFactory({ db }) ]) const [projectModel, newAutomation] = await Promise.all([ - getLatestStreamBranch(testUserStream.id), + getLatestStreamBranchFactory({ db })(testUserStream.id), createAutomation({ userId: testUser.id, projectId: testUserStream.id, diff --git a/packages/server/modules/core/domain/branches/operations.ts b/packages/server/modules/core/domain/branches/operations.ts index 0fbab01a3..a84abd70f 100644 --- a/packages/server/modules/core/domain/branches/operations.ts +++ b/packages/server/modules/core/domain/branches/operations.ts @@ -152,3 +152,30 @@ export type DeleteBranchAndNotify = ( input: BranchDeleteInput | DeleteModelInput, userId: string ) => Promise + +export type GetStreamBranchCounts = ( + streamIds: string[], + options?: Partial<{ + skipEmptyMain: boolean + }> +) => Promise> + +export type GetStreamBranchCount = ( + streamId: string, + options?: Partial<{ + skipEmptyMain: boolean + }> +) => Promise + +export type GetBranchCommitCounts = (branchIds: string[]) => Promise< + { + count: number + id: string + }[] +> + +export type GetBranchCommitCount = (branchId: string) => Promise + +export type MarkCommitBranchUpdated = (commitId: string) => Promise + +export type GetLatestStreamBranch = (streamId: string) => Promise diff --git a/packages/server/modules/core/loaders.ts b/packages/server/modules/core/loaders.ts index 840bc42c0..d56c0aec1 100644 --- a/packages/server/modules/core/loaders.ts +++ b/packages/server/modules/core/loaders.ts @@ -42,10 +42,10 @@ import { getStreamCommentCountsFactory } from '@/modules/comments/repositories/comments' import { - getBranchCommitCounts, + getBranchCommitCountsFactory, getBranchesByIdsFactory, getBranchLatestCommitsFactory, - getStreamBranchCounts, + getStreamBranchCountsFactory, getStreamBranchesByNameFactory } from '@/modules/core/repositories/branches' import { CommentRecord } from '@/modules/comments/helpers/types' @@ -108,6 +108,8 @@ const getCommentParents = getCommentParentsFactory({ db }) const getBranchesByIds = getBranchesByIdsFactory({ db }) const getStreamBranchesByName = getStreamBranchesByNameFactory({ db }) const getBranchLatestCommits = getBranchLatestCommitsFactory({ db }) +const getStreamBranchCounts = getStreamBranchCountsFactory({ db }) +const getBranchCommitCounts = getBranchCommitCountsFactory({ db }) /** * TODO: Lazy load DataLoaders to reduce memory usage diff --git a/packages/server/modules/core/repositories/branches.ts b/packages/server/modules/core/repositories/branches.ts index 4addd3c53..79d8932f3 100644 --- a/packages/server/modules/core/repositories/branches.ts +++ b/packages/server/modules/core/repositories/branches.ts @@ -6,7 +6,11 @@ import { ProjectModelsTreeArgs } from '@/modules/core/graph/generated/graphql' import { ModelsTreeItemGraphQLReturn } from '@/modules/core/helpers/graphTypes' -import { BranchRecord, CommitRecord } from '@/modules/core/helpers/types' +import { + BranchCommitRecord, + BranchRecord, + CommitRecord +} from '@/modules/core/helpers/types' import { BatchedSelectOptions, executeBatchedSelect @@ -19,8 +23,11 @@ import { DeleteBranchById, GenerateBranchId, GetBranchById, + GetBranchCommitCount, + GetBranchCommitCounts, GetBranchesByIds, GetBranchLatestCommits, + GetLatestStreamBranch, GetModelTreeItems, GetModelTreeItemsFiltered, GetModelTreeItemsFilteredTotalCount, @@ -28,8 +35,11 @@ import { GetPaginatedProjectModelsItems, GetPaginatedProjectModelsTotalCount, GetStreamBranchByName, + GetStreamBranchCount, + GetStreamBranchCounts, GetStreamBranchesByName, GetStructuredProjectModels, + MarkCommitBranchUpdated, StoreBranch, UpdateBranch } from '@/modules/core/domain/branches/operations' @@ -38,7 +48,8 @@ import { ModelTreeItem } from '@/modules/core/domain/branches/types' const tables = { branches: (db: Knex) => db(Branches.name), - commits: (db: Knex) => db(Commits.name) + commits: (db: Knex) => db(Commits.name), + branchCommits: (db: Knex) => db(BranchCommits.name) } export const generateBranchId: GenerateBranchId = () => crs({ length: 10 }) @@ -129,72 +140,84 @@ export async function insertBranches( return await q } -export async function getStreamBranchCounts( - streamIds: string[], - options?: Partial<{ - /** - * In FE2 we skip main branches in our queries, if they don't have any commits - */ - skipEmptyMain: boolean - }> -) { - const { skipEmptyMain } = options || {} - if (!streamIds?.length) return [] +export const getStreamBranchCountsFactory = + (deps: { db: Knex }): GetStreamBranchCounts => + async ( + streamIds: string[], + options?: Partial<{ + /** + * In FE2 we skip main branches in our queries, if they don't have any commits + */ + skipEmptyMain: boolean + }> + ) => { + const { skipEmptyMain } = options || {} + if (!streamIds?.length) return [] - const q = Branches.knex() - .select(Branches.col.streamId) - .whereIn(Branches.col.streamId, streamIds) - .count() - .groupBy(Branches.col.streamId) + const q = tables + .branches(deps.db) + .select(Branches.col.streamId) + .whereIn(Branches.col.streamId, streamIds) + .count() + .groupBy(Branches.col.streamId) - if (skipEmptyMain) { - q.andWhere((w) => { - w.whereNot(Branches.col.name, 'main').orWhere( - 0, - '<', - BranchCommits.knex() - .count() - .where(BranchCommits.col.branchId, knex.raw(Branches.col.id)) - ) - }) + if (skipEmptyMain) { + q.andWhere((w) => { + w.whereNot(Branches.col.name, 'main').orWhere( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 0 as any, + '<', + tables + .branchCommits(deps.db) + .count() + .where(BranchCommits.col.branchId, knex.raw(Branches.col.id)) + ) + }) + } + + const results = (await q) as { streamId: string; count: string }[] + return results.map((r) => ({ ...r, count: parseInt(r.count) })) } - const results = (await q) as { streamId: string; count: string }[] - return results.map((r) => ({ ...r, count: parseInt(r.count) })) -} +export const getStreamBranchCountFactory = + (deps: { db: Knex }): GetStreamBranchCount => + async ( + streamId: string, + options?: Partial<{ + /** + * In FE2 we skip main branches in our queries, if they don't have any commits + */ + skipEmptyMain: boolean + }> + ) => { + const [res] = await getStreamBranchCountsFactory(deps)([streamId], options) + return res?.count || 0 + } -export async function getStreamBranchCount( - streamId: string, - options?: Partial<{ - /** - * In FE2 we skip main branches in our queries, if they don't have any commits - */ - skipEmptyMain: boolean - }> -) { - const [res] = await getStreamBranchCounts([streamId], options) - return res?.count || 0 -} +export const getBranchCommitCountsFactory = + (deps: { db: Knex }): GetBranchCommitCounts => + async (branchIds: string[]) => { + if (!branchIds?.length) return [] -export async function getBranchCommitCounts(branchIds: string[]) { - if (!branchIds?.length) return [] + const q = tables + .branches(deps.db) + .select(Branches.col.id) + .whereIn(Branches.col.id, branchIds) + .innerJoin(BranchCommits.name, BranchCommits.col.branchId, Branches.col.id) + .innerJoin(Commits.name, Commits.col.id, BranchCommits.col.commitId) + .count() + .groupBy(Branches.col.id) - const q = Branches.knex() - .select(Branches.col.id) - .whereIn(Branches.col.id, branchIds) - .innerJoin(BranchCommits.name, BranchCommits.col.branchId, Branches.col.id) - .innerJoin(Commits.name, Commits.col.id, BranchCommits.col.commitId) - .count() - .groupBy(Branches.col.id) + const results = (await q) as { id: string; count: string }[] + return results.map((r) => ({ ...r, count: parseInt(r.count) })) + } - const results = (await q) as { id: string; count: string }[] - return results.map((r) => ({ ...r, count: parseInt(r.count) })) -} - -export async function getBranchCommitCount(branchId: string) { - const [res] = await getBranchCommitCounts([branchId]) - return res?.count || 0 -} +export const getBranchCommitCountFactory = + (deps: { db: Knex }): GetBranchCommitCount => + async (branchId: string) => { + const [res] = await getBranchCommitCountsFactory(deps)([branchId]) + return res?.count || 0 + } export const getBranchLatestCommitsFactory = (deps: { db: Knex }): GetBranchLatestCommits => @@ -698,23 +721,29 @@ export const deleteBranchByIdFactory = return await tables.branches(deps.db).where(Branches.col.id, branchId).del() } -export async function markCommitBranchUpdated(commitId: string) { - const q = Branches.knex() - .whereIn(Branches.col.id, (w) => { - w.select(BranchCommits.col.branchId) - .from(BranchCommits.name) - .where(BranchCommits.col.commitId, commitId) - }) - .update(Branches.withoutTablePrefix.col.updatedAt, new Date(), '*') - const [branch] = (await q) as BranchRecord[] - return branch -} +export const markCommitBranchUpdatedFactory = + (deps: { db: Knex }): MarkCommitBranchUpdated => + async (commitId: string) => { + const q = tables + .branches(deps.db) + .whereIn(Branches.col.id, (w) => { + w.select(BranchCommits.col.branchId) + .from(BranchCommits.name) + .where(BranchCommits.col.commitId, commitId) + }) + .update(Branches.withoutTablePrefix.col.updatedAt, new Date(), '*') + const [branch] = (await q) as BranchRecord[] + return branch + } -export async function getLatestStreamBranch(streamId: string) { - const q = Branches.knex() - .where(Branches.col.streamId, streamId) - .orderBy(Branches.col.updatedAt, 'desc') - .limit(1) - const [branch] = await q - return branch -} +export const getLatestStreamBranchFactory = + (deps: { db: Knex }): GetLatestStreamBranch => + async (streamId: string) => { + const q = tables + .branches(deps.db) + .where(Branches.col.streamId, streamId) + .orderBy(Branches.col.updatedAt, 'desc') + .limit(1) + const [branch] = await q + return branch + } diff --git a/packages/server/modules/core/services/branches.js b/packages/server/modules/core/services/branches.js index 295388b5a..3d813a4c2 100644 --- a/packages/server/modules/core/services/branches.js +++ b/packages/server/modules/core/services/branches.js @@ -1,6 +1,6 @@ 'use strict' const knex = require('@/db/knex') -const { getStreamBranchCount } = require('@/modules/core/repositories/branches') +const { getStreamBranchCountFactory } = require('@/modules/core/repositories/branches') const Branches = () => knex('branches') @@ -19,16 +19,12 @@ module.exports = { if (cursor) query.andWhere('createdAt', '>', cursor) query.orderBy('createdAt').limit(limit) - const totalCount = await getStreamBranchCount(streamId) + const totalCount = await getStreamBranchCountFactory({ db: knex })(streamId) const rows = await query return { items: rows, cursor: rows.length > 0 ? rows[rows.length - 1].updatedAt.toISOString() : null, totalCount } - }, - - async getBranchesByStreamIdTotalCount({ streamId }) { - return await getStreamBranchCount(streamId) } } diff --git a/packages/server/modules/core/services/commit/management.ts b/packages/server/modules/core/services/commit/management.ts index f10cbd3d6..d4aee3271 100644 --- a/packages/server/modules/core/services/commit/management.ts +++ b/packages/server/modules/core/services/commit/management.ts @@ -22,7 +22,7 @@ import { CommitRecord } from '@/modules/core/helpers/types' import { getBranchByIdFactory, getStreamBranchByNameFactory, - markCommitBranchUpdated + markCommitBranchUpdatedFactory } from '@/modules/core/repositories/branches' import { createCommit, @@ -133,7 +133,7 @@ export async function createCommitByBranchId( await Promise.all([ markCommitStreamUpdated(id), - markCommitBranchUpdated(id), + markCommitBranchUpdatedFactory({ db })(id), VersionsEmitter.emit(VersionEvents.Created, { projectId: streamId, modelId: branchId, @@ -309,7 +309,7 @@ export async function updateCommitAndNotify( await Promise.all([ markCommitStreamUpdated(commit.id), - markCommitBranchUpdated(commit.id) + markCommitBranchUpdatedFactory({ db })(commit.id) ]) } @@ -336,7 +336,7 @@ export async function deleteCommitAndNotify( const [, updatedBranch] = await Promise.all([ markCommitStreamUpdated(commit.id), - markCommitBranchUpdated(commit.id) + markCommitBranchUpdatedFactory({ db })(commit.id) ]) const isDeleted = await deleteCommit(commitId) diff --git a/packages/server/test/speckle-helpers/automationHelper.ts b/packages/server/test/speckle-helpers/automationHelper.ts index 6b6370cb6..294b28e98 100644 --- a/packages/server/test/speckle-helpers/automationHelper.ts +++ b/packages/server/test/speckle-helpers/automationHelper.ts @@ -15,7 +15,7 @@ import cryptoRandomString from 'crypto-random-string' import { createAutomation as clientCreateAutomation } from '@/modules/automate/clients/executionEngine' import { getBranchesByIdsFactory, - getLatestStreamBranch + getLatestStreamBranchFactory } from '@/modules/core/repositories/branches' import { @@ -46,6 +46,7 @@ const storeAutomation = storeAutomationFactory({ db }) const storeAutomationToken = storeAutomationTokenFactory({ db }) const storeAutomationRevision = storeAutomationRevisionFactory({ db }) const getAutomation = getAutomationFactory({ db }) +const getLatestStreamBranch = getLatestStreamBranchFactory({ db }) export const generateFunctionId = () => cryptoRandomString({ length: 10 }) export const generateFunctionReleaseId = () => cryptoRandomString({ length: 10 })