Skip to content
Merged
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
2 changes: 2 additions & 0 deletions apps/sim/lib/table/__tests__/service-filter-threading.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ vi.mock('@/lib/table/workflow-columns', () => ({
vi.mock('@/lib/table/validation', () => ({
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
coerceRowToSchema: vi.fn(() => ({ valid: true, errors: [] })),
coerceRowValues: vi.fn(),
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
getUniqueColumns: vi.fn(() => []),
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/lib/table/__tests__/update-row.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ vi.mock('@sim/db', () => dbChainMock)
vi.mock('@/lib/table/validation', () => ({
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
coerceRowToSchema: vi.fn(() => ({ valid: true, errors: [] })),
coerceRowValues: vi.fn(),
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
getUniqueColumns: vi.fn(() => []),
Expand Down
123 changes: 123 additions & 0 deletions apps/sim/lib/table/__tests__/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { describe, expect, it } from 'vitest'
import { TABLE_LIMITS } from '../constants'
import {
type ColumnDefinition,
coerceRowToSchema,
coerceRowValues,
getUniqueColumns,
type TableSchema,
validateColumnDefinition,
Expand Down Expand Up @@ -277,6 +279,127 @@ describe('Validation', () => {
})
})

describe('coerceRowToSchema', () => {
const schema: TableSchema = {
columns: [
{ name: 'name', type: 'string', required: true },
{ name: 'age', type: 'number' },
{ name: 'founded', type: 'number', required: true },
{ name: 'active', type: 'boolean' },
{ name: 'created', type: 'date' },
{ name: 'metadata', type: 'json' },
],
}

it('coerces a numeric string to a number in place', () => {
const data = { name: 'Acme', founded: '1999' }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.founded).toBe(1999)
})

it('nulls an un-coercible value for an optional number column', () => {
const data = { name: 'Acme', founded: 2000, age: 'unknown' }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.age).toBeNull()
})

it('rejects an un-coercible value for a required number column', () => {
const data = { name: 'Acme', founded: 'unknown' }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(false)
expect(result.errors[0]).toContain('founded must be number')
expect(data.founded).toBe('unknown')
})

it('coerces a number to a string for a string column', () => {
const data = { name: 12345, founded: 2000 }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.name).toBe('12345')
})

it('coerces "true"/"false" strings to booleans', () => {
const data = { name: 'Acme', founded: 2000, active: 'false' }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.active).toBe(false)
})

it('coerces an epoch number to an ISO date string', () => {
const epoch = Date.parse('2024-01-15T00:00:00Z')
const data = { name: 'Acme', founded: 2000, created: epoch }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.created).toBe(new Date(epoch).toISOString())
})

it('coerces a Date instance to an ISO date string', () => {
const date = new Date('2024-01-15T00:00:00Z')
const data = { name: 'Acme', founded: 2000, created: date }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.created).toBe(date.toISOString())
})

it('nulls an out-of-range epoch number for an optional date column without throwing', () => {
const data = { name: 'Acme', founded: 2000, created: 1e20 }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.created).toBeNull()
})

it('nulls an invalid Date instance for an optional date column without throwing', () => {
const data = { name: 'Acme', founded: 2000, created: new Date('not-a-date') }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data.created).toBeNull()
})

it('leaves already-correct values untouched and passes through json', () => {
const data = { name: 'Acme', founded: 2000, metadata: { k: 'v' } }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(true)
expect(data).toEqual({ name: 'Acme', founded: 2000, metadata: { k: 'v' } })
})

it('still rejects a missing required field', () => {
const data = { name: 'Acme' }
const result = coerceRowToSchema(data, schema)
expect(result.valid).toBe(false)
expect(result.errors).toContain('Missing required field: founded')
})
})

describe('coerceRowValues', () => {
const schema: TableSchema = {
columns: [
{ name: 'name', type: 'string', required: true },
{ name: 'founded', type: 'number', required: true },
{ name: 'age', type: 'number' },
],
}

it('coerces a partial patch in place without flagging absent required fields', () => {
const patch = { age: '42' }
coerceRowValues(patch, schema)
expect(patch.age).toBe(42)
})

it('nulls an un-coercible optional value in a patch', () => {
const patch: { age: unknown } = { age: 'nope' }
coerceRowValues(patch as never, schema)
expect(patch.age).toBeNull()
})

it('leaves an un-coercible required value in place for downstream validation', () => {
const patch: { founded: unknown } = { founded: 'nope' }
coerceRowValues(patch as never, schema)
expect(patch.founded).toBe('nope')
})
})

