diff --git a/packages/node-cache/src/index.ts b/packages/node-cache/src/index.ts index 13be31b1..19259295 100644 --- a/packages/node-cache/src/index.ts +++ b/packages/node-cache/src/index.ts @@ -228,13 +228,8 @@ export class NodeCache extends Hookified { if (result) { if (result.ttl > 0) { if (result.ttl < Date.now()) { - if (this.options.deleteOnExpire) { - this.del(key); - } - this._stats.incrementMisses(); - // Event - this.emit("expired", this.formatKey(key), result.value); + this.handleExpired(key, result); return undefined; } @@ -443,12 +438,24 @@ export class NodeCache extends Hookified { } /** - * Returns boolean indicating if the key is cached. + * Returns boolean indicating if the key is cached. If the key has expired, it + * will return false and the key will be removed from the cache when + * `deleteOnExpire` is enabled (matches the original node-cache behavior). * @param {string | number} key if the key is a number it will convert it to a string - * @returns {boolean} true if the key is cached + * @returns {boolean} true if the key is cached and not expired */ public has(key: string | number): boolean { - return this.store.has(this.formatKey(key)); + const result = this.store.get(this.formatKey(key)); + if (!result) { + return false; + } + + if (result.ttl > 0 && result.ttl < Date.now()) { + this.handleExpired(key, result); + return false; + } + + return true; } /** @@ -587,15 +594,25 @@ export class NodeCache extends Hookified { private checkData(): void { for (const [key, value] of this.store.entries()) { if (value.ttl > 0 && value.ttl < Date.now()) { - if (this.options.deleteOnExpire) { - this.del(key); - } - - this.emit("expired", this.formatKey(key), value.value); + this.handleExpired(key, value); } } } + /** + * Handles expiration for a cache entry. Deletes the entry when + * `deleteOnExpire` is enabled and emits the "expired" event. + */ + private handleExpired(key: string | number, entry: NodeCacheItem): void { + const keyValue = this.formatKey(key); + + if (this.options.deleteOnExpire) { + this.del(key); + } + + this.emit("expired", keyValue, entry.value); + } + private createError(errorCode: string, key?: string): Error { let error = errorCode; /* v8 ignore next -- @preserve */ diff --git a/packages/node-cache/test/index.test.ts b/packages/node-cache/test/index.test.ts index de67af9e..d7df09be 100644 --- a/packages/node-cache/test/index.test.ts +++ b/packages/node-cache/test/index.test.ts @@ -73,10 +73,9 @@ describe("NodeCache", () => { const result = cache.mset(list); expect(result).toBe(true); expect(cache.get(goodKey)).toBe(goodValue); - // Negative TTL item is stored but expires immediately on get - expect(cache.has(badKey)).toBe(true); + // Negative TTL item is stored but has() already sees it as expired + expect(cache.has(badKey)).toBe(false); expect(cache.get(badKey)).toBe(undefined); - // After get triggers expiration + deleteOnExpire, key is removed expect(cache.has(badKey)).toBe(false); }); @@ -241,6 +240,72 @@ describe("NodeCache", () => { expect(has).toBe(true); }); + test("has() should return false for expired keys (issue #1617)", async () => { + const cache = new NodeCache({ checkperiod: 0 }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + expect(cache.has(key)).toBe(true); + await sleep(100); + expect(cache.has(key)).toBe(false); + }); + + test("has() should emit expired and delete when deleteOnExpire is true", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: true, + }); + const key = faker.string.uuid(); + const value = faker.lorem.word(); + cache.set(key, value, 0.05); + let expiredKey: string | undefined; + let expiredValue: string | undefined; + cache.on("expired", (k, v) => { + expiredKey = k; + expiredValue = v; + }); + await sleep(100); + expect(cache.has(key)).toBe(false); + expect(expiredKey).toBe(key); + expect(expiredValue).toBe(value); + expect(cache.keys()).not.toContain(key); + }); + + test("has() should not delete expired keys when deleteOnExpire is false", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: false, + }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + await sleep(100); + // has() still reports false because the key is expired + expect(cache.has(key)).toBe(false); + // But the underlying entry is still in the store + expect(cache.keys()).toContain(key); + }); + + test("expired event fires on every access while deleteOnExpire is false", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: false, + }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + let count = 0; + cache.on("expired", () => { + count++; + }); + await sleep(100); + cache.has(key); + cache.has(key); + cache.get(key); + cache.get(key); + // Exercise the interval sweep path explicitly + // biome-ignore lint/complexity/useLiteralKeys: accessing private method for testing + (cache as unknown as { checkData: () => void })["checkData"](); + expect(count).toBe(5); + }); + test("should return the stats of the cache", () => { const cache = new NodeCache({ checkperiod: 0 }); const key1 = faker.string.uuid(); @@ -324,11 +389,10 @@ describe("NodeCache", () => { const key = faker.string.uuid(); const result = cache.set(key, faker.lorem.word(), -1); expect(result).toBe(true); - // Key is stored with a past expiration timestamp - expect(cache.has(key)).toBe(true); - // get() sees it's expired and returns undefined + // has() treats the expired key as absent (matches node-cache behavior) + expect(cache.has(key)).toBe(false); + // get() also sees it's expired and returns undefined expect(cache.get(key)).toBe(undefined); - // After get triggers expiration + deleteOnExpire, key is removed expect(cache.has(key)).toBe(false); }); @@ -338,8 +402,8 @@ describe("NodeCache", () => { cache.set(key, faker.lorem.word()); const result = cache.ttl(key, -1); expect(result).toBe(true); - // Key exists but will expire on next access - expect(cache.has(key)).toBe(true); + // has() treats the expired key as absent + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); }); @@ -349,7 +413,7 @@ describe("NodeCache", () => { const key = faker.string.uuid(); const result = cache.set(key, faker.lorem.word(), "-1"); expect(result).toBe(true); - expect(cache.has(key)).toBe(true); + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); }); @@ -360,7 +424,7 @@ describe("NodeCache", () => { cache.set(key, faker.lorem.word()); const result = cache.ttl(key, "-5"); expect(result).toBe(true); - expect(cache.has(key)).toBe(true); + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); });