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): typed predicate filter grammar, cursor pagination, and the v2 table surface#6067
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
47492ab4c63dcfd74c0c0a048d02da060340696861ded16c48f686225d11720740ca2c2106332295d98fde74b64eaf41799b5fe21ead6391705ea6f0df4d58b3f31961564fdd4f9c104e885854f2d964f79d0dd0e2685621d502082546a093dfaaadd62ad329d8fa6c3ebb639c66ecef808bc0d96d49b8File 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
Large diffs are not rendered by default.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -5,6 +5,8 @@ import { parseRequest } from '@/lib/api/server' | ||
| import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' | ||
| import { generateRequestId } from '@/lib/core/utils/request' | ||
| 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' | ||
| @@ -25,13 +27,23 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| const parsed = await parseRequest(runColumnContract, request, { params }) | ||
| if (!parsed.success) return parsed.response | ||
| const { tableId } = parsed.data.params | ||
| const { workspaceId, groupIds, runMode, rowIds, filter, excludeRowIds, limit } = | ||
| parsed.data.body | ||
| const { | ||
| workspaceId, | ||
| groupIds, | ||
| runMode, | ||
| rowIds, | ||
| filter: wireFilter, | ||
| excludeRowIds, | ||
| limit, | ||
| } = parsed.data.body | ||
| // Dual-grammar wire: downgrade a predicate to the legacy Filter the | ||
| // dispatcher and scheduled runs still compile. | ||
| const filter = toLegacyFilter(wireFilter) | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const access = await checkAccess(tableId, auth.userId, 'write') | ||
| if (!access.ok) return accessError(access, requestId, tableId) | ||
| // Validate the filter up front (the dispatcher reuses it) so a bad field fails fast. | ||
| const filterError = tableFilterError(filter, access.table.schema.columns) | ||
| const filterError = tableFilterError(wireFilter, access.table.schema.columns) | ||
| if (filterError) return filterError | ||
| const { dispatchId } = await runWorkflowColumn({ | ||
| @@ -49,6 +61,11 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| return NextResponse.json({ success: true, data: { dispatchId } }) | ||
| } catch (error) { | ||
| // A predicate that Zod accepts but the downgrade rejects (hybrid node, | ||
| // eq-with-array, valueless op) is caller error, not a server fault. | ||
| 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 }) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,9 +8,12 @@ import { isTriggerDevEnabled } from '@/lib/core/config/env-flags' | ||
| import { runDetached } from '@/lib/core/utils/background' | ||
| import { generateRequestId } from '@/lib/core/utils/request' | ||
| import { withRouteHandler } from '@/lib/core/utils/with-route-handler' | ||
| import type { Filter } from '@/lib/table' | ||
| import { markTableDeleteFailed, runTableDelete } from '@/lib/table/delete-runner' | ||
| import { TableQueryValidationError } from '@/lib/table/errors' | ||
| import { markTableJobRunning, releaseJobClaim } from '@/lib/table/jobs/service' | ||
| import { assertRowDelete } from '@/lib/table/mutation-locks' | ||
| import { toLegacyFilter } from '@/lib/table/query-builder/converters' | ||
| import type { TableDeleteJobPayload } from '@/lib/table/types' | ||
| import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils' | ||
| @@ -43,7 +46,20 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| const parsed = await parseRequest(deleteTableRowsAsyncContract, request, { params }) | ||
| if (!parsed.success) return parsed.response | ||
| const { tableId } = parsed.data.params | ||
| const { workspaceId, filter, excludeRowIds, estimatedCount } = parsed.data.body | ||
| const { workspaceId, filter: wireFilter, excludeRowIds, estimatedCount } = parsed.data.body | ||
| // Dual-grammar wire: a predicate downgrades losslessly-or-throws to the | ||
| // legacy Filter the runners/persisted payloads still compile. A shape the | ||
| // union accepted but the downgrade rejects (hybrid node, eq-with-array) is | ||
| // caller error — 400, never the generic 500. | ||
| let filter: Filter | undefined | ||
| try { | ||
| filter = toLegacyFilter(wireFilter) | ||
| } catch (error) { | ||
| if (error instanceof TableQueryValidationError) { | ||
| return NextResponse.json({ error: error.message }, { status: 400 }) | ||
| } | ||
| throw error | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const access = await checkAccess(tableId, userId, 'write') | ||
| if (!access.ok) return accessError(access, requestId, tableId) | ||
| @@ -62,7 +78,7 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro | ||
| assertRowDelete(table) | ||
| // Validate the filter up front so the caller gets immediate feedback (the worker reuses it). | ||
| const filterError = tableFilterError(filter, table.schema.columns) | ||
| const filterError = tableFilterError(wireFilter, table.schema.columns) | ||
| if (filterError) return filterError | ||
| // Rows inserted after this instant are spared (created_at <= cutoff in the worker). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| * | ||
| * v2 query route: predicate parsing, unconditional name→id translation | ||
| * (session auth included — the string grammar is name-keyed for every caller), | ||
| * cursor validation, and the response envelope. | ||
| */ | ||
| import { hybridAuthMockFns } from '@sim/testing' | ||
| import { NextRequest } from 'next/server' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import type { TableDefinition } from '@/lib/table/types' | ||
| const { mockCheckAccess, mockQueryRows, mockGate } = vi.hoisted(() => ({ | ||
| mockCheckAccess: vi.fn(), | ||
| mockQueryRows: vi.fn(), | ||
| mockGate: vi.fn(), | ||
| })) | ||
| vi.mock('@/app/api/table/utils', async () => { | ||
| const { NextResponse } = await import('next/server') | ||
| return { | ||
| checkAccess: mockCheckAccess, | ||
| accessError: (result: { status: number }) => | ||
| NextResponse.json({ error: 'Access denied' }, { status: result.status }), | ||
| tablesV2GateError: mockGate, | ||
| } | ||
| }) | ||
| vi.mock('@/lib/table', async () => { | ||
| // row-wire pulls the column-keys helpers through this barrel. | ||
| const columnKeys = await import('@/lib/table/column-keys') | ||
| return { ...columnKeys } | ||
| }) | ||
| vi.mock('@/lib/table/rows/service', () => ({ | ||
| queryRows: mockQueryRows, | ||
| })) | ||
| import { encodeCursor } from '@/lib/table/rows/cursor' | ||
| import { POST } from '@/app/api/table/[tableId]/query/route' | ||
| function buildTable(): TableDefinition { | ||
| return { | ||
| id: 'tbl_1', | ||
| name: 'People', | ||
| description: null, | ||
| schema: { | ||
| columns: [ | ||
| { id: 'col_aaa', name: 'name', type: 'string' }, | ||
| { id: 'col_bbb', name: 'wins', type: 'number' }, | ||
| ], | ||
| }, | ||
| metadata: null, | ||
| rowCount: 0, | ||
| maxRows: 100, | ||
| workspaceId: 'workspace-1', | ||
| createdBy: 'user-1', | ||
| archivedAt: null, | ||
| createdAt: new Date('2024-01-01'), | ||
| updatedAt: new Date('2024-01-01'), | ||
| } | ||
| } | ||
| function authAs(authType: 'session' | 'internal_jwt') { | ||
| hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ | ||
| success: true, | ||
| userId: 'user-1', | ||
| authType, | ||
| }) | ||
| } | ||
| function callQuery(body: Record<string, unknown>) { | ||
| const req = new NextRequest('http://localhost:3000/api/table/tbl_1/query', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(body), | ||
| }) | ||
| return POST(req, { params: Promise.resolve({ tableId: 'tbl_1' }) }) | ||
| } | ||
| const EMPTY_RESULT = { | ||
| rows: [], | ||
| rowCount: 0, | ||
| totalCount: 0, | ||
| limit: 0, | ||
| offset: 0, | ||
| nextCursor: null, | ||
| } | ||
| describe('POST /api/table/[tableId]/query', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) | ||
| mockQueryRows.mockResolvedValue(EMPTY_RESULT) | ||
| mockGate.mockResolvedValue(null) | ||
| }) | ||
| it('returns 404 when the tables-v2-api flag is off', async () => { | ||
| const { NextResponse } = await import('next/server') | ||
| authAs('session') | ||
| mockGate.mockResolvedValue(NextResponse.json({ error: 'Not found' }, { status: 404 })) | ||
| const res = await callQuery({ workspaceId: 'workspace-1' }) | ||
| expect(res.status).toBe(404) | ||
| expect(mockQueryRows).not.toHaveBeenCalled() | ||
| }) | ||
| it('runs the flag gate only after the access check, so it cannot leak a cohort oracle', async () => { | ||
| authAs('session') | ||
| mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) | ||
| const res = await callQuery({ workspaceId: 'workspace-1' }) | ||
| expect(res.status).toBe(403) | ||
| expect(mockGate).not.toHaveBeenCalled() | ||
| }) | ||
| it('translates predicate/sort column names to storage ids for SESSION auth too', async () => { | ||
| authAs('session') | ||
| const res = await callQuery({ | ||
| workspaceId: 'workspace-1', | ||
| predicate: { | ||
| all: [ | ||
| { field: 'name', op: 'eq', value: 'John' }, | ||
| { field: 'wins', op: 'gte', value: 10 }, | ||
| ], | ||
| }, | ||
| sort: [{ field: 'wins', direction: 'desc' }], | ||
| }) | ||
| expect(res.status).toBe(200) | ||
| const options = mockQueryRows.mock.calls[0][1] | ||
| expect(options.predicate).toEqual({ | ||
| all: [ | ||
| { field: 'col_aaa', op: 'eq', value: 'John' }, | ||
| { field: 'col_bbb', op: 'gte', value: 10 }, | ||
| ], | ||
| }) | ||
| expect(options.sort).toEqual({ col_bbb: 'desc' }) | ||
| expect(options.withExecutions).toBe(false) | ||
| }) | ||
| it('rejects a keyset cursor combined with a custom sort', async () => { | ||
| authAs('internal_jwt') | ||
| const cursor = encodeCursor({ | ||
| lastRow: { id: 'row_1', orderKey: 'a1' }, | ||
| keysetValid: true, | ||
| nextOffset: 1, | ||
| }) | ||
| const res = await callQuery({ | ||
| workspaceId: 'workspace-1', | ||
| sort: [{ field: 'wins', direction: 'desc' }], | ||
| cursor, | ||
| }) | ||
| expect(res.status).toBe(400) | ||
| const body = await res.json() | ||
| expect(body.error).toMatch(/not valid for a sorted query/) | ||
| expect(body.code).toBe('CURSOR_SORT_CONFLICT') | ||
| expect(mockQueryRows).not.toHaveBeenCalled() | ||
| }) | ||
| it('returns 400 (not 500) for a cursor that decodes to a JSON primitive', async () => { | ||
| authAs('internal_jwt') | ||
| const res = await callQuery({ | ||
| workspaceId: 'workspace-1', | ||
| cursor: Buffer.from('42').toString('base64url'), | ||
| }) | ||
| expect(res.status).toBe(400) | ||
| const body = await res.json() | ||
| expect(body.error).toBe('Invalid cursor') | ||
| expect(body.code).toBe('INVALID_CURSOR') | ||
| }) | ||
| it('returns 400 for a predicate referencing an unknown column', async () => { | ||
| authAs('internal_jwt') | ||
| const res = await callQuery({ | ||
| workspaceId: 'workspace-1', | ||
| predicate: { all: [{ field: 'nope', op: 'eq', value: 1 }] }, | ||
| }) | ||
| expect(res.status).toBe(400) | ||
| expect((await res.json()).error).toMatch(/Unknown filter column/) | ||
| }) | ||
| it('passes nextCursor through the response envelope and skips the count on later pages', async () => { | ||
| authAs('internal_jwt') | ||
| mockQueryRows.mockResolvedValue({ ...EMPTY_RESULT, nextCursor: 'tok' }) | ||
| const cursor = encodeCursor({ | ||
| lastRow: { id: 'row_1', orderKey: 'a1' }, | ||
| keysetValid: true, | ||
| nextOffset: 1, | ||
| }) | ||
| const res = await callQuery({ workspaceId: 'workspace-1', cursor }) | ||
| expect(res.status).toBe(200) | ||
| const body = await res.json() | ||
| expect(body.data.nextCursor).toBe('tok') | ||
| const options = mockQueryRows.mock.calls[0][1] | ||
| expect(options.includeTotal).toBe(false) | ||
| expect(options.after).toEqual({ orderKey: 'a1', id: 'row_1' }) | ||
| }) | ||
| }) |
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.