node-cache - fix: ttl was not defaulting to 0

packages/node-cache/src/index.ts

@@ -114,11 +114,7 @@ export class NodeCache<T> extends Hookified {
 	 * @param {number | string} [ttl] - this is in seconds and undefined will use the default ttl
 	 * @returns {boolean}
 	 */
-	public set(
-		key: string | number,
-		value: T,
-		ttl: number | string = 0,
-	): boolean {
+	public set(key: string | number, value: T, ttl?: number | string): boolean {
 		// Check on key type
 		/* v8 ignore next -- @preserve */
 		if (typeof key !== "string" && typeof key !== "number") {
@@ -127,24 +123,34 @@ export class NodeCache<T> extends Hookified {
 
 		// Check on ttl type
 		/* v8 ignore next -- @preserve */
-		if (ttl && typeof ttl !== "number" && typeof ttl !== "string") {
+		if (
+			ttl !== undefined &&
+			typeof ttl !== "number" &&
+			typeof ttl !== "string"
+		) {
 			throw this.createError(NodeCacheErrors.ETTLTYPE, this.formatKey(key));
 		}
 
-		const keyValue = this.formatKey(key);
-		let ttlValue = 0;
-		if (this.options.stdTTL) {
-			ttlValue = this.getExpirationTimestamp(this.options.stdTTL);
-		}
-
-		if (ttl) {
-			ttlValue = this.getExpirationTimestamp(ttl);
+		// Reject negative TTL values (numeric or numeric string)
+		if (this.isNegativeTtl(ttl)) {
+			return false;
 		}
 
-		let expirationTimestamp = 0; // Never delete
-		if (ttlValue && ttlValue > 0) {
-			expirationTimestamp = ttlValue;
+		const keyValue = this.formatKey(key);
+		let expirationTimestamp = 0; // 0 = never delete
+
+		if (ttl !== undefined && (typeof ttl === "string" || ttl > 0)) {
+			// Explicit positive TTL or string shorthand overrides stdTTL
+			expirationTimestamp = this.resolveExpiration(ttl);
+		} else if (
+			ttl === undefined &&
+			this.options.stdTTL !== undefined &&
+			this.options.stdTTL !== 0
+		) {
+			// ttl omitted, fall back to stdTTL if set and non-zero
+			expirationTimestamp = this.resolveExpiration(this.options.stdTTL);
 		}
+		// ttl === 0 means cache indefinitely (expirationTimestamp stays 0)
 
 		// Check on max key size
 		/* v8 ignore next -- @preserve */
@@ -162,7 +168,7 @@ export class NodeCache<T> extends Hookified {
 		});
 
 		// Event
-		this.emit("set", keyValue, value, ttlValue);
+		this.emit("set", keyValue, value, expirationTimestamp);
 
 		// Add the bytes to the stats
 		this._stats.incrementKSize(keyValue);
@@ -183,11 +189,14 @@ export class NodeCache<T> extends Hookified {
 			throw this.createError(NodeCacheErrors.EKEYSTYPE);
 		}
 
+		let success = true;
 		for (const item of data) {
-			this.set(item.key, item.value, item.ttl);
+			if (!this.set(item.key, item.value, item.ttl)) {
+				success = false;
+			}
 		}
 
-		return true;
+		return success;
 	}
 
 	/**
@@ -321,11 +330,30 @@ export class NodeCache<T> extends Hookified {
 	 * @returns {boolean} true if the key has been found and changed. Otherwise returns false.
 	 */
 	public ttl(key: string | number, ttl?: number | string): boolean {
+		// Reject negative TTL values (numeric or numeric string)
+		if (this.isNegativeTtl(ttl)) {
+			return false;
+		}
+
 		const result = this.store.get(this.formatKey(key));
 		if (result) {
-			// biome-ignore lint/style/noNonNullAssertion: need to fix
-			const ttlValue = ttl ?? this.options.stdTTL!;
-			result.ttl = this.getExpirationTimestamp(ttlValue);
+			if (ttl !== undefined && (typeof ttl === "string" || ttl > 0)) {
+				// Explicit positive TTL or string shorthand
+				result.ttl = this.resolveExpiration(ttl);
+			} else if (ttl === 0) {
+				// Explicit 0 = unlimited
+				result.ttl = 0;
+			} else if (
+				this.options.stdTTL !== undefined &&
+				this.options.stdTTL !== 0
+			) {
+				// ttl omitted, fall back to stdTTL if set and non-zero
+				result.ttl = this.resolveExpiration(this.options.stdTTL);
+			} else {
+				// No ttl, no stdTTL = unlimited
+				result.ttl = 0;
+			}
+
 			this.store.set(this.formatKey(key), result);
 			return true;
 		}
@@ -476,6 +504,38 @@ export class NodeCache<T> extends Hookified {
 		return expirationTimestamp;
 	}
 
+	/**
+	 * Resolves a TTL value to an expiration timestamp, returning 0 (unlimited) if the
+	 * resolved timestamp is not in the future (e.g. "0s" or a zero-duration string).
+	 */
+	private resolveExpiration(ttl: number | string): number {
+		const timestamp = this.getExpirationTimestamp(ttl);
+		if (timestamp <= Date.now()) {
+			return 0;
+		}
+
+		return timestamp;
+	}
+
+	/**
+	 * Checks whether a TTL value is negative. Handles both numbers and
+	 * purely numeric strings (e.g. "-1").
+	 */
+	private isNegativeTtl(ttl?: number | string): boolean {
+		if (typeof ttl === "number") {
+			return ttl < 0;
+		}
+
+		if (typeof ttl === "string") {
+			const num = Number(ttl);
+			if (!Number.isNaN(num) && num < 0) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	private checkData(): void {
 		for (const [key, value] of this.store.entries()) {
 			if (value.ttl > 0 && value.ttl < Date.now()) {

packages/node-cache/test/index.test.ts

@@ -48,6 +48,18 @@ describe("NodeCache", () => {
 		expect(cache.get("baz")).toBe("qux");
 	});
 
+	test("should return false from mset when any item has a negative ttl", () => {
+		const cache = new NodeCache({ checkperiod: 0 });
+		const list = [
+			{ key: "good", value: "ok" },
+			{ key: "bad", value: "nope", ttl: -1 },
+		];
+		const result = cache.mset(list);
+		expect(result).toBe(false);
+		expect(cache.get("good")).toBe("ok");
+		expect(cache.has("bad")).toBe(false);
+	});
+
 	test("should get multiple cache items", () => {
 		const cache = new NodeCache({ checkperiod: 0 });
 		cache.set("foo", "bar");
@@ -127,12 +139,12 @@ describe("NodeCache", () => {
 
 	test("ttl should default to 0 if no ttl is set", () => {
 		const cache = new NodeCache({ checkperiod: 0 });
-		cache.set("foo", "bar"); // Set to 10 by stdTTL
+		cache.set("foo", "bar");
 		const ttl = cache.getTtl("foo");
 		expect(ttl).toBe(0);
-		cache.ttl("foo");
+		cache.ttl("foo"); // No args, stdTTL is 0 → stays unlimited
 		const ttl2 = cache.getTtl("foo");
-		expect(ttl2).toBeGreaterThan(ttl!);
+		expect(ttl2).toBe(0);
 	});
 
 	test("should return 0 if there is no key to delete", () => {
@@ -235,6 +247,71 @@ describe("NodeCache", () => {
 		expect(cache.get("moo")).toBe(undefined);
 	});
 
+	test("should cache indefinitely when ttl is explicitly 0 even with stdTTL set", async () => {
+		const cache = new NodeCache({ checkperiod: 0, stdTTL: 0.5 });
+		cache.set("withStdTTL", "expires"); // omitted ttl → uses stdTTL
+		cache.set("unlimited", "stays", 0); // explicit 0 → cache indefinitely
+		await sleep(600);
+		expect(cache.get("withStdTTL")).toBe(undefined);
+		expect(cache.get("unlimited")).toBe("stays");
+	});
+
+	test("should return false and not store key when ttl is negative", () => {
+		const cache = new NodeCache({ checkperiod: 0 });
+		const result = cache.set("foo", "bar", -1);
+		expect(result).toBe(false);
+		expect(cache.has("foo")).toBe(false);
+		expect(cache.get("foo")).toBe(undefined);
+	});
+
+	test("should return false on ttl() method when ttl is negative", () => {
+		const cache = new NodeCache({ checkperiod: 0 });
+		cache.set("foo", "bar");
+		const result = cache.ttl("foo", -1);
+		expect(result).toBe(false);
+		expect(cache.get("foo")).toBe("bar");
+	});
+
+	test("should reject negative TTL passed as a numeric string in set()", () => {
+		const cache = new NodeCache({ checkperiod: 0 });
+		const result = cache.set("foo", "bar", "-1");
+		expect(result).toBe(false);
+		expect(cache.has("foo")).toBe(false);
+	});
+
+	test("should reject negative TTL passed as a numeric string in ttl()", () => {
+		const cache = new NodeCache({ checkperiod: 0 });
+		cache.set("foo", "bar");
+		const result = cache.ttl("foo", "-5");
+		expect(result).toBe(false);
+		expect(cache.get("foo")).toBe("bar");
+	});
+
+	test("should set unlimited expiration on ttl() method when ttl is 0", async () => {
+		const cache = new NodeCache({ checkperiod: 0, stdTTL: 0.5 });
+		cache.set("foo", "bar"); // uses stdTTL (0.5s)
+		cache.ttl("foo", 0); // override to unlimited
+		await sleep(600);
+		expect(cache.get("foo")).toBe("bar");
+		expect(cache.getTtl("foo")).toBe(0);
+	});
+
+	test("should treat zero-duration string stdTTL as unlimited", () => {
+		const cache = new NodeCache({ checkperiod: 0, stdTTL: "0ms" });
+		cache.set("foo", "bar");
+		expect(cache.getTtl("foo")).toBe(0);
+		expect(cache.get("foo")).toBe("bar");
+	});
+
+	test("should use stdTTL when ttl() is called without ttl argument and stdTTL is set", () => {
+		const cache = new NodeCache({ checkperiod: 0, stdTTL: 60 });
+		cache.set("foo", "bar", 0); // explicit 0 = unlimited
+		expect(cache.getTtl("foo")).toBe(0);
+		cache.ttl("foo"); // no ttl arg → fall back to stdTTL (60s)
+		const ttl = cache.getTtl("foo");
+		expect(ttl).toBeGreaterThan(0);
+	});
+
 	test("should get the internal id and stop the interval", () => {
 		const cache = new NodeCache();
 		expect(cache.getIntervalId()).toBeDefined();