Skip to content

Commit 0761363

Browse files
committed
refactor(uploads): keep sweepStoragePrefix beside the listing it pages
Moves the prefix sweep step into lib/uploads/core/prefix-sweep.ts with the listing and delete helpers it drives, and deletes sweep-prefix.ts. The provider paging cases (a short or empty non-final page, a stalled listing) now test the step itself against a mocked S3 listing, since a same-module call to listStorageKeyPage cannot be intercepted.
1 parent a53aadc commit 0761363

6 files changed

Lines changed: 91 additions & 89 deletions

File tree

‎apps/sim/lib/cleanup/chat-backend-cleanup-outbox.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ import {
1717
INLINE_CHAT_IMAGE_PREFIX,
1818
inlineImageRequestIdSchema,
1919
} from '@/lib/mothership/chat/inline-image-reference'
20-
import { outboxDeadlineNear } from '@/lib/uploads/core/prefix-sweep'
20+
import { outboxDeadlineNear, sweepStoragePrefix } from '@/lib/uploads/core/prefix-sweep'
2121
import { deleteFile } from '@/lib/uploads/core/storage-service'
22-
import { sweepStoragePrefix } from '@/lib/uploads/core/sweep-prefix'
2322

2423
const FILE_DELETE_CONCURRENCY = 10
2524

‎apps/sim/lib/projects/files/prefix-cleanup.test.ts‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1010
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
1111
vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
1212
vi.mock('@/lib/uploads/server/metadata', () => uploadsMetadataMock)
13-
vi.mock('@/lib/uploads/core/prefix-sweep', async (importOriginal) => {
14-
const actual = await importOriginal<typeof import('@/lib/uploads/core/prefix-sweep')>()
15-
return { ...actual, listStorageKeyPage: vi.fn(actual.listStorageKeyPage) }
16-
})
1713

1814
import type { OutboxEventContext } from '@/lib/core/outbox/service'
1915
import { PROJECT_FILE_PREFIX_CLEANUP_EVENT } from '@/lib/projects/files/outbox-events'
2016
import { projectFilePrefixCleanupOutboxHandlers } from '@/lib/projects/files/prefix-cleanup'
21-
import { listStorageKeyPage } from '@/lib/uploads/core/prefix-sweep'
2217

2318
const cleanup = projectFilePrefixCleanupOutboxHandlers[PROJECT_FILE_PREFIX_CLEANUP_EVENT]
2419

@@ -83,46 +78,4 @@ describe('Project prefix cleanup on local storage', () => {
8378

8479
expect(await exists('project/p1/zz.txt')).toBe(false)
8580
})
86-
87-
it('keeps sweeping after a short page that is not the last', async () => {
88-
await put('project/p1/a.txt')
89-
await put('project/p1/b.txt')
90-
vi.mocked(listStorageKeyPage).mockResolvedValueOnce({
91-
keys: ['project/p1/a.txt'],
92-
done: false,
93-
nextPageToken: 'next',
94-
})
95-
96-
const result = await cleanup({ projectId: 'p1' }, outboxContext())
97-
98-
expect(result).toMatchObject({ outcome: 'deferred', consumeAttempt: false })
99-
expect(await exists('project/p1/a.txt')).toBe(false)
100-
expect(await exists('project/p1/b.txt')).toBe(true)
101-
})
102-
103-
it('follows an empty page that is not the last to the objects after it', async () => {
104-
await put('project/p1/a.txt')
105-
vi.mocked(listStorageKeyPage)
106-
.mockResolvedValueOnce({ keys: [], done: false, nextPageToken: 'next' })
107-
.mockResolvedValueOnce({ keys: ['project/p1/a.txt'], done: true })
108-
109-
await expect(cleanup({ projectId: 'p1' }, outboxContext())).resolves.toBeUndefined()
110-
111-
expect(vi.mocked(listStorageKeyPage).mock.calls[1]?.[3]).toBe('next')
112-
expect(await exists('project/p1/a.txt')).toBe(false)
113-
})
114-
115-
it('spends an attempt when its listing stops advancing without removing anything', async () => {
116-
vi.mocked(listStorageKeyPage).mockResolvedValue({
117-
keys: [],
118-
done: false,
119-
nextPageToken: 'stuck',
120-
})
121-
122-
const result = await cleanup({ projectId: 'p1' }, outboxContext())
123-
124-
expect(result).toMatchObject({ outcome: 'deferred' })
125-
expect(result).not.toMatchObject({ consumeAttempt: false })
126-
expect(listStorageKeyPage).toHaveBeenCalledTimes(2)
127-
})
12881
})

