Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions packages/appkit/src/cache/index.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -202,8 +202,7 @@ export class CacheManager {
},
async (span) => {
try {
// check if the value is in the cache
const cached = await this.storage.get<T>(cacheKey);
const cached = await this.getValid<T>(cacheKey);
if (cached !== null) {
span.setAttribute("cache.hit", true);
span.setStatus({ code: SpanStatusCode.OK });
Expand All @@ -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
Expand Down Expand Up @@ -308,14 +307,26 @@ export class CacheManager {
// probabilistic cleanup trigger
this.maybeCleanup();

const entry = await this.getValid<T>(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<T>(key: string): Promise<CacheEntry<T> | null> {
const entry = await this.storage.get<T>(key);
if (!entry) return null;

if (Date.now() > entry.expiry) {
await this.storage.delete(key);
return null;
}
return entry.value as T;
return entry;
}

/** Probabilistically trigger cleanup of expired entries (fire-and-forget) */
Expand Down Expand Up @@ -386,14 +397,8 @@ export class CacheManager {
async has(key: string): Promise<boolean> {
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;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/appkit/src/cache/tests/cache-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Loading