From cc7de5ef49f5c2bc25aa206730054c8609639d7c Mon Sep 17 00:00:00 2001 From: Hoid Date: Wed, 11 Mar 2026 11:06:09 +0100 Subject: [PATCH] feat: add periodic database cleanup every 6 hours (TDD) - Cleans expired verifications and orphaned usage rows - Previously only ran once on startup (13d+ uptime = accumulation) - Interval uses .unref() to not block graceful shutdown - Stopped during shutdown before pool.end() - Idempotent start (safe to call multiple times) - 6 TDD tests added (periodic-cleanup.test.ts) - 663 tests total, all passing --- src/__tests__/periodic-cleanup.test.ts | 98 ++++++++++++++++++++++++++ src/index.ts | 7 ++ src/utils/periodic-cleanup.ts | 30 ++++++++ 3 files changed, 135 insertions(+) create mode 100644 src/__tests__/periodic-cleanup.test.ts create mode 100644 src/utils/periodic-cleanup.ts diff --git a/src/__tests__/periodic-cleanup.test.ts b/src/__tests__/periodic-cleanup.test.ts new file mode 100644 index 0000000..a4d0121 --- /dev/null +++ b/src/__tests__/periodic-cleanup.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// Mock the db module +vi.mock("../services/db.js", () => ({ + cleanupStaleData: vi.fn().mockResolvedValue({ expiredVerifications: 0, orphanedUsage: 0 }), +})); + +vi.mock("../services/logger.js", () => ({ + default: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +import { + startPeriodicCleanup, + stopPeriodicCleanup, + CLEANUP_INTERVAL_MS, +} from "../utils/periodic-cleanup.js"; +import { cleanupStaleData } from "../services/db.js"; +import logger from "../services/logger.js"; + +const mockCleanupStaleData = vi.mocked(cleanupStaleData); + +describe("periodic-cleanup", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + stopPeriodicCleanup(); + }); + + afterEach(() => { + stopPeriodicCleanup(); + vi.useRealTimers(); + }); + + it("should export a 6-hour cleanup interval constant", () => { + expect(CLEANUP_INTERVAL_MS).toBe(6 * 60 * 60 * 1000); + }); + + it("should call cleanupStaleData after one interval elapses", async () => { + startPeriodicCleanup(); + expect(mockCleanupStaleData).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + + expect(mockCleanupStaleData).toHaveBeenCalledTimes(1); + }); + + it("should call cleanupStaleData multiple times over multiple intervals", async () => { + startPeriodicCleanup(); + + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + + expect(mockCleanupStaleData).toHaveBeenCalledTimes(2); + }); + + it("should log errors but not throw when cleanupStaleData fails", async () => { + mockCleanupStaleData.mockRejectedValueOnce(new Error("DB connection lost")); + + startPeriodicCleanup(); + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + + expect(logger.error).toHaveBeenCalledWith( + expect.objectContaining({ err: expect.any(Error) }), + expect.stringContaining("Periodic database cleanup failed") + ); + + // Next interval should still work + mockCleanupStaleData.mockResolvedValueOnce({ expiredVerifications: 1, orphanedUsage: 0 }); + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + + expect(mockCleanupStaleData).toHaveBeenCalledTimes(2); + }); + + it("should stop cleanup when stopPeriodicCleanup is called", async () => { + startPeriodicCleanup(); + + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + expect(mockCleanupStaleData).toHaveBeenCalledTimes(1); + + stopPeriodicCleanup(); + + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + expect(mockCleanupStaleData).toHaveBeenCalledTimes(1); + }); + + it("should be idempotent — calling startPeriodicCleanup twice doesn't create two intervals", async () => { + startPeriodicCleanup(); + startPeriodicCleanup(); + + await vi.advanceTimersByTimeAsync(CLEANUP_INTERVAL_MS); + + expect(mockCleanupStaleData).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/index.ts b/src/index.ts index 151738e..0659e2d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,6 +24,7 @@ import { loadKeys, getAllKeys } from "./services/keys.js"; import { pagesRouter } from "./routes/pages.js"; import { initDatabase, pool, cleanupStaleData } from "./services/db.js"; +import { startPeriodicCleanup, stopPeriodicCleanup } from "./utils/periodic-cleanup.js"; const app = express(); @@ -244,12 +245,18 @@ async function start() { } }, 30_000); + // Run database cleanup every 6 hours (expired verifications, orphaned usage) + startPeriodicCleanup(); + let shuttingDown = false; const shutdown = async (signal: string) => { if (shuttingDown) return; shuttingDown = true; logger.info(`Received ${signal}, starting graceful shutdown...`); + // 0. Stop periodic cleanup timer + stopPeriodicCleanup(); + // 1. Stop accepting new connections, wait for in-flight requests (max 10s) await new Promise((resolve) => { const forceTimeout = setTimeout(() => { diff --git a/src/utils/periodic-cleanup.ts b/src/utils/periodic-cleanup.ts new file mode 100644 index 0000000..6eaae2d --- /dev/null +++ b/src/utils/periodic-cleanup.ts @@ -0,0 +1,30 @@ +import { cleanupStaleData } from "../services/db.js"; +import logger from "../services/logger.js"; + +export const CLEANUP_INTERVAL_MS = 6 * 60 * 60 * 1000; // 6 hours + +let intervalHandle: ReturnType | null = null; + +export function startPeriodicCleanup(): void { + // Idempotent — don't create duplicate intervals + if (intervalHandle !== null) return; + + intervalHandle = setInterval(async () => { + try { + logger.info("Running periodic database cleanup..."); + await cleanupStaleData(); + } catch (err: unknown) { + logger.error({ err }, "Periodic database cleanup failed (non-fatal)"); + } + }, CLEANUP_INTERVAL_MS); + + // Don't prevent graceful shutdown + intervalHandle.unref(); +} + +export function stopPeriodicCleanup(): void { + if (intervalHandle !== null) { + clearInterval(intervalHandle); + intervalHandle = null; + } +}