Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 197 additions & 1 deletion apps/sim/background/cleanup-soft-deletes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
* @vitest-environment node
*/

import { dbChainMock, dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing'
import {
dbChainMock,
dbChainMockFns,
queueTableRows,
resetDbChainMock,
schemaMock,
} from '@sim/testing'
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'

const {
Expand All @@ -18,7 +24,13 @@ const {
mockPrepareChatCleanup,
mockResolveStorageBillingContext,
mockSelectRowsByIdChunks,
mockDeduplicateWorkflowName,
mockAllocateUniqueWorkspaceFileName,
mockDeduplicateFolderName,
} = vi.hoisted(() => ({
mockDeduplicateFolderName: vi.fn(async (_tx, _ws, _parent, name: string) => name),
mockDeduplicateWorkflowName: vi.fn(async (name: string) => name),
mockAllocateUniqueWorkspaceFileName: vi.fn(async (_ws: string, name: string) => name),
mockBatchDeleteByWorkspaceAndTimestamp: vi.fn(async () => ({ deleted: 0, failed: 0 })),
mockChunkedBatchDelete: vi.fn(async () => ({ deleted: 0, failed: 0 })),
mockDecrementStorageUsageForBillingContextInTx: vi.fn(async () => undefined),
Expand Down Expand Up @@ -66,6 +78,16 @@ vi.mock('@/lib/uploads', () => ({

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

vi.mock('@/lib/workflows/utils', () => ({
deduplicateWorkflowName: mockDeduplicateWorkflowName,
}))

vi.mock('@/lib/folders/naming', () => ({ deduplicateFolderName: mockDeduplicateFolderName }))

vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
allocateUniqueWorkspaceFileName: mockAllocateUniqueWorkspaceFileName,
}))

import { runCleanupSoftDeletes } from '@/background/cleanup-soft-deletes'

const basePayload = {
Expand Down Expand Up @@ -269,6 +291,7 @@ interface BatchDeleteOptions {
tableName: string
requireTimestampNotNull?: boolean
additionalPredicate?: { type: string; column: unknown; values: unknown[] }
onBatch?: (rows: { id: string }[]) => Promise<void>
}

/**
Expand Down Expand Up @@ -331,4 +354,177 @@ describe('folder cleanup target', () => {
.map(([options]) => options.tableName)
expect(filtered).toEqual(['free/1/folder'])
})

/**
* `folder_id` is `ON DELETE SET NULL`, so Postgres re-roots surviving children on its own —
* but `workflow` and `workspace_files` each carry a partial unique index keyed on
* `coalesce(folder_id, '')`, so an implicit SET NULL can land a child on a name the workspace
* root already holds. That aborts the whole DELETE with a 23505, which `chunkedBatchDelete`
* turns into `hasMore = false` — folder retention then stalls permanently for that chunk,
* re-failing on every later run. `onBatch` renames first so the SET NULL is a no-op.
*/
describe('re-rooting active children before the delete', () => {
async function getFolderOnBatch() {
const target = await runAndFindFolderTarget()
expect(target?.onBatch).toBeTypeOf('function')
return target!.onBatch!
}

it('is the only cleanup target that re-roots children', async () => {
await runCleanupSoftDeletes(basePayload)
const calls = mockBatchDeleteByWorkspaceAndTimestamp.mock.calls as unknown as Array<
[BatchDeleteOptions]
>

const withOnBatch = calls
.filter(([options]) => options.onBatch !== undefined)
.map(([options]) => options.tableName)
expect(withOnBatch).toEqual(['free/1/folder'])
})

it('re-roots an active workflow under a deduplicated name', async () => {
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
queueTableRows(schemaMock.workflow, [{ id: 'w1', name: 'Report', workspaceId: 'ws-1' }])
queueTableRows(schemaMock.workspaceFiles, [])
mockDeduplicateWorkflowName.mockResolvedValueOnce('Report (2)')

await onBatch([{ id: 'folder-1' }])

// Deduped against the workspace ROOT (folderId null), which is where SET NULL would put it.
expect(mockDeduplicateWorkflowName).toHaveBeenCalledWith(
'Report',
'ws-1',
null,
expect.anything()
)
expect(dbChainMockFns.set).toHaveBeenCalledWith({ folderId: null, name: 'Report (2)' })
})

it('re-roots an active workspace file under a deduplicated name', async () => {
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
queueTableRows(schemaMock.workflow, [])
queueTableRows(schemaMock.workspaceFiles, [
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
])
mockAllocateUniqueWorkspaceFileName.mockResolvedValueOnce('report (2).pdf')

await onBatch([{ id: 'folder-1' }])

expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith('ws-1', 'report.pdf', null)
expect(dbChainMockFns.set).toHaveBeenCalledWith({
folderId: null,
originalName: 'report (2).pdf',
})
})

it('falls back to an id-suffixed name when the copy-suffix range is exhausted', async () => {
// Letting the allocator throw would abort the sweep — the exact stall this guards against.
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
queueTableRows(schemaMock.workflow, [])
queueTableRows(schemaMock.workspaceFiles, [
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
])
mockAllocateUniqueWorkspaceFileName.mockRejectedValueOnce(new Error('conflict'))

await expect(onBatch([{ id: 'folder-1' }])).resolves.toBeUndefined()

expect(dbChainMockFns.set).toHaveBeenCalledWith({
folderId: null,
originalName: 'report.pdf (f1)',
})
})

it('re-roots an active SUBFOLDER, which hits the same unique index', async () => {
/**
* `folder.parentId` is also ON DELETE SET NULL and
* `folder_workspace_resource_parent_name_active_unique` keys on `coalesce(parent_id,'')`,
* so purging a parent can collide a surviving child at the root exactly like a workflow
* or file. Covering only those two would leave the class half-closed.
*/
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
queueTableRows(schemaMock.workflow, [])
queueTableRows(schemaMock.workspaceFiles, [])
queueTableRows(schemaMock.folder, [
{ id: 'sub-1', name: 'Reports', workspaceId: 'ws-1', resourceType: 'knowledge_base' },
])
mockDeduplicateFolderName.mockResolvedValueOnce('Reports (1)')

await onBatch([{ id: 'folder-1' }])

// Deduped against the ROOT of the child's OWN resourceType, which is where SET NULL lands it.
expect(mockDeduplicateFolderName).toHaveBeenCalledWith(
expect.anything(),
'ws-1',
null,
'Reports',
'knowledge_base'
)
expect(dbChainMockFns.set).toHaveBeenCalledWith({ parentId: null, name: 'Reports (1)' })
})

it('recovers when the allocator RETURNS a colliding name and the update raises', async () => {
/**
* `allocateUniqueWorkspaceFileName` fails open — `fileExistsInWorkspace` swallows query
* errors and returns false — so it can hand back a name already taken at the root. Only
* the UPDATE discovers that, and an uncaught 23505 aborts the batch: the exact stall this
* hook prevents. Guarding the name lookup alone is not enough.
*/
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
queueTableRows(schemaMock.workflow, [])
queueTableRows(schemaMock.workspaceFiles, [
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
])
mockAllocateUniqueWorkspaceFileName.mockResolvedValueOnce('taken.pdf')
dbChainMockFns.update.mockImplementationOnce(() => ({
set: () => ({ where: () => Promise.reject(new Error('duplicate key value (23505)')) }),
}))

await expect(onBatch([{ id: 'folder-1' }])).resolves.toBeUndefined()

expect(dbChainMockFns.set).toHaveBeenCalledWith({
folderId: null,
originalName: 'report.pdf (f1)',
})
})

it('leaves children alone when the folder was restored between select and onBatch', async () => {
/**
* The DELETE re-asserts eligibility and so correctly skips a restored folder. Without the
* same re-assertion here, this hook would still strip and rename that folder's children —
* leaving a live folder emptied out. This is the one side effect in the sweep that mutates
* rows which survive, so losing the race is user-visible.
*/
const onBatch = await getFolderOnBatch()
queueTableRows(schemaMock.folder, []) // restored: no longer soft-deleted past retention
// Children ARE queued: without the eligibility re-check these would be re-rooted and
// renamed, so the assertions below fail rather than passing for want of candidate rows.
queueTableRows(schemaMock.workflow, [{ id: 'w1', name: 'Report', workspaceId: 'ws-1' }])
queueTableRows(schemaMock.workspaceFiles, [
{ id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
])
dbChainMockFns.update.mockClear()

await onBatch([{ id: 'folder-1' }])

expect(mockDeduplicateWorkflowName).not.toHaveBeenCalled()
expect(mockAllocateUniqueWorkspaceFileName).not.toHaveBeenCalled()
expect(dbChainMockFns.update).not.toHaveBeenCalled()
})

it('touches nothing when the batch is empty', async () => {
const onBatch = await getFolderOnBatch()
dbChainMockFns.select.mockClear()
dbChainMockFns.update.mockClear()

await onBatch([])

expect(dbChainMockFns.select).not.toHaveBeenCalled()
expect(dbChainMockFns.update).not.toHaveBeenCalled()
})
})
})
Loading
Loading