cacheable - fix: ttl in documentation and cascading ttl in set and setMany

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
 

packages/cacheable/src/index.ts

@@ -516,14 +516,23 @@ export class Cacheable extends Hookified {
 		ttl?: number | string,
 	): Promise<boolean> {
 		let result = false;
-		const finalTtl = shorthandToMilliseconds(ttl ?? this._ttl);
+		const explicitTtl = shorthandToMilliseconds(ttl);
 		try {
-			const item = { key, value, ttl: finalTtl };
+			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) {
-				promises.push(this._secondary.set(item.key, item.value, item.ttl));
+				const secondaryTtl = hookOverridden
+					? item.ttl
+					: getCascadingTtl(this._ttl, this._secondary.ttl, explicitTtl);
+				promises.push(this._secondary.set(item.key, item.value, secondaryTtl));
 			}
 
 			if (this._nonBlocking) {
@@ -938,7 +947,11 @@ export class Cacheable extends Hookified {
 	): Promise<boolean> {
 		const entries: KeyvEntry[] = [];
 		for (const item of items) {
-			const finalTtl = shorthandToMilliseconds(item.ttl ?? this._ttl);
+			const finalTtl = getCascadingTtl(
+				this._ttl,
+				keyv.ttl,
+				shorthandToMilliseconds(item.ttl),
+			);
 			entries.push({ key: item.key, value: item.value, ttl: finalTtl });
 		}
 

packages/cacheable/test/secondary-primary.test.ts

@@ -121,3 +121,125 @@ 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);
+});
+
+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);
+});