diff --git a/__tests__/unit/services/current-user-id-cache.test.ts b/__tests__/unit/services/current-user-id-cache.test.ts new file mode 100644 index 000000000..19fa3b4a3 --- /dev/null +++ b/__tests__/unit/services/current-user-id-cache.test.ts @@ -0,0 +1,70 @@ +/** + * Learning who is reading costs a network call, so do it once. + * + * `supabase.auth.getUser()` validates the token against `/auth/v1/user` — it is + * not a local read. Enrichment calls it once per pass, and enrichment runs once + * per node while a reply tree is built, so opening a thread fired one + * round-trip per reply just to re-learn the same id. Production 2026-08-28: + * eight `/auth/v1/user` calls to open one post. + * + * The concurrency case matters as much as the repeat case: enrichment is + * deliberately parallel, so several callers arrive together and must collapse + * onto one request rather than racing to make several. + */ + +import { getCurrentUserId, __resetCurrentUserIdCache } from '@/services/timeline/processors/social-shared'; + +const getUser = jest.fn(); +jest.mock('@/lib/supabase/browser', () => ({ + __esModule: true, + default: { auth: { getUser: (...a: unknown[]) => getUser(...a) } }, +})); + +describe('getCurrentUserId', () => { + beforeEach(() => { + jest.clearAllMocks(); + __resetCurrentUserIdCache(); + getUser.mockResolvedValue({ data: { user: { id: 'u1' } } }); + }); + + it('asks the network once, however many times it is called', async () => { + expect(await getCurrentUserId()).toBe('u1'); + expect(await getCurrentUserId()).toBe('u1'); + expect(await getCurrentUserId()).toBe('u1'); + + expect(getUser).toHaveBeenCalledTimes(1); + }); + + it('collapses concurrent callers onto one request', async () => { + const [a, b, c] = await Promise.all([ + getCurrentUserId(), + getCurrentUserId(), + getCurrentUserId(), + ]); + + expect([a, b, c]).toEqual(['u1', 'u1', 'u1']); + expect(getUser).toHaveBeenCalledTimes(1); + }); + + it('does not cache a failure as "signed out"', async () => { + __resetCurrentUserIdCache(); + getUser.mockRejectedValueOnce(new Error('network blip')); + + expect(await getCurrentUserId()).toBeNull(); + + // A blip must not outlive itself and render the whole timeline as + // signed-out for the rest of the page. + getUser.mockResolvedValue({ data: { user: { id: 'u1' } } }); + expect(await getCurrentUserId()).toBe('u1'); + }); + + it('caches a genuine signed-out answer', async () => { + __resetCurrentUserIdCache(); + getUser.mockResolvedValue({ data: { user: null } }); + + expect(await getCurrentUserId()).toBeNull(); + expect(await getCurrentUserId()).toBeNull(); + + expect(getUser).toHaveBeenCalledTimes(1); + }); +}); diff --git a/__tests__/unit/services/reply-tree-query-count.test.ts b/__tests__/unit/services/reply-tree-query-count.test.ts new file mode 100644 index 000000000..057a90135 --- /dev/null +++ b/__tests__/unit/services/reply-tree-query-count.test.ts @@ -0,0 +1,142 @@ +/** + * Opening a thread must not cost a round-trip per reply. + * + * `getReplies` recursed: one query AND one `enrichEventsForDisplay` per node. + * Enrichment is itself several requests — profiles, projects, the reader's id, + * and the three reaction tables — so a thread cost roughly six round-trips per + * reply. Measured in production 2026-08-28 by opening one post: eight + * `/auth/v1/user` calls alone, for a thread with three replies. + * + * The cost is what these tests hold. Asserting only "the tree is correct" is + * what let the shape regress in the first place — the recursive version built a + * perfectly correct tree. + */ + +import { getReplies } from '@/services/timeline/queries/eventQueries'; + +const from = jest.fn(); +const enrich = jest.fn(); + +jest.mock('@/lib/supabase/browser', () => ({ + __esModule: true, + default: { from: (...a: unknown[]) => from(...a) }, +})); +jest.mock('@/services/timeline/processors/enrichment', () => ({ + enrichEventsForDisplay: (...a: unknown[]) => enrich(...a), +})); +jest.mock('@/services/timeline/processors/reaction-state', () => ({ + attachReactionState: async (x: T) => x, + EMPTY_REACTION_STATE: {}, +})); +jest.mock('@/lib/supabase/untyped', () => ({ callRpc: jest.fn() })); + +/** + * A fake table that actually honours the parent filter. + * + * An argument-blind mock returns the same rows whatever is asked for, which + * makes "one query per level" and "one query per node" indistinguishable — the + * first version of this file had one, and a mutation replacing `.in(parents)` + * with `.eq(parents[0])` stayed green. The filter has to be real for the count + * assertions below to mean anything. + */ +function tableOf(rows: Array>) { + from.mockImplementation(() => { + let parents: string[] = []; + const chain: Record = {}; + for (const m of ['select', 'eq', 'order']) { + chain[m] = (col?: string, val?: unknown) => { + if (col === 'parent_event_id') { + parents = [String(val)]; + } + return chain; + }; + } + chain.in = (col: string, vals: string[]) => { + if (col === 'parent_event_id') { + parents = vals; + } + return chain; + }; + chain.limit = () => + Promise.resolve({ + data: rows.filter(r => parents.includes(String(r.parent_event_id))), + error: null, + }); + return chain; + }); +} + +describe('getReplies query cost', () => { + beforeEach(() => { + jest.clearAllMocks(); + // Enrichment echoes its input, mapped to the display shape the tree needs. + enrich.mockImplementation(async (rows: Array>) => + rows.map(r => ({ id: r.id, parentEventId: r.parent_event_id })) + ); + }); + + it('asks once per DEPTH, not once per reply', async () => { + // Three replies to the root, none nested. + tableOf([ + { id: 'r1', parent_event_id: 'root' }, + { id: 'r2', parent_event_id: 'root' }, + { id: 'r3', parent_event_id: 'root' }, + ]); + + const result = await getReplies('root'); + + expect(result.success).toBe(true); + expect(result.replies).toHaveLength(3); + // Two queries: the level with replies, and the one that came back empty. + // The recursive version made four for this shape, and one more for every + // additional reply. + expect(from).toHaveBeenCalledTimes(2); + }); + + it('asks for every sibling in ONE query, not one per sibling', async () => { + // Two branches, each with a child. A per-node fetch would either take four + // queries or — asking only about the first parent — silently lose r2's + // child, so this pins both the count and the completeness. + tableOf([ + { id: 'r1', parent_event_id: 'root' }, + { id: 'r2', parent_event_id: 'root' }, + { id: 'r1a', parent_event_id: 'r1' }, + { id: 'r2a', parent_event_id: 'r2' }, + ]); + + const result = await getReplies('root'); + + // depth 0 (root), depth 1 (r1+r2 together), depth 2 (empty) = 3. + expect(from).toHaveBeenCalledTimes(3); + expect(result.replies?.map(r => r.id)).toEqual(['r1', 'r2']); + expect(result.replies?.[0].replies?.[0].id).toBe('r1a'); + expect(result.replies?.[1].replies?.[0].id).toBe('r2a'); + }); + + it('enriches the whole tree exactly once', async () => { + tableOf([ + { id: 'r1', parent_event_id: 'root' }, + { id: 'r2', parent_event_id: 'root' }, + { id: 'r3', parent_event_id: 'r1' }, + ]); + + await getReplies('root'); + + expect(enrich).toHaveBeenCalledTimes(1); + // And it got every node, so nothing is left un-enriched. + expect(enrich.mock.calls[0][0]).toHaveLength(3); + }); + + it('still nests replies under the right parent', async () => { + tableOf([ + { id: 'r1', parent_event_id: 'root' }, + { id: 'r2', parent_event_id: 'r1' }, + ]); + + const result = await getReplies('root'); + + expect(result.replies?.[0].id).toBe('r1'); + expect(result.replies?.[0].replies?.[0].id).toBe('r2'); + expect(result.replies?.[0].replyCount).toBe(1); + }); +}); diff --git a/src/services/timeline/processors/social-shared.ts b/src/services/timeline/processors/social-shared.ts index 975e10660..17c88c7c5 100644 --- a/src/services/timeline/processors/social-shared.ts +++ b/src/services/timeline/processors/social-shared.ts @@ -10,14 +10,52 @@ import { logger } from '@/utils/logger'; // and custom RPCs (like/unlike/comment) are also absent — cast required. export const db = supabase as any; +/** + * Who is reading, cached for the page. + * + * `supabase.auth.getUser()` is a NETWORK call — it validates the token against + * `/auth/v1/user`. This is called once per enrichment pass, and enrichment runs + * once per node while a reply tree is built, so opening a thread fired one + * round-trip per reply just to re-learn the same id. Measured on a three-reply + * thread: eight `/auth/v1/user` calls for one page. + * + * Cached as the in-flight PROMISE, not the value, so concurrent callers — and + * enrichment is deliberately concurrent — collapse onto one request instead of + * racing to make several. Only a resolved id is kept; a failure is not cached, + * because caching "nobody is signed in" would outlive the blip that caused it + * and quietly render the whole timeline as signed-out. + * + * The id cannot change without a sign-in or sign-out, and both replace the + * page, so a page-lifetime cache is the correct scope. + */ +let cachedUserId: string | null | undefined; +let userIdInFlight: Promise | null = null; + export async function getCurrentUserId(): Promise { - try { - const { - data: { user }, - } = await supabase.auth.getUser(); - return user?.id || null; - } catch (error) { - logger.error('Error getting current user ID', error, 'Timeline'); - return null; + if (cachedUserId !== undefined) { + return cachedUserId; } + if (!userIdInFlight) { + userIdInFlight = (async () => { + try { + const { + data: { user }, + } = await supabase.auth.getUser(); + cachedUserId = user?.id || null; + return cachedUserId; + } catch (error) { + logger.error('Error getting current user ID', error, 'Timeline'); + return null; + } finally { + userIdInFlight = null; + } + })(); + } + return userIdInFlight; +} + +/** Test seam, and the hook a sign-out would use if this ever needs clearing. */ +export function __resetCurrentUserIdCache(): void { + cachedUserId = undefined; + userIdInFlight = null; } diff --git a/src/services/timeline/queries/eventQueries.ts b/src/services/timeline/queries/eventQueries.ts index 7e9c31a7c..e0bc973b9 100644 --- a/src/services/timeline/queries/eventQueries.ts +++ b/src/services/timeline/queries/eventQueries.ts @@ -74,42 +74,62 @@ export async function getReplies( limit: number = 50 ): Promise<{ success: boolean; replies?: TimelineDisplayEvent[]; error?: string }> { try { - const buildTree = async (parentId: string, depth: number): Promise => { - // Limit depth to avoid accidental cycles - if (depth > 3) { - return []; - } + // Fetch the tree a LEVEL at a time, then enrich the whole thing once. + // + // This used to recurse per node: one query and one enrichEventsForDisplay + // per reply. Enrichment is itself several round-trips — profiles, projects, + // the reader's id, and the three reaction tables — so a thread cost roughly + // six requests per reply, and opening a three-reply thread fired eight + // `/auth/v1/user` calls alone. Level-order asks once per depth regardless of + // width, and enriches once regardless of both. + const MAX_DEPTH = 3; + const levels: Array>[] = []; + let frontier = [eventId]; + for (let depth = 0; depth <= MAX_DEPTH && frontier.length > 0; depth++) { const { data: childEvents, error } = await supabase .from(TIMELINE_TABLES.EVENTS) .select('*') - .eq('parent_event_id', parentId) + .in('parent_event_id', frontier) .eq('is_deleted', false) .order('created_at', { ascending: true }) - .limit(depth === 0 ? limit : 50); + .limit(depth === 0 ? limit : 200); if (error) { logger.error('Error fetching replies', error, 'Timeline'); - return []; + break; + } + + const rows = (childEvents || []) as Array>; + if (rows.length === 0) { + break; } + levels.push(rows); + frontier = rows.map(row => String(row.id)); + } + + const enriched = await enrichEventsForDisplay(levels.flat()); - const enrichedChildren = await enrichEventsForDisplay(childEvents || []); + // Assemble parent → children from the flat list. Every node is visited + // once, and a reply whose parent did not come back (deleted mid-read) + // simply does not attach, rather than orphaning the whole branch. + const byParent = new Map(); + for (const reply of enriched) { + const parentId = reply.parentEventId ?? ''; + byParent.set(parentId, [...(byParent.get(parentId) ?? []), reply]); + } - // Recursively fetch children for each reply, in parallel — a sequential - // loop here made thread latency grow linearly with reply count - return Promise.all( - enrichedChildren.map(async reply => { - const nestedReplies = await buildTree(reply.id, depth + 1); - return { - ...reply, - replies: nestedReplies, - replyCount: nestedReplies.length, - }; - }) - ); + const attach = (parentId: string, depth: number): TimelineDisplayEvent[] => { + if (depth > MAX_DEPTH) { + return []; + } + return (byParent.get(parentId) ?? []).map(reply => { + const nested = attach(reply.id, depth + 1); + return { ...reply, replies: nested, replyCount: nested.length }; + }); }; - const replies = await buildTree(eventId, 0); + const replies = attach(eventId, 0); return { success: true, replies }; } catch (error) { logger.error('Error fetching replies', error, 'Timeline');