diff --git a/lib/AbstractFlatCache.ts b/lib/AbstractFlatCache.ts index 251603d..48e8082 100644 --- a/lib/AbstractFlatCache.ts +++ b/lib/AbstractFlatCache.ts @@ -20,11 +20,14 @@ export abstract class AbstractFlatCache { - const key = this.cacheKeyFromLoadParamsResolver(loadParams) + return this.getAsyncOnlyResolved(this.cacheKeyFromLoadParamsResolver(loadParams), loadParams) + } + + private getAsyncOnlyResolved(key: string, loadParams: LoadParams): Promise { const existingLoad = this.runningLoads.get(key) if (existingLoad) { return existingLoad @@ -86,12 +92,13 @@ export abstract class AbstractFlatCache { - const inMemoryValue = this.getInMemoryOnly(loadParams) + const key = this.cacheKeyFromLoadParamsResolver(loadParams) + const inMemoryValue = this.getInMemoryOnlyResolved(key, loadParams) if (inMemoryValue !== undefined) { return Promise.resolve(inMemoryValue) } - return this.getAsyncOnly(loadParams) + return this.getAsyncOnlyResolved(key, loadParams) } public getMany(keys: string[], loadParams?: LoadManyParams): Promise { @@ -103,7 +110,12 @@ export abstract class AbstractFlatCache { - return [...inMemoryValues.resolvedValues, ...asyncRetrievedValues.resolvedValues] + // in-memory caches always return a fresh array, so it is safe to append to it in place + const mergedValues = inMemoryValues.resolvedValues + for (let i = 0; i < asyncRetrievedValues.resolvedValues.length; i++) { + mergedValues.push(asyncRetrievedValues.resolvedValues[i]) + } + return mergedValues }) } diff --git a/lib/AbstractGroupCache.ts b/lib/AbstractGroupCache.ts index 90f5e14..c3100e8 100644 --- a/lib/AbstractGroupCache.ts +++ b/lib/AbstractGroupCache.ts @@ -42,12 +42,19 @@ export abstract class AbstractGroupCache { - const key = this.cacheKeyFromLoadParamsResolver(loadParams) + return this.getAsyncOnlyResolved(this.cacheKeyFromLoadParamsResolver(loadParams), loadParams, group) + } + + private getAsyncOnlyResolved( + key: string, + loadParams: LoadParams, + group: string, + ): Promise { const groupLoads = this.resolveGroupLoads(group) const existingLoad = groupLoads.get(key) if (existingLoad) { @@ -111,12 +125,12 @@ export abstract class AbstractGroupCache { const key = this.cacheKeyFromLoadParamsResolver(loadParams) - const inMemoryValue = this.getInMemoryOnly(loadParams, group) + const inMemoryValue = this.getInMemoryOnlyResolved(key, loadParams, group) if (inMemoryValue !== undefined) { return Promise.resolve(inMemoryValue) } - return this.getAsyncOnly(loadParams, group) + return this.getAsyncOnlyResolved(key, loadParams, group) } public getMany( @@ -133,7 +147,12 @@ export abstract class AbstractGroupCache { - return [...inMemoryValues.resolvedValues, ...asyncRetrievedValues.resolvedValues] + // in-memory caches always return a fresh array, so it is safe to append to it in place + const mergedValues = inMemoryValues.resolvedValues + for (let i = 0; i < asyncRetrievedValues.resolvedValues.length; i++) { + mergedValues.push(asyncRetrievedValues.resolvedValues[i]) + } + return mergedValues }, ) } diff --git a/lib/GroupLoader.ts b/lib/GroupLoader.ts index d7b4cb4..509ec95 100644 --- a/lib/GroupLoader.ts +++ b/lib/GroupLoader.ts @@ -154,12 +154,13 @@ export class GroupLoader[] = loadValues.map((loadValue) => { - return { - key: this.cacheKeyFromValueResolver(loadValue), - value: loadValue, - } - }) + const cacheEntries: CacheEntry[] = [] + for (let i = 0; i < loadValues.length; i++) { + cacheEntries.push({ + key: this.cacheKeyFromValueResolver(loadValues[i]), + value: loadValues[i], + }) + } await this.asyncCache.setManyForGroup(cacheEntries, group).catch((err) => { this.cacheUpdateErrorHandler( @@ -172,7 +173,8 @@ export class GroupLoader { - this.loadErrorHandler(err, key, this.dataSources[index], this.logger) + const dataSource = this.dataSources[index] + const resolvedValue = await dataSource.getFromGroup(loadParams, group).catch((err) => { + this.loadErrorHandler(err, key, dataSource, this.logger) if (this.throwIfLoadError) { throw err } @@ -209,8 +212,9 @@ export class GroupLoader { - this.loadErrorHandler(err, keys.toString(), this.dataSources[index], this.logger) + const dataSource = this.dataSources[index] + lastResolvedValues = await dataSource.getManyFromGroup(keys, group, loadParams).catch((err) => { + this.loadErrorHandler(err, keys.toString(), dataSource, this.logger) if (this.throwIfLoadError) { throw err } diff --git a/lib/Loader.ts b/lib/Loader.ts index 76fa4ff..499bdbb 100644 --- a/lib/Loader.ts +++ b/lib/Loader.ts @@ -182,8 +182,9 @@ export class Loader { - this.loadErrorHandler(err, key, this.dataSources[index], this.logger) + const dataSource = this.dataSources[index] + const resolvedValue = await dataSource.get(loadParams).catch((err) => { + this.loadErrorHandler(err, key, dataSource, this.logger) if (this.throwIfLoadError) { throw err } @@ -222,12 +223,13 @@ export class Loader[] = loadValues.map((loadValue) => { - return { - key: this.cacheKeyFromValueResolver(loadValue), - value: loadValue, - } - }) + const cacheEntries: CacheEntry[] = [] + for (let i = 0; i < loadValues.length; i++) { + cacheEntries.push({ + key: this.cacheKeyFromValueResolver(loadValues[i]), + value: loadValues[i], + }) + } await this.asyncCache.setMany(cacheEntries).catch((err) => { this.cacheUpdateErrorHandler( @@ -240,7 +242,8 @@ export class Loader { - this.loadErrorHandler(err, keys.toString(), this.dataSources[index], this.logger) + const dataSource = this.dataSources[index] + lastResolvedValues = await dataSource.getMany(keys, loadParams).catch((err) => { + this.loadErrorHandler(err, keys.toString(), dataSource, this.logger) if (this.throwIfLoadError) { throw err } diff --git a/lib/redis/AbstractRedisCache.ts b/lib/redis/AbstractRedisCache.ts index db9781f..0d3a474 100644 --- a/lib/redis/AbstractRedisCache.ts +++ b/lib/redis/AbstractRedisCache.ts @@ -18,6 +18,7 @@ export const DEFAULT_REDIS_CACHE_CONFIGURATION: RedisCacheConfiguration = { export abstract class AbstractRedisCache { protected readonly redis: Redis protected readonly config: ConfigType + protected readonly keyPrefix: string constructor(redis: Redis, config: Partial) { this.redis = redis @@ -26,6 +27,7 @@ export abstract class AbstractRedisCache extends AbstractRedisCache implements GroupCache { public readonly expirationTimeLoadingGroupedOperation: GroupLoader public ttlLeftBeforeRefreshInMsecs?: number + private readonly groupIndexPrefix: string name = 'Redis group cache' constructor(redis: Redis, config: Partial = {}) { super(redis, config) + this.groupIndexPrefix = `${this.keyPrefix}${GROUP_INDEX_KEY}${this.config.separator}` + this.ttlLeftBeforeRefreshInMsecs = config.ttlLeftBeforeRefreshInMsecs this.redis.defineCommand('getOrSetZeroWithTtl', { lua: GET_OR_SET_ZERO_WITH_TTL, @@ -88,7 +91,8 @@ export class RedisGroupCache extends AbstractRedisCache this.resolveKeyWithGroup(key, groupId, currentGroupKey)) + const entryPrefix = this.resolveGroupEntryPrefix(groupId, currentGroupKey) + const transformedKeys = keys.map((key) => entryPrefix + key) const resolvedValues: T[] = [] const unresolvedKeys: string[] = [] @@ -182,13 +186,14 @@ export class RedisGroupCache extends AbstractRedisCache extends AbstractRedisCache { + it('returns the same array reference for an empty array', () => { + const input: string[] = [] + expect(unique(input)).toBe(input) + }) + + it('returns the same array reference for a single-element array', () => { + const input = ['key'] + expect(unique(input)).toBe(input) + }) + it('returns the same array reference when there are no duplicates', () => { const input = [1, 2, 3, 'a', 'b'] const result = unique(input) diff --git a/lib/util/unique.ts b/lib/util/unique.ts index 269cc42..e0bcf3e 100644 --- a/lib/util/unique.ts +++ b/lib/util/unique.ts @@ -1,4 +1,7 @@ export const unique = (arr: T[]): T[] => { + if (arr.length <= 1) { + return arr + } const set = new Set(arr) return set.size === arr.length ? arr : Array.from(set) } diff --git a/package.json b/package.json index ae7bc58..11c4a74 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "homepage": "https://github.com/kibertoad/layered-loader", "dependencies": { "ioredis": "^5.10.1", - "toad-cache": "^3.7.1" + "toad-cache": "^3.7.4" }, "devDependencies": { "@biomejs/biome": "^2.4.16", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4274466..2623faa 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -106,8 +106,8 @@ importers: specifier: ^5.10.1 version: 5.10.1 toad-cache: - specifier: ^3.7.1 - version: 3.7.1 + specifier: ^3.7.4 + version: 3.7.4 devDependencies: '@biomejs/biome': specifier: ^2.4.16 @@ -1568,14 +1568,14 @@ packages: resolution: {integrity: sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==} engines: {node: '>=8.0'} - toad-cache@3.7.0: - resolution: {integrity: sha512-/m8M+2BJUpoJdgAHoG+baCwBT+tf2VraSfkBgl0Y00qIWt41DJ8R5B8nsEw0I58YwF5IZH6z24/2TobDKnqSWw==} - engines: {node: '>=12'} - toad-cache@3.7.1: resolution: {integrity: sha512-5DXWzE4Vz7xNHsv+xQ+MGfJYyC78Aok3tEr0MNwHoRf7vZnga1mQXZ4/Nsodld4VR6Wd+VhfmqnNrsRJyYPfrQ==} engines: {node: '>=20'} + toad-cache@3.7.4: + resolution: {integrity: sha512-m1TdR/rvT7kgGJZhspNtXdsdYk0fddFpJJFlG5s+UkPFo6lkLoZ3YLOaovPYjq1R75NP5JfeTlSHaOsE09peCg==} + engines: {node: '>=20'} + tslib@2.8.1: resolution: {integrity: sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==} @@ -2549,7 +2549,7 @@ snapshots: '@fastify/cors@11.2.0': dependencies: fastify-plugin: 5.1.0 - toad-cache: 3.7.0 + toad-cache: 3.7.1 '@fastify/error@4.2.0': {} @@ -2598,7 +2598,7 @@ snapshots: fast-equals: 6.0.0 json-stream-stringify: 3.1.6 tmp: 0.2.5 - toad-cache: 3.7.0 + toad-cache: 3.7.1 zod: 4.4.3 '@message-queue-toolkit/schemas@7.1.0(zod@4.4.3)': @@ -3197,7 +3197,7 @@ snapshots: rfdc: 1.4.1 secure-json-parse: 4.1.0 semver: 7.7.4 - toad-cache: 3.7.0 + toad-cache: 3.7.1 fastq@1.20.1: dependencies: @@ -3211,7 +3211,7 @@ snapshots: '@fastify/cors': 11.2.0 '@smithy/node-http-handler': 4.7.0 fastify: 5.8.5 - toad-cache: 3.7.0 + toad-cache: 3.7.1 valibot: 1.4.0(typescript@6.0.3) transitivePeerDependencies: - aws-crt @@ -3586,10 +3586,10 @@ snapshots: dependencies: is-number: 7.0.0 - toad-cache@3.7.0: {} - toad-cache@3.7.1: {} + toad-cache@3.7.4: {} + tslib@2.8.1: {} type-fest@5.6.0: diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 97558bc..e755fee 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,5 +1,8 @@ packages: - 'packages/*' +minimumReleaseAgeExclude: + - toad-cache@3.7.4 + onlyBuiltDependencies: - '@biomejs/biome' diff --git a/test/GroupLoader-main.spec.ts b/test/GroupLoader-main.spec.ts index 98b2afe..b273cbf 100644 --- a/test/GroupLoader-main.spec.ts +++ b/test/GroupLoader-main.spec.ts @@ -602,6 +602,28 @@ describe('GroupLoader Main', () => { }) describe('getMany', () => { + it('serves cached null value without hitting the data source again', async () => { + const userValuesNull = { + [user1.companyId]: { + [user1.userId]: null, + }, + } + const loader = new CountingGroupedLoader(userValuesNull as unknown as typeof userValues) + const operation = new GroupLoader({ + inMemoryCache: IN_MEMORY_CACHE_CONFIG, + dataSources: [loader], + cacheKeyFromValueResolver: idResolver, + }) + + const value = await operation.get(user1.userId, user1.companyId) + expect(value).toBeNull() + expect(loader.counter).toBe(1) + + const values = await operation.getMany([user1.userId], user1.companyId) + expect(values).toEqual([null]) + expect(loader.counter).toBe(1) + }) + it('returns empty array when fails to resolve value', async () => { const operation = new GroupLoader({ cacheKeyFromValueResolver: idResolver, diff --git a/test/Loader-main.spec.ts b/test/Loader-main.spec.ts index 4fa6639..136d1dc 100644 --- a/test/Loader-main.spec.ts +++ b/test/Loader-main.spec.ts @@ -693,6 +693,23 @@ describe('Loader Main', () => { }) describe('getMany', () => { + it('serves cached null value without hitting the data source again', async () => { + const dataSource = new CountingDataSource(null) + const operation = new Loader({ + inMemoryCache: IN_MEMORY_CACHE_CONFIG, + dataSources: [dataSource], + cacheKeyFromValueResolver: idResolver, + }) + + const value = await operation.get('key1') + expect(value).toBeNull() + expect(dataSource.counter).toBe(1) + + const values = await operation.getMany(['key1']) + expect(values).toEqual([null]) + expect(dataSource.counter).toBe(1) + }) + it('returns empty list when fails to resolve value', async () => { const operation = new Loader({ cacheKeyFromValueResolver: idResolver,