Skip to content

Commit 345278b

Browse files
committed
test(cleanup): cover re-rooting active children out of purged folders
Covers the one path this PR added that production exercises on zero rows: the onBatch that renames active children before the folder hard-delete, so the new ON DELETE SET NULL cannot collide on the workflow / workspace_files active-unique indexes and stall retention permanently. Each assertion was mutation-checked — dropping the dedup, the id-suffixed fallback, or the onBatch wiring each fails the corresponding test.
1 parent bab6d30 commit 345278b

1 file changed

Lines changed: 111 additions & 1 deletion

File tree

apps/sim/background/cleanup-soft-deletes.test.ts

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22
* @vitest-environment node
33
*/
44

5-
import { dbChainMock, dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing'
5+
import {
6+
dbChainMock,
7+
dbChainMockFns,
8+
queueTableRows,
9+
resetDbChainMock,
10+
schemaMock,
11+
} from '@sim/testing'
612
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
713

814
const {
@@ -18,7 +24,11 @@ const {
1824
mockPrepareChatCleanup,
1925
mockResolveStorageBillingContext,
2026
mockSelectRowsByIdChunks,
27+
mockDeduplicateWorkflowName,
28+
mockAllocateUniqueWorkspaceFileName,
2129
} = vi.hoisted(() => ({
30+
mockDeduplicateWorkflowName: vi.fn(async (name: string) => name),
31+
mockAllocateUniqueWorkspaceFileName: vi.fn(async (_ws: string, name: string) => name),
2232
mockBatchDeleteByWorkspaceAndTimestamp: vi.fn(async () => ({ deleted: 0, failed: 0 })),
2333
mockChunkedBatchDelete: vi.fn(async () => ({ deleted: 0, failed: 0 })),
2434
mockDecrementStorageUsageForBillingContextInTx: vi.fn(async () => undefined),
@@ -66,6 +76,14 @@ vi.mock('@/lib/uploads', () => ({
6676

6777
vi.mock('@/lib/uploads/server/metadata', () => ({ deleteFileMetadata: mockDeleteFileMetadata }))
6878

79+
vi.mock('@/lib/workflows/utils', () => ({
80+
deduplicateWorkflowName: mockDeduplicateWorkflowName,
81+
}))
82+
83+
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
84+
allocateUniqueWorkspaceFileName: mockAllocateUniqueWorkspaceFileName,
85+
}))
86+
6987
import { runCleanupSoftDeletes } from '@/background/cleanup-soft-deletes'
7088

7189
const basePayload = {
@@ -269,6 +287,7 @@ interface BatchDeleteOptions {
269287
tableName: string
270288
requireTimestampNotNull?: boolean
271289
additionalPredicate?: { type: string; column: unknown; values: unknown[] }
290+
onBatch?: (rows: { id: string }[]) => Promise<void>
272291
}
273292

274293
/**
@@ -331,4 +350,95 @@ describe('folder cleanup target', () => {
331350
.map(([options]) => options.tableName)
332351
expect(filtered).toEqual(['free/1/folder'])
333352
})
353+
354+
/**
355+
* `folder_id` is `ON DELETE SET NULL`, so Postgres re-roots surviving children on its own —
356+
* but `workflow` and `workspace_files` each carry a partial unique index keyed on
357+
* `coalesce(folder_id, '')`, so an implicit SET NULL can land a child on a name the workspace
358+
* root already holds. That aborts the whole DELETE with a 23505, which `chunkedBatchDelete`
359+
* turns into `hasMore = false` — folder retention then stalls permanently for that chunk,
360+
* re-failing on every later run. `onBatch` renames first so the SET NULL is a no-op.
361+
*/
362+
describe('re-rooting active children before the delete', () => {
363+
async function getFolderOnBatch() {
364+
const target = await runAndFindFolderTarget()
365+
expect(target?.onBatch).toBeTypeOf('function')
366+
return target!.onBatch!
367+
}
368+
369+
it('is the only cleanup target that re-roots children', async () => {
370+
await runCleanupSoftDeletes(basePayload)
371+
const calls = mockBatchDeleteByWorkspaceAndTimestamp.mock.calls as unknown as Array<
372+
[BatchDeleteOptions]
373+
>
374+
375+
const withOnBatch = calls
376+
.filter(([options]) => options.onBatch !== undefined)
377+
.map(([options]) => options.tableName)
378+
expect(withOnBatch).toEqual(['free/1/folder'])
379+
})
380+
381+
it('re-roots an active workflow under a deduplicated name', async () => {
382+
const onBatch = await getFolderOnBatch()
383+
queueTableRows(schemaMock.workflow, [{ id: 'w1', name: 'Report', workspaceId: 'ws-1' }])
384+
queueTableRows(schemaMock.workspaceFiles, [])
385+
mockDeduplicateWorkflowName.mockResolvedValueOnce('Report (2)')
386+
387+
await onBatch([{ id: 'folder-1' }])
388+
389+
// Deduped against the workspace ROOT (folderId null), which is where SET NULL would put it.
390+
expect(mockDeduplicateWorkflowName).toHaveBeenCalledWith(
391+
'Report',
392+
'ws-1',
393+
null,
394+
expect.anything()
395+
)
396+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ folderId: null, name: 'Report (2)' })
397+
})
398+
399+
it('re-roots an active workspace file under a deduplicated name', async () => {
400+
const onBatch = await getFolderOnBatch()
401+
queueTableRows(schemaMock.workflow, [])
402+
queueTableRows(schemaMock.workspaceFiles, [
403+
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
404+
])
405+
mockAllocateUniqueWorkspaceFileName.mockResolvedValueOnce('report (2).pdf')
406+
407+
await onBatch([{ id: 'folder-1' }])
408+
409+
expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith('ws-1', 'report.pdf', null)
410+
expect(dbChainMockFns.set).toHaveBeenCalledWith({
411+
folderId: null,
412+
originalName: 'report (2).pdf',
413+
})
414+
})
415+
416+
it('falls back to an id-suffixed name when the copy-suffix range is exhausted', async () => {
417+
// Letting the allocator throw would abort the sweep — the exact stall this guards against.
418+
const onBatch = await getFolderOnBatch()
419+
queueTableRows(schemaMock.workflow, [])
420+
queueTableRows(schemaMock.workspaceFiles, [
421+
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
422+
])
423+
mockAllocateUniqueWorkspaceFileName.mockRejectedValueOnce(new Error('conflict'))
424+
425+
await expect(onBatch([{ id: 'folder-1' }])).resolves.toBeUndefined()
426+
427+
expect(dbChainMockFns.set).toHaveBeenCalledWith({
428+
folderId: null,
429+
originalName: 'report.pdf (f1)',
430+
})
431+
})
432+
433+
it('touches nothing when the batch is empty', async () => {
434+
const onBatch = await getFolderOnBatch()
435+
dbChainMockFns.select.mockClear()
436+
dbChainMockFns.update.mockClear()
437+
438+
await onBatch([])
439+
440+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
441+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
442+
})
443+
})
334444
})

0 commit comments

Comments
 (0)