‎apps/sim/lib/projects/files/prefix-cleanup.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@ import {
1616
PROJECT_STORAGE_RECONCILE_EVENT,
1717
} from '@/lib/projects/files/outbox-events'
1818
import { isObjectNotFoundError } from '@/lib/uploads/core/errors'
19-
import { outboxDeadlineNear, storageCleanupAvailableAt } from '@/lib/uploads/core/prefix-sweep'
20-
import { sweepStoragePrefix } from '@/lib/uploads/core/sweep-prefix'
19+
import {
20+
outboxDeadlineNear,
21+
storageCleanupAvailableAt,
22+
sweepStoragePrefix,
23+
} from '@/lib/uploads/core/prefix-sweep'
2124
import { abortProviderUpload } from '@/lib/uploads/upload-session/provider'
2225
import type { UploadSessionRecord } from '@/lib/uploads/upload-session/service'
2326
import { PROJECT_FILE_UPLOAD_BINDING_KEY } from '@/lib/uploads/upload-session/types'
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock'
2+
import { setUploadsConfig, uploadsConfigMock } from '@sim/testing/mocks/uploads-config.mock'
3+
import { uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock'
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockListS3ObjectKeyPage } = vi.hoisted(() => ({ mockListS3ObjectKeyPage: vi.fn() }))
7+
8+
vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
9+
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
10+
vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock)
11+
vi.mock('@/lib/uploads/providers/s3/client', () => ({
12+
listS3ObjectKeyPage: mockListS3ObjectKeyPage,
13+
}))
14+
15+
import { sweepStoragePrefix } from '@/lib/uploads/core/prefix-sweep'
16+
17+
const outbox = { signal: new AbortController().signal }
18+
const deletedKeys = () => storageServiceMockFns.mockDeleteFile.mock.calls.map(([{ key }]) => key)
19+
20+
/** A provider (Azure Blob, GCS, S3-compatibles) may return a short or empty page that is not the last. */
21+
describe('sweepStoragePrefix over a provider listing', () => {
22+
beforeEach(() => {
23+
setUploadsConfig({ USE_S3_STORAGE: true, S3_CONFIG: { bucket: 'bucket', region: 'r' } })
24+
mockListS3ObjectKeyPage.mockReset()
25+
storageServiceMockFns.mockDeleteFile.mockReset().mockResolvedValue(undefined)
26+
})
27+
28+
it('continues after a short page that is not the last', async () => {
29+
mockListS3ObjectKeyPage.mockResolvedValueOnce({
30+
keys: ['project/p1/a.txt'],
31+
nextContinuationToken: 'next',
32+
})
33+
34+
expect(await sweepStoragePrefix('project/p1/', 'project', outbox)).toBe('continue')
35+
expect(deletedKeys()).toEqual(['project/p1/a.txt'])
36+
})
37+
38+
it('follows an empty page that is not the last to the objects after it', async () => {
39+
mockListS3ObjectKeyPage
40+
.mockResolvedValueOnce({ keys: [], nextContinuationToken: 'next' })
41+
.mockResolvedValueOnce({ keys: ['project/p1/a.txt'] })
42+
43+
expect(await sweepStoragePrefix('project/p1/', 'project', outbox)).toBe('done')
44+
expect(mockListS3ObjectKeyPage.mock.calls[1]?.[0]).toMatchObject({ continuationToken: 'next' })
45+
expect(deletedKeys()).toEqual(['project/p1/a.txt'])
46+
})
47+
48+
it('stalls when its listing stops advancing without removing anything', async () => {
49+
mockListS3ObjectKeyPage.mockResolvedValue({ keys: [], nextContinuationToken: 'stuck' })
50+
51+
expect(await sweepStoragePrefix('project/p1/', 'project', outbox)).toBe('stalled')
52+
expect(mockListS3ObjectKeyPage).toHaveBeenCalledTimes(2)
53+
})
54+
})

‎apps/sim/lib/uploads/core/prefix-sweep.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,34 @@ export async function deleteStorageKeysUnderPrefix(
215215
}
216216
return { deleted, complete: true }
217217
}
218+
219+
/**
220+
* One outbox run's step of deleting every object under `prefix`: deletes one listing page's
221+
* objects, re-listing from the prefix start on the next step. A provider (Azure Blob, GCS,
222+
* S3-compatibles) may return a short or empty page that is not the last, so only `page.done` ends
223+
* the sweep; a non-final page that removes nothing is followed to the next one within the step, so
224+
* objects behind it are still reached. `continue` after a step that removed something with more
225+
* left; `stalled` when no page removed anything before the listing stopped advancing or the
226+
* deadline neared.
227+
*/
228+
export async function sweepStoragePrefix(
229+
prefix: string,
230+
context: StorageContext,
231+
outbox: Pick<OutboxEventContext, 'signal' | 'deadlineAt'>
232+
): Promise<'done' | 'continue' | 'stalled'> {
233+
let pageToken: string | undefined
234+
for (;;) {
235+
const page = await listStorageKeyPage(prefix, context, outbox.signal, pageToken)
236+
const { deleted, complete } = await deleteStorageKeysUnderPrefix(
237+
prefix,
238+
context,
239+
page.keys,
240+
outbox
241+
)
242+
if (complete && page.done) return 'done'
243+
if (deleted > 0) return 'continue'
244+
const advanced = page.nextPageToken !== undefined && page.nextPageToken !== pageToken
245+
if (!complete || !advanced || outboxDeadlineNear(outbox)) return 'stalled'
246+
pageToken = page.nextPageToken
247+
}
248+
}

‎apps/sim/lib/uploads/core/sweep-prefix.ts‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)