From b21b8ac3808948bac8c22ed70bd2399146ad31bc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 11:43:44 +0000 Subject: [PATCH 1/3] fix: respect per-store TTL in Cacheable set operations (#1619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `set()` and `setManyKeyv()` methods now correctly follow the documented TTL priority: explicit function TTL >> store adapter TTL >> cacheable TTL. Previously, both primary and secondary stores received the same TTL (cacheable default), ignoring any TTL configured on individual Keyv store instances. Also fixes README examples that incorrectly passed `{ ttl }` to KeyvRedis (which doesn't accept it) — the correct approach is to wrap KeyvRedis in a Keyv instance with the ttl option. https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57 --- packages/cacheable/README.md | 7 +- packages/cacheable/src/index.ts | 14 ++- .../cacheable/test/secondary-primary.test.ts | 89 +++++++++++++++++++ 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/packages/cacheable/README.md b/packages/cacheable/README.md index ca78dc71..07f66a0f 100644 --- a/packages/cacheable/README.md +++ b/packages/cacheable/README.md @@ -171,8 +171,9 @@ When `Getting Data` if the value does not exist in the primary store it will try ```javascript import { Cacheable } from 'cacheable'; +import {Keyv} from 'keyv'; import KeyvRedis from '@keyv/redis'; -const secondary = new KeyvRedis('redis://user:pass@localhost:6379', { ttl: 1000 }); +const secondary = new Keyv({ store: new KeyvRedis('redis://user:pass@localhost:6379'), ttl: 1000 }); const cache = new Cacheable({secondary, ttl: 100}); await cache.set('key', 'value'); // sets the value in the primary store with a ttl of 100 ms and secondary store with a ttl of 1000 ms @@ -189,10 +190,10 @@ import { Cacheable } from 'cacheable'; import {Keyv} from 'keyv'; import KeyvRedis from '@keyv/redis'; const primary = new Keyv({ ttl: 200 }); -const secondary = new KeyvRedis('redis://user:pass@localhost:6379', { ttl: 1000 }); +const secondary = new Keyv({ store: new KeyvRedis('redis://user:pass@localhost:6379'), ttl: 1000 }); const cache = new Cacheable({primary, secondary}); -await cache.set('key', 'value'); // sets the value in the primary store with a ttl of 100 ms and secondary store with a ttl of 1000 ms +await cache.set('key', 'value'); // sets the value in the primary store with a ttl of 200 ms and secondary store with a ttl of 1000 ms await sleep(200); // wait for .2 seconds diff --git a/packages/cacheable/src/index.ts b/packages/cacheable/src/index.ts index 5a06fb41..7c053b3b 100644 --- a/packages/cacheable/src/index.ts +++ b/packages/cacheable/src/index.ts @@ -516,14 +516,17 @@ export class Cacheable extends Hookified { ttl?: number | string, ): Promise { let result = false; - const finalTtl = shorthandToMilliseconds(ttl ?? this._ttl); + const explicitTtl = shorthandToMilliseconds(ttl); + const cacheableTtl = shorthandToMilliseconds(this._ttl); + const primaryTtl = explicitTtl ?? this._primary.ttl ?? cacheableTtl; try { - const item = { key, value, ttl: finalTtl }; + const item = { key, value, ttl: primaryTtl }; await this.hook(CacheableHooks.BEFORE_SET, item); const promises = []; promises.push(this._primary.set(item.key, item.value, item.ttl)); if (this._secondary) { - promises.push(this._secondary.set(item.key, item.value, item.ttl)); + const secondaryTtl = explicitTtl ?? this._secondary.ttl ?? cacheableTtl; + promises.push(this._secondary.set(item.key, item.value, secondaryTtl)); } if (this._nonBlocking) { @@ -938,7 +941,10 @@ export class Cacheable extends Hookified { ): Promise { const entries: KeyvEntry[] = []; for (const item of items) { - const finalTtl = shorthandToMilliseconds(item.ttl ?? this._ttl); + const finalTtl = + shorthandToMilliseconds(item.ttl) ?? + keyv.ttl ?? + shorthandToMilliseconds(this._ttl); entries.push({ key: item.key, value: item.value, ttl: finalTtl }); } diff --git a/packages/cacheable/test/secondary-primary.test.ts b/packages/cacheable/test/secondary-primary.test.ts index 5e9303b1..ce014072 100644 --- a/packages/cacheable/test/secondary-primary.test.ts +++ b/packages/cacheable/test/secondary-primary.test.ts @@ -121,3 +121,92 @@ test("should use the secondary ttl on secondary -> primary", async () => { expect(ttlFromExpires).toBeGreaterThan(45); expect(ttlFromExpires).toBeLessThan(55); }); + +test("should respect per-store ttl on set when secondary has its own ttl", async () => { + const data = { + key: faker.string.uuid(), + value: faker.string.uuid(), + }; + + const secondary = new Keyv({ ttl: 500 }); + const primary = new Keyv(); + const cacheable = new Cacheable({ secondary, primary, ttl: 100 }); + + // Set the value via cacheable.set (no explicit ttl) + await cacheable.set(data.key, data.value); + + // Primary should use cacheable ttl (100ms) since it has no own ttl + const primaryResult = await cacheable.primary.get(data.key, { raw: true }); + expect(primaryResult?.value).toEqual(data.value); + const primaryTtl = getTtlFromExpires(primaryResult?.expires); + expect(primaryTtl).toBeGreaterThan(90); + expect(primaryTtl).toBeLessThan(110); + + // Secondary should use its own ttl (500ms) instead of cacheable ttl (100ms) + const secondaryResult = await cacheable.secondary?.get(data.key, { + raw: true, + }); + expect(secondaryResult?.value).toEqual(data.value); + const secondaryTtl = getTtlFromExpires(secondaryResult?.expires); + expect(secondaryTtl).toBeGreaterThan(450); + expect(secondaryTtl).toBeLessThan(510); +}); + +test("should respect per-store ttl on set when primary has its own ttl", async () => { + const data = { + key: faker.string.uuid(), + value: faker.string.uuid(), + }; + + const secondary = new Keyv(); + const primary = new Keyv({ ttl: 200 }); + const cacheable = new Cacheable({ secondary, primary, ttl: 500 }); + + // Set the value via cacheable.set (no explicit ttl) + await cacheable.set(data.key, data.value); + + // Primary should use its own ttl (200ms) instead of cacheable ttl (500ms) + const primaryResult = await cacheable.primary.get(data.key, { raw: true }); + expect(primaryResult?.value).toEqual(data.value); + const primaryTtl = getTtlFromExpires(primaryResult?.expires); + expect(primaryTtl).toBeGreaterThan(190); + expect(primaryTtl).toBeLessThan(210); + + // Secondary should use cacheable ttl (500ms) since it has no own ttl + const secondaryResult = await cacheable.secondary?.get(data.key, { + raw: true, + }); + expect(secondaryResult?.value).toEqual(data.value); + const secondaryTtl = getTtlFromExpires(secondaryResult?.expires); + expect(secondaryTtl).toBeGreaterThan(490); + expect(secondaryTtl).toBeLessThan(510); +}); + +test("should use explicit ttl over store and cacheable ttl", async () => { + const data = { + key: faker.string.uuid(), + value: faker.string.uuid(), + }; + + const secondary = new Keyv({ ttl: 500 }); + const primary = new Keyv({ ttl: 200 }); + const cacheable = new Cacheable({ secondary, primary, ttl: 100 }); + + // Set the value with an explicit ttl of 50ms + await cacheable.set(data.key, data.value, 50); + + // Both stores should use the explicit ttl (50ms) + const primaryResult = await cacheable.primary.get(data.key, { raw: true }); + expect(primaryResult?.value).toEqual(data.value); + const primaryTtl = getTtlFromExpires(primaryResult?.expires); + expect(primaryTtl).toBeGreaterThan(40); + expect(primaryTtl).toBeLessThan(55); + + const secondaryResult = await cacheable.secondary?.get(data.key, { + raw: true, + }); + expect(secondaryResult?.value).toEqual(data.value); + const secondaryTtl = getTtlFromExpires(secondaryResult?.expires); + expect(secondaryTtl).toBeGreaterThan(40); + expect(secondaryTtl).toBeLessThan(55); +}); From 259a43236d681e1195c92cdb79dddd0abd25f2d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 11:58:17 +0000 Subject: [PATCH 2/3] refactor: use getCascadingTtl helper for per-store TTL computation Replace inlined TTL priority logic with the existing getCascadingTtl utility from @cacheable/utils in both set() and setManyKeyv(). https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57 --- packages/cacheable/src/index.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/cacheable/src/index.ts b/packages/cacheable/src/index.ts index 7c053b3b..3c1c4c9c 100644 --- a/packages/cacheable/src/index.ts +++ b/packages/cacheable/src/index.ts @@ -517,15 +517,13 @@ export class Cacheable extends Hookified { ): Promise { let result = false; const explicitTtl = shorthandToMilliseconds(ttl); - const cacheableTtl = shorthandToMilliseconds(this._ttl); - const primaryTtl = explicitTtl ?? this._primary.ttl ?? cacheableTtl; try { - const item = { key, value, ttl: primaryTtl }; + const item = { key, value, ttl: getCascadingTtl(this._ttl, this._primary.ttl, explicitTtl) }; await this.hook(CacheableHooks.BEFORE_SET, item); const promises = []; promises.push(this._primary.set(item.key, item.value, item.ttl)); if (this._secondary) { - const secondaryTtl = explicitTtl ?? this._secondary.ttl ?? cacheableTtl; + const secondaryTtl = getCascadingTtl(this._ttl, this._secondary.ttl, explicitTtl); promises.push(this._secondary.set(item.key, item.value, secondaryTtl)); } @@ -941,10 +939,7 @@ export class Cacheable extends Hookified { ): Promise { const entries: KeyvEntry[] = []; for (const item of items) { - const finalTtl = - shorthandToMilliseconds(item.ttl) ?? - keyv.ttl ?? - shorthandToMilliseconds(this._ttl); + const finalTtl = getCascadingTtl(this._ttl, keyv.ttl, shorthandToMilliseconds(item.ttl)); entries.push({ key: item.key, value: item.value, ttl: finalTtl }); } From 8b6366f552994cf97f8090a5a95f8d9d25284975 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 12:31:41 +0000 Subject: [PATCH 3/3] fix: respect BEFORE_SET hook TTL override for secondary store When the BEFORE_SET hook modifies item.ttl, that override now applies to the secondary store as well. Previously the secondary TTL was computed independently, ignoring the hook's modification. https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57 --- packages/cacheable/src/index.ts | 18 ++++++++-- .../cacheable/test/secondary-primary.test.ts | 33 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/cacheable/src/index.ts b/packages/cacheable/src/index.ts index 3c1c4c9c..f3ec3c83 100644 --- a/packages/cacheable/src/index.ts +++ b/packages/cacheable/src/index.ts @@ -518,12 +518,20 @@ export class Cacheable extends Hookified { let result = false; const explicitTtl = shorthandToMilliseconds(ttl); try { - const item = { key, value, ttl: getCascadingTtl(this._ttl, this._primary.ttl, explicitTtl) }; + const primaryTtl = getCascadingTtl( + this._ttl, + this._primary.ttl, + explicitTtl, + ); + const item = { key, value, ttl: primaryTtl }; await this.hook(CacheableHooks.BEFORE_SET, item); + const hookOverridden = item.ttl !== primaryTtl; const promises = []; promises.push(this._primary.set(item.key, item.value, item.ttl)); if (this._secondary) { - const secondaryTtl = getCascadingTtl(this._ttl, this._secondary.ttl, explicitTtl); + const secondaryTtl = hookOverridden + ? item.ttl + : getCascadingTtl(this._ttl, this._secondary.ttl, explicitTtl); promises.push(this._secondary.set(item.key, item.value, secondaryTtl)); } @@ -939,7 +947,11 @@ export class Cacheable extends Hookified { ): Promise { const entries: KeyvEntry[] = []; for (const item of items) { - const finalTtl = getCascadingTtl(this._ttl, keyv.ttl, shorthandToMilliseconds(item.ttl)); + const finalTtl = getCascadingTtl( + this._ttl, + keyv.ttl, + shorthandToMilliseconds(item.ttl), + ); entries.push({ key: item.key, value: item.value, ttl: finalTtl }); } diff --git a/packages/cacheable/test/secondary-primary.test.ts b/packages/cacheable/test/secondary-primary.test.ts index ce014072..9c36cd4d 100644 --- a/packages/cacheable/test/secondary-primary.test.ts +++ b/packages/cacheable/test/secondary-primary.test.ts @@ -210,3 +210,36 @@ test("should use explicit ttl over store and cacheable ttl", async () => { expect(secondaryTtl).toBeGreaterThan(40); expect(secondaryTtl).toBeLessThan(55); }); + +test("should apply BEFORE_SET hook ttl override to both stores", async () => { + const data = { + key: faker.string.uuid(), + value: faker.string.uuid(), + }; + + const secondary = new Keyv({ ttl: 500 }); + const primary = new Keyv({ ttl: 200 }); + const cacheable = new Cacheable({ secondary, primary, ttl: 100 }); + + // Hook overrides TTL to 30ms for all stores + cacheable.onHook(CacheableHooks.BEFORE_SET, async (item) => { + item.ttl = 30; + }); + + await cacheable.set(data.key, data.value); + + // Both stores should use the hook-overridden ttl (30ms) + const primaryResult = await cacheable.primary.get(data.key, { raw: true }); + expect(primaryResult?.value).toEqual(data.value); + const primaryTtl = getTtlFromExpires(primaryResult?.expires); + expect(primaryTtl).toBeGreaterThan(20); + expect(primaryTtl).toBeLessThan(35); + + const secondaryResult = await cacheable.secondary?.get(data.key, { + raw: true, + }); + expect(secondaryResult?.value).toEqual(data.value); + const secondaryTtl = getTtlFromExpires(secondaryResult?.expires); + expect(secondaryTtl).toBeGreaterThan(20); + expect(secondaryTtl).toBeLessThan(35); +});