perf(activities): bulk-fetch list serialisation (N+1 → constant)
The /api/activities list endpoint serialised each row by calling tagsFor, heartsFor, viewerBookmarked, and ownerAttribution — that's ~5 queries per row. For a list of N activities the endpoint issued 5N queries; for a hundred-row list, hundreds of round-trips. Add buildBulkLookups(rows, viewerId) that runs one IN-query per concern (tags, heart counts, viewer-hearted, viewer-bookmarked, owner attribution) and returns precomputed maps. serialize() now accepts an optional bulk arg; single-row paths (GET /:id, POST, PATCH, heart/bookmark toggles) pass undefined and keep their per-row helpers — fine for one row, wrong for a list. Also add bulkTagsFor() to server/tags.ts as a reusable helper, and apply the same treatment to server/users.ts (public-list endpoint had a 2N pattern on tags + heart counts). Surfaced by /audit simplify. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
45ad3ea3bb
commit
c7365b46ed
3 changed files with 167 additions and 41 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { Hono } from 'hono';
|
||||
import { getDb } from './db';
|
||||
import { requireAuth, currentUserId, type AppVariables, type AppContext } from './session';
|
||||
import { setActivityTags, clearActivityTags, tagsFor } from './tags';
|
||||
import { setActivityTags, clearActivityTags, tagsFor, bulkTagsFor } from './tags';
|
||||
import { isModerator } from './roles';
|
||||
import type {
|
||||
Activity, ActivityPublic, ActivitySemi, ActivityPrivate,
|
||||
|
|
@ -115,7 +115,89 @@ function b64ToBuf(s: string): Buffer {
|
|||
return Buffer.from(s, 'base64');
|
||||
}
|
||||
|
||||
function serialize(row: ActivityRow, viewerId: string | null): Activity {
|
||||
/**
|
||||
* Per-list precomputed lookups. The list endpoint builds these once via
|
||||
* buildBulkLookups so per-row serialisation can do constant work instead of
|
||||
* issuing 4-5 queries per row. Single-row paths (POST, PATCH, GET /:id, heart
|
||||
* + bookmark toggles) pass undefined and serialize() falls back to the
|
||||
* per-row helpers — fine for one row, terrible for 100.
|
||||
*/
|
||||
interface BulkLookups {
|
||||
tags: Map<string, string[]>;
|
||||
hearts: Map<string, { count: number; hearted: boolean }>;
|
||||
bookmarked: Set<string>;
|
||||
attribution: Map<string, { display: string | null; username: string | null }>;
|
||||
}
|
||||
|
||||
function buildBulkLookups(rows: ActivityRow[], viewerId: string | null): BulkLookups {
|
||||
const db = getDb();
|
||||
const ids = rows.map((r) => r.id);
|
||||
const tags = bulkTagsFor(ids);
|
||||
const hearts = new Map<string, { count: number; hearted: boolean }>();
|
||||
const bookmarked = new Set<string>();
|
||||
const attribution = new Map<string, { display: string | null; username: string | null }>();
|
||||
|
||||
if (ids.length === 0) return { tags, hearts, bookmarked, attribution };
|
||||
const ph = ids.map(() => '?').join(',');
|
||||
|
||||
const heartCounts = db
|
||||
.prepare(`
|
||||
SELECT activity_id, COUNT(*) AS n FROM activity_hearts
|
||||
WHERE activity_id IN (${ph})
|
||||
GROUP BY activity_id
|
||||
`)
|
||||
.all(...ids) as { activity_id: string; n: number }[];
|
||||
const viewerHeartSet = new Set<string>();
|
||||
if (viewerId) {
|
||||
const vh = db
|
||||
.prepare(`
|
||||
SELECT activity_id FROM activity_hearts
|
||||
WHERE activity_id IN (${ph}) AND user_id = ?
|
||||
`)
|
||||
.all(...ids, viewerId) as { activity_id: string }[];
|
||||
for (const r of vh) viewerHeartSet.add(r.activity_id);
|
||||
}
|
||||
for (const r of heartCounts) {
|
||||
hearts.set(r.activity_id, { count: r.n, hearted: viewerHeartSet.has(r.activity_id) });
|
||||
}
|
||||
|
||||
if (viewerId) {
|
||||
const bm = db
|
||||
.prepare(`
|
||||
SELECT activity_id FROM bookmarks
|
||||
WHERE activity_id IN (${ph}) AND user_id = ?
|
||||
`)
|
||||
.all(...ids, viewerId) as { activity_id: string }[];
|
||||
for (const r of bm) bookmarked.add(r.activity_id);
|
||||
}
|
||||
|
||||
const ownerIds = [...new Set(rows.map((r) => r.owner_id))];
|
||||
if (ownerIds.length > 0) {
|
||||
const op = ownerIds.map(() => '?').join(',');
|
||||
const userRows = db
|
||||
.prepare(`
|
||||
SELECT id, display_name, username, public_list_enabled FROM users
|
||||
WHERE id IN (${op})
|
||||
`)
|
||||
.all(...ownerIds) as {
|
||||
id: string;
|
||||
display_name: string | null;
|
||||
username: string | null;
|
||||
public_list_enabled: number | null;
|
||||
}[];
|
||||
for (const u of userRows) {
|
||||
const display = (u.display_name && u.display_name.trim())
|
||||
? u.display_name
|
||||
: (u.username && u.username.trim()) ? u.username : null;
|
||||
const username = u.public_list_enabled === 1 ? u.username : null;
|
||||
attribution.set(u.id, { display, username });
|
||||
}
|
||||
}
|
||||
|
||||
return { tags, hearts, bookmarked, attribution };
|
||||
}
|
||||
|
||||
function serialize(row: ActivityRow, viewerId: string | null, bulk?: BulkLookups): Activity {
|
||||
// Effective sort position: custom if the viewer has dragged this row,
|
||||
// otherwise -created_at so unsorted rows float to the top by recency.
|
||||
// Matches the SQL ORDER BY in the list query.
|
||||
|
|
@ -137,9 +219,13 @@ function serialize(row: ActivityRow, viewerId: string | null): Activity {
|
|||
};
|
||||
return a;
|
||||
}
|
||||
const tags = tagsFor(row.id);
|
||||
const hearts = heartsFor(row.id, viewerId);
|
||||
const bookmarked = viewerBookmarked(row.id, viewerId);
|
||||
const tags = bulk ? (bulk.tags.get(row.id) ?? []) : tagsFor(row.id);
|
||||
const hearts = bulk
|
||||
? (bulk.hearts.get(row.id) ?? { count: 0, hearted: false })
|
||||
: heartsFor(row.id, viewerId);
|
||||
const bookmarked = bulk
|
||||
? bulk.bookmarked.has(row.id)
|
||||
: viewerBookmarked(row.id, viewerId);
|
||||
if (row.visibility === 'semi') {
|
||||
// owner_id is included ONLY when the viewer IS the owner — that lets the
|
||||
// client render Edit/Delete on the user's own semi rows without leaking
|
||||
|
|
@ -164,12 +250,14 @@ function serialize(row: ActivityRow, viewerId: string | null): Activity {
|
|||
if (viewerId === row.owner_id) a.owner_id = row.owner_id;
|
||||
return a;
|
||||
}
|
||||
const attrib = ownerAttribution(row.owner_id);
|
||||
const attrib = bulk
|
||||
? (bulk.attribution.get(row.owner_id) ?? { display: null, username: null })
|
||||
: ownerAttribution(row.owner_id);
|
||||
if (row.visibility === 'friends') {
|
||||
// Visibility check already happened upstream; we only get here if the
|
||||
// viewer is allowed to see the row. Attribution is fine because being a
|
||||
// friend implies knowing the owner.
|
||||
const attribF = ownerAttribution(row.owner_id);
|
||||
const attribF = attrib;
|
||||
return {
|
||||
id: row.id,
|
||||
visibility: 'friends',
|
||||
|
|
@ -294,7 +382,8 @@ activitiesRoutes.get('/', (c) => {
|
|||
`)
|
||||
.all(...orderParams) as ActivityRow[];
|
||||
|
||||
return c.json(rows.map((r) => serialize(r, viewerId)));
|
||||
const bulk = buildBulkLookups(rows, viewerId);
|
||||
return c.json(rows.map((r) => serialize(r, viewerId, bulk)));
|
||||
});
|
||||
|
||||
// --- PATCH /api/activities/:id/sort -----------------------------------------
|
||||
|
|
|
|||
|
|
@ -90,6 +90,31 @@ export function tagsFor(activityId: string): string[] {
|
|||
).map((r) => r.name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Bulk variant of tagsFor for list endpoints. Returns a Map keyed by activity
|
||||
* id; activities with no tags are absent from the map, callers should default
|
||||
* to []. One query regardless of input size.
|
||||
*/
|
||||
export function bulkTagsFor(activityIds: string[]): Map<string, string[]> {
|
||||
const out = new Map<string, string[]>();
|
||||
if (activityIds.length === 0) return out;
|
||||
const placeholders = activityIds.map(() => '?').join(',');
|
||||
const rows = getDb()
|
||||
.prepare(`
|
||||
SELECT at.activity_id, t.name FROM activity_tags at
|
||||
JOIN tags t ON t.id = at.tag_id
|
||||
WHERE at.activity_id IN (${placeholders})
|
||||
ORDER BY t.name
|
||||
`)
|
||||
.all(...activityIds) as { activity_id: string; name: string }[];
|
||||
for (const r of rows) {
|
||||
const arr = out.get(r.activity_id);
|
||||
if (arr) arr.push(r.name);
|
||||
else out.set(r.activity_id, [r.name]);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// --- Routes ------------------------------------------------------------------
|
||||
export const tagsRoutes = new Hono();
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { Hono } from 'hono';
|
||||
import { getDb } from './db';
|
||||
import { tagsFor } from './tags';
|
||||
import { bulkTagsFor } from './tags';
|
||||
import { loadUserLinks } from './auth';
|
||||
import type { PublicListResponse, ActivityPublic } from '../shared/types';
|
||||
|
||||
|
|
@ -51,38 +51,50 @@ usersRoutes.get('/:username/list', (c) => {
|
|||
`)
|
||||
.all(user.id) as ActivityRow[];
|
||||
|
||||
const activities: ActivityPublic[] = rows.map((r) => {
|
||||
const count = (db
|
||||
.prepare('SELECT COUNT(*) AS n FROM activity_hearts WHERE activity_id = ?')
|
||||
.get(r.id) as { n: number }).n;
|
||||
return {
|
||||
id: r.id,
|
||||
visibility: 'public',
|
||||
owner_id: r.owner_id,
|
||||
// Prefer the display name; fall back to the username slug (which is
|
||||
// what the URL already shows). Never falls through to email/id.
|
||||
owner_display: (user.display_name && user.display_name.trim()) || username,
|
||||
// The list itself is at /<username>/list, so we already know the slug.
|
||||
// Surfacing it on each row keeps ActivityRow's rendering uniform.
|
||||
owner_username: username,
|
||||
title: r.title ?? '',
|
||||
description: r.description,
|
||||
tags: tagsFor(r.id),
|
||||
loc_label: r.loc_label,
|
||||
loc_lat: r.loc_lat,
|
||||
loc_lng: r.loc_lng,
|
||||
scheduled_at: r.scheduled_at,
|
||||
heart_count: count,
|
||||
// The public-list endpoint is unauthenticated; we don't know who the
|
||||
// viewer is to fill viewer_hearted/bookmarked truthfully. Always false.
|
||||
viewer_hearted: false,
|
||||
viewer_bookmarked: false,
|
||||
// No personal sort here — anonymous view always sorts by recency.
|
||||
sort_position: -r.created_at,
|
||||
created_at: r.created_at,
|
||||
updated_at: r.updated_at,
|
||||
};
|
||||
});
|
||||
// Bulk lookups so we make 2 queries instead of 2N. The endpoint is public,
|
||||
// so viewer_hearted/bookmarked are always false — no per-viewer queries.
|
||||
const ids = rows.map((r) => r.id);
|
||||
const tags = bulkTagsFor(ids);
|
||||
const heartCounts = ids.length === 0
|
||||
? new Map<string, number>()
|
||||
: new Map<string, number>(
|
||||
(db
|
||||
.prepare(`
|
||||
SELECT activity_id, COUNT(*) AS n FROM activity_hearts
|
||||
WHERE activity_id IN (${ids.map(() => '?').join(',')})
|
||||
GROUP BY activity_id
|
||||
`)
|
||||
.all(...ids) as { activity_id: string; n: number }[]
|
||||
).map((r) => [r.activity_id, r.n]),
|
||||
);
|
||||
|
||||
const activities: ActivityPublic[] = rows.map((r) => ({
|
||||
id: r.id,
|
||||
visibility: 'public',
|
||||
owner_id: r.owner_id,
|
||||
// Prefer the display name; fall back to the username slug (which is
|
||||
// what the URL already shows). Never falls through to email/id.
|
||||
owner_display: (user.display_name && user.display_name.trim()) || username,
|
||||
// The list itself is at /<username>/list, so we already know the slug.
|
||||
// Surfacing it on each row keeps ActivityRow's rendering uniform.
|
||||
owner_username: username,
|
||||
title: r.title ?? '',
|
||||
description: r.description,
|
||||
tags: tags.get(r.id) ?? [],
|
||||
loc_label: r.loc_label,
|
||||
loc_lat: r.loc_lat,
|
||||
loc_lng: r.loc_lng,
|
||||
scheduled_at: r.scheduled_at,
|
||||
heart_count: heartCounts.get(r.id) ?? 0,
|
||||
// The public-list endpoint is unauthenticated; we don't know who the
|
||||
// viewer is to fill viewer_hearted/bookmarked truthfully. Always false.
|
||||
viewer_hearted: false,
|
||||
viewer_bookmarked: false,
|
||||
// No personal sort here — anonymous view always sorts by recency.
|
||||
sort_position: -r.created_at,
|
||||
created_at: r.created_at,
|
||||
updated_at: r.updated_at,
|
||||
}));
|
||||
|
||||
const resp: PublicListResponse = {
|
||||
username,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue