Skip to content

Commit 8a224be

Browse files
authored
fix(files): read the Files prefetch from the data layer and bound invitation previews (#6415)
Two unrelated load-time fixes. On a hard refresh of /files the folders painted first and the files a beat later. Both are prefetched and hydrated together, so the files entry was not reaching the client. Each read went to its own route over an internal HTTP request; prefetchQuery swallows a rejection and shouldDehydrateQuery drops the errored entry, so a failure there silently shipped a page with that list missing, and the files read is the heavier of the two. Those two reads now call the data layer. Note the staging logs show no errors from that route, so this removes the failure mode without proving it was the one firing — the request it drops from the render path, and the shape fix below, stand on their own. listWorkspaceFilesWithShares is shared by the route and the prefetch and shapes its result through the route contract's response schema. listWorkspaceFiles returns contentUpdatedAt, which the schema neither declares nor passes through, so the prefetch was caching a field a client fetch never has and that vanished on the next refetch. The reads carry no authorization of their own now that they bypass the route, so the prefetch proves the viewer first. It reuses the layout's cached host-context lookup rather than re-deriving the permission, so the gate costs no extra queries. Separately, GET /api/invitations computed join previews in a serial loop and each preview issues up to three queries of its own, putting all of them on the critical path of the workspace switcher opening. Bounded with mapWithConcurrency; the mapper was already total, which is what that helper requires.
1 parent 3b0651e commit 8a224be

9 files changed

Lines changed: 332 additions & 82 deletions

File tree

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mockGetInvitationJoinPreview, mockGetSession, mockListPendingInvitationsForEmail } =
8+
vi.hoisted(() => ({
9+
mockGetInvitationJoinPreview: vi.fn(),
10+
mockGetSession: vi.fn(),
11+
mockListPendingInvitationsForEmail: vi.fn(),
12+
}))
13+
14+
vi.mock('@/lib/auth', () => ({
15+
auth: { api: { getSession: vi.fn() } },
16+
getSession: mockGetSession,
17+
}))
18+
19+
vi.mock('@/lib/invitations/core', () => ({
20+
getInvitationJoinPreview: mockGetInvitationJoinPreview,
21+
listPendingInvitationsForEmail: mockListPendingInvitationsForEmail,
22+
}))
23+
24+
import { GET } from '@/app/api/invitations/route'
25+
26+
function invitation(id: string) {
27+
return {
28+
id,
29+
kind: 'organization',
30+
email: 'invitee@example.com',
31+
organizationId: 'org-1',
32+
organizationName: 'Org',
33+
membershipIntent: 'member',
34+
role: 'member',
35+
status: 'pending',
36+
expiresAt: new Date('2026-02-01T00:00:00.000Z'),
37+
createdAt: new Date('2026-01-01T00:00:00.000Z'),
38+
inviterName: 'Ada',
39+
inviterEmail: 'ada@example.com',
40+
grants: [{ workspaceId: 'ws-1', workspaceName: 'WS', permission: 'read' }],
41+
}
42+
}
43+
44+
describe('GET /api/invitations', () => {
45+
beforeEach(() => {
46+
vi.clearAllMocks()
47+
mockGetSession.mockResolvedValue({
48+
user: { id: 'user-1', email: 'invitee@example.com' },
49+
})
50+
})
51+
52+
it('pairs each row with its own preview, in order', async () => {
53+
mockListPendingInvitationsForEmail.mockResolvedValue(['a', 'b', 'c'].map(invitation))
54+
mockGetInvitationJoinPreview.mockImplementation(async (_userId, inv) => ({ for: inv.id }))
55+
56+
const { invitations } = await (await GET(createMockRequest('GET'))).json()
57+
58+
expect(invitations.map((i: { id: string }) => i.id)).toEqual(['a', 'b', 'c'])
59+
expect(invitations.map((i: { joinPreview: unknown }) => i.joinPreview)).toEqual([
60+
{ for: 'a' },
61+
{ for: 'b' },
62+
{ for: 'c' },
63+
])
64+
})
65+
66+
/**
67+
* The preview is disclosure-only, so one failing row degrades to `null` rather than hiding
68+
* the invitation — which is what makes the bounded mapper safe, since it fails
69+
* all-or-nothing on a throwing mapper.
70+
*/
71+
it('degrades a failing preview to null without dropping the invitation', async () => {
72+
mockListPendingInvitationsForEmail.mockResolvedValue(['a', 'b'].map(invitation))
73+
mockGetInvitationJoinPreview.mockImplementation(async (_userId, inv) => {
74+
if (inv.id === 'a') throw new Error('preview blew up')
75+
return { for: inv.id }
76+
})
77+
78+
const { invitations } = await (await GET(createMockRequest('GET'))).json()
79+
80+
expect(invitations).toHaveLength(2)
81+
expect(invitations[0].joinPreview).toBeNull()
82+
expect(invitations[1].joinPreview).toEqual({ for: 'b' })
83+
})
84+
85+
/**
86+
* Each preview issues several queries of its own, so the fan-out stays bounded rather than
87+
* holding one pooled connection per pending invitation.
88+
*/
89+
it('runs previews concurrently, up to a bound', async () => {
90+
mockListPendingInvitationsForEmail.mockResolvedValue(
91+
Array.from({ length: 12 }, (_, i) => invitation(`inv-${i}`))
92+
)
93+
let inFlight = 0
94+
let peak = 0
95+
mockGetInvitationJoinPreview.mockImplementation(async () => {
96+
inFlight++
97+
peak = Math.max(peak, inFlight)
98+
await Promise.resolve()
99+
inFlight--
100+
return null
101+
})
102+
103+
await GET(createMockRequest('GET'))
104+
105+
expect(mockGetInvitationJoinPreview).toHaveBeenCalledTimes(12)
106+
expect(peak).toBe(4)
107+
})
108+
})

