Skip to content

Commit f5f0fdf

Browse files
fix(v2-tables): route single-row delete through the service
The v2 handler did a raw db.delete on user_table_rows, skipping assertRowDelete and deleteOrderedRow — so a delete-locked table returned 200 and the row-count and order bookkeeping never ran. v1 carries a comment warning against exactly this. Routes through deleteRow like v1, and adds v2TableLockError so a lock renders as the 423 LOCKED envelope instead of a 500. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 00e6c56 commit f5f0fdf

3 files changed

Lines changed: 127 additions & 15 deletions

File tree

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Public v2 single-row delete: goes through the row service so the delete lock
5+
* and row-count bookkeeping are enforced, and renders lock/not-found in the v2
6+
* error envelope.
7+
*/
8+
import { NextRequest } from 'next/server'
9+
import { beforeEach, describe, expect, it, vi } from 'vitest'
10+
11+
const { mockCheckRateLimit, mockResolveWorkspaceScope, mockCheckAccess, mockDeleteRow } =
12+
vi.hoisted(() => ({
13+
mockCheckRateLimit: vi.fn(),
14+
mockResolveWorkspaceScope: vi.fn(),
15+
mockCheckAccess: vi.fn(),
16+
mockDeleteRow: vi.fn(),
17+
}))
18+
19+
vi.mock('@/app/api/v1/middleware', () => ({
20+
checkRateLimit: mockCheckRateLimit,
21+
resolveWorkspaceScope: mockResolveWorkspaceScope,
22+
}))
23+
24+
vi.mock('@/app/api/table/utils', () => ({
25+
checkAccess: mockCheckAccess,
26+
normalizeColumn: (col: Record<string, unknown>) => col,
27+
rootErrorMessage: (error: unknown) => String(error),
28+
rowWriteErrorResponse: () => null,
29+
}))
30+
31+
vi.mock('@/lib/table', async () => {
32+
const actual = await import('@/lib/table/column-keys')
33+
return {
34+
...actual,
35+
deleteRow: mockDeleteRow,
36+
updateRow: vi.fn(),
37+
rowDataNameToId: vi.fn(),
38+
buildIdByName: vi.fn(),
39+
}
40+
})
41+
42+
vi.mock('@/app/api/v2/lib/gate', () => ({
43+
v2ApiGateError: vi.fn().mockResolvedValue(null),
44+
}))
45+
46+
import { TableLockedError } from '@/lib/table/mutation-locks'
47+
import { DELETE } from '@/app/api/v2/tables/[tableId]/rows/[rowId]/route'
48+
49+
const TABLE = { id: 'table-1', workspaceId: 'ws-1', schema: { columns: [] } }
50+
51+
function callDelete() {
52+
const req = new NextRequest(
53+
'http://localhost:3000/api/v2/tables/table-1/rows/row-1?workspaceId=ws-1',
54+
{ method: 'DELETE' }
55+
)
56+
return DELETE(req, { params: Promise.resolve({ tableId: 'table-1', rowId: 'row-1' }) })
57+
}
58+
59+
describe('DELETE /api/v2/tables/[tableId]/rows/[rowId]', () => {
60+
beforeEach(() => {
61+
vi.clearAllMocks()
62+
mockCheckRateLimit.mockResolvedValue({
63+
allowed: true,
64+
userId: 'user-1',
65+
keyType: 'workspace',
66+
workspaceId: 'ws-1',
67+
limit: 100,
68+
remaining: 99,
69+
resetAt: new Date('2026-01-01T01:00:00Z'),
70+
})
71+
mockResolveWorkspaceScope.mockResolvedValue(null)
72+
mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE })
73+
})
74+
75+
it('deletes through the row service rather than a raw db.delete', async () => {
76+
mockDeleteRow.mockResolvedValue(undefined)
77+
78+
const res = await callDelete()
79+
80+
expect(res.status).toBe(200)
81+
expect((await res.json()).data).toEqual({ deletedCount: 1, deletedRowIds: ['row-1'] })
82+
// The service applies the delete lock and the row-count decrement; the raw
83+
// delete this replaced skipped both.
84+
expect(mockDeleteRow).toHaveBeenCalledWith(TABLE, 'row-1', expect.any(String))
85+
})
86+
87+
it('returns 423 LOCKED when the table forbids deletes', async () => {
88+
mockDeleteRow.mockRejectedValue(new TableLockedError('Table is locked for deletes'))
89+
90+
const res = await callDelete()
91+
92+
expect(res.status).toBe(423)
93+
expect((await res.json()).error.code).toBe('LOCKED')
94+
})
95+
96+
it('returns 404 for a missing row', async () => {
97+
mockDeleteRow.mockRejectedValue(new Error('Row not found'))
98+
99+
const res = await callDelete()
100+
101+
expect(res.status).toBe(404)
102+
expect((await res.json()).error.code).toBe('NOT_FOUND')
103+
})
104+
})

apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { isZodError, parseRequest } from '@/lib/api/server'
1313
import { generateRequestId } from '@/lib/core/utils/request'
1414
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1515
import type { RowData, TableSchema } from '@/lib/table'
16-
import { buildIdByName, rowDataNameToId, updateRow } from '@/lib/table'
16+
import { buildIdByName, deleteRow, rowDataNameToId, updateRow } from '@/lib/table'
1717
import { namedRowMapper } from '@/lib/table/cell-format'
1818
import { checkAccess } from '@/app/api/table/utils'
1919
import { checkRateLimit, resolveWorkspaceScope } from '@/app/api/v1/middleware'
@@ -25,7 +25,7 @@ import {
2525
v2ValidationError,
2626
v2WorkspaceAccessError,
2727
} from '@/app/api/v2/lib/response'
28-
import { toApiRow, v2TableAccessError } from '@/app/api/v2/tables/utils'
28+
import { toApiRow, v2TableAccessError, v2TableLockError } from '@/app/api/v2/tables/utils'
2929

3030
const logger = createLogger('V2TableRowAPI')
3131

@@ -214,22 +214,19 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row
214214
return v2Error('NOT_FOUND', 'Table not found')
215215
}
216216

217-
const [deletedRow] = await db
218-
.delete(userTableRows)
219-
.where(
220-
and(
221-
eq(userTableRows.id, rowId),
222-
eq(userTableRows.tableId, tableId),
223-
eq(userTableRows.workspaceId, workspaceId)
224-
)
225-
)
226-
.returning({ id: userTableRows.id })
227-
228-
if (!deletedRow) return v2Error('NOT_FOUND', 'Row not found')
217+
// Route through the service (not a raw `db.delete`) so the delete lock is
218+
// enforced and the row-count/order bookkeeping runs — the raw path returned
219+
// 200 on a locked table and left the count stale.
220+
await deleteRow(result.table, rowId, requestId)
229221

230222
// v2 mirrors the bulk delete shape: always returns `deletedRowIds`.
231-
return v2Data({ deletedCount: 1, deletedRowIds: [deletedRow.id] }, { rateLimit })
223+
return v2Data({ deletedCount: 1, deletedRowIds: [rowId] }, { rateLimit })
232224
} catch (error) {
225+
const lockError = v2TableLockError(error)
226+
if (lockError) return lockError
227+
if (error instanceof Error && error.message === 'Row not found') {
228+
return v2Error('NOT_FOUND', 'Row not found')
229+
}
233230
logger.error(`[${requestId}] Error deleting row`, {
234231
error: getErrorMessage(error, 'Unknown error'),
235232
})

apps/sim/app/api/v2/tables/utils.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { NextResponse } from 'next/server'
22
import type { RowData, TableDefinition, TablePredicate, TableSchema } from '@/lib/table'
3+
import { TableLockedError } from '@/lib/table/mutation-locks'
34
import { predicateToFilter } from '@/lib/table/query-builder/converters'
45
import {
56
validatePredicateShape,
@@ -95,6 +96,16 @@ export function v2TableAccessError(result: { ok: false; status: 404 | 403 }): Ne
9596
: v2Error('FORBIDDEN', 'Access denied')
9697
}
9798

99+
/**
100+
* Maps a delete/write rejected by a table lock to the v2 `LOCKED` envelope,
101+
* mirroring v1's {@link tableLockErrorResponse}. Returns `null` for anything
102+
* else so the caller falls through to its own classification.
103+
*/
104+
export function v2TableLockError(error: unknown): NextResponse | null {
105+
if (error instanceof TableLockedError) return v2Error('LOCKED', error.message)
106+
return null
107+
}
108+
98109
/**
99110
* Maps a known user-facing row-write failure (schema/size/unique/limit) to a v2
100111
* `BAD_REQUEST`, reusing v1's {@link rowWriteErrorResponse} classifier as the

0 commit comments

Comments
 (0)