From a32fcd8c6d1be20da1663b3707e153e94e05d2e8 Mon Sep 17 00:00:00 2001 From: Jinho Ayden Jeong <144667387+ayden94@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:50:33 +0900 Subject: [PATCH] Resolve #2276: cover cache-manager edge regressions --- .../cache-manager/src/interceptor.test.ts | 95 +++++++++++--- packages/cache-manager/src/module.test.ts | 116 +++++++++++++++++- .../src/stores/redis-store.test.ts | 18 +++ 3 files changed, 206 insertions(+), 23 deletions(-) diff --git a/packages/cache-manager/src/interceptor.test.ts b/packages/cache-manager/src/interceptor.test.ts index 1c4a050ba..fb92a67c4 100644 --- a/packages/cache-manager/src/interceptor.test.ts +++ b/packages/cache-manager/src/interceptor.test.ts @@ -311,6 +311,29 @@ describe('CacheInterceptor', () => { await expect(cacheService.get('/events')).resolves.toBeUndefined(); }); + it('does not cache undefined GET handler results', async () => { + class ProductController { + @CacheTTL(120) + optional() {} + } + + const { interceptor, cacheService } = createInterceptor({ ttl: 120 }); + const firstContext = createContext(ProductController, 'optional', createRequestContext('GET', '/products/optional', '/products/optional')); + const secondContext = createContext(ProductController, 'optional', createRequestContext('GET', '/products/optional', '/products/optional')); + const next: CallHandler = { + handle: vi + .fn() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined), + }; + + await expect(interceptor.intercept(firstContext, next)).resolves.toBeUndefined(); + await expect(interceptor.intercept(secondContext, next)).resolves.toBeUndefined(); + + expect(next.handle).toHaveBeenCalledTimes(2); + await expect(cacheService.get('/products/optional')).resolves.toBeUndefined(); + }); + it('does not cache non-success HTTP responses returned by GET handlers', async () => { class ProductController { @CacheTTL(120) @@ -520,28 +543,30 @@ describe('CacheInterceptor', () => { const unhandledRejection = vi.fn(); process.once('unhandledRejection', unhandledRejection); - class ProductController { - @CacheEvict(async () => { - throw new Error('evict metadata failed'); - }) - refresh() {} - } - - const { interceptor } = createInterceptor(); - const requestContext = createRequestContext('POST', '/products/refresh'); - const context = createContext(ProductController, 'refresh', requestContext, 'POST'); - const next: CallHandler = { - handle: vi.fn(async () => ({ refreshed: true })), - }; + try { + class ProductController { + @CacheEvict(async () => { + throw new Error('evict metadata failed'); + }) + refresh() {} + } - const value = await interceptor.intercept(context, next); - await expect(requestContext.response.send(value)).resolves.toBeUndefined(); - await new Promise((resolve) => setImmediate(resolve)); + const { interceptor } = createInterceptor(); + const requestContext = createRequestContext('POST', '/products/refresh'); + const context = createContext(ProductController, 'refresh', requestContext, 'POST'); + const next: CallHandler = { + handle: vi.fn(async () => ({ refreshed: true })), + }; - expect(value).toEqual({ refreshed: true }); - expect(unhandledRejection).not.toHaveBeenCalled(); + const value = await interceptor.intercept(context, next); + await expect(requestContext.response.send(value)).resolves.toBeUndefined(); + await new Promise((resolve) => setImmediate(resolve)); - process.removeListener('unhandledRejection', unhandledRejection); + expect(value).toEqual({ refreshed: true }); + expect(unhandledRejection).not.toHaveBeenCalled(); + } finally { + process.removeListener('unhandledRejection', unhandledRejection); + } }); it('evicts immediately when the handler already committed the response before returning', async () => { @@ -592,6 +617,38 @@ describe('CacheInterceptor', () => { await expect(cacheService.get('GET:/products')).resolves.toBeUndefined(); }); + it('contains fallback-timer deferred eviction failures', async () => { + vi.useFakeTimers(); + const unhandledRejection = vi.fn(); + process.once('unhandledRejection', unhandledRejection); + + try { + class ProductController { + @CacheEvict('GET:/products') + refresh() {} + } + + const { cacheService, interceptor } = createInterceptor(); + await cacheService.set('GET:/products', { count: 1 }, 120); + vi.spyOn(cacheService, 'del').mockRejectedValueOnce(new Error('redis down during fallback eviction')); + + const requestContext = createRequestContext('POST', '/products/refresh'); + const context = createContext(ProductController, 'refresh', requestContext, 'POST'); + const next: CallHandler = { + handle: vi.fn(async () => ({ refreshed: true })), + }; + + await expect(interceptor.intercept(context, next)).resolves.toEqual({ refreshed: true }); + await vi.advanceTimersByTimeAsync(5_000); + await Promise.resolve(); + + expect(unhandledRejection).not.toHaveBeenCalled(); + await expect(cacheService.get('GET:/products')).resolves.toEqual({ count: 1 }); + } finally { + process.removeListener('unhandledRejection', unhandledRejection); + } + }); + it('cancels deferred eviction when response.send rejects', async () => { vi.useFakeTimers(); diff --git a/packages/cache-manager/src/module.test.ts b/packages/cache-manager/src/module.test.ts index 16643074e..02f6f873b 100644 --- a/packages/cache-manager/src/module.test.ts +++ b/packages/cache-manager/src/module.test.ts @@ -1,15 +1,14 @@ -import { describe, expect, it, vi } from 'vitest'; - import { Inject } from '@fluojs/core'; import { getModuleMetadata } from '@fluojs/core/internal'; -import { Controller, Get, Post, UseInterceptors, getCurrentRequestContext, type FrameworkRequest, type FrameworkResponse } from '@fluojs/http'; +import { Controller, type FrameworkRequest, type FrameworkResponse, Get, getCurrentRequestContext, Post, UseInterceptors } from '@fluojs/http'; import { getRedisClientToken, REDIS_CLIENT } from '@fluojs/redis'; import { bootstrapApplication, defineModule } from '@fluojs/runtime'; +import { describe, expect, it, vi } from 'vitest'; import { CacheEvict } from './decorators.js'; import { CacheInterceptor } from './interceptor.js'; -import { CacheService } from './service.js'; import { CacheModule } from './module.js'; +import { CacheService } from './service.js'; import { CACHE_OPTIONS } from './tokens.js'; import type { CacheStore, RedisCompatibleClient } from './types.js'; @@ -212,6 +211,115 @@ describe('CacheModule.forRoot', () => { expect(store.close).toHaveBeenCalledTimes(1); }); + it('routes custom stores through CacheService and CacheInterceptor operations', async () => { + type StoreOperation = + | { key: string; ttlSeconds?: number; type: 'set' } + | { key: string; type: 'get' | 'del' } + | { type: 'reset' }; + + class OperationStore implements CacheStore { + readonly operations: StoreOperation[] = []; + private readonly entries = new Map(); + + clearOperations(): void { + this.operations.length = 0; + } + + async get(key: string): Promise { + this.operations.push({ key, type: 'get' }); + return this.entries.get(key) as T | undefined; + } + + async set(key: string, value: T, ttlSeconds?: number): Promise { + this.operations.push({ key, ttlSeconds, type: 'set' }); + this.entries.set(key, value); + } + + async del(key: string): Promise { + this.operations.push({ key, type: 'del' }); + this.entries.delete(key); + } + + async reset(): Promise { + this.operations.push({ type: 'reset' }); + this.entries.clear(); + } + } + + @Inject(CacheService) + class Consumer { + constructor(readonly cache: CacheService) {} + } + + const listHandler = vi.fn(() => ({ count: 1 })); + + @Controller('/custom-store') + class ProductController { + @Get('/') + @UseInterceptors(CacheInterceptor) + list() { + return listHandler(); + } + + @Post('/refresh') + @UseInterceptors(CacheInterceptor) + @CacheEvict('/custom-store') + refresh() { + return { refreshed: true }; + } + } + + const store = new OperationStore(); + + class AppModule {} + defineModule(AppModule, { + controllers: [ProductController], + imports: [CacheModule.forRoot({ store, ttl: 120 })], + providers: [Consumer], + }); + + const app = await bootstrapApplication({ rootModule: AppModule }); + + try { + const consumer = await app.container.resolve(Consumer); + + await consumer.cache.set('/manual', { ok: true }, 45); + await expect(consumer.cache.get('/manual')).resolves.toEqual({ ok: true }); + await consumer.cache.reset(); + + expect(store.operations).toEqual([ + { key: '/manual', ttlSeconds: 45, type: 'set' }, + { key: '/manual', type: 'get' }, + { type: 'reset' }, + ]); + + store.clearOperations(); + + const firstGetResponse = createResponse(); + await app.dispatch(createRequest('/custom-store', 'GET'), firstGetResponse); + + const secondGetResponse = createResponse(); + await app.dispatch(createRequest('/custom-store', 'GET'), secondGetResponse); + + expect(firstGetResponse.body).toEqual({ count: 1 }); + expect(secondGetResponse.body).toEqual(firstGetResponse.body); + expect(listHandler).toHaveBeenCalledTimes(1); + + const refreshResponse = createResponse(); + await app.dispatch(createRequest('/custom-store/refresh', 'POST'), refreshResponse); + + expect(refreshResponse.body).toEqual({ refreshed: true }); + expect(store.operations).toEqual([ + { key: '/custom-store', type: 'get' }, + { key: '/custom-store', ttlSeconds: 120, type: 'set' }, + { key: '/custom-store', type: 'get' }, + { key: '/custom-store', type: 'del' }, + ]); + } finally { + await app.close(); + } + }); + it('fails fast at bootstrap when redis store is selected but redis client is unavailable', async () => { class AppModule {} defineModule(AppModule, { diff --git a/packages/cache-manager/src/stores/redis-store.test.ts b/packages/cache-manager/src/stores/redis-store.test.ts index 146e9735d..9e784fe8d 100644 --- a/packages/cache-manager/src/stores/redis-store.test.ts +++ b/packages/cache-manager/src/stores/redis-store.test.ts @@ -112,6 +112,24 @@ describe('RedisStore', () => { expect(client.storage.get('external:owned-by-app')).toBe(JSON.stringify({ value: 'keep' })); }); + it('limits empty keyPrefix reset ownership to each recreated RedisStore instance', async () => { + const client = new MockRedisClient(); + const firstStore = new RedisStore(client, { keyPrefix: '' }); + + await firstStore.set('cache:first-instance', { value: 'preserve' }); + client.storage.set('external:owned-by-app', JSON.stringify({ value: 'keep' })); + + const recreatedStore = new RedisStore(client, { keyPrefix: '' }); + await recreatedStore.set('cache:recreated-instance', { value: 'remove' }); + + await recreatedStore.reset(); + + expect(client.scanCalls).toEqual([]); + await expect(firstStore.get('cache:first-instance')).resolves.toEqual({ value: 'preserve' }); + await expect(recreatedStore.get('cache:recreated-instance')).resolves.toBeUndefined(); + expect(client.storage.get('external:owned-by-app')).toBe(JSON.stringify({ value: 'keep' })); + }); + it('deletes an owned empty-string key when keyPrefix is empty', async () => { const client = new MockRedisClient(); const store = new RedisStore(client, { keyPrefix: '' });