Skip to content

Commit e3ab7d2

Browse files
author
Theodore Li
committed
Enforce duplicate file name
1 parent 5ca1d5b commit e3ab7d2

File tree

9 files changed

+1128
-373
lines changed

9 files changed

+1128
-373
lines changed

apps/sim/app/api/v1/files/route.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { z } from 'zod'
44
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
55
import { generateRequestId } from '@/lib/core/utils/request'
66
import {
7+
FileConflictError,
78
getWorkspaceFile,
89
listWorkspaceFiles,
910
uploadWorkspaceFile,
@@ -182,7 +183,8 @@ export async function POST(request: NextRequest) {
182183
})
183184
} catch (error) {
184185
const errorMessage = error instanceof Error ? error.message : 'Failed to upload file'
185-
const isDuplicate = errorMessage.includes('already exists')
186+
const isDuplicate =
187+
error instanceof FileConflictError || errorMessage.includes('already exists')
186188

187189
if (isDuplicate) {
188190
return NextResponse.json({ error: errorMessage }, { status: 409 })

apps/sim/app/api/workspaces/[id]/files/[fileId]/restore/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { getSession } from '@/lib/auth'
44
import { generateRequestId } from '@/lib/core/utils/request'
5-
import { restoreWorkspaceFile } from '@/lib/uploads/contexts/workspace'
5+
import { FileConflictError, restoreWorkspaceFile } from '@/lib/uploads/contexts/workspace'
66
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
77

88
const logger = createLogger('RestoreWorkspaceFileAPI')
@@ -31,6 +31,9 @@ export async function POST(
3131

3232
return NextResponse.json({ success: true })
3333
} catch (error) {
34+
if (error instanceof FileConflictError) {
35+
return NextResponse.json({ error: error.message }, { status: 409 })
36+
}
3437
logger.error(`[${requestId}] Error restoring workspace file ${fileId}`, error)
3538
return NextResponse.json(
3639
{ error: error instanceof Error ? error.message : 'Internal server error' },

apps/sim/app/api/workspaces/[id]/files/route.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
44
import { getSession } from '@/lib/auth'
55
import { generateRequestId } from '@/lib/core/utils/request'
66
import {
7+
FileConflictError,
78
listWorkspaceFiles,
89
uploadWorkspaceFile,
910
type WorkspaceFileScope,
@@ -135,9 +136,9 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
135136
} catch (error) {
136137
logger.error(`[${requestId}] Error uploading workspace file:`, error)
137138

138-
// Check if it's a duplicate file error
139139
const errorMessage = error instanceof Error ? error.message : 'Failed to upload file'
140-
const isDuplicate = errorMessage.includes('already exists')
140+
const isDuplicate =
141+
error instanceof FileConflictError || errorMessage.includes('already exists')
141142

142143
return NextResponse.json(
143144
{

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

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
uploadFile,
2020
} from '@/lib/uploads/core/storage-service'
2121
import { getFileMetadataByKey, insertFileMetadata } from '@/lib/uploads/server/metadata'
22+
import { getPostgresErrorCode } from '@/lib/core/utils/pg-error'
2223
import { generateRestoreName } from '@/lib/core/utils/restore-name'
2324
import { isUuid, sanitizeFileName } from '@/executor/constants'
2425
import type { UserFile } from '@/executor/types'
@@ -110,7 +111,7 @@ export async function uploadWorkspaceFile(
110111

111112
const exists = await fileExistsInWorkspace(workspaceId, fileName)
112113
if (exists) {
113-
throw new Error(`A file named "${fileName}" already exists in this workspace`)
114+
throw new FileConflictError(fileName)
114115
}
115116

116117
const quotaCheck = await checkStorageQuota(userId, fileBuffer.length)
@@ -204,6 +205,12 @@ export async function uploadWorkspaceFile(
204205
context: 'workspace',
205206
}
206207
} catch (error) {
208+
if (error instanceof FileConflictError) {
209+
throw error
210+
}
211+
if (getPostgresErrorCode(error) === '23505') {
212+
throw new FileConflictError(fileName)
213+
}
207214
logger.error(`Failed to upload workspace file ${fileName}:`, error)
208215
throw new Error(
209216
`Failed to upload file: ${error instanceof Error ? error.message : 'Unknown error'}`
@@ -573,17 +580,25 @@ export async function renameWorkspaceFile(
573580
throw new FileConflictError(trimmedName)
574581
}
575582

576-
const updated = await db
577-
.update(workspaceFiles)
578-
.set({ originalName: trimmedName })
579-
.where(
580-
and(
581-
eq(workspaceFiles.id, fileId),
582-
eq(workspaceFiles.workspaceId, workspaceId),
583-
eq(workspaceFiles.context, 'workspace')
583+
let updated: { id: string }[]
584+
try {
585+
updated = await db
586+
.update(workspaceFiles)
587+
.set({ originalName: trimmedName })
588+
.where(
589+
and(
590+
eq(workspaceFiles.id, fileId),
591+
eq(workspaceFiles.workspaceId, workspaceId),
592+
eq(workspaceFiles.context, 'workspace')
593+
)
584594
)
585-
)
586-
.returning({ id: workspaceFiles.id })
595+
.returning({ id: workspaceFiles.id })
596+
} catch (error: unknown) {
597+
if (getPostgresErrorCode(error) === '23505') {
598+
throw new FileConflictError(trimmedName)
599+
}
600+
throw error
601+
}
587602

588603
if (updated.length === 0) {
589604
throw new Error('File not found or could not be renamed')
@@ -651,22 +666,43 @@ export async function restoreWorkspaceFile(workspaceId: string, fileId: string):
651666
throw new Error('Cannot restore file into an archived workspace')
652667
}
653668

654-
const newName = await generateRestoreName(
655-
fileRecord.name,
656-
(candidate) => fileExistsInWorkspace(workspaceId, candidate),
657-
{ hasExtension: true }
658-
)
669+
/**
670+
* A concurrent upload/rename can claim the chosen name after `generateRestoreName`'s check (MVCC).
671+
* Retries pick a new random suffix; 23505 maps to {@link FileConflictError} after exhaustion.
672+
*/
673+
const maxUniqueViolationRetries = 8
674+
let attemptedRestoreName = ''
659675

660-
await db
661-
.update(workspaceFiles)
662-
.set({ deletedAt: null, originalName: newName })
663-
.where(
664-
and(
665-
eq(workspaceFiles.id, fileId),
666-
eq(workspaceFiles.workspaceId, workspaceId),
667-
eq(workspaceFiles.context, 'workspace')
676+
for (let attempt = 0; attempt < maxUniqueViolationRetries; attempt++) {
677+
attemptedRestoreName = ''
678+
try {
679+
const newName = await generateRestoreName(
680+
fileRecord.name,
681+
(candidate) => fileExistsInWorkspace(workspaceId, candidate),
682+
{ hasExtension: true }
668683
)
669-
)
684+
attemptedRestoreName = newName
685+
686+
await db
687+
.update(workspaceFiles)
688+
.set({ deletedAt: null, originalName: newName })
689+
.where(
690+
and(
691+
eq(workspaceFiles.id, fileId),
692+
eq(workspaceFiles.workspaceId, workspaceId),
693+
eq(workspaceFiles.context, 'workspace')
694+
)
695+
)
670696

671-
logger.info(`Successfully restored workspace file: ${newName}`)
697+
logger.info(`Successfully restored workspace file: ${newName}`)
698+
return
699+
} catch (error: unknown) {
700+
if (getPostgresErrorCode(error) !== '23505') {
701+
throw error
702+
}
703+
if (attempt === maxUniqueViolationRetries - 1) {
704+
throw new FileConflictError(attemptedRestoreName || fileRecord.name)
705+
}
706+
}
707+
}
672708
}

packages/db/migrations/0178_chunky_juggernaut.sql

Lines changed: 0 additions & 21 deletions
This file was deleted.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
-- Dedupe blocks prepended after `bunx drizzle-kit generate` so partial unique indexes can apply.
2+
--
3+
-- Knowledge bases: one active name per workspace (see schema `kb_workspace_name_active_unique`).
4+
-- Keeps the most recently updated row with its original name; older duplicates get " (2)", " (3)", etc.
5+
WITH kb_duplicates AS (
6+
SELECT id, name, workspace_id,
7+
ROW_NUMBER() OVER (
8+
PARTITION BY workspace_id, name
9+
ORDER BY updated_at DESC, created_at DESC
10+
) AS rn
11+
FROM knowledge_base
12+
WHERE deleted_at IS NULL
13+
AND workspace_id IS NOT NULL
14+
)
15+
UPDATE knowledge_base
16+
SET name = knowledge_base.name || ' (' || kb_duplicates.rn || ')'
17+
FROM kb_duplicates
18+
WHERE knowledge_base.id = kb_duplicates.id
19+
AND kb_duplicates.rn > 1;--> statement-breakpoint
20+
--
21+
-- Workspace files: one active display name per workspace for context `workspace`
22+
-- (`workspace_files_workspace_name_active_unique`). Keeps the most recently uploaded row; others
23+
-- get " (2)", " (3)", etc.
24+
WITH file_name_duplicates AS (
25+
SELECT id, original_name, workspace_id,
26+
ROW_NUMBER() OVER (
27+
PARTITION BY workspace_id, original_name
28+
ORDER BY uploaded_at DESC
29+
) AS rn
30+
FROM workspace_files
31+
WHERE deleted_at IS NULL
32+
AND context = 'workspace'
33+
AND workspace_id IS NOT NULL
34+
)
35+
UPDATE workspace_files
36+
SET original_name = workspace_files.original_name || ' (' || file_name_duplicates.rn || ')'
37+
FROM file_name_duplicates
38+
WHERE workspace_files.id = file_name_duplicates.id
39+
AND file_name_duplicates.rn > 1;--> statement-breakpoint
40+
CREATE UNIQUE INDEX "kb_workspace_name_active_unique" ON "knowledge_base" USING btree ("workspace_id","name") WHERE "knowledge_base"."deleted_at" IS NULL;--> statement-breakpoint
41+
CREATE UNIQUE INDEX "workspace_files_workspace_name_active_unique" ON "workspace_files" USING btree ("workspace_id","original_name") WHERE "workspace_files"."deleted_at" IS NULL AND "workspace_files"."context" = 'workspace' AND "workspace_files"."workspace_id" IS NOT NULL;--> statement-breakpoint

0 commit comments

Comments
 (0)