Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(tables): per-table mutation locks (schema/insert/update/delete)#5960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
c173eb9b348cd071b87469369f0abbebfa01cfd3c80f13ee26f48eb67a6f37662779c8ed0d9f5468097a1aa9424fca76b043e497dFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -10,6 +10,7 @@ import { generateRequestId } from '@/lib/core/utils/request' | ||
| import { withRouteHandler } from '@/lib/core/utils/with-route-handler' | ||
| import { runTableImport, type TableImportPayload } from '@/lib/table/import-runner' | ||
| import { markTableJobRunning, releaseJobClaim } from '@/lib/table/jobs/service' | ||
| import { assertRowDelete, assertRowInsert, assertSchemaMutable } from '@/lib/table/mutation-locks' | ||
| import { getUserSettings } from '@/lib/users/queries' | ||
| import { accessError, checkAccess } from '@/app/api/table/utils' | ||
| @@ -53,6 +54,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| return NextResponse.json({ error: 'Cannot import into an archived table' }, { status: 400 }) | ||
| } | ||
| // Gate the locks before claiming the single write-job slot, so a locked table | ||
| // reports 423 here instead of holding the slot and failing inside the worker. | ||
| assertRowInsert(table) | ||
| if (mode === 'replace') assertRowDelete(table) | ||
TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (createColumns && createColumns.length > 0) assertSchemaMutable(table) | ||
| const ext = fileName.split('.').pop()?.toLowerCase() | ||
| if (ext !== 'csv' && ext !== 'tsv') { | ||
| return NextResponse.json({ error: 'Only CSV and TSV files are supported' }, { status: 400 }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -44,6 +44,7 @@ import { | ||
| checkAccess, | ||
| csvProxyBodyCapResponse, | ||
| multipartErrorResponse, | ||
| tableLockErrorResponse, | ||
| } from '@/app/api/table/utils' | ||
| const logger = createLogger('TableImportCSVExisting') | ||
| @@ -336,6 +337,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| }, | ||
| }) | ||
| } catch (err) { | ||
| // This branch returns rather than rethrowing, so the outer catch's | ||
| // mapper is unreachable from here — map the lock error first or a 423 | ||
| // degrades into a generic 500 (replace mode rethrows and maps fine). | ||
| const lockError = tableLockErrorResponse(err) | ||
| if (lockError) return lockError | ||
| const message = toError(err).message | ||
| logger.warn(`[${requestId}] Append failed for table ${tableId}`, { | ||
| total: coerced.length, | ||
| @@ -408,6 +415,8 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| throw err | ||
| } | ||
| } catch (error) { | ||
| const lockError = tableLockErrorResponse(error) | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (lockError) return lockError | ||
| if (isMultipartError(error)) return multipartErrorResponse(error) | ||
| const message = toError(error).message | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,29 @@ | ||
| import { createLogger } from '@sim/logger' | ||
| import { getErrorMessage } from '@sim/utils/errors' | ||
| import { type NextRequest, NextResponse } from 'next/server' | ||
| import { getTableQuerySchema, renameTableContract } from '@/lib/api/contracts/tables' | ||
| import { getTableQuerySchema, updateTableContract } from '@/lib/api/contracts/tables' | ||
| import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' | ||
| import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' | ||
| import { isFeatureEnabled } from '@/lib/core/config/feature-flags' | ||
| import { generateRequestId } from '@/lib/core/utils/request' | ||
| import { withRouteHandler } from '@/lib/core/utils/with-route-handler' | ||
| import { captureServerEvent } from '@/lib/posthog/server' | ||
| import { deleteTable, renameTable, TableConflictError, type TableSchema } from '@/lib/table' | ||
| import { | ||
| deleteTable, | ||
| getTableById, | ||
| renameTable, | ||
| TableConflictError, | ||
| type TableSchema, | ||
| updateTableLocks, | ||
| } from '@/lib/table' | ||
| import { getWorkspaceTableLimits } from '@/lib/table/billing' | ||
| import { accessError, checkAccess, normalizeColumn } from '@/app/api/table/utils' | ||
| import { TABLE_LOCK_FLAGS, TABLE_LOCK_KINDS } from '@/lib/table/types' | ||
| import { | ||
| accessError, | ||
| checkAccess, | ||
| normalizeColumn, | ||
| tableLockErrorResponse, | ||
| } from '@/app/api/table/utils' | ||
| const logger = createLogger('TableDetailAPI') | ||
| @@ -65,6 +79,7 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Tab | ||
| metadata: table.metadata ?? null, | ||
| rowCount: table.rowCount, | ||
| maxRows: maxRowsPerTable, | ||
| locks: table.locks, | ||
| createdAt: | ||
| table.createdAt instanceof Date | ||
| ? table.createdAt.toISOString() | ||
| @@ -104,7 +119,7 @@ export const PATCH = withRouteHandler( | ||
| } | ||
| const parsed = await parseRequest( | ||
| renameTableContract, | ||
| updateTableContract, | ||
| request, | ||
| { params }, | ||
| { | ||
| @@ -116,6 +131,8 @@ export const PATCH = withRouteHandler( | ||
| const { tableId } = parsed.data.params | ||
| const validated = parsed.data.body | ||
| // `write` is the floor for either operation; a `locks` change additionally | ||
| // requires `admin` (checked below), matching the workflow-lock precedent. | ||
| const result = await checkAccess(tableId, authResult.userId, 'write') | ||
| if (!result.ok) return accessError(result, requestId, tableId) | ||
| @@ -125,20 +142,56 @@ export const PATCH = withRouteHandler( | ||
| return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) | ||
| } | ||
| const updated = await renameTable(tableId, validated.name, requestId, authResult.userId) | ||
| if (validated.locks !== undefined) { | ||
| // With the flag off you may still CLEAR locks — otherwise flipping the | ||
| // kill switch would strand an already-locked table with no way to | ||
| // unlock it, while enforcement of those stored locks keeps running. | ||
| // Only a lock actually transitioning off→on needs the feature enabled; | ||
| // comparing against the stored state (rather than "every value is | ||
| // false") is what lets the settings UI, which always submits the full | ||
| // four-flag draft, clear one lock while another stays on. | ||
| const enablesALock = TABLE_LOCK_KINDS.some((kind) => { | ||
| const flag = TABLE_LOCK_FLAGS[kind] | ||
| return validated.locks?.[flag] === true && !table.locks[flag] | ||
| }) | ||
| if (enablesALock && !(await isFeatureEnabled('table-locks'))) { | ||
| return NextResponse.json({ error: 'Table locks are not enabled' }, { status: 403 }) | ||
TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const adminResult = await checkAccess(tableId, authResult.userId, 'admin') | ||
| if (!adminResult.ok) { | ||
| return NextResponse.json( | ||
| { error: 'Admin access required to change table locks' }, | ||
| { status: 403 } | ||
| ) | ||
| } | ||
| await updateTableLocks(tableId, validated.locks, authResult.userId, requestId) | ||
| } | ||
| if (validated.name !== undefined) { | ||
| await renameTable(tableId, validated.name, requestId, authResult.userId) | ||
| } | ||
| // Re-read so the response reflects both a rename and a lock change. | ||
| const updated = await getTableById(tableId) | ||
| if (!updated) { | ||
| return NextResponse.json({ error: 'Table not found' }, { status: 404 }) | ||
| } | ||
| return NextResponse.json({ | ||
| success: true, | ||
| data: { table: updated }, | ||
| }) | ||
| } catch (error) { | ||
| const lockError = tableLockErrorResponse(error) | ||
| if (lockError) return lockError | ||
| if (error instanceof TableConflictError) { | ||
| return NextResponse.json({ error: error.message }, { status: 409 }) | ||
| } | ||
| logger.error(`[${requestId}] Error renaming table:`, error) | ||
| logger.error(`[${requestId}] Error updating table:`, error) | ||
| return NextResponse.json( | ||
| { error: getErrorMessage(error, 'Failed to rename table') }, | ||
| { error: getErrorMessage(error, 'Failed to update table') }, | ||
| { status: 500 } | ||
| ) | ||
| } | ||
| @@ -188,6 +241,8 @@ export const DELETE = withRouteHandler( | ||
| }, | ||
| }) | ||
| } catch (error) { | ||
| const lockError = tableLockErrorResponse(error) | ||
| if (lockError) return lockError | ||
| if (isZodError(error)) { | ||
| return validationErrorResponse(error) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -21,6 +21,7 @@ import { | ||
| checkAccess, | ||
| rootErrorMessage, | ||
| rowWriteErrorResponse, | ||
| tableLockErrorResponse, | ||
| } from '@/app/api/table/utils' | ||
| const logger = createLogger('TableRowAPI') | ||
| @@ -211,7 +212,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row | ||
| return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) | ||
| } | ||
| await deleteRow(tableId, rowId, validated.workspaceId, requestId) | ||
| await deleteRow(table, rowId, requestId) | ||
TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return NextResponse.json({ | ||
| success: true, | ||
| @@ -221,6 +222,9 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row | ||
| }, | ||
| }) | ||
| } catch (error) { | ||
| const lockError = tableLockErrorResponse(error) | ||
| if (lockError) return lockError | ||
| const errorMessage = toError(error).message | ||
| if (errorMessage === 'Row not found') { | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.