apps/sim/app/api/invitations/route.ts

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ import { createLogger } from '@sim/logger'
22
import { NextResponse } from 'next/server'
33
import type { MyInvitation } from '@/lib/api/contracts/invitations'
44
import { getSession } from '@/lib/auth'
5+
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
56
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
67
import { getInvitationJoinPreview, listPendingInvitationsForEmail } from '@/lib/invitations/core'
78

89
const logger = createLogger('MyInvitationsAPI')
910

11+
/** Caps how many pooled connections one request can hold; a list is a handful of rows. */
12+
const INVITATION_PREVIEW_CONCURRENCY = 4
13+
1014
/**
1115
* Pending invitations addressed to the session's email — the invitee-facing
1216
* list behind the workspace switcher's Invitations section. Acceptance is
@@ -29,24 +33,25 @@ export const GET = withRouteHandler(async () => {
2933
* Disclosure-only, so a preview failure degrades to `null` (the client
3034
* shows a generic notice) rather than hiding the invitation.
3135
*
32-
* Sequential on purpose: each preview issues several queries, and this
33-
* endpoint is hit whenever the workspace switcher opens. Fanning them out
34-
* with `Promise.all` would hold one pooled connection per pending
35-
* invitation for the length of the slowest one. The list is a handful of
36-
* rows, so the added latency is not worth the pool pressure.
36+
* Each preview issues up to three queries of its own, so a serial loop put
37+
* every one of them on the critical path of the switcher opening. The mapper
38+
* must stay total — `mapWithConcurrency` fails the whole batch on a throw.
3739
*/
38-
const previews: Array<Awaited<ReturnType<typeof getInvitationJoinPreview>> | null> = []
39-
for (const inv of invitations) {
40-
try {
41-
previews.push(await getInvitationJoinPreview(session.user.id, inv))
42-
} catch (previewError) {
43-
logger.warn('Failed to compute join preview for pending invitation', {
44-
invitationId: inv.id,
45-
error: previewError,
46-
})
47-
previews.push(null)
40+
const previews = await mapWithConcurrency(
41+
invitations,
42+
INVITATION_PREVIEW_CONCURRENCY,
43+
async (inv) => {
44+
try {
45+
return await getInvitationJoinPreview(session.user.id, inv)
46+
} catch (previewError) {
47+
logger.warn('Failed to compute join preview for pending invitation', {
48+
invitationId: inv.id,
49+
error: previewError,
50+
})
51+
return null
52+
}
4853
}
49-
}
54+
)
5055

5156
return NextResponse.json({
5257
invitations: invitations.map(

apps/sim/app/api/workspaces/[id]/files/route.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,10 @@ import {
1616
} from '@/lib/core/utils/stream-limits'
1717
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1818
import { captureServerEvent } from '@/lib/posthog/server'
19-
import { getWorkspaceShares } from '@/lib/public-shares/share-manager'
20-
import {
21-
FileConflictError,
22-
listWorkspaceFiles,
23-
uploadWorkspaceFile,
24-
} from '@/lib/uploads/contexts/workspace'
19+
import { FileConflictError, uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace'
2520
import { EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
2621
import { MAX_WORKSPACE_FORMDATA_FILE_SIZE } from '@/lib/uploads/shared/types'
22+
import { listWorkspaceFilesWithShares } from '@/lib/workspace-files/queries'
2723
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
2824
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
2925

@@ -73,15 +69,11 @@ export const GET = withRouteHandler(
7369
}
7470
const { scope } = queryResult.data
7571

76-
const files = await listWorkspaceFiles(workspaceId, { scope })
77-
78-
const shares = await getWorkspaceShares('file', workspaceId)
79-
const filesWithShares = files.map((file) => ({
80-
...file,
81-
share: shares.get(file.id) ?? null,
82-
}))
72+
const filesWithShares = await listWorkspaceFilesWithShares(workspaceId, scope)
8373

84-
logger.info(`[${requestId}] Listed ${files.length} files for workspace ${workspaceId}`)
74+
logger.info(
75+
`[${requestId}] Listed ${filesWithShares.length} files for workspace ${workspaceId}`
76+
)
8577

8678
return NextResponse.json({
8779
success: true,

apps/sim/app/workspace/[workspaceId]/files/page.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Suspense } from 'react'
22
import { dehydrate, HydrationBoundary } from '@tanstack/react-query'
33
import type { Metadata } from 'next'
4+
import { getSession } from '@/lib/auth'
45
import { getQueryClient } from '@/app/_shell/providers/get-query-client'
56
import { prefetchFilesBrowser } from '@/app/workspace/[workspaceId]/files/prefetch'
67
import { Files } from './files'
@@ -19,10 +20,12 @@ export const metadata: Metadata = {
1920
* `loading.tsx` covers the navigation/chunk-load transition the same way.
2021
*/
2122
export default async function FilesPage({ params }: { params: Promise<{ workspaceId: string }> }) {
22-
const { workspaceId } = await params
23+
const [{ workspaceId }, session] = await Promise.all([params, getSession()])
2324

2425
const queryClient = getQueryClient()
25-
await prefetchFilesBrowser(queryClient, workspaceId)
26+
if (session?.user?.id) {
27+
await prefetchFilesBrowser(queryClient, workspaceId, session.user.id)
28+
}
2629

2730
return (
2831
<HydrationBoundary state={dehydrate(queryClient)}>

apps/sim/app/workspace/[workspaceId]/files/prefetch.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { QueryClient } from '@tanstack/react-query'
2-
import type { WorkspaceFileFolderApi } from '@/lib/api/contracts/workspace-file-folders'
3-
import type { ListWorkspaceFilesResponse } from '@/lib/api/contracts/workspace-files'
4-
import { prefetchInternalJson } from '@/app/workspace/[workspaceId]/lib/prefetch-internal-fetch'
2+
import { listWorkspaceFileFolders } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
3+
import { listWorkspaceFilesWithShares } from '@/lib/workspace-files/queries'
4+
import { getWorkspaceHostContextForViewer } from '@/lib/workspaces/host-context'
55
import { prefetchResourceListChrome } from '@/app/workspace/[workspaceId]/lib/prefetch-resource-list-chrome'
66
import {
77
WORKSPACE_FILE_FOLDERS_STALE_TIME,
@@ -20,32 +20,32 @@ import {
2020
* `useWorkspaceFileFolders`) use (scope `active`), so the browser paints
2121
* populated on first render.
2222
*
23-
* Both payloads carry `Date` fields, so they go through their routes and cache
24-
* the serialized wire shape — see {@link prefetchInternalJson}.
23+
* Files and folders read the data layer; both payloads are shaped to their route contract so
24+
* a hydrated entry matches a client fetch. Everything else still goes through its route —
25+
* see {@link prefetchInternalJson}.
26+
*
27+
* Those two reads carry no authorization of their own, so the viewer is proved first. This
28+
* reuses the layout's `cache`d host-context lookup rather than re-deriving the permission,
29+
* so it costs no additional queries; a viewer without access caches nothing and the client
30+
* fetch reaches the route for the real 403.
2531
*/
2632
export async function prefetchFilesBrowser(
2733
queryClient: QueryClient,
28-
workspaceId: string
34+
workspaceId: string,
35+
userId: string
2936
): Promise<void> {
37+
const hostContext = await getWorkspaceHostContextForViewer(workspaceId, userId)
38+
if (!hostContext) return
39+
3040
await Promise.all([
3141
queryClient.prefetchQuery({
3242
queryKey: workspaceFilesKeys.list(workspaceId, 'active'),
33-
queryFn: async () => {
34-
const data = await prefetchInternalJson<ListWorkspaceFilesResponse>(
35-
`/api/workspaces/${workspaceId}/files?scope=active`
36-
)
37-
return data.success ? data.files : []
38-
},
43+
queryFn: () => listWorkspaceFilesWithShares(workspaceId, 'active'),
3944
staleTime: WORKSPACE_FILES_LIST_STALE_TIME,
4045
}),
4146
queryClient.prefetchQuery({
4247
queryKey: workspaceFileFolderKeys.list(workspaceId, 'active'),
43-
queryFn: async () => {
44-
const data = await prefetchInternalJson<{ folders?: WorkspaceFileFolderApi[] }>(
45-
`/api/workspaces/${workspaceId}/files/folders?scope=active`
46-
)
47-
return data.folders ?? []
48-
},
48+
queryFn: () => listWorkspaceFileFolders(workspaceId, { scope: 'active' }),
4949
staleTime: WORKSPACE_FILE_FOLDERS_STALE_TIME,
5050
}),
5151
prefetchResourceListChrome(queryClient, workspaceId, 'file'),

apps/sim/app/workspace/[workspaceId]/lib/prefetch-internal-fetch.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
55
* Server-side GET against an internal `/api` route, forwarding the incoming
66
* request's cookie so the route authenticates as the current user.
77
*
8-
* List prefetches go through the route (rather than the data layer) when the
9-
* payload carries `Date` fields: `NextResponse.json` serializes them to the
10-
* string wire shape the client caches via `requestJson`, so the
11-
* server-hydrated entry byte-matches the client-fetched one through
12-
* dehydration. Calling the data layer directly would cache raw `Date` objects
13-
* and drift from that wire shape. Mirrors the settings/subscription prefetch.
8+
* The legacy path. Reading the data layer and shaping the result through the
9+
* route's response contract — as `files/prefetch.ts` does — is canonical: it
10+
* drops a server-to-server request and its duplicate auth, and the contract
11+
* parse is what guarantees the hydrated entry matches a client fetch. Prefetches
12+
* still on this helper have not been converted; a converted one must prove the
13+
* viewer itself, since the route's own authorization no longer runs.
1414
*/
1515
export async function prefetchInternalJson<T>(path: string): Promise<T> {
1616
const cookie = (await headers()).get('cookie')

0 commit comments

Comments
 (0)