Review a cache backend migration

Review exercise · TypeScript

Advanced45 min

Review brief

A multi-tenant service is introducing a shared cache while retaining its process-local cache as a rollout fallback. The project repository remains authoritative, and caching must be transparent to request correctness. Review whether the abstraction preserves identity, expiration, invalidation, and outage behavior across both implementations.

Behavioral contract

Every cache key is scoped by both tenant ID and project ID. Identifiers are normalized and cannot contain the namespace separator used by the local cache.
`ProjectCache.set` expresses TTL in milliseconds, while `SharedCacheClient.setEx` accepts whole seconds; the adapter must convert at that boundary.
A cached `null` is a valid negative-cache entry and suppresses repository reads until expiry. Only `undefined` means a cache miss.
During rollout, reads prefer the shared backend and then the process-local fallback. Successful repository loads fill both layers on a best-effort basis.
A mutation must invalidate both layers so a shared miss cannot resurrect an older fallback value.
Cache get, set, or delete failures must never fail a project lookup or rename; fall back to another layer or the authoritative repository and expose cache failures only through operations reporting.
Shared cache payloads are written only by this adapter and are guaranteed to be valid JSON.

Review instructions

1Review only the changed code against the migration's observable cache contract.
2Trace reads and invalidations through both layers, including normal misses and backend errors.
3Prioritize no more than six findings, with test feedback tied to the behaviors it fails to protect.

Learning objectives

• Review abstractions for semantic compatibility
• Preserve isolation and invalidation across migrations
• Test fallback behavior rather than only the preferred backend

src/project-cache-types.tsTypeScript

@@ -0,0 +1,29 @@

1+export interface Project {

2+ id: string;

3+ tenantId: string;

4+ name: string;

5+}

7+export type CachedProject = Project | null;

9+export interface ProjectCache {

10+ get(tenantId: string, projectId: string): Promise<CachedProject | undefined>;

11+ set(

12+ tenantId: string,

13+ projectId: string,

14+ value: CachedProject,

15+ ttlMilliseconds: number,

16+ ): Promise<void>;

17+ delete(tenantId: string, projectId: string): Promise<void>;

18+}

19+

20+export interface SharedCacheClient {

21+ get(key: string): Promise<string | null>;

22+ setEx(key: string, ttlSeconds: number, value: string): Promise<void>;

23+ delete(key: string): Promise<void>;

24+}

25+

26+export interface ProjectRepository {

27+ find(tenantId: string, projectId: string): Promise<Project | null>;

28+ updateName(tenantId: string, projectId: string, name: string): Promise<Project>;

29+}

Reference solution

The findings below are ordered by priority.

Findings

1. Shared cache keys collide across tenants

Location: src/migrating-project-cache.ts:49-51

The shared key discards tenantId and uses only the project ID. Two tenants with the same project ID read and overwrite one cache entry, which can return one customer's project data to another customer.

Include the normalized tenant and project identities in every shared key, under a stable namespace, and keep the same identity semantics in all cache implementations.

2. Invalidation leaves stale fallback entries that can reappear

Location: src/migrating-project-cache.ts:80-82

delete clears only the shared backend. The process-local fallback retains its old value; the next shared miss reads that fallback at lines 57-63 and resurrects the pre-rename project until its long local TTL expires.

Invalidate both layers as one best-effort operation. A shared miss after a mutation must not make an older fallback value authoritative again.

3. Shared-cache errors fail otherwise valid project requests

Location: src/migrating-project-cache.ts:53-81 and src/project-service.ts:11-35

Shared get, setEx, and delete calls are awaited without recovery. An outage prevents reads from reaching the local fallback or repository, turns a successful repository lookup into an error during cache fill, and can make a completed rename appear failed during invalidation.

Treat each cache operation as best effort: recover from shared errors, use the fallback and repository as appropriate, and report cache health separately without changing the repository-backed request result.

4. Millisecond TTLs are passed to an API that expects seconds

Location: src/migrating-project-cache.ts:66-77

The ProjectCache abstraction supplies ttlMilliseconds, but SharedCacheClient.setEx expects seconds. Passing the number unchanged keeps shared entries for roughly 1,000 times the intended lifetime, increasing stale-data exposure after the fallback window.

Convert the duration at the adapter boundary, with an explicit rounding policy, while continuing to pass milliseconds to the process-local implementation.

5. Negative cache entries are treated as misses

Location: src/project-service.ts:11-18

The cache contract distinguishes null (a cached not-found result) from undefined (no entry), but the truthiness check rejects both. Every request for a missing project therefore reloads the repository despite a valid negative entry.

Check specifically for cached !== undefined and return the cached null until its TTL expires.

6. The fake bypasses the migration semantics under review

Location: src/project-service.test.ts:6-44

The fake keys only by project ID, never expires entries, and tests ProjectService without the shared/fallback composition. The happy-path assertion can pass while tenant isolation, TTL conversion, fallback invalidation, negative caching, and outage behavior are all broken.

Add adapter and migration tests with two tenants sharing a project ID, a controllable clock, both cache layers, injected shared failures, and cached not-found values.

Reasonable non-findings

Lazy expiry in ProcessLocalProjectCache is acceptable; eager cleanup and a maximum entry count are outside this exercise's contract.
Serializing a cached null as JSON preserves a distinct negative entry. The defect is the service's truthiness check, not JSON serialization.
The shared-first, local-fallback read order is the intended rollout order when the shared backend returns a normal miss.
Cache payloads are written only by this adapter and guaranteed to contain valid JSON, so malformed payload recovery is out of scope.