diff --git a/packages/appkit/src/cache/index.ts b/packages/appkit/src/cache/index.ts index 37bd1659e..873aada18 100644 --- a/packages/appkit/src/cache/index.ts +++ b/packages/appkit/src/cache/index.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { ApiError, WorkspaceClient } from "@databricks/sdk-experimental"; -import type { CacheConfig, CacheStorage } from "shared"; +import type { CacheConfig, CacheEntry, CacheStorage } from "shared"; import { createLakebasePool } from "../connectors/lakebase"; import { AppKitError, ExecutionError, InitializationError } from "../errors"; import { createLogger } from "../logging/logger"; @@ -202,8 +202,7 @@ export class CacheManager { }, async (span) => { try { - // check if the value is in the cache - const cached = await this.storage.get(cacheKey); + const cached = await this.getValid(cacheKey); if (cached !== null) { span.setAttribute("cache.hit", true); span.setStatus({ code: SpanStatusCode.OK }); @@ -216,7 +215,7 @@ export class CacheManager { cache_key: cacheKey, }); - return cached.value as T; + return cached.value; } // check if the value is being processed by another request @@ -308,6 +307,18 @@ export class CacheManager { // probabilistic cleanup trigger this.maybeCleanup(); + const entry = await this.getValid(key); + return entry?.value ?? null; + } + + /** + * Get a cached entry only if it has not expired. + * Returns null on miss or expired (and deletes the expired entry). + * + * Storage implementations return entries unconditionally — expiry handling + * lives at the CacheManager layer. + */ + private async getValid(key: string): Promise | null> { const entry = await this.storage.get(key); if (!entry) return null; @@ -315,7 +326,7 @@ export class CacheManager { await this.storage.delete(key); return null; } - return entry.value as T; + return entry; } /** Probabilistically trigger cleanup of expired entries (fire-and-forget) */ @@ -386,14 +397,8 @@ export class CacheManager { async has(key: string): Promise { if (!this.config.enabled) return false; - const entry = await this.storage.get(key); - if (!entry) return false; - - if (Date.now() > entry.expiry) { - await this.storage.delete(key); - return false; - } - return true; + const entry = await this.getValid(key); + return entry !== null; } /** diff --git a/packages/appkit/src/cache/tests/cache-manager.test.ts b/packages/appkit/src/cache/tests/cache-manager.test.ts index dcf94f097..3916ef1eb 100644 --- a/packages/appkit/src/cache/tests/cache-manager.test.ts +++ b/packages/appkit/src/cache/tests/cache-manager.test.ts @@ -354,6 +354,30 @@ describe("CacheManager", () => { expect(result1).toBe("user1-data"); expect(result2).toBe("user2-data"); }); + + test("should re-execute function when cached entry has expired", async () => { + const cache = await CacheManager.getInstance({ + storage: createMockStorage(), + }); + let calls = 0; + const fn = vi.fn().mockImplementation(async () => `result-${++calls}`); + + // First call - populates cache with a 1ms TTL + const r1 = await cache.getOrExecute(["key"], fn, "user1", { + ttl: 0.001, + }); + expect(r1).toBe("result-1"); + + // Wait for expiry + await new Promise((resolve) => setTimeout(resolve, 10)); + + // Second call - cached entry is past its expiry, fn must run again + const r2 = await cache.getOrExecute(["key"], fn, "user1", { + ttl: 0.001, + }); + expect(r2).toBe("result-2"); + expect(fn).toHaveBeenCalledTimes(2); + }); }); describe("disabled cache", () => {