Skip to content

Commit 80ce6ae

Browse files
committed
perf(table): start the workspace load with the table load when a workspace is asserted
resolveActiveTableContext ran two sequential round trips: load the table, then load the workspace it turned out to live in. When the caller asserts a workspace the second input is already in hand, so both can start together. What makes that safe is unchanged: requireTable still compares the table's canonical workspaceId against the assertion and reports a mismatch as not_found. The table outcome is inspected first and unconditionally, so any path that returns has proven the assertion equal to the canonical id, and a failing workspace load can never replace the concealing not_found. A final identity check on the loaded context restates the invariant at the point of return, so even with the first check removed the function cannot hand back a foreign workspace. Promise.allSettled keeps the discarded branch from surfacing as an unhandled rejection. With no asserted workspace the path stays sequential — the table load is what reveals which workspace to load, so there is nothing to start early. One existing assertion changed: it required the workspace load not to have been issued yet on a mismatched assertion, which is internal sequencing rather than caller-observable behaviour and is definitionally untrue once the loads overlap. The observable half is kept, and two timing tests now cover the sequencing directly. Verified to fail: removing either mismatch check, reading the workspace outcome first, and swapping allSettled for bare awaits each turn the corresponding tests red.
1 parent 1dbecd4 commit 80ce6ae

2 files changed

Lines changed: 193 additions & 10 deletions

File tree

apps/sim/lib/table/application/context.test.ts

Lines changed: 154 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44

5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

