Skip to content

Commit b06840e

Browse files
committed
fix(files): index-shape the move conflict probe and cover execution-context URL parses
1 parent 4b22934 commit b06840e

4 files changed

Lines changed: 19 additions & 14 deletions

File tree

‎apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
workspaceFileSecretProvenanceMock,
1515
workspaceFileSecretProvenanceMockFns,
1616
} from '@sim/testing/mocks/workspace-file-secret-provenance.mock'
17+
import { generateShortId } from '@sim/utils/id'
1718
import { beforeEach, describe, expect, it, vi } from 'vitest'
1819

1920
/** The `workspace_files` columns {@link fileRows} enforces its unique indexes on. */
@@ -36,7 +37,7 @@ interface WorkspaceFileRow {
3637
*/
3738
const { fileRows, allocateFromFileRows } = vi.hoisted(() => {
3839
const fileRows: WorkspaceFileRow[] = []
39-
const withCopySuffix = (name: string, n: number) => {
40+
const withCopySuffix = (name: string, n: number | string) => {
4041
const lastDot = name.lastIndexOf('.')
4142
return lastDot > 0 && lastDot < name.length - 1
4243
? `${name.slice(0, lastDot)} (${n})${name.slice(lastDot)}`
@@ -63,11 +64,11 @@ const { fileRows, allocateFromFileRows } = vi.hoisted(() => {
6364
row.originalName === name
6465
)
6566
if (!taken(baseName)) return baseName
66-
for (let n = 1; n <= 1000; n++) {
67+
for (let n = 1; n <= 20; n++) {
6768
const candidate = withCopySuffix(baseName, n)
6869
if (!taken(candidate)) return candidate
6970
}
70-
throw new Error(`A file named "${baseName}" already exists in this workspace`)
71+
return withCopySuffix(baseName, generateShortId(8))
7172
},
7273
}
7374
})

‎apps/sim/lib/uploads/archive.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ function craftCentralDirectory(records: number, extraPerRecord: number): Buffer
114114
return buffer
115115
}
116116

117-
/** Mirrors `allocateUniqueWorkspaceFileName`'s " (n)" suffixing. */
117+
/** Numbered " (n)" suffixing in the style of `allocateUniqueWorkspaceFileName`'s first candidates. */
118118
function allocateUniqueName(folderKey: string, name: string): string {
119119
const dot = name.lastIndexOf('.')
120120
const base = dot > 0 ? name.slice(0, dot) : name

‎apps/sim/lib/uploads/contexts/workspace/__integration__/file-names.integration.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('workspace file names in PostgreSQL', () => {
6666
})
6767
}
6868

69-
it('parses an external URL without saving a copy to workspace Files', async () => {
69+
async function parseExternalUrl(executionId?: string) {
7070
const fixture = await seedWorkspace()
7171
const url = 'https://example.com/page.txt'
7272
vi.spyOn(inputValidation, 'validateUrlWithDNS').mockResolvedValue({
@@ -96,9 +96,11 @@ describe('workspace file names in PostgreSQL', () => {
9696
subjectUserId: fixture.aliceId,
9797
workspaceId: fixture.workspaceId,
9898
delegationId: generateId(),
99+
executionId,
99100
}),
100101
workspaceId: fixture.workspaceId,
101102
workflowId: generateId(),
103+
executionId,
102104
attributedUserId: fixture.aliceId,
103105
fileAccessUserId: fixture.aliceId,
104106
}
@@ -107,11 +109,18 @@ describe('workspace file names in PostgreSQL', () => {
107109

108110
expect(response.status).toBe(200)
109111
expect(body.output.content).toContain('fetched page body')
110-
const rows = await db
111-
.select({ id: workspaceFiles.id })
112+
return db
113+
.select({ context: workspaceFiles.context })
112114
.from(workspaceFiles)
113115
.where(eq(workspaceFiles.workspaceId, fixture.workspaceId))
114-
expect(rows).toEqual([])
116+
}
117+
118+
it('parses an external URL without saving a copy to workspace Files', async () => {
119+
expect(await parseExternalUrl()).toEqual([])
120+
})
121+
122+
it('keeps an external URL parsed during an execution as an execution file only', async () => {
123+
expect(await parseExternalUrl(generateId())).toEqual([{ context: 'execution' }])
115124
})
116125

117126
it('scopes name lookups to root or folder through the unique name index', async () => {

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,6 @@ function folderParentCondition(parentId?: string | null) {
224224
return normalized ? eq(folderTable.parentId, normalized) : isNull(folderTable.parentId)
225225
}
226226

227-
function fileFolderCondition(folderId?: string | null) {
228-
const normalized = normalizeParentId(folderId)
229-
return normalized ? eq(workspaceFiles.folderId, normalized) : isNull(workspaceFiles.folderId)
230-
}
231-
232227
/**
233228
* Folder predicate for active-name lookups, spelled exactly as the
234229
* `workspace_files_workspace_folder_name_active_unique` index expression so the
@@ -1059,7 +1054,7 @@ export async function moveWorkspaceFileItems(params: {
10591054
eq(workspaceFiles.workspaceId, params.workspaceId),
10601055
eq(workspaceFiles.originalName, file.name),
10611056
eq(workspaceFiles.context, 'workspace'),
1062-
fileFolderCondition(targetFolderId),
1057+
workspaceFileNameFolderCondition(targetFolderId),
10631058
isNull(workspaceFiles.deletedAt)
10641059
)
10651060
)

0 commit comments

Comments
 (0)