Skip to content

Commit b2c7c61

Browse files
TheodoreSpeaksTheodore Li
TheodoreSpeaks
and
Theodore Li
authored
Enforce name uniqueness (#3679)
* Enforce name uniqueness * Use established pattern for error handling * Fix lint * Fix lint * Add kb name uniqueness to db * Fix lint * Handle name getting taken before restore * Enforce duplicate file name * Fix lint --------- Co-authored-by: Theodore Li <theo@sim.ai>
1 parent abb29e5 commit b2c7c61

File tree

30 files changed

+15144
-186
lines changed

30 files changed

+15144
-186
lines changed

‎apps/sim/app/_shell/providers/get-query-client.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ function makeQueryClient() {
1111
retryOnMount: false,
1212
},
1313
mutations: {
14-
retry: 1,
14+
retry: false,
1515
},
1616
dehydrate: {
1717
shouldDehydrateQuery: (query) =>

‎apps/sim/app/api/knowledge/[id]/restore/route.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { eq } from 'drizzle-orm'
55
import { type NextRequest, NextResponse } from 'next/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
77
import { generateRequestId } from '@/lib/core/utils/request'
8-
import { restoreKnowledgeBase } from '@/lib/knowledge/service'
8+
import { KnowledgeBaseConflictError, restoreKnowledgeBase } from '@/lib/knowledge/service'
99
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1010

1111
const logger = createLogger('RestoreKnowledgeBaseAPI')
@@ -49,6 +49,10 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
4949

5050
return NextResponse.json({ success: true })
5151
} catch (error) {
52+
if (error instanceof KnowledgeBaseConflictError) {
53+
return NextResponse.json({ error: error.message }, { status: 409 })
54+
}
55+
5256
logger.error(`[${requestId}] Error restoring knowledge base ${id}`, error)
5357
return NextResponse.json(
5458
{ error: error instanceof Error ? error.message : 'Internal server error' },

‎apps/sim/app/api/knowledge/[id]/route.test.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,15 @@ vi.mock('@sim/db/schema', () => ({
9898

9999
vi.mock('@/lib/audit/log', () => auditMock)
100100

101-
vi.mock('@/lib/knowledge/service', () => ({
102-
getKnowledgeBaseById: vi.fn(),
103-
updateKnowledgeBase: vi.fn(),
104-
deleteKnowledgeBase: vi.fn(),
105-
}))
101+
vi.mock('@/lib/knowledge/service', async (importOriginal) => {
102+
const actual = await importOriginal<typeof import('@/lib/knowledge/service')>()
103+
return {
104+
...actual,
105+
getKnowledgeBaseById: vi.fn(),
106+
updateKnowledgeBase: vi.fn(),
107+
deleteKnowledgeBase: vi.fn(),
108+
}
109+
})
106110

107111
vi.mock('@/app/api/knowledge/utils', () => ({
108112
checkKnowledgeBaseAccess: vi.fn(),

‎apps/sim/app/api/knowledge/[id]/route.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
88
import {
99
deleteKnowledgeBase,
1010
getKnowledgeBaseById,
11+
KnowledgeBaseConflictError,
1112
updateKnowledgeBase,
1213
} from '@/lib/knowledge/service'
1314
import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils'
@@ -166,6 +167,10 @@ export async function PUT(req: NextRequest, { params }: { params: Promise<{ id:
166167
throw validationError
167168
}
168169
} catch (error) {
170+
if (error instanceof KnowledgeBaseConflictError) {
171+
return NextResponse.json({ error: error.message }, { status: 409 })
172+
}
173+
169174
logger.error(`[${requestId}] Error updating knowledge base`, error)
170175
return NextResponse.json({ error: 'Failed to update knowledge base' }, { status: 500 })
171176
}

‎apps/sim/app/api/knowledge/route.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const { mockGetSession, mockDbChain } = vi.hoisted(() => {
1515
where: vi.fn().mockReturnThis(),
1616
groupBy: vi.fn().mockReturnThis(),
1717
orderBy: vi.fn().mockResolvedValue([]),
18+
limit: vi.fn().mockResolvedValue([]),
1819
insert: vi.fn().mockReturnThis(),
1920
values: vi.fn().mockResolvedValue(undefined),
2021
}
@@ -113,7 +114,7 @@ describe('Knowledge Base API Route', () => {
113114
Object.values(mockDbChain).forEach((fn) => {
114115
if (typeof fn === 'function') {
115116
fn.mockClear()
116-
if (fn !== mockDbChain.orderBy && fn !== mockDbChain.values) {
117+
if (fn !== mockDbChain.orderBy && fn !== mockDbChain.values && fn !== mockDbChain.limit) {
117118
fn.mockReturnThis()
118119
}
119120
}

‎apps/sim/app/api/knowledge/route.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
88
import {
99
createKnowledgeBase,
1010
getKnowledgeBases,
11+
KnowledgeBaseConflictError,
1112
type KnowledgeBaseScope,
1213
} from '@/lib/knowledge/service'
1314

@@ -149,6 +150,10 @@ export async function POST(req: NextRequest) {
149150
throw validationError
150151
}
151152
} catch (error) {
153+
if (error instanceof KnowledgeBaseConflictError) {
154+
return NextResponse.json({ error: error.message }, { status: 409 })
155+
}
156+
152157
logger.error(`[${requestId}] Error creating knowledge base`, error)
153158
return NextResponse.json({ error: 'Failed to create knowledge base' }, { status: 500 })
154159
}

‎apps/sim/app/api/table/[tableId]/restore/route.ts‎

Lines changed: 5 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 { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
44
import { generateRequestId } from '@/lib/core/utils/request'
5-
import { getTableById, restoreTable } from '@/lib/table'
5+
import { getTableById, restoreTable, TableConflictError } from '@/lib/table'
66
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
77

88
const logger = createLogger('RestoreTableAPI')
@@ -36,6 +36,10 @@ export async function POST(
3636

3737
return NextResponse.json({ success: true })
3838
} catch (error) {
39+
if (error instanceof TableConflictError) {
40+
return NextResponse.json({ error: error.message }, { status: 409 })
41+
}
42+
3943
logger.error(`[${requestId}] Error restoring table ${tableId}`, error)
4044
return NextResponse.json(
4145
{ error: error instanceof Error ? error.message : 'Internal server error' },

‎apps/sim/app/api/table/[tableId]/route.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@ import { type NextRequest, NextResponse } from 'next/server'
33
import { z } from 'zod'
44
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
55
import { generateRequestId } from '@/lib/core/utils/request'
6-
import { deleteTable, NAME_PATTERN, renameTable, TABLE_LIMITS, type TableSchema } from '@/lib/table'
6+
import {
7+
deleteTable,
8+
NAME_PATTERN,
9+
renameTable,
10+
TABLE_LIMITS,
11+
TableConflictError,
12+
type TableSchema,
13+
} from '@/lib/table'
714
import { accessError, checkAccess, normalizeColumn } from '@/app/api/table/utils'
815

916
const logger = createLogger('TableDetailAPI')
@@ -136,6 +143,10 @@ export async function PATCH(request: NextRequest, { params }: TableRouteParams)
136143
)
137144
}
138145

146+
if (error instanceof TableConflictError) {
147+
return NextResponse.json({ error: error.message }, { status: 409 })
148+
}
149+
139150
logger.error(`[${requestId}] Error renaming table:`, error)
140151
return NextResponse.json(
141152
{ error: error instanceof Error ? error.message : 'Failed to rename table' },

‎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' },

0 commit comments

Comments
 (0)