From 0fb81ad772fa1dac91b7960c96841ae0b4bb90e7 Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Sat, 4 Jul 2026 20:36:15 +0200 Subject: [PATCH] refactor(api): replace file-based statistics cache with in-memory Map Removes fs, path, env, getLocalDataDirectory imports from statistics service. A module-level Map gives the same TTL semantics without touching disk. Adds clearStatisticsCache() for test isolation and updates the test to call it in afterEach instead of relying on unique LOCAL_STORAGE_PATH temp dirs (finding 5). Claude-Session: https://claude.ai/code/session_012Bnvt1MghwHj4qQRxuQUGa --- .../src/features/statistics/service.test.ts | 22 +++------ .../api/src/features/statistics/service.ts | 46 +++++-------------- 2 files changed, 18 insertions(+), 50 deletions(-) diff --git a/packages/api/src/features/statistics/service.test.ts b/packages/api/src/features/statistics/service.test.ts index b3f3d2774..f356a2439 100644 --- a/packages/api/src/features/statistics/service.test.ts +++ b/packages/api/src/features/statistics/service.test.ts @@ -1,13 +1,5 @@ -import { mkdtempSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const envMock = vi.hoisted(() => ({ - // Filled in by beforeEach so caches do not bleed between tests. - LOCAL_STORAGE_PATH: "" as string, -})); - const dbResult = vi.hoisted(() => ({ count: 0 })); const dbMock = vi.hoisted(() => { const select = vi.fn(); @@ -15,7 +7,6 @@ const dbMock = vi.hoisted(() => { return { select }; }); -vi.mock("@reactive-resume/env/server", () => ({ env: envMock })); vi.mock("@reactive-resume/db/client", () => ({ db: dbMock })); vi.mock("@reactive-resume/db/schema", () => ({ user: { __table: "user" }, resume: { __table: "resume" } })); vi.mock("drizzle-orm", () => ({ count: () => "count(*)" })); @@ -27,19 +18,17 @@ beforeEach(() => { afterEach(() => { vi.unstubAllGlobals(); fetchMock.mockReset(); - dbMock.select.mockClear(); + dbMock.select.mockReset(); + // ponytail: clear in-memory cache so each test starts with a fresh fetch + clearStatisticsCache(); }); -const { statisticsService } = await import("./service"); - -// Each test gets a unique LOCAL_STORAGE_PATH to avoid cross-test cache hits. -beforeEach(() => { - envMock.LOCAL_STORAGE_PATH = mkdtempSync(join(tmpdir(), "rr-statistics-test-")); -}); +const { statisticsService, clearStatisticsCache } = await import("./service"); describe("statisticsService.user.getCount", () => { it("returns the DB count when the fetcher succeeds", async () => { dbResult.count = 42; + dbMock.select.mockReturnValue({ from: () => Promise.resolve([dbResult]) }); await expect(statisticsService.user.getCount()).resolves.toBe(42); }); @@ -56,6 +45,7 @@ describe("statisticsService.user.getCount", () => { describe("statisticsService.resume.getCount", () => { it("returns the DB count for resume", async () => { dbResult.count = 7; + dbMock.select.mockReturnValue({ from: () => Promise.resolve([dbResult]) }); await expect(statisticsService.resume.getCount()).resolves.toBe(7); }); }); diff --git a/packages/api/src/features/statistics/service.ts b/packages/api/src/features/statistics/service.ts index 2b055706c..606a7f72c 100644 --- a/packages/api/src/features/statistics/service.ts +++ b/packages/api/src/features/statistics/service.ts @@ -1,10 +1,6 @@ -import fs from "node:fs/promises"; -import { dirname, join } from "node:path"; import { count } from "drizzle-orm"; import { db } from "@reactive-resume/db/client"; import * as schema from "@reactive-resume/db/schema"; -import { env } from "@reactive-resume/env/server"; -import { getLocalDataDirectory } from "@reactive-resume/utils/monorepo.node"; const CACHE_DURATION_MS = 6 * 60 * 60 * 1000; // 6 hours const GITHUB_API_URL = "https://api.github.com/repos/amruthpillai/reactive-resume"; @@ -17,38 +13,20 @@ const LAST_KNOWN = { stars: 34_073, } as const; -const getCachePath = (key: string) => join(getLocalDataDirectory(env.LOCAL_STORAGE_PATH), "statistics", `${key}.txt`); +// ponytail: file-based disk cache replaced with module-level memo; LAST_KNOWN fallbacks cover restarts +const memCache = new Map(); -const readCache = async (key: string): Promise => { - try { - const filePath = getCachePath(key); - const [stats, contents] = await Promise.all([fs.stat(filePath), fs.readFile(filePath, "utf-8")]); +/** Clear all cached statistics. Exposed for test isolation only. */ +export const clearStatisticsCache = () => memCache.clear(); - if (stats.mtimeMs < Date.now() - CACHE_DURATION_MS) return null; - - const value = Number.parseInt(contents, 10); - return Number.isFinite(value) && value >= 0 ? value : null; - } catch { - return null; - } +const getCached = (key: string): number | null => { + const entry = memCache.get(key); + if (!entry || Date.now() - entry.cachedAt >= CACHE_DURATION_MS) return null; + return entry.value; }; -const writeCache = async (key: string, value: number) => { - try { - const filePath = getCachePath(key); - await fs.mkdir(dirname(filePath), { recursive: true }); - - const contents = String(value); - try { - if ((await fs.readFile(filePath, "utf-8")) === contents) return; - } catch { - // Cache file does not exist yet. - } - - await fs.writeFile(filePath, contents, "utf-8"); - } catch { - // Ignore errors, cache is not critical - } +const setCached = (key: string, value: number) => { + memCache.set(key, { value, cachedAt: Date.now() }); }; const getCachedCount = async ( @@ -56,13 +34,13 @@ const getCachedCount = async ( lastKnown: number, fetcher: () => Promise, ): Promise => { - const cached = await readCache(key); + const cached = getCached(key); if (cached !== null) return cached; try { const value = await fetcher(); if (value !== null) { - await writeCache(key, value); + setCached(key, value); return value; } } catch {