Skip to content

Commit a7af48a

Browse files
jaredwrayjaredwray
authored
node-cache - fix: ttl was not defaulting to 0 (#1591)
* node-cache - fix: ttl was not defaulting to 0 * handle the ttl better * adding resolveExpiration * isNegativeTtl * mset fixes
1 parent 649b534 commit a7af48a

2 files changed

Lines changed: 163 additions & 26 deletions

File tree

‎packages/node-cache/src/index.ts‎

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,7 @@ export class NodeCache<T> extends Hookified {
114114
* @param {number | string} [ttl] - this is in seconds and undefined will use the default ttl
115115
* @returns {boolean}
116116
*/
117-
public set(
118-
key: string | number,
119-
value: T,
120-
ttl: number | string = 0,
121-
): boolean {
117+
public set(key: string | number, value: T, ttl?: number | string): boolean {
122118
// Check on key type
123119
/* v8 ignore next -- @preserve */
124120
if (typeof key !== "string" && typeof key !== "number") {
@@ -127,24 +123,34 @@ export class NodeCache<T> extends Hookified {
127123

128124
// Check on ttl type
129125
/* v8 ignore next -- @preserve */
130-
if (ttl && typeof ttl !== "number" && typeof ttl !== "string") {
126+
if (
127+
ttl !== undefined &&
128+
typeof ttl !== "number" &&
129+
typeof ttl !== "string"
130+
) {
131131
throw this.createError(NodeCacheErrors.ETTLTYPE, this.formatKey(key));
132132
}
133133

134-
const keyValue = this.formatKey(key);
135-
let ttlValue = 0;
136-
if (this.options.stdTTL) {
137-
ttlValue = this.getExpirationTimestamp(this.options.stdTTL);
138-
}
139-
140-
if (ttl) {
141-
ttlValue = this.getExpirationTimestamp(ttl);
134+
// Reject negative TTL values (numeric or numeric string)
135+
if (this.isNegativeTtl(ttl)) {
136+
return false;
142137
}
143138

144-
let expirationTimestamp = 0; // Never delete
145-
if (ttlValue && ttlValue > 0) {
146-
expirationTimestamp = ttlValue;
139+
const keyValue = this.formatKey(key);
140+
let expirationTimestamp = 0; // 0 = never delete
141+
142+
if (ttl !== undefined && (typeof ttl === "string" || ttl > 0)) {
143+
// Explicit positive TTL or string shorthand overrides stdTTL
144+
expirationTimestamp = this.resolveExpiration(ttl);
145+
} else if (
146+
ttl === undefined &&
147+
this.options.stdTTL !== undefined &&
148+
this.options.stdTTL !== 0
149+
) {
150+
// ttl omitted, fall back to stdTTL if set and non-zero
151+
expirationTimestamp = this.resolveExpiration(this.options.stdTTL);
147152
}
153+
// ttl === 0 means cache indefinitely (expirationTimestamp stays 0)
148154

149155
// Check on max key size
150156
/* v8 ignore next -- @preserve */
@@ -162,7 +168,7 @@ export class NodeCache<T> extends Hookified {
162168
});
163169

164170
// Event
165-
this.emit("set", keyValue, value, ttlValue);
171+
this.emit("set", keyValue, value, expirationTimestamp);
166172

167173
// Add the bytes to the stats
168174
this._stats.incrementKSize(keyValue);
@@ -183,11 +189,14 @@ export class NodeCache<T> extends Hookified {
183189
throw this.createError(NodeCacheErrors.EKEYSTYPE);
184190
}
185191

192+
let success = true;
186193
for (const item of data) {
187-
this.set(item.key, item.value, item.ttl);
194+
if (!this.set(item.key, item.value, item.ttl)) {
195+
success = false;
196+
}
188197
}
189198

190-
return true;
199+
return success;
191200
}
192201

193202
/**
@@ -321,11 +330,30 @@ export class NodeCache<T> extends Hookified {
321330
* @returns {boolean} true if the key has been found and changed. Otherwise returns false.
322331
*/
323332
public ttl(key: string | number, ttl?: number | string): boolean {
333+
// Reject negative TTL values (numeric or numeric string)
334+
if (this.isNegativeTtl(ttl)) {
335+
return false;
336+
}
337+
324338
const result = this.store.get(this.formatKey(key));
325339
if (result) {
326-
// biome-ignore lint/style/noNonNullAssertion: need to fix
327-
const ttlValue = ttl ?? this.options.stdTTL!;
328-
result.ttl = this.getExpirationTimestamp(ttlValue);
340+
if (ttl !== undefined && (typeof ttl === "string" || ttl > 0)) {
341+
// Explicit positive TTL or string shorthand
342+
result.ttl = this.resolveExpiration(ttl);
343+
} else if (ttl === 0) {
344+
// Explicit 0 = unlimited
345+
result.ttl = 0;
346+
} else if (
347+
this.options.stdTTL !== undefined &&
348+
this.options.stdTTL !== 0
349+
) {
350+
// ttl omitted, fall back to stdTTL if set and non-zero
351+
result.ttl = this.resolveExpiration(this.options.stdTTL);
352+
} else {
353+
// No ttl, no stdTTL = unlimited
354+
result.ttl = 0;
355+
}
356+
329357
this.store.set(this.formatKey(key), result);
330358
return true;
331359
}
@@ -476,6 +504,38 @@ export class NodeCache<T> extends Hookified {
476504
return expirationTimestamp;
477505
}
478506

507+
/**
508+
* Resolves a TTL value to an expiration timestamp, returning 0 (unlimited) if the
509+
* resolved timestamp is not in the future (e.g. "0s" or a zero-duration string).
510+
*/
511+
private resolveExpiration(ttl: number | string): number {
512+
const timestamp = this.getExpirationTimestamp(ttl);
513+
if (timestamp <= Date.now()) {
514+
return 0;
515+
}
516+
517+
return timestamp;
518+
}
519+
520+
/**
521+
* Checks whether a TTL value is negative. Handles both numbers and
522+
* purely numeric strings (e.g. "-1").
523+
*/
524+
private isNegativeTtl(ttl?: number | string): boolean {
525+
if (typeof ttl === "number") {
526+
return ttl < 0;
527+
}
528+
529+
if (typeof ttl === "string") {
530+
const num = Number(ttl);
531+
if (!Number.isNaN(num) && num < 0) {
532+
return true;
533+
}
534+
}
535+
536+
return false;
537+
}
538+
479539
private checkData(): void {
480540
for (const [key, value] of this.store.entries()) {
481541
if (value.ttl > 0 && value.ttl < Date.now()) {

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

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@ describe("NodeCache", () => {
4848
expect(cache.get("baz")).toBe("qux");
4949
});
5050

51+
test("should return false from mset when any item has a negative ttl", () => {
52+
const cache = new NodeCache({ checkperiod: 0 });
53+
const list = [
54+
{ key: "good", value: "ok" },
55+
{ key: "bad", value: "nope", ttl: -1 },
56+
];
57+
const result = cache.mset(list);
58+
expect(result).toBe(false);
59+
expect(cache.get("good")).toBe("ok");
60+
expect(cache.has("bad")).toBe(false);
61+
});
62+
5163
test("should get multiple cache items", () => {
5264
const cache = new NodeCache({ checkperiod: 0 });
5365
cache.set("foo", "bar");
@@ -127,12 +139,12 @@ describe("NodeCache", () => {
127139

128140
test("ttl should default to 0 if no ttl is set", () => {
129141
const cache = new NodeCache({ checkperiod: 0 });
130-
cache.set("foo", "bar"); // Set to 10 by stdTTL
142+
cache.set("foo", "bar");
131143
const ttl = cache.getTtl("foo");
132144
expect(ttl).toBe(0);
133-
cache.ttl("foo");
145+
cache.ttl("foo"); // No args, stdTTL is 0 → stays unlimited
134146
const ttl2 = cache.getTtl("foo");
135-
expect(ttl2).toBeGreaterThan(ttl!);
147+
expect(ttl2).toBe(0);
136148
});
137149

138150
test("should return 0 if there is no key to delete", () => {
@@ -235,6 +247,71 @@ describe("NodeCache", () => {
235247
expect(cache.get("moo")).toBe(undefined);
236248
});
237249

250+
test("should cache indefinitely when ttl is explicitly 0 even with stdTTL set", async () => {
251+
const cache = new NodeCache({ checkperiod: 0, stdTTL: 0.5 });
252+
cache.set("withStdTTL", "expires"); // omitted ttl → uses stdTTL
253+
cache.set("unlimited", "stays", 0); // explicit 0 → cache indefinitely
254+
await sleep(600);
255+
expect(cache.get("withStdTTL")).toBe(undefined);
256+
expect(cache.get("unlimited")).toBe("stays");
257+
});
258+
259+
test("should return false and not store key when ttl is negative", () => {
260+
const cache = new NodeCache({ checkperiod: 0 });
261+
const result = cache.set("foo", "bar", -1);
262+
expect(result).toBe(false);
263+
expect(cache.has("foo")).toBe(false);
264+
expect(cache.get("foo")).toBe(undefined);
265+
});
266+
267+
test("should return false on ttl() method when ttl is negative", () => {
268+
const cache = new NodeCache({ checkperiod: 0 });
269+
cache.set("foo", "bar");
270+
const result = cache.ttl("foo", -1);
271+
expect(result).toBe(false);
272+
expect(cache.get("foo")).toBe("bar");
273+
});
274+
275+
test("should reject negative TTL passed as a numeric string in set()", () => {
276+
const cache = new NodeCache({ checkperiod: 0 });
277+
const result = cache.set("foo", "bar", "-1");
278+
expect(result).toBe(false);
279+
expect(cache.has("foo")).toBe(false);
280+
});
281+
282+
test("should reject negative TTL passed as a numeric string in ttl()", () => {
283+
const cache = new NodeCache({ checkperiod: 0 });
284+
cache.set("foo", "bar");
285+
const result = cache.ttl("foo", "-5");
286+
expect(result).toBe(false);
287+
expect(cache.get("foo")).toBe("bar");
288+
});
289+
290+
test("should set unlimited expiration on ttl() method when ttl is 0", async () => {
291+
const cache = new NodeCache({ checkperiod: 0, stdTTL: 0.5 });
292+
cache.set("foo", "bar"); // uses stdTTL (0.5s)
293+
cache.ttl("foo", 0); // override to unlimited
294+
await sleep(600);
295+
expect(cache.get("foo")).toBe("bar");
296+
expect(cache.getTtl("foo")).toBe(0);
297+
});
298+
299+
test("should treat zero-duration string stdTTL as unlimited", () => {
300+
const cache = new NodeCache({ checkperiod: 0, stdTTL: "0ms" });
301+
cache.set("foo", "bar");
302+
expect(cache.getTtl("foo")).toBe(0);
303+
expect(cache.get("foo")).toBe("bar");
304+
});
305+
306+
test("should use stdTTL when ttl() is called without ttl argument and stdTTL is set", () => {
307+
const cache = new NodeCache({ checkperiod: 0, stdTTL: 60 });
308+
cache.set("foo", "bar", 0); // explicit 0 = unlimited
309+
expect(cache.getTtl("foo")).toBe(0);
310+
cache.ttl("foo"); // no ttl arg → fall back to stdTTL (60s)
311+
const ttl = cache.getTtl("foo");
312+
expect(ttl).toBeGreaterThan(0);
313+
});
314+
238315
test("should get the internal id and stop the interval", () => {
239316
const cache = new NodeCache();
240317
expect(cache.getIntervalId()).toBeDefined();

0 commit comments

Comments
 (0)