describe('getUniqueColumns', () => {
it('should return only columns with unique=true', () => {
const schema: TableSchema = {
Expand Down
39 changes: 24 additions & 15 deletions apps/sim/lib/table/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ import type {
import {
checkBatchUniqueConstraintsDb,
checkUniqueConstraintsDb,
coerceRowToSchema,
coerceRowValues,
getUniqueColumns,
validateRowAgainstSchema,
validateRowSize,
validateTableName,
validateTableSchema,
Expand Down Expand Up @@ -913,7 +914,7 @@ export async function insertRow(
}

// Validate against schema
const schemaValidation = validateRowAgainstSchema(data.data, table.schema)
const schemaValidation = coerceRowToSchema(data.data, table.schema)
if (!schemaValidation.valid) {
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -1060,7 +1061,7 @@ export async function batchInsertRowsWithTx(
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
}

const schemaValidation = validateRowAgainstSchema(row, table.schema)
const schemaValidation = coerceRowToSchema(row, table.schema)
if (!schemaValidation.valid) {
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -1201,7 +1202,7 @@ export async function replaceTableRowsWithTx(
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
}

const schemaValidation = validateRowAgainstSchema(row, table.schema)
const schemaValidation = coerceRowToSchema(row, table.schema)
if (!schemaValidation.valid) {
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -1331,22 +1332,24 @@ export async function upsertRow(
)
}

const targetValue = data.data[targetColumnName]
if (targetValue === undefined || targetValue === null) {
throw new Error(`Upsert requires a value for the conflict target column "${targetColumnName}"`)
}

// Validate row data
const sizeValidation = validateRowSize(data.data)
if (!sizeValidation.valid) {
throw new Error(sizeValidation.errors.join(', '))
}

const schemaValidation = validateRowAgainstSchema(data.data, schema)
const schemaValidation = coerceRowToSchema(data.data, schema)
Comment thread
cursor[bot] marked this conversation as resolved.
if (!schemaValidation.valid) {
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
}

// Read the conflict-target value *after* coercion so `matchFilter` branches on
// the persisted type (e.g. a coerced `"123"` → `123` matches existing rows).
const targetValue = data.data[targetColumnName]
if (targetValue === undefined || targetValue === null) {
throw new Error(`Upsert requires a value for the conflict target column "${targetColumnName}"`)
}

// `data->` and `data->>` accept the JSON key as a parameterized text value;
// no need for `sql.raw` interpolation.
const matchFilter =
Expand Down Expand Up @@ -1957,7 +1960,7 @@ export async function updateRow(
}

// Validate against schema
const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
if (!schemaValidation.valid) {
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -2167,6 +2170,12 @@ export async function updateRowsByFilter(
return { affectedCount: 0, affectedRowIds: [] }
}

// Coerce the patch itself in place — the write below persists `data.data`
// (as `patchJson`), so coercing only the per-row merged copies would be
// discarded. The merged validation in the loop still enforces required
// fields against the full row.
coerceRowValues(data.data, table.schema)

for (const row of matchingRows) {
const existingData = row.data as RowData
const mergedData = { ...existingData, ...data.data }
Expand All @@ -2176,7 +2185,7 @@ export async function updateRowsByFilter(
throw new Error(`Row ${row.id}: ${sizeValidation.errors.join(', ')}`)
}

const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
Comment thread
cursor[bot] marked this conversation as resolved.
if (!schemaValidation.valid) {
throw new Error(`Row ${row.id}: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -2334,7 +2343,7 @@ export async function batchUpdateRows(
throw new Error(`Row ${update.rowId}: ${sizeValidation.errors.join(', ')}`)
}

const schemaValidation = validateRowAgainstSchema(merged, table.schema)
const schemaValidation = coerceRowToSchema(merged, table.schema)
if (!schemaValidation.valid) {
throw new Error(`Row ${update.rowId}: ${schemaValidation.errors.join(', ')}`)
}
Expand Down Expand Up @@ -3247,8 +3256,8 @@ export async function updateWorkflowGroup(

// Resolve the new leaf type for each remap so the column's declared type
// matches what the new mapping produces. Without this, a string→number
// remap would keep `type: 'string'` and validateRowAgainstSchema would
// reject every backfilled value.
// remap would keep `type: 'string'` and coerceRowToSchema would coerce
// every backfilled value toward the wrong type.
try {
const [
{ loadWorkflowFromNormalizedTables },
Expand Down
96 changes: 93 additions & 3 deletions apps/sim/lib/table/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { userTableRows } from '@sim/db/schema'
import { and, eq, or, sql } from 'drizzle-orm'
import { NextResponse } from 'next/server'
import { COLUMN_TYPES, NAME_PATTERN, TABLE_LIMITS } from './constants'
import type { ColumnDefinition, RowData, TableSchema, ValidationResult } from './types'
import type { ColumnDefinition, JsonValue, RowData, TableSchema, ValidationResult } from './types'

export type { ColumnDefinition, TableSchema, ValidationResult }

Expand Down Expand Up @@ -57,7 +57,7 @@ export async function validateRowData(
}
}

const schemaValidation = validateRowAgainstSchema(rowData, schema)
const schemaValidation = coerceRowToSchema(rowData, schema)
if (!schemaValidation.valid) {
return {
valid: false,
Expand Down Expand Up @@ -105,7 +105,7 @@ export async function validateBatchRows(
continue
}

const schemaValidation = validateRowAgainstSchema(rowData, schema)
const schemaValidation = coerceRowToSchema(rowData, schema)
if (!schemaValidation.valid) {
errors.push({ row: i, errors: schemaValidation.errors })
}
Expand Down Expand Up @@ -255,6 +255,96 @@ export function validateRowAgainstSchema(data: RowData, schema: TableSchema): Va
return { valid: errors.length === 0, errors }
}

/**
* Attempts to coerce a non-null value to a column's declared type. Returns the
* coerced value when the value already matches or can be converted without
* ambiguity (e.g. the string `"1999"` to the number `1999`), and `ok: false`
* when no safe conversion exists.
*/
function coerceValueToColumnType(
value: JsonValue,
type: ColumnDefinition['type']
): { ok: true; value: JsonValue } | { ok: false } {
switch (type) {
case 'string':
if (typeof value === 'string') return { ok: true, value }
if (typeof value === 'number' || typeof value === 'boolean') {
return { ok: true, value: String(value) }
}
return { ok: false }
case 'number':
if (typeof value === 'number') {
return Number.isFinite(value) ? { ok: true, value } : { ok: false }
}
if (typeof value === 'string' && value.trim() !== '') {
const parsed = Number(value)
return Number.isFinite(parsed) ? { ok: true, value: parsed } : { ok: false }
}
return { ok: false }
case 'boolean':
if (typeof value === 'boolean') return { ok: true, value }
if (typeof value === 'string') {
const normalized = value.trim().toLowerCase()
if (normalized === 'true') return { ok: true, value: true }
if (normalized === 'false') return { ok: true, value: false }
}
return { ok: false }
case 'date': {
if (typeof value === 'string' && !Number.isNaN(Date.parse(value))) return { ok: true, value }
// Date instances and epoch numbers may still be out of the representable
// range (>±8.64e15ms) — guard `toISOString()`, which throws RangeError on
// an Invalid Date, so an over-range value degrades to `{ ok: false }`
// rather than crashing the write.
const date =
value instanceof Date ? value : typeof value === 'number' ? new Date(value) : null
if (date && !Number.isNaN(date.getTime())) return { ok: true, value: date.toISOString() }
return { ok: false }
}
default:
return { ok: true, value }
}
}

/**
* Coerces each present value in `data` toward its column's declared type **in
* place**. Values that already match are untouched; unambiguous conversions
* (e.g. `"1999"` → `1999`) are applied; values that cannot be coerced are set to
* `null` when the column is optional, or left in place when required (so a
* subsequent {@link validateRowAgainstSchema} reports them).
*
* Operates per-present-column, so it is safe on a partial patch (columns absent
* from `data` are skipped — it never invents a missing-required-field error).
*/
export function coerceRowValues(data: RowData, schema: TableSchema): void {
for (const column of schema.columns) {
const value = data[column.name]
if (value === null || value === undefined) continue

const coerced = coerceValueToColumnType(value, column.type)
if (coerced.ok) {
data[column.name] = coerced.value
} else if (!column.required) {
data[column.name] = null
}
}
}

/**
* Coerces a full row toward its schema **in place** (see {@link coerceRowValues})
* then validates the result.
*
* This is the write-path entry point — callers that persist a complete row use
* it instead of {@link validateRowAgainstSchema} so a single off-type field (a
* tool returning `"unknown"` for a numeric column, say) nulls that one cell
* rather than failing the entire row write. Callers persisting only a partial
* patch should use {@link coerceRowValues} on the patch and validate the merged
* row separately.
*/
export function coerceRowToSchema(data: RowData, schema: TableSchema): ValidationResult {
coerceRowValues(data, schema)
return validateRowAgainstSchema(data, schema)
}

/** Validates row data size is within limits. */
export function validateRowSize(data: RowData): ValidationResult {
const size = JSON.stringify(data).length
Expand Down
Loading