From 46a68df8c167ca661a96b00f51ea8c15c03f19d8 Mon Sep 17 00:00:00 2001 From: sur-ser Date: Sun, 9 Aug 2026 16:36:57 -0700 Subject: [PATCH 1/4] fix(interceptor): correct cache hits, keys and failure handling --- .../src/cache-ttl.decorator.ts | 2 +- .../cacheable.interceptor.regression.spec.ts | 186 ++++++++++++++++++ .../src/cacheable.interceptor.spec.ts | 31 ++- .../src/cacheable.interceptor.ts | 138 +++++++++++-- 4 files changed, 334 insertions(+), 23 deletions(-) create mode 100644 libs/nestjs-cacheable/src/cacheable.interceptor.regression.spec.ts diff --git a/libs/nestjs-cacheable/src/cache-ttl.decorator.ts b/libs/nestjs-cacheable/src/cache-ttl.decorator.ts index 13a3eda..b52eca6 100644 --- a/libs/nestjs-cacheable/src/cache-ttl.decorator.ts +++ b/libs/nestjs-cacheable/src/cache-ttl.decorator.ts @@ -1,4 +1,4 @@ import { SetMetadata } from '@nestjs/common' export const CACHE_TTL_KEY = 'cache_ttl' -export const CacheTTL = (ttl: number) => SetMetadata(CACHE_TTL_KEY, ttl) +export const CacheTTL = (ttl: number | string) => SetMetadata(CACHE_TTL_KEY, ttl) diff --git a/libs/nestjs-cacheable/src/cacheable.interceptor.regression.spec.ts b/libs/nestjs-cacheable/src/cacheable.interceptor.regression.spec.ts new file mode 100644 index 0000000..e928fbd --- /dev/null +++ b/libs/nestjs-cacheable/src/cacheable.interceptor.regression.spec.ts @@ -0,0 +1,186 @@ +import { CallHandler, ExecutionContext } from '@nestjs/common' +import { Reflector } from '@nestjs/core' +import { Cacheable } from 'cacheable' +import { firstValueFrom, of, toArray } from 'rxjs' +import { CacheableInterceptor } from './cacheable.interceptor' +import { NestjsCacheableService } from './nestjs-cacheable.service' + +function contextFor(request: Record): ExecutionContext { + return { + switchToHttp: () => ({ getRequest: () => request }), + getHandler: () => function handler() {}, + } as unknown as ExecutionContext +} + +function handlerFor(value: unknown): { + handler: CallHandler + calls: () => number +} { + const handle = jest.fn(() => of(value)) + return { + handler: { handle } as CallHandler, + calls: () => handle.mock.calls.length, + } +} + +describe('CacheableInterceptor (regressions)', () => { + let service: NestjsCacheableService + let interceptor: CacheableInterceptor + + beforeEach(() => { + service = new NestjsCacheableService(new Cacheable()) + interceptor = new CacheableInterceptor(service, new Reflector()) + }) + + it('serves a cached falsy value instead of running the handler again', async () => { + const context = contextFor({ url: '/count', method: 'GET', headers: {} }) + const first = handlerFor(0) + + await firstValueFrom(await interceptor.intercept(context, first.handler)) + + const second = handlerFor(0) + const body = await firstValueFrom( + await interceptor.intercept(context, second.handler), + ) + + expect(body).toBe(0) + expect(second.calls()).toBe(0) + }) + + it('never serves a mutation response to a read of the same url', async () => { + const post = handlerFor({ created: true }) + await firstValueFrom( + await interceptor.intercept( + contextFor({ url: '/items', method: 'POST', headers: {} }), + post.handler, + ), + ) + + const get = handlerFor({ items: ['real'] }) + const body = await firstValueFrom( + await interceptor.intercept( + contextFor({ url: '/items', method: 'GET', headers: {} }), + get.handler, + ), + ) + + expect(body).toEqual({ items: ['real'] }) + }) + + it('does not share a response between callers that send credentials', async () => { + const alice = handlerFor({ balance: 'alice' }) + await firstValueFrom( + await interceptor.intercept( + contextFor({ + url: '/me', + method: 'GET', + headers: { authorization: 'Bearer alice' }, + }), + alice.handler, + ), + ) + + const bob = handlerFor({ balance: 'bob' }) + const body = await firstValueFrom( + await interceptor.intercept( + contextFor({ + url: '/me', + method: 'GET', + headers: { authorization: 'Bearer bob' }, + }), + bob.handler, + ), + ) + + expect(body).toEqual({ balance: 'bob' }) + }) + + it('runs the handler once for concurrent identical requests', async () => { + const context = contextFor({ url: '/slow', method: 'GET', headers: {} }) + const handle = jest.fn(() => of({ ok: true })) + const handler = { handle } as CallHandler + + await Promise.all([ + interceptor + .intercept(context, handler) + .then((response$) => firstValueFrom(response$)), + interceptor + .intercept(context, handler) + .then((response$) => firstValueFrom(response$)), + interceptor + .intercept(context, handler) + .then((response$) => firstValueFrom(response$)), + ]) + + expect(handle).toHaveBeenCalledTimes(1) + }) + + it('passes every emission of the handler through', async () => { + const context = contextFor({ url: '/stream', method: 'GET', headers: {} }) + const handle = jest.fn(() => of(1, 2, 3)) + + const emitted = await firstValueFrom( + (await interceptor.intercept(context, { handle } as CallHandler)).pipe( + toArray(), + ), + ) + + expect(emitted).toEqual([1, 2, 3]) + }) + + it('answers the request when the cache fails to read', async () => { + const failing = { + get: jest.fn().mockRejectedValue(new Error('ECONNREFUSED')), + set: jest.fn().mockResolvedValue(true), + } as unknown as NestjsCacheableService + const local = new CacheableInterceptor(failing, new Reflector()) + const { handler, calls } = handlerFor({ ok: true }) + + const body = await firstValueFrom( + await local.intercept( + contextFor({ url: '/data', method: 'GET', headers: {} }), + handler, + ), + ) + + expect(body).toEqual({ ok: true }) + expect(calls()).toBe(1) + }) + + it('answers the request when the cache fails to write', async () => { + const failing = { + get: jest.fn().mockResolvedValue(undefined), + set: jest.fn().mockRejectedValue(new Error('ECONNREFUSED')), + } as unknown as NestjsCacheableService + const local = new CacheableInterceptor(failing, new Reflector()) + + const body = await firstValueFrom( + await local.intercept( + contextFor({ url: '/data', method: 'GET', headers: {} }), + handlerFor({ ok: true }).handler, + ), + ) + + expect(body).toEqual({ ok: true }) + }) +}) + +describe('NestjsCacheableService (regressions)', () => { + it('caches a value that cannot be serialised to JSON', async () => { + const service = new NestjsCacheableService(new Cacheable()) + const circular: Record = { name: 'node' } + circular.self = circular + + await expect(service.set('circular', circular)).resolves.toBe(true) + }) + + it('disconnects the primary store as well as the secondary', async () => { + const cache = new Cacheable() + const service = new NestjsCacheableService(cache) + const primary = jest.spyOn(cache.primary, 'disconnect') + + await service.disconnect() + + expect(primary).toHaveBeenCalled() + }) +}) diff --git a/libs/nestjs-cacheable/src/cacheable.interceptor.spec.ts b/libs/nestjs-cacheable/src/cacheable.interceptor.spec.ts index 71d81df..4f6ee2e 100644 --- a/libs/nestjs-cacheable/src/cacheable.interceptor.spec.ts +++ b/libs/nestjs-cacheable/src/cacheable.interceptor.spec.ts @@ -23,6 +23,8 @@ describe('CacheableInterceptor', () => { switchToHttp: () => ({ getRequest: () => ({ url: '/test', + method: 'GET', + headers: {}, }), }), getHandler: () => ({}), @@ -61,8 +63,11 @@ describe('CacheableInterceptor', () => { }) it('should return cached value if it exists', async () => { - mockCacheService.get.mockResolvedValue('cached_value') - const result$ = await interceptor.intercept(mockExecutionContext, mockCallHandler) + mockCacheService.get.mockResolvedValue({ value: 'cached_value' }) + const result$ = await interceptor.intercept( + mockExecutionContext, + mockCallHandler, + ) const result = await firstValueFrom(result$) expect(result).toBe('cached_value') expect(mockCallHandler.handle).not.toHaveBeenCalled() @@ -71,18 +76,32 @@ describe('CacheableInterceptor', () => { it('should call handler, cache the result, and return it if no cached value', async () => { mockCacheService.get.mockResolvedValue(undefined) mockCallHandler.handle.mockReturnValue(of('new_value')) - const result$ = await interceptor.intercept(mockExecutionContext, mockCallHandler) + const result$ = await interceptor.intercept( + mockExecutionContext, + mockCallHandler, + ) const value = await firstValueFrom(result$) expect(value).toBe('new_value') - expect(mockCacheService.set).toHaveBeenCalledWith('/test', 'new_value', undefined) + expect(mockCacheService.set).toHaveBeenCalledWith( + 'GET:/test', + { value: 'new_value' }, + undefined, + ) }) it('should use TTL from decorator if present', async () => { mockCacheService.get.mockResolvedValue(undefined) mockCallHandler.handle.mockReturnValue(of('new_value')) mockReflector.get.mockReturnValue(5000) - const result$ = await interceptor.intercept(mockExecutionContext, mockCallHandler) + const result$ = await interceptor.intercept( + mockExecutionContext, + mockCallHandler, + ) await firstValueFrom(result$) - expect(mockCacheService.set).toHaveBeenCalledWith('/test', 'new_value', 5000) + expect(mockCacheService.set).toHaveBeenCalledWith( + 'GET:/test', + { value: 'new_value' }, + 5000, + ) }) }) diff --git a/libs/nestjs-cacheable/src/cacheable.interceptor.ts b/libs/nestjs-cacheable/src/cacheable.interceptor.ts index 05f9d5a..7cd794a 100644 --- a/libs/nestjs-cacheable/src/cacheable.interceptor.ts +++ b/libs/nestjs-cacheable/src/cacheable.interceptor.ts @@ -1,35 +1,141 @@ -import { Injectable, NestInterceptor, ExecutionContext, CallHandler } from '@nestjs/common' -import { Observable, of, firstValueFrom } from 'rxjs' +import { + Injectable, + NestInterceptor, + ExecutionContext, + CallHandler, + Logger, +} from '@nestjs/common' +import { Observable, of } from 'rxjs' +import { finalize, shareReplay, tap } from 'rxjs/operators' import { NestjsCacheableService } from './nestjs-cacheable.service' import { Reflector } from '@nestjs/core' import { CACHE_TTL_KEY } from './cache-ttl.decorator' +/** + * Envelope stored in the cache so that a cached `0`, `''`, `false` or `null` + * is still recognised as a hit. + */ +type CacheEnvelope = { value: unknown } + +/** Methods whose responses may be cached. */ +const CACHEABLE_METHODS = new Set(['GET', 'HEAD']) + @Injectable() export class CacheableInterceptor implements NestInterceptor { + private readonly logger = new Logger(CacheableInterceptor.name) + + /** Responses being produced right now, keyed by cache key. */ + private readonly inFlight = new Map>() + constructor( private readonly cacheService: NestjsCacheableService, private readonly reflector: Reflector, ) {} - async intercept(context: ExecutionContext, next: CallHandler): Promise> { + async intercept( + context: ExecutionContext, + next: CallHandler, + ): Promise> { const key = this.getCacheKey(context) - const cachedValue = await this.cacheService.get(key) - if (cachedValue) { - return of(cachedValue) + if (key === undefined) { + return next.handle() + } + + const cached = await this.read(key) + + if (cached !== undefined) { + return of(cached.value) + } + + const pending = this.inFlight.get(key) + + if (pending) { + return pending + } + + const ttl = this.reflector.get( + CACHE_TTL_KEY, + context.getHandler(), + ) + + const response$ = next.handle().pipe( + tap((value) => { + void this.write(key, value, ttl) + }), + finalize(() => { + this.inFlight.delete(key) + }), + shareReplay({ bufferSize: 1, refCount: false }), + ) + + this.inFlight.set(key, response$) + + return response$ + } + + /** + * Cache key for a request, or `undefined` when the request must not be + * cached. + * + * The default keys by method and url, which is only correct for responses + * that are identical for every caller. Requests that carry credentials are + * therefore not cached at all by default: keying them by url alone would + * serve one user's response to the next. Override this method to cache them + * under a key that includes the user. + */ + protected getCacheKey(context: ExecutionContext): string | undefined { + const request = context.switchToHttp().getRequest() + const method: string = request?.method ?? 'GET' + + if (!CACHEABLE_METHODS.has(method.toUpperCase())) { + return undefined } - const ttl = this.reflector.get(CACHE_TTL_KEY, context.getHandler()) + if (this.isPerCaller(request)) { + return undefined + } - const value = await firstValueFrom(next.handle()) - await this.cacheService.set(key, value, ttl) - return of(value) + return `${method.toUpperCase()}:${request.url}` } - private getCacheKey(context: ExecutionContext): string { - // A simple key generation strategy. This can be improved later. - const httpContext = context.switchToHttp() - const request = httpContext.getRequest() - return request.url + /** True when the response may depend on who is asking. */ + protected isPerCaller(request: { + user?: unknown + headers?: Record + }): boolean { + const headers = request?.headers ?? {} + + return ( + request?.user !== undefined || + headers['authorization'] !== undefined || + headers['cookie'] !== undefined + ) + } + + /** A cache outage must never fail a request. */ + private async read(key: string): Promise { + try { + return await this.cacheService.get(key) + } catch (error) { + this.logger.debug( + `Cache read failed for key ${key}: ${(error as Error).message}`, + ) + return undefined + } + } + + private async write( + key: string, + value: unknown, + ttl?: number | string, + ): Promise { + try { + await this.cacheService.set(key, { value }, ttl) + } catch (error) { + this.logger.debug( + `Cache write failed for key ${key}: ${(error as Error).message}`, + ) + } } -} \ No newline at end of file +} From ef3ca790c69b8353c7359e5c11a894b8889fbaa4 Mon Sep 17 00:00:00 2001 From: sur-ser Date: Sun, 9 Aug 2026 16:36:57 -0700 Subject: [PATCH 2/4] fix(service): disconnect the primary store and stop dumping values into logs --- .../src/nestjs-cacheable.service.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libs/nestjs-cacheable/src/nestjs-cacheable.service.ts b/libs/nestjs-cacheable/src/nestjs-cacheable.service.ts index 4bebbb2..3fa0006 100644 --- a/libs/nestjs-cacheable/src/nestjs-cacheable.service.ts +++ b/libs/nestjs-cacheable/src/nestjs-cacheable.service.ts @@ -23,8 +23,10 @@ export class NestjsCacheableService implements OnModuleDestroy { } async disconnect() { - if (this.cache.secondary && typeof this.cache.secondary.disconnect === 'function') { - await this.cache.secondary.disconnect() + for (const store of [this.cache.primary, this.cache.secondary]) { + if (store && typeof store.disconnect === 'function') { + await store.disconnect() + } } } @@ -32,14 +34,12 @@ export class NestjsCacheableService implements OnModuleDestroy { return this.cache.get(key) } - async set(key: string, value: any, ttl?: number): Promise { - this.logger.log(`Setting cache for key: ${key}, value: ${JSON.stringify(value)}, ttl: ${ttl}`) - if (ttl !== undefined) { - await this.cache.set(key, value, ttl) - } else { - await this.cache.set(key, value) - } - return true + async set(key: string, value: any, ttl?: number | string): Promise { + this.logger.debug(`Setting cache for key: ${key}, ttl: ${ttl}`) + + return ttl === undefined + ? this.cache.set(key, value) + : this.cache.set(key, value, ttl) } async del(key: string): Promise { From cf9ad7b6b9e7f88a7f41e76525354308b01cfae6 Mon Sep 17 00:00:00 2001 From: sur-ser Date: Sun, 9 Aug 2026 16:36:57 -0700 Subject: [PATCH 3/4] fix(package): declare the nestjs peer dependencies --- libs/nestjs-cacheable/package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/nestjs-cacheable/package.json b/libs/nestjs-cacheable/package.json index 828401b..293b2ad 100644 --- a/libs/nestjs-cacheable/package.json +++ b/libs/nestjs-cacheable/package.json @@ -16,6 +16,8 @@ }, "peerDependencies": { "@keyv/redis": "^5.0.2", + "@nestjs/common": "^10.0.0 || ^11.0.0", + "@nestjs/core": "^10.0.0 || ^11.0.0", "cacheable": "^1.0.0", "ioredis": "^5.0.0", "keyv": "^5.0.0", From d113e595b0d27110107842b53e968f6288d97076 Mon Sep 17 00:00:00 2001 From: sur-ser Date: Sun, 9 Aug 2026 16:36:57 -0700 Subject: [PATCH 4/4] docs: fix the package name in examples and document caching rules --- libs/nestjs-cacheable/README.md | 34 +++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/libs/nestjs-cacheable/README.md b/libs/nestjs-cacheable/README.md index deb2564..ce5ab1b 100644 --- a/libs/nestjs-cacheable/README.md +++ b/libs/nestjs-cacheable/README.md @@ -21,7 +21,7 @@ For static configuration, use the `register()` method. ```typescript // app.module.ts import { Module } from '@nestjs/common' -import { NestjsCacheableModule } from '@m8a-io/nestjs-cacheable' +import { NestjsCacheableModule } from '@m8a/nestjs-cacheable' import KeyvRedis from '@keyv/redis' @Module({ @@ -41,7 +41,7 @@ For asynchronous configuration, use the `registerAsync()` method. This is useful ```typescript // app.module.ts import { Module } from '@nestjs/common' -import { NestjsCacheableModule } from '@m8a-io/nestjs-cacheable' +import { NestjsCacheableModule } from '@m8a/nestjs-cacheable' import { ConfigModule, ConfigService } from '@nestjs/config' import KeyvRedis from '@keyv/redis' @@ -66,7 +66,7 @@ The easiest way to use the cache is with the `CacheableInterceptor`. You can app ```typescript // app.controller.ts import { Controller, Get, UseInterceptors } from '@nestjs/common' -import { CacheableInterceptor, CacheTTL } from '@m8a-io/nestjs-cacheable' +import { CacheableInterceptor, CacheTTL } from '@m8a/nestjs-cacheable' @Controller() export class AppController { @@ -82,6 +82,32 @@ export class AppController { } ``` +#### What the interceptor caches + +The default key is `METHOD:url`, and only `GET` and `HEAD` responses are cached. + +Requests that carry credentials (a `user` on the request, an `Authorization` +header or a `Cookie`) are **not** cached by default, because keying them by url +alone would serve one user's response to the next. To cache them, subclass the +interceptor and key by the caller: + +```typescript +@Injectable() +export class UserScopedCacheInterceptor extends CacheableInterceptor { + protected getCacheKey(context: ExecutionContext): string | undefined { + const request = context.switchToHttp().getRequest() + + if (request.method !== 'GET') return undefined + + return `GET:${request.user.id}:${request.url}` + } +} +``` + +Concurrent requests that miss the same key run the handler once and share the +result. A cache backend that is unreachable is logged at debug level and the +request is answered from the handler. + ### Using the Service Directly You can also inject the `NestjsCacheableService` to interact with the cache programmatically. @@ -89,7 +115,7 @@ You can also inject the `NestjsCacheableService` to interact with the cache prog ```typescript // my.service.ts import { Injectable } from '@nestjs/common' -import { NestjsCacheableService } from '@m8a-io/nestjs-cacheable' +import { NestjsCacheableService } from '@m8a/nestjs-cacheable' @Injectable() export class MyService {