Fix domain cache never expiring under steady traffic

apps/web/lib/__tests__/domain-cache.test.ts

@@ -11,7 +11,12 @@ jest.mock("../../models/Domain", () => ({
 }));
 
 import DomainModel from "../../models/Domain";
-import { getDomainFromHost } from "../domain-cache";
+import {
+    cacheDomainByName,
+    getCachedDomain,
+    getDomainFromHost,
+    invalidateDomainCache,
+} from "../domain-cache";
 
 describe("domain-cache host resolution", () => {
     const originalDomain = process.env.DOMAIN;
@@ -53,3 +58,53 @@ describe("domain-cache host resolution", () => {
         expect(domain).toBe(school);
     });
 });
+
+describe("domain-cache expiry", () => {
+    const mockFindOne = (DomainModel as any).findOne as jest.Mock;
+
+    beforeEach(() => {
+        mockFindOne.mockReset();
+        jest.useFakeTimers();
+    });
+
+    afterEach(() => {
+        invalidateDomainCache("main");
+        jest.useRealTimers();
+    });
+
+    it("reloads from the database after the TTL even when the entry is re-cached in between", async () => {
+        mockFindOne.mockResolvedValueOnce({ name: "main", version: 1 });
+
+        const first = await getCachedDomain("main");
+        expect(first).toEqual({ name: "main", version: 1 });
+        expect(mockFindOne).toHaveBeenCalledTimes(1);
+
+        // A request 30 seconds later reads the cached copy and caches it again,
+        // the way the verify-domain route does.
+        jest.advanceTimersByTime(30_000);
+        const cached = await getCachedDomain("main");
+        cacheDomainByName(cached!);
+        expect(mockFindOne).toHaveBeenCalledTimes(1);
+
+        // 61 seconds after the first load the entry has expired, so the next
+        // read picks up a change made directly in the database.
+        jest.advanceTimersByTime(31_000);
+        mockFindOne.mockResolvedValueOnce({ name: "main", version: 2 });
+
+        const reloaded = await getCachedDomain("main");
+        expect(mockFindOne).toHaveBeenCalledTimes(2);
+        expect(reloaded).toEqual({ name: "main", version: 2 });
+    });
+
+    it("reloads from the database right after invalidation", async () => {
+        mockFindOne.mockResolvedValueOnce({ name: "main", version: 1 });
+        await getCachedDomain("main");
+
+        invalidateDomainCache("main");
+        mockFindOne.mockResolvedValueOnce({ name: "main", version: 2 });
+
+        const reloaded = await getCachedDomain("main");
+        expect(mockFindOne).toHaveBeenCalledTimes(2);
+        expect(reloaded).toEqual({ name: "main", version: 2 });
+    });
+});

apps/web/lib/domain-cache.ts

@@ -75,9 +75,19 @@ export function cacheDomainByName(domain: Domain): void {
         return;
     }
 
-    domainCacheByName.set(normalizeDomainName(domain.name), {
+    const normalizedDomainName = normalizeDomainName(domain.name);
+    const existing = domainCacheByName.get(normalizedDomainName);
+    const now = Date.now();
+
+    domainCacheByName.set(normalizedDomainName, {
         data: (domain as any).toObject ? (domain as any).toObject() : domain,
-        expiresAt: Date.now() + TTL,
+        // Keep the expiry of an unexpired entry. Callers often re-cache the
+        // copy they just read from this cache, and resetting the expiry then
+        // would stop the entry from ever reloading from the database.
+        expiresAt:
+            existing && existing.expiresAt > now
+                ? existing.expiresAt
+                : now + TTL,
     });
 }