diff --git a/src/app/api/metrics/pinned-repos/route.ts b/src/app/api/metrics/pinned-repos/route.ts index 1d606956f..fb8722101 100644 --- a/src/app/api/metrics/pinned-repos/route.ts +++ b/src/app/api/metrics/pinned-repos/route.ts @@ -1,6 +1,19 @@ import { getServerSession } from "next-auth"; +import type { NextRequest } from "next/server"; import { authOptions } from "@/lib/auth"; -import { GitHubAuthError, githubAuthErrorResponse } from "@/lib/github-fetch"; +import { + GitHubApiError, + GitHubAuthError, + GitHubRateLimitError, + githubAuthErrorResponse, + githubGraphQL, +} from "@/lib/github-fetch"; +import { + isMetricsCacheBypassed, + METRICS_CACHE_TTL_SECONDS, + metricsCacheKey, + withMetricsCache, +} from "@/lib/metrics-cache"; export const dynamic = "force-dynamic"; @@ -13,6 +26,14 @@ interface PinnedRepo { primaryLanguage: { name: string; color: string } | null; } +interface PinnedReposQueryResult { + viewer?: { + pinnedItems?: { + nodes?: Array; + }; + }; +} + const PINNED_REPOS_QUERY = ` query { viewer { @@ -35,47 +56,59 @@ const PINNED_REPOS_QUERY = ` } `; -export async function GET() { +export async function GET(req?: NextRequest) { const session = await getServerSession(authOptions); + if (!session?.accessToken) { return Response.json({ error: "Unauthorized" }, { status: 401 }); } + if (session.error === "TokenRevoked") { return githubAuthErrorResponse(); } + const accessToken = session.accessToken; + const cacheUserId = session.githubId ?? session.githubLogin; + + if (!cacheUserId) { + return Response.json({ error: "Unauthorized" }, { status: 401 }); + } + + const bypass = req ? isMetricsCacheBypassed(req) : false; + const key = metricsCacheKey(cacheUserId, "pinned-repos"); + try { - const response = await fetch("https://api.github.com/graphql", { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${session.accessToken}`, + const result = await withMetricsCache( + { + bypass, + key, + ttlSeconds: METRICS_CACHE_TTL_SECONDS["pinned-repos"], + fallbackToStaleOnError: (error) => + error instanceof GitHubRateLimitError, }, - body: JSON.stringify({ query: PINNED_REPOS_QUERY }), - cache: "no-store", - }); - - if (!response.ok) { - if (response.status === 401) return githubAuthErrorResponse(); - return Response.json({ error: "GitHub API error" }, { status: 502 }); - } + async () => { + const data = await githubGraphQL( + PINNED_REPOS_QUERY, + accessToken + ); - const data = (await response.json()) as { - data?: { - viewer?: { - pinnedItems?: { - nodes?: Array; - }; - }; - }; - }; + const nodes = (data.viewer?.pinnedItems?.nodes ?? []).filter( + (node): node is PinnedRepo => node != null + ); - const nodes = (data.data?.viewer?.pinnedItems?.nodes ?? []).filter( - (node): node is PinnedRepo => node != null + return { pinnedRepos: nodes }; + } ); - return Response.json({ pinnedRepos: nodes }); - } catch (e) { + return Response.json(result); + } catch (error) { + if ( + error instanceof GitHubAuthError || + (error instanceof GitHubApiError && error.status === 401) + ) { + return githubAuthErrorResponse(); + } + return Response.json({ error: "GitHub API error" }, { status: 502 }); } } diff --git a/src/lib/metrics-cache.ts b/src/lib/metrics-cache.ts index ff22e2691..e42e646ee 100644 --- a/src/lib/metrics-cache.ts +++ b/src/lib/metrics-cache.ts @@ -6,6 +6,7 @@ export const METRICS_CACHE_TTL_SECONDS = { "productive-hours": 5 * 60, discussions: 10 * 60, repos: 10 * 60, + "pinned-repos": 10 * 60, "inactive-repos": 10 * 60, prs: 10 * 60, "pr-review-time": 10 * 60, @@ -25,6 +26,19 @@ export const METRICS_CACHE_TTL_SECONDS = { type MetricsCacheEndpoint = keyof typeof METRICS_CACHE_TTL_SECONDS; type CacheParamValue = boolean | number | string | null | undefined; type MemoryCacheEntry = { value: unknown; expiresAt: number }; +export const DEFAULT_METRICS_STALE_GRACE_SECONDS = 24 * 60 * 60; + +export type MetricsCacheOptions = { + bypass: boolean; + key: string; + ttlSeconds: number; + staleGraceSeconds?: number; + fallbackToStaleOnError?: (error: unknown) => boolean; +}; + +function staleMetricsCacheKey(key: string): string { + return `${key}:stale`; +} let redisClient: Redis | null | undefined; const MAX_MEMORY_CACHE_ENTRIES = 500; @@ -132,7 +146,7 @@ export function metricsCacheKey( Object.entries(params) .filter(([, value]) => value !== undefined && value !== null) - .sort(([a], [b]) => a < b ? -1 : a > b ? 1 : 0) + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) .forEach(([key, value]) => cacheParams.set(key, String(value))); return `metrics:${userId}:${endpoint}:${cacheParams.toString() || "default"}`; @@ -148,7 +162,7 @@ export async function cacheGet( } const redis = getRedisClient(); - + if (redis) { try { const redisValue = await redis.get(key); @@ -160,7 +174,7 @@ export async function cacheGet( return null; } } - + return null; } @@ -174,7 +188,7 @@ export async function cacheSet( } const redis = getRedisClient(); - + if (redis) { try { await redis.set(key, value, { ex: ttlSeconds }); @@ -187,23 +201,49 @@ export async function cacheSet( } export async function withMetricsCache( - options: { - bypass: boolean; - key: string; - ttlSeconds: number; - }, + options: MetricsCacheOptions, loadFresh: () => Promise ): Promise { + let staleValue: T | null = null; + if (!options.bypass) { const cached = await cacheGet(options.key, options.ttlSeconds); + if (cached !== null) { return cached; } + + if (options.fallbackToStaleOnError) { + staleValue = await cacheGet(staleMetricsCacheKey(options.key)); + } } - const fresh = await loadFresh(); - await cacheSet(options.key, fresh, options.ttlSeconds); - return fresh; + try { + const fresh = await loadFresh(); + + await cacheSet(options.key, fresh, options.ttlSeconds); + + if (options.fallbackToStaleOnError) { + const staleGraceSeconds = + options.staleGraceSeconds ?? DEFAULT_METRICS_STALE_GRACE_SECONDS; + + if (Number.isFinite(staleGraceSeconds) && staleGraceSeconds > 0) { + await cacheSet( + staleMetricsCacheKey(options.key), + fresh, + options.ttlSeconds + staleGraceSeconds + ); + } + } + + return fresh; + } catch (error) { + if (staleValue !== null && options.fallbackToStaleOnError?.(error)) { + return staleValue; + } + + throw error; + } } /** @@ -212,19 +252,25 @@ export async function withMetricsCache( * scanning all keys the way invalidateUserMetricsCache does for per-user data. */ export async function cacheDelete(key: string): Promise { - memoryCache.delete(key); + const keys = [key, staleMetricsCacheKey(key)]; + + for (const cacheKey of keys) { + memoryCache.delete(cacheKey); + } const redis = getRedisClient(); if (!redis) return; try { - await redis.del(key); + await redis.del(...keys); } catch { // Cache invalidation failures must not surface to callers. } } -export async function invalidateUserMetricsCache(userId: string): Promise { +export async function invalidateUserMetricsCache( + userId: string +): Promise { const prefix = `metrics:${userId}:`; for (const key of memoryCache.keys()) { @@ -239,7 +285,10 @@ export async function invalidateUserMetricsCache(userId: string): Promise try { let cursor = 0; do { - const [nextCursor, keys] = await redis.scan(cursor, { match: `${prefix}*`, count: 100 }); + const [nextCursor, keys] = await redis.scan(cursor, { + match: `${prefix}*`, + count: 100, + }); if (keys.length > 0) { await redis.del(...keys); } @@ -265,7 +314,10 @@ export async function invalidateLeaderboardCache(): Promise { try { let cursor = 0; do { - const [nextCursor, keys] = await redis.scan(cursor, { match: `${prefix}*`, count: 100 }); + const [nextCursor, keys] = await redis.scan(cursor, { + match: `${prefix}*`, + count: 100, + }); if (keys.length > 0) { await redis.del(...keys); } diff --git a/test/metrics-cache.test.ts b/test/metrics-cache.test.ts index c76cc528d..281fe61ba 100644 --- a/test/metrics-cache.test.ts +++ b/test/metrics-cache.test.ts @@ -1,23 +1,26 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { NextRequest } from 'next/server'; -import { - metricsCacheKey, - isMetricsCacheBypassed, - cacheGet, - cacheSet, - withMetricsCache -} from '../src/lib/metrics-cache'; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { NextRequest } from "next/server"; +import { + metricsCacheKey, + isMetricsCacheBypassed, + cacheDelete, + cacheGet, + cacheSet, + withMetricsCache, +} from "../src/lib/metrics-cache"; declare global { // eslint-disable-next-line no-var - var metricsMemoryCache: Map | undefined; + var metricsMemoryCache: + | Map + | undefined; } // Mock Redis const mockRedisGet = vi.fn(); const mockRedisSet = vi.fn(); -vi.mock('@upstash/redis', () => { +vi.mock("@upstash/redis", () => { return { Redis: vi.fn(() => ({ get: mockRedisGet, @@ -26,7 +29,7 @@ vi.mock('@upstash/redis', () => { }; }); -describe('metrics-cache', () => { +describe("metrics-cache", () => { beforeEach(() => { vi.clearAllMocks(); delete globalThis.metricsMemoryCache; @@ -34,63 +37,97 @@ describe('metrics-cache', () => { delete process.env.UPSTASH_REDIS_REST_TOKEN; }); - describe('metricsCacheKey', () => { - it('verify key format includes userId, endpoint, and params', () => { - const key = metricsCacheKey('user123', 'activity', { year: 2023 }); - expect(key).toBe('metrics:user123:activity:year=2023'); + describe("metricsCacheKey", () => { + it("verify key format includes userId, endpoint, and params", () => { + const key = metricsCacheKey("user123", "activity", { year: 2023 }); + expect(key).toBe("metrics:user123:activity:year=2023"); }); - it('verify params are sorted and serialized', () => { - const key1 = metricsCacheKey('user123', 'activity', { b: 2, a: 1 }); - const key2 = metricsCacheKey('user123', 'activity', { a: 1, b: 2 }); - - expect(key1).toBe('metrics:user123:activity:a=1&b=2'); - expect(key2).toBe('metrics:user123:activity:a=1&b=2'); + it("verify params are sorted and serialized", () => { + const key1 = metricsCacheKey("user123", "activity", { b: 2, a: 1 }); + const key2 = metricsCacheKey("user123", "activity", { a: 1, b: 2 }); + + expect(key1).toBe("metrics:user123:activity:a=1&b=2"); + expect(key2).toBe("metrics:user123:activity:a=1&b=2"); }); }); - describe('isMetricsCacheBypassed', () => { - it('verify refresh, bypassCache, and sync params', () => { - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=true'))).toBe(true); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?bypassCache=1'))).toBe(true); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?sync=yes'))).toBe(true); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=false'))).toBe(false); + describe("isMetricsCacheBypassed", () => { + it("verify refresh, bypassCache, and sync params", () => { + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=true")) + ).toBe(true); + expect( + isMetricsCacheBypassed( + new NextRequest("http://localhost?bypassCache=1") + ) + ).toBe(true); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?sync=yes")) + ).toBe(true); + expect( + isMetricsCacheBypassed( + new NextRequest("http://localhost?refresh=false") + ) + ).toBe(false); }); - it('verify x-devtrack-cache-bypass header', () => { - const req = new NextRequest('http://localhost', { - headers: new Headers({ 'x-devtrack-cache-bypass': 'on' }) + it("verify x-devtrack-cache-bypass header", () => { + const req = new NextRequest("http://localhost", { + headers: new Headers({ "x-devtrack-cache-bypass": "on" }), }); expect(isMetricsCacheBypassed(req)).toBe(true); }); - it('verify non-bypass values are rejected', () => { - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=false'))).toBe(false); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=0'))).toBe(false); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=no'))).toBe(false); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?bypassCache=off'))).toBe(false); + it("verify non-bypass values are rejected", () => { + expect( + isMetricsCacheBypassed( + new NextRequest("http://localhost?refresh=false") + ) + ).toBe(false); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=0")) + ).toBe(false); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=no")) + ).toBe(false); + expect( + isMetricsCacheBypassed( + new NextRequest("http://localhost?bypassCache=off") + ) + ).toBe(false); }); - it('verify missing parameters do not bypass', () => { - expect(isMetricsCacheBypassed(new NextRequest('http://localhost'))).toBe(false); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?other=value'))).toBe(false); + it("verify missing parameters do not bypass", () => { + expect(isMetricsCacheBypassed(new NextRequest("http://localhost"))).toBe( + false + ); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?other=value")) + ).toBe(false); }); - it('verify case insensitive bypass values', () => { - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=TRUE'))).toBe(true); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=YES'))).toBe(true); - expect(isMetricsCacheBypassed(new NextRequest('http://localhost?refresh=ON'))).toBe(true); + it("verify case insensitive bypass values", () => { + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=TRUE")) + ).toBe(true); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=YES")) + ).toBe(true); + expect( + isMetricsCacheBypassed(new NextRequest("http://localhost?refresh=ON")) + ).toBe(true); }); - it('verify combination of query param and header bypass', () => { - const req = new NextRequest('http://localhost?refresh=true', { - headers: new Headers({ 'x-devtrack-cache-bypass': '1' }) + it("verify combination of query param and header bypass", () => { + const req = new NextRequest("http://localhost?refresh=true", { + headers: new Headers({ "x-devtrack-cache-bypass": "1" }), }); expect(isMetricsCacheBypassed(req)).toBe(true); }); }); - describe('cacheGet/cacheSet', () => { + describe("cacheGet/cacheSet", () => { beforeEach(() => { vi.useFakeTimers(); }); @@ -99,37 +136,52 @@ describe('metrics-cache', () => { vi.useRealTimers(); }); - it('verify TTL expiration logic', async () => { - await cacheSet('test-ttl-key', 'data', 10); - expect(await cacheGet('test-ttl-key')).toBe('data'); - + it("verify TTL expiration logic", async () => { + await cacheSet("test-ttl-key", "data", 10); + expect(await cacheGet("test-ttl-key")).toBe("data"); + vi.advanceTimersByTime(11000); - - expect(await cacheGet('test-ttl-key')).toBeNull(); + + expect(await cacheGet("test-ttl-key")).toBeNull(); }); - it('verify MAX_CACHE_ENTRIES bound', async () => { + it("verify MAX_CACHE_ENTRIES bound", async () => { for (let i = 0; i < 505; i++) { await cacheSet(`key-${i}`, `val-${i}`, 60); } - - expect(await cacheGet('key-0')).toBeNull(); - expect(await cacheGet('key-504')).toBe('val-504'); + + expect(await cacheGet("key-0")).toBeNull(); + expect(await cacheGet("key-504")).toBe("val-504"); + }); + + it("verify invalid TTL values are handled", async () => { + await cacheSet("invalid-1", "data", -5); + expect(await cacheGet("invalid-1")).toBeNull(); + + await cacheSet("invalid-2", "data", NaN); + expect(await cacheGet("invalid-2")).toBeNull(); + + await cacheSet("invalid-3", "data", 0); + expect(await cacheGet("invalid-3")).toBeNull(); }); + it("deletes both fresh and stale cache entries", async () => { + const key = "delete-with-stale-key"; + const staleKey = `${key}:stale`; + + await cacheSet(key, "fresh-data", 60); + await cacheSet(staleKey, "stale-data", 3600); + + expect(await cacheGet(key)).toBe("fresh-data"); + expect(await cacheGet(staleKey)).toBe("stale-data"); - it('verify invalid TTL values are handled', async () => { - await cacheSet('invalid-1', 'data', -5); - expect(await cacheGet('invalid-1')).toBeNull(); + await cacheDelete(key); - await cacheSet('invalid-2', 'data', NaN); - expect(await cacheGet('invalid-2')).toBeNull(); - - await cacheSet('invalid-3', 'data', 0); - expect(await cacheGet('invalid-3')).toBeNull(); + expect(await cacheGet(key)).toBeNull(); + expect(await cacheGet(staleKey)).toBeNull(); }); }); - describe('withMetricsCache', () => { + describe("withMetricsCache", () => { beforeEach(() => { vi.useFakeTimers(); }); @@ -138,43 +190,119 @@ describe('metrics-cache', () => { vi.useRealTimers(); }); - it('verify bypass skips cache', async () => { + it("verify bypass skips cache", async () => { let loadCount = 0; const loadFresh = async () => { loadCount++; - return 'fresh-data'; + return "fresh-data"; }; - const options = { bypass: true, key: 'bypass-key', ttlSeconds: 60 }; - + const options = { bypass: true, key: "bypass-key", ttlSeconds: 60 }; + await withMetricsCache(options, loadFresh); expect(loadCount).toBe(1); - + await withMetricsCache(options, loadFresh); expect(loadCount).toBe(2); - + options.bypass = false; const val = await withMetricsCache(options, loadFresh); - expect(val).toBe('fresh-data'); - expect(loadCount).toBe(2); + expect(val).toBe("fresh-data"); + expect(loadCount).toBe(2); }); - it('verify fallback to loadFresh on cache miss', async () => { - const loadFresh = vi.fn().mockResolvedValue('new-data'); - const options = { bypass: false, key: 'miss-key', ttlSeconds: 60 }; - + it("verify fallback to loadFresh on cache miss", async () => { + const loadFresh = vi.fn().mockResolvedValue("new-data"); + const options = { bypass: false, key: "miss-key", ttlSeconds: 60 }; + const val1 = await withMetricsCache(options, loadFresh); - expect(val1).toBe('new-data'); + expect(val1).toBe("new-data"); expect(loadFresh).toHaveBeenCalledTimes(1); - + const val2 = await withMetricsCache(options, loadFresh); - expect(val2).toBe('new-data'); + expect(val2).toBe("new-data"); expect(loadFresh).toHaveBeenCalledTimes(1); - + vi.advanceTimersByTime(61000); const val3 = await withMetricsCache(options, loadFresh); - expect(val3).toBe('new-data'); + expect(val3).toBe("new-data"); expect(loadFresh).toHaveBeenCalledTimes(2); }); + it("returns stale data when refresh fails with an approved error", async () => { + const rateLimitError = new Error("GitHub rate limit exceeded"); + rateLimitError.name = "GitHubRateLimitError"; + + const options = { + bypass: false, + key: "stale-rate-limit-key", + ttlSeconds: 60, + staleGraceSeconds: 3600, + fallbackToStaleOnError: (error: unknown) => + error instanceof Error && error.name === "GitHubRateLimitError", + }; + + const firstLoader = vi.fn().mockResolvedValue("cached-data"); + + await expect(withMetricsCache(options, firstLoader)).resolves.toBe( + "cached-data" + ); + + vi.advanceTimersByTime(61_000); + + const failingLoader = vi.fn().mockRejectedValue(rateLimitError); + + await expect(withMetricsCache(options, failingLoader)).resolves.toBe( + "cached-data" + ); + + expect(failingLoader).toHaveBeenCalledTimes(1); + }); + + it("does not return stale data for unrelated errors", async () => { + const options = { + bypass: false, + key: "stale-unrelated-error-key", + ttlSeconds: 60, + staleGraceSeconds: 3600, + fallbackToStaleOnError: (error: unknown) => + error instanceof Error && error.name === "GitHubRateLimitError", + }; + + await withMetricsCache(options, vi.fn().mockResolvedValue("cached-data")); + + vi.advanceTimersByTime(61_000); + + await expect( + withMetricsCache( + options, + vi.fn().mockRejectedValue(new Error("database unavailable")) + ) + ).rejects.toThrow("database unavailable"); + }); + + it("does not use stale data when cache bypass is requested", async () => { + const rateLimitError = new Error("GitHub rate limit exceeded"); + rateLimitError.name = "GitHubRateLimitError"; + + const options = { + bypass: false, + key: "stale-bypass-key", + ttlSeconds: 60, + staleGraceSeconds: 3600, + fallbackToStaleOnError: (error: unknown) => + error instanceof Error && error.name === "GitHubRateLimitError", + }; + + await withMetricsCache(options, vi.fn().mockResolvedValue("cached-data")); + + vi.advanceTimersByTime(61_000); + + await expect( + withMetricsCache( + { ...options, bypass: true }, + vi.fn().mockRejectedValue(rateLimitError) + ) + ).rejects.toThrow("GitHub rate limit exceeded"); + }); }); });