77
const { getTableById, loadWorkspace } = vi.hoisted(() => ({
88
getTableById: vi.fn(),
@@ -16,6 +16,37 @@ vi.mock('@/lib/workspaces/application/workspace-context', () => ({
1616

1717
import { resolveActiveTableContext } from '@/lib/table/application/context'
1818

19+
const WORKSPACE_ONE = {
20+
workspaceId: 'workspace-1',
21+
workspaceOrganizationId: 'organization-1',
22+
allowPersonalApiKeys: true,
23+
billedAccountUserId: 'billing-user-1',
24+
}
25+
26+
const WORKSPACE_TWO = {
27+
workspaceId: 'workspace-2',
28+
workspaceOrganizationId: 'organization-2',
29+
allowPersonalApiKeys: false,
30+
billedAccountUserId: 'billing-user-2',
31+
}
32+
33+
/** Runs `body` while capturing any unhandled promise rejection it provokes. */
34+
async function withUnhandledRejectionWatch(body: () => Promise<void>): Promise<unknown[]> {
35+
const seen: unknown[] = []
36+
const onUnhandled = (reason: unknown) => {
37+
seen.push(reason)
38+
}
39+
process.on('unhandledRejection', onUnhandled)
40+
try {
41+
await body()
42+
await new Promise((resolve) => setImmediate(resolve))
43+
await new Promise((resolve) => setImmediate(resolve))
44+
} finally {
45+
process.off('unhandledRejection', onUnhandled)
46+
}
47+
return seen
48+
}
49+
1950
describe('table application context', () => {
2051
beforeEach(() => {
2152
vi.clearAllMocks()
@@ -24,12 +55,13 @@ describe('table application context', () => {
2455
workspaceId: 'workspace-1',
2556
name: 'Contacts',
2657
})
27-
loadWorkspace.mockResolvedValue({
28-
workspaceId: 'workspace-1',
29-
workspaceOrganizationId: 'organization-1',
30-
allowPersonalApiKeys: true,
31-
billedAccountUserId: 'billing-user-1',
32-
})
58+
loadWorkspace.mockImplementation(async (workspaceId: string) =>
59+
workspaceId === 'workspace-1' ? WORKSPACE_ONE : WORKSPACE_TWO
60+
)
61+
})
62+
63+
afterEach(() => {
64+
vi.useRealTimers()
3365
})
3466

3567
it('derives workspace scope from the canonical active table', async () => {
@@ -44,13 +76,106 @@ describe('table application context', () => {
4476
expect(loadWorkspace).toHaveBeenCalledWith('workspace-1')
4577
})
4678

47-
it('conceals an asserted cross-workspace table before workspace resolution', async () => {
79+
it('starts the workspace load without waiting for the table when a workspace is asserted', async () => {
80+
let releaseTable: (table: unknown) => void = () => {}
81+
getTableById.mockImplementationOnce(
82+
() =>
83+
new Promise((resolve) => {
84+
releaseTable = resolve
85+
})
86+
)
87+
88+
const pending = resolveActiveTableContext({
89+
tableId: 'table-1',
90+
assertedWorkspaceId: 'workspace-1',
91+
})
92+
await Promise.resolve()
93+
await Promise.resolve()
94+
95+
expect(loadWorkspace).toHaveBeenCalledWith('workspace-1')
96+
97+
releaseTable({ id: 'table-1', workspaceId: 'workspace-1', name: 'Contacts' })
98+
await expect(pending).resolves.toMatchObject({ tableId: 'table-1', workspaceId: 'workspace-1' })
99+
})
100+
101+
it('waits for the table before loading a workspace when none is asserted', async () => {
102+
let releaseTable: (table: unknown) => void = () => {}
103+
getTableById.mockImplementationOnce(
104+
() =>
105+
new Promise((resolve) => {
106+
releaseTable = resolve
107+
})
108+
)
109+
110+
const pending = resolveActiveTableContext({ tableId: 'table-1' })
111+
await Promise.resolve()
112+
await Promise.resolve()
113+
114+
expect(loadWorkspace).not.toHaveBeenCalled()
115+
116+
releaseTable({ id: 'table-1', workspaceId: 'workspace-1', name: 'Contacts' })
117+
await expect(pending).resolves.toMatchObject({ tableId: 'table-1', workspaceId: 'workspace-1' })
118+
expect(loadWorkspace).toHaveBeenCalledWith('workspace-1')
119+
})
120+
121+
it('conceals an asserted cross-workspace table as not found', async () => {
48122
await expect(
49123
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-2' })
50124
).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' })
125+
})
126+
127+
it('conceals a table that does not exist at all', async () => {
128+
getTableById.mockResolvedValueOnce(null)
129+
130+
await expect(
131+
resolveActiveTableContext({ tableId: 'missing', assertedWorkspaceId: 'workspace-1' })
132+
).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' })
133+
})
134+
135+
it('conceals a missing table with no asserted workspace', async () => {
136+
getTableById.mockResolvedValueOnce(null)
137+
138+
await expect(resolveActiveTableContext({ tableId: 'missing' })).rejects.toMatchObject({
139+
code: 'not_found',
140+
message: 'Table not found',
141+
})
51142
expect(loadWorkspace).not.toHaveBeenCalled()
52143
})
53144

145+
it('surfaces not_found rather than a failing workspace load on a mismatched assertion', async () => {
146+
const failure = new Error('workspace database unavailable')
147+
loadWorkspace.mockRejectedValueOnce(failure)
148+
149+
const unhandled = await withUnhandledRejectionWatch(async () => {
150+
await expect(
151+
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-2' })
152+
).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' })
153+
})
154+
155+
expect(unhandled).toEqual([])
156+
})
157+
158+
it('surfaces not_found rather than a failing table load on a matched assertion', async () => {
159+
const failure = new Error('table database unavailable')
160+
getTableById.mockRejectedValueOnce(failure)
161+
162+
const unhandled = await withUnhandledRejectionWatch(async () => {
163+
await expect(
164+
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' })
165+
).rejects.toBe(failure)
166+
})
167+
168+
expect(unhandled).toEqual([])
169+
})
170+
171+
it('refuses a workspace context that is not the canonical workspace of the table', async () => {
172+
loadWorkspace.mockResolvedValueOnce(WORKSPACE_TWO)
173+
174+
await expect(
175+
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' })
176+
).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' })
177+
})
178+
54179
it('fails when the canonical workspace is unavailable', async () => {
55180
loadWorkspace.mockResolvedValueOnce(null)
56181

@@ -60,10 +185,31 @@ describe('table application context', () => {
60185
})
61186
})
62187

188+
it('fails when the canonical workspace is unavailable on the asserted path', async () => {
189+
loadWorkspace.mockResolvedValueOnce(null)
190+
191+
await expect(
192+
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' })
193+
).rejects.toMatchObject({ code: 'not_found', message: 'Workspace not found' })
194+
})
195+
63196
it('propagates canonical workspace database failures', async () => {
64197
const failure = new Error('workspace database unavailable')
65198
loadWorkspace.mockRejectedValueOnce(failure)
66199

67200
await expect(resolveActiveTableContext({ tableId: 'table-1' })).rejects.toBe(failure)
68201
})
202+
203+
it('propagates canonical workspace database failures on the asserted path', async () => {
204+
const failure = new Error('workspace database unavailable')
205+
loadWorkspace.mockRejectedValueOnce(failure)
206+
207+
const unhandled = await withUnhandledRejectionWatch(async () => {
208+
await expect(
209+
resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' })
210+
).rejects.toBe(failure)
211+
})
212+
213+
expect(unhandled).toEqual([])
214+
})
69215
})

apps/sim/lib/table/application/context.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,49 @@ async function requireTable(tableId: string, workspaceId: string | undefined) {
3232
return table
3333
}
3434

35+
/**
36+
* Loads the canonical context a table use case authorizes against.
37+
*
38+
* When the caller asserts a workspace, the workspace load no longer has to wait for the table
39+
* load: the asserted id is already in hand, so both round trips start together. What makes that
40+
* safe is that {@link requireTable} still compares the table's canonical `workspaceId` against
41+
* the assertion and reports any mismatch as `not_found`. The table outcome is inspected first and
42+
* unconditionally, so on every path that returns, the assertion has been *proven* equal to the
43+
* canonical workspace id — the context handed back is the table's own workspace, never a
44+
* workspace the caller merely named. A mismatch throws before the workspace outcome is read, so a
45+
* failing workspace load can never replace the concealing `not_found`. `Promise.allSettled` keeps
46+
* the branch that is thrown away from surfacing as an unhandled rejection. A final identity check
47+
* on the loaded context restates the invariant at the point of return, so the value handed back is
48+
* only ever a context whose own `workspaceId` is the table's.
49+
*
50+
* Without an asserted id there is nothing to start early — the table load is what reveals which
51+
* workspace to load — so that path stays sequential.
52+
*/
3553
export async function resolveActiveTableContext(input: {
3654
tableId: string
3755
assertedWorkspaceId?: string
3856
}): Promise<ActiveTableContext> {
39-
const table = await requireTable(input.tableId, input.assertedWorkspaceId)
40-
const workspaceContext = await resolveTableWorkspaceContext(table.workspaceId)
57+
const { tableId, assertedWorkspaceId } = input
58+
59+
if (assertedWorkspaceId === undefined) {
60+
const table = await requireTable(tableId, undefined)
61+
const workspaceContext = await resolveTableWorkspaceContext(table.workspaceId)
62+
return { ...workspaceContext, tableId: table.id, table }
63+
}
64+
65+
const [tableOutcome, workspaceOutcome] = await Promise.allSettled([
66+
requireTable(tableId, assertedWorkspaceId),
67+
resolveTableWorkspaceContext(assertedWorkspaceId),
68+
])
69+
70+
if (tableOutcome.status === 'rejected') throw tableOutcome.reason
71+
if (workspaceOutcome.status === 'rejected') throw workspaceOutcome.reason
72+
73+
const table = tableOutcome.value
74+
const workspaceContext = workspaceOutcome.value
75+
if (workspaceContext.workspaceId !== table.workspaceId) {
76+
throw new OrchestrationError('not_found', 'Table not found')
77+
}
4178
return { ...workspaceContext, tableId: table.id, table }
4279
}
4380

0 commit comments

Comments
 (0)