Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion apps/sim/app/api/table/[tableId]/columns/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ vi.mock('@/lib/table', () => ({
updateColumnOptions: mockUpdateColumnOptions,
updateColumnType: mockUpdateColumnType,
}))
vi.mock('@/lib/table/columns/service', () => ({
renameColumn: mockRenameColumn,
updateColumnConstraints: mockUpdateColumnConstraints,
updateColumnCurrency: mockUpdateColumnCurrency,
updateColumnOptions: mockUpdateColumnOptions,
updateColumnType: mockUpdateColumnType,
}))
vi.mock('@/app/api/table/utils', () => ({
accessError: () => new Response('denied', { status: 403 }),
checkAccess: mockCheckAccess,
Expand All @@ -50,6 +57,7 @@ vi.mock('@/app/api/table/utils', () => ({
tableLockErrorResponse: () => null,
}))

import { OrchestrationError } from '@/lib/core/orchestration/types'
import { PATCH } from '@/app/api/table/[tableId]/columns/route'

const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111'
Expand Down Expand Up @@ -159,7 +167,10 @@ describe('PATCH /api/table/[tableId]/columns — pre-flight guards', () => {
// Stands in for the race the guards cannot close: the column stopped being
// a currency between the snapshot the guards read and this write.
mockUpdateColumnCurrency.mockRejectedValue(
new Error('Cannot set currency on column "amount" of type "string"')
new OrchestrationError(
'validation',
'Cannot set currency on column "amount" of type "string"'
)
)

const response = await patch({ name: 'renamed', currencyCode: 'USD' })
Expand Down
222 changes: 15 additions & 207 deletions apps/sim/app/api/table/[tableId]/columns/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,11 @@ import {
import { parseRequest } from '@/lib/api/server'
import { isZodError, validationErrorResponse } from '@/lib/api/server/validation'
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
import { statusForOrchestrationError } from '@/lib/core/orchestration/types'
import { generateRequestId } from '@/lib/core/utils/request'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import {
addTableColumn,
deleteColumn,
renameColumn,
updateColumnConstraints,
updateColumnCurrency,
updateColumnOptions,
updateColumnType,
} from '@/lib/table'
import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
import { columnTypeById } from '@/lib/table/column-types'
import { isSupportedCurrencyCode } from '@/lib/table/currency'
import { addTableColumn, deleteColumn } from '@/lib/table'
import { performUpdateTableColumn } from '@/lib/table/orchestration'
import {
accessError,
checkAccess,
Expand Down Expand Up @@ -120,215 +111,32 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
}

const { updates } = validated
let updatedTable = null

// A payload that repeats the current type must not go through
// `updateColumnType` — it early-returns on an unchanged type and would drop
// any `options` alongside it. Only a real type change routes there; an
// unchanged type with options routes to the options-only update.
const currentColumn = table.schema.columns.find((c) =>
columnMatchesRef(c, validated.columnName)
)
// Address every write below by the stable id, not the name: a rename folded
// into one of them must not break the next one's lookup.
const columnRef = currentColumn ? getColumnId(currentColumn) : validated.columnName
// The constraints write below is a separate, unconditional step, so it is
// the last one whenever it runs — that is the write the rename rides on.
const typeChanging = updates.type !== undefined && updates.type !== currentColumn?.type
if (!currentColumn) {
return NextResponse.json(
{ error: `Column "${validated.columnName}" not found` },
{ status: 404 }
)
}

// A retype applies and validates the constraints itself, so the separate
// constraint write only runs when the type is unchanged. The rename rides
// whichever write actually runs last.
const typedWriteRuns =
typeChanging ||
updates.currencyCode !== undefined ||
updates.options !== undefined ||
updates.multiple !== undefined
const constraintsWriteRuns =
!typedWriteRuns && (updates.required !== undefined || updates.unique !== undefined)
const renameWithTypedWrite =
updates.name && !constraintsWriteRuns ? { newName: updates.name } : {}

// Every write below is its own locked transaction, so one that is going to
// fail leaves the earlier ones committed. These guards reject the knowable
// cases up front, before any write at all.
// Gate on the type the column ENDS UP with, not on whether the type is
// changing: an options-only update on an existing select column carries the
// same hazard as a conversion does.
const resultingType = updates.type ?? currentColumn?.type
if (updates.currencyCode !== undefined) {
if (resultingType !== 'currency') {
return NextResponse.json(
{
error: `Cannot set currency on column "${validated.columnName}" of type "${resultingType}"`,
},
{ status: 400 }
)
}
if (!isSupportedCurrencyCode(updates.currencyCode)) {
return NextResponse.json(
{
error: `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`,
},
{ status: 400 }
)
}
}
// The rename runs last (see below), so a name already taken would fail after
// the typed write committed. This is the only rename failure a caller can
// cause; catching it here leaves just the concurrent-collision race, which
// no pre-flight check can close.
if (
updates.name &&
table.schema.columns.some(
(c) =>
c.name.toLowerCase() === updates.name?.toLowerCase() &&
!columnMatchesRef(c, validated.columnName)
)
) {
return NextResponse.json(
{ error: `Column "${updates.name}" already exists` },
{ status: 400 }
)
}
if (
currentColumn?.workflowGroupId &&
(updates.required !== undefined || updates.unique !== undefined)
) {
return NextResponse.json(
{
error: `Cannot change constraints on workflow-output column "${currentColumn.name}". Constraints aren't applicable to columns whose values come from workflow execution.`,
},
{ status: 400 }
)
}
if (updates.unique === true && !columnTypeById(resultingType).supportsUnique) {
const outcome = await performUpdateTableColumn({
table,
columnName: validated.columnName,
userId: authResult.userId,
updates: validated.updates,
requestId,
request,
})
if (!outcome.success || !outcome.table) {
return NextResponse.json(
{ error: `Cannot set a ${resultingType} column as unique` },
{ status: 400 }
{ error: outcome.error ?? 'Failed to update column' },
{ status: statusForOrchestrationError(outcome.errorCode) }
)
}

if (typeChanging) {
updatedTable = await updateColumnType(
{
tableId,
columnName: columnRef,
newType: updates.type as NonNullable<typeof updates.type>,
...(updates.options !== undefined ? { options: updates.options } : {}),
...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}),
...(updates.currencyCode !== undefined ? { currencyCode: updates.currencyCode } : {}),
// Forwarded so the conversion validates against the constraint this
// same request is about to set, not the column's current one.
...(updates.required !== undefined ? { required: updates.required } : {}),
...(updates.unique !== undefined ? { unique: updates.unique } : {}),
...renameWithTypedWrite,
},
requestId
)
} else if (updates.currencyCode !== undefined) {
// Re-denominating an existing currency column: schema-only, no cell
// rewrite. Reached only when the type is unchanged — a conversion INTO
// currency carries the code through `updateColumnType` above.
updatedTable = await updateColumnCurrency(
{
tableId,
columnName: columnRef,
currencyCode: updates.currencyCode,
...(updates.required !== undefined ? { required: updates.required } : {}),
...(updates.unique !== undefined ? { unique: updates.unique } : {}),
...renameWithTypedWrite,
},
requestId
)
} else if (updates.options !== undefined || updates.multiple !== undefined) {
updatedTable = await updateColumnOptions(
{
tableId,
columnName: columnRef,
options: updates.options ?? currentColumn?.options ?? [],
...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}),
// Forwarded so the removal guard validates against the constraint this
// same request is about to set, not the column's current one.
...(updates.required !== undefined ? { required: updates.required } : {}),
...(updates.unique !== undefined ? { unique: updates.unique } : {}),
...renameWithTypedWrite,
},
requestId
)
}

// Skipped whenever a typed write ran: that write already applied and
// validated these, in one transaction with the change they accompany.
if (constraintsWriteRuns) {
updatedTable = await updateColumnConstraints(
{
tableId,
columnName: columnRef,
...(updates.required !== undefined ? { required: updates.required } : {}),
...(updates.unique !== undefined ? { unique: updates.unique } : {}),
...(updates.name ? { newName: updates.name } : {}),
},
requestId
)
}

// A rename rides along with the LAST write above, inside that write's
// transaction — a rename is metadata-only (rows key on the stable column
// id), so nothing forces it to be its own write, and folding it in is what
// stops a combined request from committing one half and then failing. Only
// a rename with nothing to ride on runs standalone.
if (updates.name && !updatedTable) {
updatedTable = await renameColumn(
{ tableId, oldName: columnRef, newName: updates.name },
requestId
)
}

if (!updatedTable) {
return NextResponse.json({ error: 'No updates specified' }, { status: 400 })
}

return NextResponse.json({
success: true,
data: {
columns: updatedTable.schema.columns.map(normalizeColumn),
columns: outcome.table.schema.columns.map(normalizeColumn),
},
})
} catch (error) {
const lockError = tableLockErrorResponse(error)
if (lockError) return lockError
if (isZodError(error)) {
return validationErrorResponse(error, 'Invalid request data')
}

const msg = rootErrorMessage(error)
if (msg.includes('not found') || msg.includes('Table not found')) {
return NextResponse.json({ error: msg }, { status: 404 })
}
if (
msg.includes('already exists') ||
msg.includes('Cannot delete the last column') ||
msg.includes('Cannot set column') ||
msg.includes('Cannot set unique column') ||
msg.includes('Invalid column') ||
msg.includes('exceeds maximum') ||
msg.includes('incompatible') ||
msg.includes('duplicate') ||
msg.includes('option') ||
msg.includes('currency') ||
msg.includes('is already type')
) {
return NextResponse.json({ error: msg }, { status: 400 })
}

logger.error(`[${requestId}] Error updating column in table ${tableId}:`, error)
return NextResponse.json({ error: 'Failed to update column' }, { status: 500 })
}
Expand Down
12 changes: 8 additions & 4 deletions apps/sim/app/api/table/[tableId]/columns/run/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { TableQueryValidationError } from '@/lib/table/errors'
import { toLegacyFilter } from '@/lib/table/query-builder/converters'
import { runWorkflowColumn } from '@/lib/table/workflow-columns'
import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils'
import {
accessError,
checkAccess,
orchestrationErrorResponse,
tableFilterError,
} from '@/app/api/table/utils'

const logger = createLogger('TableRunColumnAPI')

Expand Down Expand Up @@ -66,9 +71,8 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
if (error instanceof TableQueryValidationError) {
return NextResponse.json({ error: error.message }, { status: 400 })
}
if (error instanceof Error && error.message === 'Invalid workspace ID') {
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
}
const classified = orchestrationErrorResponse(error)
if (classified) return classified
logger.error(`run-column failed:`, error)
return NextResponse.json({ error: 'Failed to run columns' }, { status: 500 })
}
Expand Down
14 changes: 11 additions & 3 deletions apps/sim/app/api/table/[tableId]/import/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ vi.mock('@/lib/table/billing', () => ({
limit >= 0 && current + added > limit,
}))

import { OrchestrationError } from '@/lib/core/orchestration/types'
import { TableLockedError } from '@/lib/table/mutation-locks'
import { POST } from '@/app/api/table/[tableId]/import/route'

Expand Down Expand Up @@ -372,7 +373,10 @@ describe('POST /api/table/[tableId]/import', () => {

it('surfaces unique violations from importAppendRows as 400', async () => {
mockImportAppendRows.mockRejectedValueOnce(
new Error('Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx')
new OrchestrationError(
'validation',
'Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx'
)
)
const response = await callPost(
createFormData(createCsvFile('name,age\nAlice,30'), { mode: 'append' })
Expand Down Expand Up @@ -516,7 +520,9 @@ describe('POST /api/table/[tableId]/import', () => {
})

it('surfaces column-creation failures from importAppendRows as 400', async () => {
mockImportAppendRows.mockRejectedValueOnce(new Error('Column "email" already exists'))
mockImportAppendRows.mockRejectedValueOnce(
new OrchestrationError('validation', 'Column "email" already exists')
)
const response = await callPost(
createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), {
mode: 'append',
Expand All @@ -529,7 +535,9 @@ describe('POST /api/table/[tableId]/import', () => {
})

it('surfaces row insert failures without success when schema was mutated', async () => {
mockImportAppendRows.mockRejectedValueOnce(new Error('must be unique'))
mockImportAppendRows.mockRejectedValueOnce(
new OrchestrationError('validation', 'must be unique')
)
const response = await callPost(
createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), {
mode: 'append',
Expand Down
Loading