From 961e354a1b0546fcc61427bf0d014c02442602d4 Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Wed, 9 Sep 2026 07:11:06 +1000 Subject: [PATCH] fix(api): cache user skills route reads per login --- server/api/users/[login]/skills.get.ts | 90 ++++++++++++------- test/unit/user-skills-cache.test.ts | 120 +++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 31 deletions(-) create mode 100644 test/unit/user-skills-cache.test.ts diff --git a/server/api/users/[login]/skills.get.ts b/server/api/users/[login]/skills.get.ts index 3dfb045e..ef5c66eb 100644 --- a/server/api/users/[login]/skills.get.ts +++ b/server/api/users/[login]/skills.get.ts @@ -1,6 +1,13 @@ +import type { H3Event } from 'h3' +import type { Platform } from '#shared/server/platform' + +import { cached } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' import { canonicalRepoSkillPath } from '#shared/skill-routes' +const USER_SKILLS_CACHE_TTL = 60 +const USER_SKILLS_CACHE_STALE_TTL = 60 * 5 + interface SkillRow { name: string owner: string @@ -24,36 +31,57 @@ export default defineApiHandler({ if (!login) throw createError({ statusCode: 400, message: 'login required' }) - const { db } = platform - const res = await db - .prepare( - `SELECT s.name, s.owner, s.repo, s.display_name, s.slug, s.description, - s.like_count AS likeCount, - s.modified_at, s.last_synced_at, s.rendered_skill_path AS skill_path, - r.source_owner, r.source_repo, r.default_branch, - (SELECT COUNT(*) FROM skills repo_skills - WHERE repo_skills.owner = s.owner - AND repo_skills.repo = s.repo - AND repo_skills.source_resolved = 1) AS repo_skill_count - FROM skills s - JOIN repos r ON r.owner = s.owner AND r.repo = s.repo - WHERE s.owner = ?1 COLLATE NOCASE - ORDER BY COALESCE(s.modified_at, s.last_synced_at, 0) DESC`, - ) - .bind(login) - .all() - - return { - ok: true as const, - items: (res.results ?? []).map(skill => ({ - ...skill, - registryPath: canonicalRepoSkillPath({ - owner: skill.owner, - repo: skill.repo, - name: skill.name, - repoSkillCount: skill.repo_skill_count, - }), - })), - } + return cached({ + storage: useStorage('cache'), + key: `user-skills:${login.toLowerCase()}`, + ttlSeconds: USER_SKILLS_CACHE_TTL, + staleSeconds: USER_SKILLS_CACHE_STALE_TTL, + compute: () => loadUserSkills(platform, login), + schedule: promise => runAfterResponse(event, promise), + }) }, }) + +async function loadUserSkills(platform: Platform, login: string) { + const res = await platform.db + .prepare( + `SELECT s.name, s.owner, s.repo, s.display_name, s.slug, s.description, + s.like_count AS likeCount, + s.modified_at, s.last_synced_at, s.rendered_skill_path AS skill_path, + r.source_owner, r.source_repo, r.default_branch, + (SELECT COUNT(*) FROM skills repo_skills + WHERE repo_skills.owner = s.owner + AND repo_skills.repo = s.repo + AND repo_skills.source_resolved = 1) AS repo_skill_count + FROM skills s + JOIN repos r ON r.owner = s.owner AND r.repo = s.repo + WHERE s.owner = ?1 COLLATE NOCASE + ORDER BY COALESCE(s.modified_at, s.last_synced_at, 0) DESC`, + ) + .bind(login) + .all() + + return { + ok: true as const, + items: (res.results ?? []).map(skill => ({ + ...skill, + registryPath: canonicalRepoSkillPath({ + owner: skill.owner, + repo: skill.repo, + name: skill.name, + repoSkillCount: skill.repo_skill_count, + }), + })), + } +} + +function runAfterResponse(event: H3Event, promise: Promise): void { + const ctx = (event.context as { cloudflare?: { context?: { waitUntil?: (p: Promise) => void } } }).cloudflare?.context + if (ctx?.waitUntil) { + ctx.waitUntil(promise) + return + } + // Local dev / non-Workers: don't block the response, but make sure the + // promise isn't an unhandled rejection. + void promise +} diff --git a/test/unit/user-skills-cache.test.ts b/test/unit/user-skills-cache.test.ts new file mode 100644 index 00000000..4712535d --- /dev/null +++ b/test/unit/user-skills-cache.test.ts @@ -0,0 +1,120 @@ +import type { H3Event } from 'h3' +import type { SqliteD1 } from './helpers/d1-sqlite' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +// GET /api/users/:login/skills ran as a bare handler, so every crawler hit +// repeated the same per-login D1 reads and the route landed as SKILLD-1K in +// the D1 overload burst. These tests pin the read-through cache the skill +// detail route already uses: one query per login per fresh window, stale +// entries served while a single refresh recomputes, and one key per login +// no matter which casing the URL used. +describe('user skills cache', () => { + const NOW_SEC = Math.floor(Date.now() / 1000) + const login = 'ericzakariasson' + const cacheKey = `user-skills:${login}` + let harness: SqliteD1 + let handler: (event: H3Event) => Promise<{ ok: boolean, items: Record[] }> + let prepareCalls: number + + vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) + vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) + vi.stubGlobal('getUserSession', () => Promise.resolve(null)) + vi.stubGlobal('getRouterParam', (event: unknown, key: string) => + (event as { context: { params?: Record } }).context.params?.[key]) + + beforeEach(async () => { + vi.resetModules() + harness = createSqliteD1(allMigrations()) + prepareCalls = 0 + const db = harness.db as unknown as { prepare: (sql: string) => unknown } + const realPrepare = db.prepare.bind(db) + db.prepare = (sql: string) => { + prepareCalls++ + return realPrepare(sql) + } + + harness.raw.prepare(`INSERT INTO repos (owner, repo) VALUES ('ericzakariasson', 'scandinavian-design')`).run() + harness.raw.prepare( + `INSERT INTO skills (owner, repo, name, slug, display_name, source_resolved, modified_at) + VALUES ('ericzakariasson', 'scandinavian-design', 'alpha', ?, 'alpha', 1, ?)`, + ).run(`${login}/scandinavian-design/alpha`, NOW_SEC) + handler = (await import('../../server/api/users/[login]/skills.get')).default + }) + + it('serves a repeat request from the cache without repeating the D1 query', async () => { + const cacheMap = new Map() + vi.stubGlobal('useStorage', () => ({ + getItem: async (key: string) => cacheMap.get(key) ?? null, + setItem: async (key: string, value: unknown) => { + cacheMap.set(key, value) + }, + })) + + const first = await handler(event()) + expect(first.ok).toBe(true) + expect(first.items).toHaveLength(1) + const singleRunQueries = prepareCalls + + const second = await handler(event()) + expect(second).toEqual(first) + expect(prepareCalls).toBe(singleRunQueries) + }) + + it('shares one cache entry between casings of the same login', async () => { + const cacheMap = new Map() + vi.stubGlobal('useStorage', () => ({ + getItem: async (key: string) => cacheMap.get(key) ?? null, + setItem: async (key: string, value: unknown) => { + cacheMap.set(key, value) + }, + })) + + await handler(event()) + expect(cacheMap.has(cacheKey)).toBe(true) + + const before = prepareCalls + const mixed = await handler(event('EricZakariasson')) + expect(mixed.items).toHaveLength(1) + expect(prepareCalls).toBe(before) + }) + + it('serves a stale entry while exactly one refresh recomputes in the background', async () => { + const cacheMap = new Map() + vi.stubGlobal('useStorage', () => ({ + getItem: async (key: string) => cacheMap.get(key) ?? null, + setItem: async (key: string, value: unknown) => { + cacheMap.set(key, value) + }, + })) + + await handler(event()) + const singleRunQueries = prepareCalls + const fresh = cacheMap.get(cacheKey) as { v: { items: unknown[] }, t: number } + expect(fresh.v.items).toHaveLength(1) + + // Age the entry past the 60s TTL, into the 5 minute stale window. + const staleT = Math.floor(Date.now() / 1000) - 61 + cacheMap.set(cacheKey, { v: fresh.v, t: staleT }) + + const before = prepareCalls + const results = await Promise.all(Array.from({ length: 8 }, () => handler(event()))) + + // Every request is served the stale value immediately. + expect(results.every(result => result === fresh.v)).toBe(true) + + // One background refresh recomputes and writes the entry back. + await vi.waitFor(() => { + const written = cacheMap.get(cacheKey) as { v: { items: unknown[] }, t: number } + expect(written.t).toBeGreaterThan(staleT) + }) + expect(prepareCalls - before).toBeLessThanOrEqual(singleRunQueries) + }) + + function event(loginParam: string = login): H3Event { + return { + context: { params: { login: loginParam }, platform: { db: harness.db } }, + node: { req: { headers: {} } }, + } as unknown as H3Event + } +})