diff --git a/packages/cli/src/services/cache.service.ts b/packages/cli/src/services/cache.service.ts index 16ba2584b..f3651d4f8 100644 --- a/packages/cli/src/services/cache.service.ts +++ b/packages/cli/src/services/cache.service.ts @@ -80,6 +80,9 @@ export class CacheService { refreshTtl?: number; } = {}, ): Promise { + if (!key || key.length === 0) { + return; + } const value = await this.cache?.store.get(key); if (value !== undefined) { return value; @@ -113,6 +116,9 @@ export class CacheService { refreshTtl?: number; } = {}, ): Promise { + if (keys.length === 0) { + return []; + } let values = await this.cache?.store.mget(...keys); if (values === undefined) { values = keys.map(() => undefined); @@ -163,6 +169,9 @@ export class CacheService { if (!this.cache) { await this.init(); } + if (!key || key.length === 0) { + return; + } if (value === undefined || value === null) { return; } @@ -183,8 +192,13 @@ export class CacheService { if (!this.cache) { await this.init(); } + if (values.length === 0) { + return; + } // eslint-disable-next-line @typescript-eslint/naming-convention - const nonNullValues = values.filter(([_key, value]) => value !== undefined && value !== null); + const nonNullValues = values.filter( + ([key, value]) => value !== undefined && value !== null && key && key.length > 0, + ); if (this.isRedisCache()) { // eslint-disable-next-line @typescript-eslint/naming-convention nonNullValues.forEach(([_key, value]) => { @@ -201,6 +215,9 @@ export class CacheService { * @param key The key to delete */ async delete(key: string): Promise { + if (!key || key.length === 0) { + return; + } await this.cache?.store.del(key); } @@ -209,6 +226,9 @@ export class CacheService { * @param keys List of keys to delete */ async deleteMany(keys: string[]): Promise { + if (keys.length === 0) { + return; + } return this.cache?.store.mdel(...keys); } diff --git a/packages/cli/test/unit/services/cache-mock.service.test.ts b/packages/cli/test/unit/services/cache-mock.service.test.ts new file mode 100644 index 000000000..925ab9fdc --- /dev/null +++ b/packages/cli/test/unit/services/cache-mock.service.test.ts @@ -0,0 +1,67 @@ +import Container from 'typedi'; +import { mock } from 'jest-mock-extended'; +import { CacheService } from '@/services/cache.service'; + +const cacheService = Container.get(CacheService); +const store = mock['store']>({ isCacheable: () => true }); +Object.assign(cacheService, { cache: { store } }); + +describe('CacheService (Mock)', () => { + beforeEach(() => jest.clearAllMocks()); + + describe('should prevent use of empty keys', () => { + test('get', async () => { + await cacheService.get(''); + expect(store.get).not.toHaveBeenCalled(); + + await cacheService.get('key'); + expect(store.get).toHaveBeenCalledWith('key'); + }); + + test('getMany', async () => { + await cacheService.getMany([]); + expect(store.mget).not.toHaveBeenCalled(); + + await cacheService.getMany(['key1', 'key2']); + expect(store.mget).toHaveBeenCalledWith('key1', 'key2'); + }); + + test('set', async () => { + await cacheService.set('', ''); + expect(store.set).not.toHaveBeenCalled(); + + await cacheService.set('key', 'value'); + expect(store.set).toHaveBeenCalledWith('key', 'value', undefined); + + await cacheService.set('key', 'value', 123); + expect(store.set).toHaveBeenCalledWith('key', 'value', 123); + }); + + test('setMany', async () => { + await cacheService.setMany([]); + expect(store.mset).not.toHaveBeenCalled(); + + await cacheService.setMany([['key', 'value']]); + expect(store.mset).toHaveBeenCalledWith([['key', 'value']], undefined); + + await cacheService.setMany([['key', 'value']], 123); + expect(store.mset).toHaveBeenCalledWith([['key', 'value']], 123); + }); + + test('delete', async () => { + await cacheService.delete(''); + expect(store.del).not.toHaveBeenCalled(); + + await cacheService.delete('key'); + expect(store.del).toHaveBeenCalledWith('key'); + }); + + test('deleteMany', async () => { + await cacheService.deleteMany([]); + expect(store.mdel).not.toHaveBeenCalled(); + + await cacheService.deleteMany(['key1', 'key2']); + expect(store.mdel).toHaveBeenCalledWith('key1', 'key2'); + }); + }); +}); diff --git a/packages/cli/test/unit/services/cache.service.test.ts b/packages/cli/test/unit/services/cache.service.test.ts index 479964e0f..f2f51b203 100644 --- a/packages/cli/test/unit/services/cache.service.test.ts +++ b/packages/cli/test/unit/services/cache.service.test.ts @@ -339,4 +339,33 @@ describe('cacheService', () => { await expect(cacheService.get('undefValue')).resolves.toBeUndefined(); await expect(cacheService.get('nullValue')).resolves.toBeUndefined(); }); + + test('should handle setting empty keys', async () => { + await cacheService.set('', null); + await expect(cacheService.get('')).resolves.toBeUndefined(); + await cacheService.setMany([ + ['', 'something'], + ['', 'something'], + ]); + await expect(cacheService.getMany([''])).resolves.toStrictEqual([undefined]); + await cacheService.setMany([]); + await expect(cacheService.getMany([])).resolves.toStrictEqual([]); + }); + + test('should handle setting empty keys (redis)', async () => { + config.set('cache.backend', 'redis'); + config.set('executions.mode', 'queue'); + await cacheService.destroy(); + await cacheService.init(); + + await cacheService.set('', null); + await expect(cacheService.get('')).resolves.toBeUndefined(); + await cacheService.setMany([ + ['', 'something'], + ['', 'something'], + ]); + await expect(cacheService.getMany([''])).resolves.toStrictEqual([undefined]); + await cacheService.setMany([]); + await expect(cacheService.getMany([])).resolves.toStrictEqual([]); + }); });