Skip to content

Commit a745600

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
refactor(api): page workflow versions in the persistence helper
listWorkflowVersions read every version row and the route filtered and sliced the result in memory, so the response was bounded but the query was not. It now takes optional limit/afterVersion, turning the cursor into a real keyset query; the route asks for limit + 1 and only trims the has-more probe. Both params are optional, so the internal, v1 admin, and copilot callers are unchanged. Also restores the untouched GET handler in [id]/route.ts to its original formatting — collapsing its signature had re-indented the whole body and buried the actual additions in whitespace churn.
1 parent c86d2fc commit a745600

4 files changed

Lines changed: 143 additions & 91 deletions

File tree

‎apps/sim/app/api/v2/workflows/[id]/route.ts‎

Lines changed: 59 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -43,71 +43,72 @@ interface RouteContext {
4343
params: Promise<{ id: string }>
4444
}
4545

46-
/** GET /api/v2/workflows/[id] — Fetch one workflow with its variables and trigger inputs. */
47-
export const GET = withRouteHandler(async (request: NextRequest, context: RouteContext) => {
48-
const requestId = generateId().slice(0, 8)
49-
50-
try {
51-
const rateLimit = await checkRateLimit(request, 'workflow-detail')
52-
if (!rateLimit.allowed) return v2RateLimitError(rateLimit)
46+
export const GET = withRouteHandler(
47+
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
48+
const requestId = generateId().slice(0, 8)
5349

54-
const userId = rateLimit.userId!
55-
56-
const gate = await v2ApiGateError(userId)
57-
if (gate) return gate
50+
try {
51+
const rateLimit = await checkRateLimit(request, 'workflow-detail')
52+
if (!rateLimit.allowed) return v2RateLimitError(rateLimit)
5853

59-
const parsed = await parseRequest(v2GetWorkflowContract, request, context, {
60-
validationErrorResponse: v2ValidationError,
61-
})
62-
if (!parsed.success) return parsed.response
63-
64-
const { id } = parsed.data.params
65-
66-
const workflowData = await getActiveWorkflowRecord(id)
67-
if (!workflowData?.workspaceId) return v2Error('NOT_FOUND', 'Workflow not found')
54+
const userId = rateLimit.userId!
6855

69-
// Mask an authorization failure as 404 so existence is not leaked.
70-
const access = await resolveWorkspaceAccess(rateLimit, userId, workflowData.workspaceId)
71-
if (access) return v2Error('NOT_FOUND', 'Workflow not found')
56+
const gate = await v2ApiGateError(userId)
57+
if (gate) return gate
7258

73-
const blockRows = await db
74-
.select({
75-
id: workflowBlocks.id,
76-
type: workflowBlocks.type,
77-
subBlocks: workflowBlocks.subBlocks,
59+
const parsed = await parseRequest(v2GetWorkflowContract, request, context, {
60+
validationErrorResponse: v2ValidationError,
7861
})
79-
.from(workflowBlocks)
80-
.where(eq(workflowBlocks.workflowId, id))
81-
82-
const blocksRecord = Object.fromEntries(
83-
blockRows.map((block) => [block.id, { type: block.type, subBlocks: block.subBlocks }])
84-
)
85-
const inputs = extractInputFieldsFromBlocks(blocksRecord)
86-
87-
const detail: V2WorkflowDetail = {
88-
id: workflowData.id,
89-
name: workflowData.name,
90-
description: workflowData.description,
91-
folderId: workflowData.folderId,
92-
workspaceId: workflowData.workspaceId,
93-
isDeployed: workflowData.isDeployed,
94-
deployedAt: workflowData.deployedAt?.toISOString() ?? null,
95-
runCount: workflowData.runCount,
96-
lastRunAt: workflowData.lastRunAt?.toISOString() ?? null,
97-
variables: (workflowData.variables as Record<string, unknown> | null) ?? {},
98-
inputs,
99-
createdAt: workflowData.createdAt.toISOString(),
100-
updatedAt: workflowData.updatedAt.toISOString(),
62+
if (!parsed.success) return parsed.response
63+
64+
const { id } = parsed.data.params
65+
66+
const workflowData = await getActiveWorkflowRecord(id)
67+
if (!workflowData?.workspaceId) return v2Error('NOT_FOUND', 'Workflow not found')
68+
69+
// Mask an authorization failure as 404 so existence is not leaked.
70+
const access = await resolveWorkspaceAccess(rateLimit, userId, workflowData.workspaceId)
71+
if (access) return v2Error('NOT_FOUND', 'Workflow not found')
72+
73+
const blockRows = await db
74+
.select({
75+
id: workflowBlocks.id,
76+
type: workflowBlocks.type,
77+
subBlocks: workflowBlocks.subBlocks,
78+
})
79+
.from(workflowBlocks)
80+
.where(eq(workflowBlocks.workflowId, id))
81+
82+
const blocksRecord = Object.fromEntries(
83+
blockRows.map((block) => [block.id, { type: block.type, subBlocks: block.subBlocks }])
84+
)
85+
const inputs = extractInputFieldsFromBlocks(blocksRecord)
86+
87+
const detail: V2WorkflowDetail = {
88+
id: workflowData.id,
89+
name: workflowData.name,
90+
description: workflowData.description,
91+
folderId: workflowData.folderId,
92+
workspaceId: workflowData.workspaceId,
93+
isDeployed: workflowData.isDeployed,
94+
deployedAt: workflowData.deployedAt?.toISOString() ?? null,
95+
runCount: workflowData.runCount,
96+
lastRunAt: workflowData.lastRunAt?.toISOString() ?? null,
97+
variables: (workflowData.variables as Record<string, unknown> | null) ?? {},
98+
inputs,
99+
createdAt: workflowData.createdAt.toISOString(),
100+
updatedAt: workflowData.updatedAt.toISOString(),
101+
}
102+
103+
return v2Data(detail, { rateLimit })
104+
} catch (error) {
105+
logger.error(`[${requestId}] Workflow details fetch error`, {
106+
error: getErrorMessage(error, 'Unknown error'),
107+
})
108+
return v2Error('INTERNAL_ERROR', 'Internal server error')
101109
}
102-
103-
return v2Data(detail, { rateLimit })
104-
} catch (error) {
105-
logger.error(`[${requestId}] Workflow details fetch error`, {
106-
error: getErrorMessage(error, 'Unknown error'),
107-
})
108-
return v2Error('INTERNAL_ERROR', 'Internal server error')
109110
}
110-
})
111+
)
111112

112113
/** PATCH /api/v2/workflows/[id] — Rename, re-describe, or move a workflow. */
113114
export const PATCH = withRouteHandler(async (request: NextRequest, context: RouteContext) => {

‎apps/sim/app/api/v2/workflows/[id]/versions/route.test.ts‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ function buildVersion(version: number, overrides: Record<string, unknown> = {})
7474
}
7575
}
7676

77+
const ALL_VERSIONS = [
78+
buildVersion(3, { isActive: true, name: 'Escalation branch', latestOperationStatus: 'active' }),
79+
buildVersion(2),
80+
buildVersion(1),
81+
]
82+
7783
const routeContext = () => ({ params: Promise.resolve({ id: 'wf-1' }) })
7884
const callGet = (query = '') =>
7985
GET(
@@ -87,17 +93,21 @@ describe('GET /api/v2/workflows/[id]/versions', () => {
8793
mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK)
8894
mockResolveWorkspaceAccess.mockResolvedValue(null)
8995
mockGetActiveWorkflowRecord.mockResolvedValue(WORKFLOW_RECORD)
90-
mockListWorkflowVersions.mockResolvedValue({
91-
versions: [
92-
buildVersion(3, {
93-
isActive: true,
94-
name: 'Escalation branch',
95-
latestOperationStatus: 'active',
96-
}),
97-
buildVersion(2),
98-
buildVersion(1),
99-
],
100-
})
96+
/**
97+
* Stands in for the keyset query the helper now runs, so the route's
98+
* has-more probe and cursor round-trip are exercised against realistic
99+
* `limit`/`afterVersion` behavior rather than a fixed array.
100+
*/
101+
mockListWorkflowVersions.mockImplementation(
102+
async (_workflowId: string, options: { limit?: number; afterVersion?: number } = {}) => {
103+
let versions = ALL_VERSIONS
104+
if (options.afterVersion !== undefined) {
105+
versions = versions.filter((row) => row.version < options.afterVersion!)
106+
}
107+
if (options.limit !== undefined) versions = versions.slice(0, options.limit)
108+
return { versions }
109+
}
110+
)
101111
})
102112

103113
it('returns 404 when the v2 API surface flag is off', async () => {
@@ -157,7 +167,25 @@ describe('GET /api/v2/workflows/[id]/versions', () => {
157167
latestOperationStatus: 'active',
158168
})
159169
expect(body.data[0]).not.toHaveProperty('createdBy')
160-
expect(mockListWorkflowVersions).toHaveBeenCalledWith('wf-1')
170+
// Paging is pushed into the helper — the route never reads the full set.
171+
expect(mockListWorkflowVersions).toHaveBeenCalledWith('wf-1', {
172+
limit: 51,
173+
afterVersion: undefined,
174+
})
175+
})
176+
177+
it('bounds the read to one page plus the has-more probe', async () => {
178+
await callGet('?limit=2')
179+
expect(mockListWorkflowVersions).toHaveBeenCalledWith('wf-1', {
180+
limit: 3,
181+
afterVersion: undefined,
182+
})
183+
})
184+
185+
it('pushes the cursor down to the helper as a keyset bound', async () => {
186+
const cursor = Buffer.from(JSON.stringify({ version: 3 })).toString('base64')
187+
await callGet(`?limit=2&cursor=${encodeURIComponent(cursor)}`)
188+
expect(mockListWorkflowVersions).toHaveBeenCalledWith('wf-1', { limit: 3, afterVersion: 3 })
161189
})
162190

163191
it('400s a structurally invalid cursor instead of silently truncating the list', async () => {

‎apps/sim/app/api/v2/workflows/[id]/versions/route.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,14 @@ export const GET = withRouteHandler(
7575
return v2Error('BAD_REQUEST', 'Invalid cursor')
7676
}
7777

78-
const { versions: rows } = await listWorkflowVersions(id)
79-
80-
const remaining = after ? rows.filter((row) => row.version < after.version) : rows
78+
// One extra row is the has-more probe, matching the other v2 cursor lists.
79+
const { versions: rows } = await listWorkflowVersions(id, {
80+
limit: limit + 1,
81+
afterVersion: after?.version,
82+
})
8183

82-
const hasMore = remaining.length > limit
83-
const page = remaining.slice(0, limit)
84+
const hasMore = rows.length > limit
85+
const page = rows.slice(0, limit)
8486

8587
const data: V2WorkflowVersion[] = page.map((row) => ({
8688
id: row.id,

‎apps/sim/lib/workflows/persistence/utils.ts‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,21 @@ export async function getWorkflowDeploymentVersion(
937937
return row ?? null
938938
}
939939

940-
export async function listWorkflowVersions(workflowId: string): Promise<{
940+
export interface ListWorkflowVersionsOptions {
941+
/** Caps the rows read. Omitted reads every version. */
942+
limit?: number
943+
/**
944+
* Keyset bound for the `version DESC` ordering: returns only versions
945+
* strictly below this number, i.e. the page *after* it. Paired with `limit`
946+
* this keeps a paginated caller off a full-table read.
947+
*/
948+
afterVersion?: number
949+
}
950+
951+
export async function listWorkflowVersions(
952+
workflowId: string,
953+
options: ListWorkflowVersionsOptions = {}
954+
): Promise<{
941955
versions: Array<{
942956
id: string
943957
version: number
@@ -952,22 +966,29 @@ export async function listWorkflowVersions(workflowId: string): Promise<{
952966
}> {
953967
const { user } = await import('@sim/db')
954968

969+
const versionConditions = [eq(workflowDeploymentVersion.workflowId, workflowId)]
970+
if (options.afterVersion !== undefined) {
971+
versionConditions.push(lt(workflowDeploymentVersion.version, options.afterVersion))
972+
}
973+
974+
const versionQuery = db
975+
.select({
976+
id: workflowDeploymentVersion.id,
977+
version: workflowDeploymentVersion.version,
978+
name: workflowDeploymentVersion.name,
979+
description: workflowDeploymentVersion.description,
980+
isActive: workflowDeploymentVersion.isActive,
981+
createdAt: workflowDeploymentVersion.createdAt,
982+
createdBy: workflowDeploymentVersion.createdBy,
983+
deployedByName: user.name,
984+
})
985+
.from(workflowDeploymentVersion)
986+
.leftJoin(user, eq(workflowDeploymentVersion.createdBy, user.id))
987+
.where(and(...versionConditions))
988+
.orderBy(desc(workflowDeploymentVersion.version))
989+
955990
const [rows, [currentOperation]] = await Promise.all([
956-
db
957-
.select({
958-
id: workflowDeploymentVersion.id,
959-
version: workflowDeploymentVersion.version,
960-
name: workflowDeploymentVersion.name,
961-
description: workflowDeploymentVersion.description,
962-
isActive: workflowDeploymentVersion.isActive,
963-
createdAt: workflowDeploymentVersion.createdAt,
964-
createdBy: workflowDeploymentVersion.createdBy,
965-
deployedByName: user.name,
966-
})
967-
.from(workflowDeploymentVersion)
968-
.leftJoin(user, eq(workflowDeploymentVersion.createdBy, user.id))
969-
.where(eq(workflowDeploymentVersion.workflowId, workflowId))
970-
.orderBy(desc(workflowDeploymentVersion.version)),
991+
options.limit !== undefined ? versionQuery.limit(options.limit) : versionQuery,
971992
/**
972993
* Only the workflow's current (latest-generation) operation carries a
973994
* status marker: a failed or in-flight attempt is live information until

0 commit comments

Comments
 (0)