feat(redis)!: remove support for legacy redis client versions by G0maa · Pull Request #12057 · typeorm/typeorm

G0maa · 2026-02-28T12:27:14Z

Description of change

Removing support for redis v3/v4 clients.
Added simple unit tests.

Pull-Request Checklist

Code is up-to-date with the master branch
This pull request links relevant issues as Fixes #00000
There are new or updated tests validating the change (tests/**.test.ts)
Documentation has been updated to reflect this change (docs/docs/**.md)

qodo-free-for-open-source-projects · 2026-02-28T12:27:28Z

User description

Description of change

removing support for redis v3/v4 clients.

Pull-Request Checklist

Code is up-to-date with the master branch
This pull request links relevant issues as Fixes #00000
There are new or updated tests validating the change (tests/**.test.ts)
Documentation has been updated to reflect this change (docs/docs/**.md)

PR Type

Enhancement

Description

Remove support for legacy Redis client versions (v3/v4)
Simplify codebase by eliminating version detection logic
Standardize on Promise-based API for all Redis operations
Add explicit type annotations for Redis client types

Diagram Walkthrough

flowchart LR
  A["Legacy Redis v3/v4<br/>Callback-based API"] -->|Remove| B["Modern Redis v5+<br/>Promise-based API"]
  C["Version Detection<br/>Logic"] -->|Remove| D["Simplified<br/>Codebase"]
  E["Dual API Paths<br/>in Methods"] -->|Consolidate| F["Single Promise<br/>Implementation"]

File Walkthrough

Relevant files

Enhancement

RedisQueryResultCache.ts `Eliminate legacy Redis version support and callbacks` src/cache/RedisQueryResultCache.ts Remove `redisMajorVersion` property and version detection methods (`detectRedisVersion()`, `isRedis5OrHigher()`) Simplify `connect()` method by removing Redis v4 legacy mode setup and version checks Update `disconnect()` to use Promise-based `quit()` exclusively Convert `getFromCache()` to async and remove callback-based API fallback Simplify `saveInCache()` to use Promise-based `set()` with options object Update `clear()` to handle both redis and ioredis clients with proper type casting Simplify `deleteKey()` to use Promise-based `del()` with array parameter Add explicit type imports for `RedisClientType` from redis package and `Redis`, `Cluster` from ioredis	+17/-131

qodo-free-for-open-source-projects · 2026-02-28T12:28:30Z

PR Code Suggestions ✨

Latest suggestions up to 69ffb7b

Category	Suggestion	Impact
Possible issue	Fix multi-key deletion behavior In the `remove` method, differentiate between `node-redis` and `ioredis` clients for the `del` command, as `ioredis` requires keys to be spread (`...identifiers`) while `node-redis` accepts an array. src/cache/RedisQueryResultCache.ts [195-200] async remove( identifiers: string[], queryRunner?: QueryRunner, ): Promise<void> { - await this.client.del(identifiers) + if (this.isNodeRedisClient(this.client)) { + await this.client.del(identifiers) + } else { + await this.client.del(...identifiers) + } } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where `remove` would fail silently for `ioredis` by attempting to delete a single stringified key instead of multiple keys.	High
Possible issue	Fix single-key deletion call In the `deleteKey` method, differentiate between `node-redis` and `ioredis` clients, passing `[key]` to `del` for `node-redis` and just `key` for `ioredis` to ensure correct deletion. src/cache/RedisQueryResultCache.ts [210-212] protected async deleteKey(key: string): Promise<void> { - await this.client.del([key]) + if (this.isNodeRedisClient(this.client)) { + await this.client.del([key]) + } else { + await this.client.del(key) + } } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where `deleteKey` would fail silently for `ioredis` by passing an array `[key]` instead of a string `key` to the `del` command.	High
General	✅ ~~Re-enable disabled unit tests~~ Suggestion Impact: The RedisQueryResultCache test suite was re-enabled by removing describe.skip, and the tests were substantially updated/expanded to match current promise-based client behavior (e.g., async stubs for set/flushdb/del, updated load error handling, and new coverage for cache methods). code diff: +import { TypeORMError } from "../../../src/error/TypeORMError" -describe.skip("RedisQueryResultCache", () => { - describe("detectRedisVersion", () => { - let sandbox: sinon.SinonSandbox - let mockDataSource: sinon.SinonStubbedInstance<DataSource> - let readPackageVersionStub: sinon.SinonStub +describe("RedisQueryResultCache", () => { + let sandbox: sinon.SinonSandbox + let mockDataSource: Pick<DataSource, "options" \| "logger"> + let loadStub: sinon.SinonStub - beforeEach(() => { - sandbox = sinon.createSandbox() + beforeEach(() => { + sandbox = sinon.createSandbox() + mockDataSource = { + options: { + cache: {}, + } as any, + logger: { + log: sandbox.stub(), + } as any, + } + loadStub = sandbox.stub(PlatformTools, "load") + }) - // Create a mock DataSource - mockDataSource = { - options: {}, - logger: { - log: sandbox.stub(), - }, + afterEach(() => { + sandbox.restore() + }) + + describe("isExpired", () => { + it("detects expiration based on duration", () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const now = Date.now() + + expect(cache.isExpired({ time: now - 2000, duration: 1000 } as any)) + .to.be.true + expect(cache.isExpired({ time: now - 200, duration: 1000 } as any)) + .to.be.false + }) + }) + + describe("storeInCache", () => { + it("stores using PX options for node-redis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const set = sandbox.stub().resolves() + ;(cache as any).client = { set } + const options = { + identifier: "node", + duration: 5000, + time: Date.now(), } as any - // Stub PlatformTools.readPackageVersion - readPackageVersionStub = sandbox.stub( - PlatformTools, - "readPackageVersion", + await cache.storeInCache(options, options) + + expect( + set.calledOnceWithExactly("node", JSON.stringify(options), { + expiration: { type: "PX", value: 5000 }, + }), + ).to.be.true + }) + + it("stores using PX arguments for ioredis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis", + ) + const set = sandbox.stub().resolves() + ;(cache as any).client = { set } + const options = { + identifier: "io", + duration: 1200, + time: Date.now(), + } as any + + await cache.storeInCache(options, options) + + expect( + set.calledOnceWithExactly( + "io", + JSON.stringify(options), + "PX", + 1200, + ), + ).to.be.true + }) + }) + + describe("clear", () => { + it("uses flushDb for node-redis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const flushDb = sandbox.stub().resolves() + ;(cache as any).client = { flushDb } + + await cache.clear() + + expect(flushDb.calledOnce).to.be.true + }) + + it("uses flushdb for ioredis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis", + ) + const flushdb = sandbox.stub().resolves() + ;(cache as any).client = { flushdb } + + await cache.clear() + + expect(flushdb.calledOnce).to.be.true + }) + }) + + describe("remove", () => { + it("delegates to redis del with identifiers", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const del = sandbox.stub().resolves() + ;(cache as any).client = { del } + + await cache.remove(["a", "b"]) + + expect(del.calledOnceWithExactly(["a", "b"])).to.be.true + }) + }) + + describe("deleteKey", () => { + it("removes a single key using del", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const del = sandbox.stub().resolves() + ;(cache as any).client = { del } + + await (cache as any).deleteKey("single") + + expect(del.calledOnceWithExactly(["single"])).to.be.true + }) + }) + + describe("loadRedis", () => { + it("loads ioredis module for cluster clients", () => { + const module = { Cluster: class {} } + loadStub.withArgs("ioredis").returns(module) + + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis/cluster", ) - // Stub PlatformTools.load to prevent actual redis loading - sandbox.stub(PlatformTools, "load").returns({}) + expect((cache as any).redis).to.equal(module) + expect(loadStub.calledOnceWithExactly("ioredis")).to.be.true }) - afterEach(() => { - sandbox.restore() - }) + it("throws a TypeORMError when dependency is missing", () => { + loadStub.callsFake(() => { + throw new Error("missing") + }) - it("should detect Redis v3.x and set redisMajorVersion to 3", () => { - readPackageVersionStub.returns("3.1.2") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - // Access the private method via any cast for testing - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(3) - expect(readPackageVersionStub.calledOnceWith("redis")).to.be.true - }) - - it("should detect Redis v4.x and set redisMajorVersion to 3 (callback-based)", () => { - readPackageVersionStub.returns("4.6.13") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(3) - }) - - it("should detect Redis v5.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("5.0.0") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) - expect(readPackageVersionStub.calledOnceWith("redis")).to.be.true - }) - - it("should detect Redis v6.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("6.2.3") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) - }) - - it("should detect Redis v7.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("7.0.0") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) + expect(() => { + return new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + }).to.throw(TypeORMError) }) }) Re-enable the skipped `RedisQueryResultCache` test suite and update the tests to align with the new promise-based client behavior, ensuring proper test coverage. test/unit/cache/redis-query-result-cache.test.ts [8] -describe.skip("RedisQueryResultCache", () => { +describe("RedisQueryResultCache", () => { `[Suggestion processed]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that skipping the entire test suite is a significant issue that hides potential regressions, and it rightly advises re-enabling and updating the tests.	Medium
Update

Previous suggestions

✅ Suggestions up to commit 2326fe3

Category	Suggestion	Impact
Possible issue	✅ ~~Fix TTL set API mismatch~~ Suggestion Impact: The commit added client-type conditional logic (via `isNodeRedisClient`) to call `set` with a node-redis-style options object including PX expiration, while retaining the positional "PX" arguments for ioredis; it also reused the same client-type check in `clear`. code diff: @@ -164,7 +163,16 @@ const value = JSON.stringify(options) const duration = options.duration - await this.client.set(key, value, "PX", duration) + if (this.isNodeRedisClient(this.client)) { + await this.client.set(key, value, { + expiration: { + type: "PX", + value: duration, + }, + }) + } else { + await this.client.set(key, value, "PX", duration) + } } /** @@ -172,12 +180,10 @@ * @param queryRunner */ async clear(queryRunner?: QueryRunner): Promise<void> { - if (this.clientType === "redis") { - const client = this.client as RedisClientType - await client.flushDb() + if (this.isNodeRedisClient(this.client)) { + await this.client.flushDb() } else { - const client = this.client as Redis \| Cluster - await client.flushdb() + await this.client.flushdb() } } @@ -190,11 +196,7 @@ identifiers: string[], queryRunner?: QueryRunner, ): Promise<void> { - await Promise.all( - identifiers.map((identifier) => { - return this.deleteKey(identifier) - }), - ) + await this.client.del(identifiers) } // ------------------------------------------------------------------------- @@ -225,5 +227,11 @@ ) } } + + private isNodeRedisClient( + client: Redis \| Cluster \| RedisClientType, + ): client is RedisClientType { + return this.clientType === "redis" + } Use conditional logic based on the client type to call the `set` method with the correct signature for `redis` (options object) and `ioredis` (positional arguments) clients. src/cache/RedisQueryResultCache.ts [167] -await this.client.set(key, value, "PX", duration) +if (this.clientType === "redis") { + const client = this.client as RedisClientType + await client.set(key, value, { PX: duration }) +} else { + const client = this.client as Redis \| Cluster + await client.set(key, value, "PX", duration) +} Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where the `set` method's signature differs between `redis` and `ioredis`, which would cause caching to fail when using the `redis` client.	High
Possible issue	Fix delete API incompatibility Use conditional logic based on the client type to call the `del` method with the correct signature for `redis` (array of keys) and `ioredis` (string key) clients. src/cache/RedisQueryResultCache.ts [209] -await this.client.del([key]) +if (this.clientType === "redis") { + const client = this.client as RedisClientType + await client.del([key]) +} else { + const client = this.client as Redis \| Cluster + await client.del(key) +} Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where the `del` method's signature differs between `redis` and `ioredis`, which would cause cache key deletion to fail when using the `ioredis` client.	High
General	✅ ~~Re-enable skipped unit tests~~ Suggestion Impact: The commit removed `describe.skip` to re-enable the RedisQueryResultCache tests and substantially rewrote/added test cases to align with the refactored Redis cache implementation (e.g., storeInCache, clear, remove, loadRedis behaviors). code diff: -describe.skip("RedisQueryResultCache", () => { - describe("detectRedisVersion", () => { - let sandbox: sinon.SinonSandbox - let mockDataSource: sinon.SinonStubbedInstance<DataSource> - let readPackageVersionStub: sinon.SinonStub +describe("RedisQueryResultCache", () => { + let sandbox: sinon.SinonSandbox + let mockDataSource: Pick<DataSource, "options" \| "logger"> + let loadStub: sinon.SinonStub - beforeEach(() => { - sandbox = sinon.createSandbox() + beforeEach(() => { + sandbox = sinon.createSandbox() + mockDataSource = { + options: { + cache: {}, + } as any, + logger: { + log: sandbox.stub(), + } as any, + } + loadStub = sandbox.stub(PlatformTools, "load") + }) - // Create a mock DataSource - mockDataSource = { - options: {}, - logger: { - log: sandbox.stub(), - }, + afterEach(() => { + sandbox.restore() + }) + + describe("isExpired", () => { + it("detects expiration based on duration", () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const now = Date.now() + + expect(cache.isExpired({ time: now - 2000, duration: 1000 } as any)) + .to.be.true + expect(cache.isExpired({ time: now - 200, duration: 1000 } as any)) + .to.be.false + }) + }) + + describe("storeInCache", () => { + it("stores using PX options for node-redis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const set = sandbox.stub().resolves() + ;(cache as any).client = { set } + const options = { + identifier: "node", + duration: 5000, + time: Date.now(), } as any - // Stub PlatformTools.readPackageVersion - readPackageVersionStub = sandbox.stub( - PlatformTools, - "readPackageVersion", + await cache.storeInCache(options, options) + + expect( + set.calledOnceWithExactly("node", JSON.stringify(options), { + expiration: { type: "PX", value: 5000 }, + }), + ).to.be.true + }) + + it("stores using PX arguments for ioredis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis", + ) + const set = sandbox.stub().resolves() + ;(cache as any).client = { set } + const options = { + identifier: "io", + duration: 1200, + time: Date.now(), + } as any + + await cache.storeInCache(options, options) + + expect( + set.calledOnceWithExactly( + "io", + JSON.stringify(options), + "PX", + 1200, + ), + ).to.be.true + }) + }) + + describe("clear", () => { + it("uses flushDb for node-redis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const flushDb = sandbox.stub().resolves() + ;(cache as any).client = { flushDb } + + await cache.clear() + + expect(flushDb.calledOnce).to.be.true + }) + + it("uses flushdb for ioredis", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis", + ) + const flushdb = sandbox.stub().resolves() + ;(cache as any).client = { flushdb } + + await cache.clear() + + expect(flushdb.calledOnce).to.be.true + }) + }) + + describe("remove", () => { + it("delegates to redis del with identifiers", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const del = sandbox.stub().resolves() + ;(cache as any).client = { del } + + await cache.remove(["a", "b"]) + + expect(del.calledOnceWithExactly(["a", "b"])).to.be.true + }) + }) + + describe("deleteKey", () => { + it("removes a single key using del", async () => { + loadStub.returns({}) + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + const del = sandbox.stub().resolves() + ;(cache as any).client = { del } + + await (cache as any).deleteKey("single") + + expect(del.calledOnceWithExactly(["single"])).to.be.true + }) + }) + + describe("loadRedis", () => { + it("loads ioredis module for cluster clients", () => { + const module = { Cluster: class {} } + loadStub.withArgs("ioredis").returns(module) + + const cache = new RedisQueryResultCache( + mockDataSource as DataSource, + "ioredis/cluster", ) - // Stub PlatformTools.load to prevent actual redis loading - sandbox.stub(PlatformTools, "load").returns({}) + expect((cache as any).redis).to.equal(module) + expect(loadStub.calledOnceWithExactly("ioredis")).to.be.true }) - afterEach(() => { - sandbox.restore() - }) + it("throws a TypeORMError when dependency is missing", () => { + loadStub.callsFake(() => { + throw new Error("missing") + }) - it("should detect Redis v3.x and set redisMajorVersion to 3", () => { - readPackageVersionStub.returns("3.1.2") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - // Access the private method via any cast for testing - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(3) - expect(readPackageVersionStub.calledOnceWith("redis")).to.be.true - }) - - it("should detect Redis v4.x and set redisMajorVersion to 3 (callback-based)", () => { - readPackageVersionStub.returns("4.6.13") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(3) - }) - - it("should detect Redis v5.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("5.0.0") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) - expect(readPackageVersionStub.calledOnceWith("redis")).to.be.true - }) - - it("should detect Redis v6.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("6.2.3") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) - }) - - it("should detect Redis v7.x and set redisMajorVersion to 5 (Promise-based)", () => { - readPackageVersionStub.returns("7.0.0") - - const cache = new RedisQueryResultCache( - mockDataSource as any, - "redis", - ) - - ;(cache as any).detectRedisVersion() - - expect((cache as any).redisMajorVersion).to.equal(5) + expect(() => { + return new RedisQueryResultCache( + mockDataSource as DataSource, + "redis", + ) + }).to.throw(TypeORMError) }) }) Re-enable the skipped `RedisQueryResultCache` test suite and update the tests to align with the refactored code instead of disabling them. test/unit/cache/redis-query-result-cache.test.ts [8] -describe.skip("RedisQueryResultCache", () => { +describe("RedisQueryResultCache", () => { Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that skipping the test suite is risky and hides potential regressions, which is especially important after a significant refactoring of the Redis cache logic.	Medium

✅ Suggestions up to commit a404fde

Category	Suggestion	Impact
Possible issue	✅ ~~Use correct arguments for `set`~~ Suggestion Impact: The commit updated the cache set logic to branch for node-redis vs ioredis, using an options object for node-redis and the "PX", duration argument form for ioredis; it also added a type-guard helper to detect node-redis. code diff: @@ -164,7 +163,16 @@ const value = JSON.stringify(options) const duration = options.duration - await this.client.set(key, value, "PX", duration) + if (this.isNodeRedisClient(this.client)) { + await this.client.set(key, value, { + expiration: { + type: "PX", + value: duration, + }, + }) + } else { + await this.client.set(key, value, "PX", duration) + } } /** @@ -172,12 +180,10 @@ * @param queryRunner */ async clear(queryRunner?: QueryRunner): Promise<void> { - if (this.clientType === "redis") { - const client = this.client as RedisClientType - await client.flushDb() + if (this.isNodeRedisClient(this.client)) { + await this.client.flushDb() } else { - const client = this.client as Redis \| Cluster - await client.flushdb() + await this.client.flushdb() } } @@ -190,11 +196,7 @@ identifiers: string[], queryRunner?: QueryRunner, ): Promise<void> { - await Promise.all( - identifiers.map((identifier) => { - return this.deleteKey(identifier) - }), - ) + await this.client.del(identifiers) } // ------------------------------------------------------------------------- @@ -225,5 +227,11 @@ ) } } + + private isNodeRedisClient( + client: Redis \| Cluster \| RedisClientType, + ): client is RedisClientType { + return this.clientType === "redis" + } Conditionally call the `set` method with the correct arguments based on the `clientType`, as the method signature differs between the `redis` and `ioredis` libraries. src/cache/RedisQueryResultCache.ts [167] -await this.client.set(key, value, "PX", duration) +if (this.clientType === "redis") { + await (this.client as RedisClientType).set(key, value, { + PX: duration, + }) +} else { + await (this.client as Redis \| Cluster).set( + key, + value, + "PX", + duration, + ) +} Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where the `set` method is called with arguments that are only valid for `ioredis`, which would cause a runtime error when using the `redis` client.	High
Possible issue	Use correct arguments for `del` Use the correct arguments for the `del` method based on the `clientType`, as the `redis` and `ioredis` libraries expect different argument formats. src/cache/RedisQueryResultCache.ts [209] -await this.client.del([key]) +if (this.clientType === "redis") { + await (this.client as RedisClientType).del([key]) +} else { + await (this.client as Redis \| Cluster).del(key) +} Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where `del([key])` is used. This is incorrect for `ioredis` and would attempt to delete a key with a literal name including brackets, instead of the intended key.	High
General	✅ ~~Convert promise chain to `await`~~ Suggestion Impact: Replaced the this.client.get(key).then(...) promise chain with an awaited call and returned the parsed result directly. code diff: - return this.client.get(key).then((result: any) => { - return result ? JSON.parse(result) : undefined - }) + const result = await this.client.get(key) + return result ? JSON.parse(result) : undefined Refactor the promise-based `.then()` chain to use `async/await` for improved readability and consistency within the `async` method. src/cache/RedisQueryResultCache.ts [137-139] -return this.client.get(key).then((result: any) => { - return result ? JSON.parse(result) : undefined -}) +const result = await this.client.get(key) +return result ? JSON.parse(result) : undefined Suggestion importance[1-10]: 4 __ Why: The suggestion improves code readability and consistency by replacing a `.then()` promise chain with `async/await`, which aligns with the `async` method signature.	Low

pkg-pr-new · 2026-02-28T12:29:03Z

typeorm-sql-js-example

npm i https://pkg.pr.new/typeorm/typeorm@12057

commit: 5c73674

qodo-free-for-open-source-projects · 2026-02-28T12:33:01Z

Code Review by Qodo

🐞 Bugs (17) 📘 Rule violations (9) 📎 Requirement gaps (0)

1. ~~Redis integration test breaks CI~~ ☑ 🐞 Bug ⛯ Reliability

Description

The new functional test unconditionally connects to Redis at localhost:6379 and creates an
ioredis-based cache, but CI does not start a Redis service and the repo doesn’t install ioredis by
default. This will fail the default test run (and likely CI) for most environments.

Code

test/functional/cache/redis-cache-integration.test.ts[R10-52]

+    beforeEach(async () => {
+        const logger = {
+            log: (level: string, message: string) => console.log(message),
+        }
+
+        const mockRedisDataSource = {
+            options: {
+                cache: {
+                    type: "redis",
+                    options: {
+                        url: "redis://localhost:6379",
+                    },
+                },
+            },
+            logger,
+        }
+
+        const mockIoRedisDataSource = {
+            options: {
+                cache: {
+                    type: "ioredis",
+                    options: {
+                        host: "localhost",
+                        port: 6379,
+                    },
+                },
+            },
+            logger,
+        }
+
+        const redisCache = new RedisQueryResultCache(
+            mockRedisDataSource as DataSource,
+            "redis",
+        )
+        const ioredisCache = new RedisQueryResultCache(
+            mockIoRedisDataSource as DataSource,
+            "ioredis",
+        )
+
+        await redisCache.connect()
+        await ioredisCache.connect()
+
+        caches = [redisCache, ioredisCache]

Evidence
The new test always creates both redis and ioredis caches and calls connect() against
localhost Redis. ioredis is only an optional peer dependency (not installed by default), and
PlatformTools.load("ioredis") requires it at runtime. The mocha config runs all test/**/*.test.*
files, so this test will run in CI. CI workflows define services for other DBs but not Redis.
test/functional/cache/redis-cache-integration.test.ts[10-52]
package.json[140-174]
src/platform/PlatformTools.ts[44-114]
.mocharc.json[1-10]
.github/workflows/tests-linux.yml[9-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added functional test unconditionally attempts to connect to a real Redis at `localhost:6379` and also instantiates an `ioredis` client, but CI does not start Redis and `ioredis` is not installed by default. This causes the default mocha test run to fail.
### Issue Context
- Mocha is configured to run all `test/**/*.test.*` files.
- `ioredis` is an optional peerDependency, not a devDependency.
- CI workflow jobs do not provision a Redis service.
### Fix Focus Areas
- test/functional/cache/redis-cache-integration.test.ts[10-63]
- package.json[140-200]
- .github/workflows/tests-linux.yml[1-120]
- .mocharc.json[1-10]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. DEL args break ioredis 🐞 Bug ✓ Correctness

Description

remove() now forwards a string[] directly into client.del(...), which is inconsistent with the
positional/variadic command style used for ioredis elsewhere in this class and can result in keys
not being deleted (stale cache). This impacts cache invalidation correctness for ioredis and
potentially cluster mode as well.

Code

src/cache/RedisQueryResultCache.ts[R199-200]
+        await this.client.del(identifiers)
}

Evidence

RedisQueryResultCache uses positional arguments for ioredis commands (e.g., set(key, value, "PX",
duration)), but remove() now passes an array to del, which is a different calling convention and can
change the command arguments sent to Redis. The same array-wrapping approach is also used in
deleteKey(), indicating the new intended (but risky) contract.

src/cache/RedisQueryResultCache.ts[155-176]
src/cache/RedisQueryResultCache.ts[195-212]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache.remove()` (and `deleteKey()`) now pass arrays into `client.del(...)`. This is inconsistent with how the ioredis branch passes command arguments elsewhere (positional/variadic), and can break cache invalidation by not deleting the intended keys.
### Issue Context
`storeInCache()` already distinguishes between node-redis and ioredis call shapes. `del` should follow the same pattern.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[195-212]
- src/cache/RedisQueryResultCache.ts[155-176]
### Suggested change
- Introduce a private helper like `delKeys(keys: string[])`:
- if `keys.length === 0` return
- if node-redis: `await this.client.del(keys)`
- else (ioredis + cluster): `await this.client.del(...keys)`
- Implement `remove()` via `delKeys(identifiers)`
- Implement `deleteKey()` via `delKeys([key])`
- Add unit tests covering ioredis `remove()` and `deleteKey()` to ensure the correct invocation shape (spread vs array)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Unit tests add as any 📘 Rule violation ✓ Correctness

Description

The updated unit tests introduce multiple as any casts and disable the no-explicit-any lint
rule, which matches the checklist’s definition of AI-like noise/type-bypass. This reduces type
safety and can mask real API incompatibilities.

Code

test/unit/cache/redis-query-result-cache.test.ts[R4-23]

import { RedisQueryResultCache } from "../../../src/cache/RedisQueryResultCache"
import { PlatformTools } from "../../../src/platform/PlatformTools"
import { DataSource } from "../../../src/data-source/DataSource"
+import { TypeORMError } from "../../../src/error/TypeORMError"
describe("RedisQueryResultCache", () => {
-    describe("detectRedisVersion", () => {
-        let sandbox: sinon.SinonSandbox
-        let mockDataSource: sinon.SinonStubbedInstance<DataSource>
-        let readPackageVersionStub: sinon.SinonStub
-
-        beforeEach(() => {
-            sandbox = sinon.createSandbox()
-
-            // Create a mock DataSource
-            mockDataSource = {
-                options: {},
-                logger: {
-                    log: sandbox.stub(),
-                },
-            } as any
+    let sandbox: sinon.SinonSandbox
+    let mockDataSource: Pick<DataSource, "options" | "logger">
+    let loadStub: sinon.SinonStub
+
+    beforeEach(() => {
+        sandbox = sinon.createSandbox()
+        mockDataSource = {
+            options: {
+                cache: {},
+            } as any,
+            logger: {
+                log: sandbox.stub(),
+            } as any,
+        }

Evidence
The compliance checklist forbids type-bypass via any casts and related noise. The new test code
adds an eslint disable plus several as any casts to force types rather than modeling minimal
interfaces/types.
Rule 4: Remove AI-generated noise
test/unit/cache/redis-query-result-cache.test.ts[1-23]
test/unit/cache/redis-query-result-cache.test.ts[40-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Unit tests introduce explicit `any` casts and disable `@typescript-eslint/no-explicit-any`, which violates the checklist requirement to avoid AI-like noise and type-bypass.
## Issue Context
`RedisQueryResultCache` uses `protected client: any`, so tests can avoid `(cache as any)` by using a small subclass (e.g., `class TestRedisQueryResultCache extends RedisQueryResultCache { setTestClient(c: unknown) { this.client = c } }`) and by typing test inputs with minimal interfaces or `unknown` + narrow casts (instead of `any`).
## Fix Focus Areas
- test/unit/cache/redis-query-result-cache.test.ts[1-186]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (22)

4. ~~disconnect clears client early~~ ☑ 🐞 Bug ⛯ Reliability

Description

disconnect() sets this.client to undefined before awaiting quit(), so if quit() rejects the cache
loses its client reference and cannot be cleanly retried/inspected. This can also cause
DataSource.destroy() to fail while leaving cleanup ambiguous.

Code

src/cache/RedisQueryResultCache.ts[R112-114]

+        const client = this.client
+        this.client = undefined
+        await client.quit()

Evidence

RedisQueryResultCache.disconnect() clears the internal client reference before quit completes.
DataSource.destroy() awaits this disconnect before marking the DataSource uninitialized, so a quit
failure propagates while the cache has already dropped its reference.

src/cache/RedisQueryResultCache.ts[111-115]
src/data-source/DataSource.ts[286-296]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache.disconnect()` clears `this.client` before awaiting `quit()`. If `quit()` rejects, the cache instance loses the client reference, making retries/cleanup/debugging difficult.
## Issue Context
`DataSource.destroy()` awaits `queryResultCache.disconnect()` during shutdown; failures here are surfaced to the caller. The disconnect implementation should not drop internal state until the underlying operation succeeds.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[108-116]
## Suggested fix
- Implement:
- early return if `this.client` is falsy.
- `await this.client.quit()` first.
- set `this.client = undefined` only after `quit()` resolves.
Example:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Redis v3/v4 removal undocumented 📘 Rule violation ✓ Correctness

Description

The PR removes support for legacy Redis clients (v3/v4) but no documentation updates are included.
This can break existing users without guidance on required Redis client versions and migration
steps.

Code

src/cache/RedisQueryResultCache.ts[R59-62]

+            this.client = this.redis.createClient(clientOptions)
if (
 typeof this.dataSource.options.cache === "object" &&

Evidence

The checklist requires docs to be updated for user-facing changes. The PR description states legacy
Redis client support is being removed, and the code changes remove legacy-version handling and now
always uses the modern connect flow, but there are no accompanying doc changes in this PR diff.

Rule 2: Docs updated for user-facing changes
src/cache/RedisQueryResultCache.ts[59-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This change removes support for legacy `redis` v3/v4 clients, but the PR does not include documentation updates to reflect the new minimum supported versions and any migration steps.
## Issue Context
Users relying on older Redis client versions may experience runtime failures or type/API mismatches after upgrading.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[59-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. Redis v3/v4 removal undocumented 📘 Rule violation ✓ Correctness

Description

The PR removes support for legacy Redis clients (v3/v4) but no documentation updates are included.
This can break existing users without guidance on required Redis client versions and migration
steps.

Code

src/cache/RedisQueryResultCache.ts[R59-62]

+            this.client = this.redis.createClient(clientOptions)
if (
typeof this.dataSource.options.cache === "object" &&

Evidence

The checklist requires docs to be updated for user-facing changes. The PR description states legacy
Redis client support is being removed, and the code changes remove legacy-version handling and now
always uses the modern connect flow, but there are no accompanying doc changes in this PR diff.

Rule 2: Docs updated for user-facing changes
src/cache/RedisQueryResultCache.ts[59-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This change removes support for legacy `redis` v3/v4 clients, but the PR does not include documentation updates to reflect the new minimum supported versions and any migration steps.
## Issue Context
Users relying on older Redis client versions may experience runtime failures or type/API mismatches after upgrading.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[59-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~Optional deps imported eagerly~~ ☑ 🐞 Bug ⛯ Reliability

Description

RedisQueryResultCache now imports types from optional peer dependencies (and ioredis as a value
import), which forces TypeScript to resolve those modules during build and leaks those types into
generated .d.ts. Because DataSource imports the cache factory, this can break TypeORM
compilation/consumption for users who do not have redis/ioredis installed (even if cache is
disabled).

Code

src/cache/RedisQueryResultCache.ts[R7-8]
+import Redis, { Cluster } from "ioredis"
+import { RedisClientType } from "redis"

Evidence
RedisQueryResultCache adds top-level imports from optional packages and uses them in an exported
class property type. TypeORM emits declarations (tsconfig.json), so the compiler must resolve these
modules/types, and consumers may also need these deps just to typecheck. This violates the existing
optional-peer-dependency/dynamic-load pattern (PlatformTools.load) and is reachable via common
entrypoints because DataSource imports QueryResultCacheFactory which imports RedisQueryResultCache.
src/cache/RedisQueryResultCache.ts[1-32]
src/cache/RedisQueryResultCache.ts[212-227]
src/platform/PlatformTools.ts[106-114]
src/cache/QueryResultCacheFactory.ts[1-45]
src/data-source/DataSource.ts[31-35]
src/data-source/DataSource.ts[141-156]
package.json[109-168]
package.json[169-185]
package.json[186-222]
tsconfig.json[1-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache` introduces top-level imports from `ioredis` and `redis` types, which breaks TypeORM’s optional peer dependency model and can cause `tsc`/declaration generation to fail when `ioredis` isn’t installed. It also leaks those types into the exported class declaration, forcing downstream typecheckers to have those packages installed.
### Issue Context
- TypeORM uses `PlatformTools.load()` to dynamically `require()` optional dependencies.
- `DataSource` imports `QueryResultCacheFactory`, which imports `RedisQueryResultCache`, so any eager import in that file is effectively part of the common entry path.
- `tsconfig.json` enables `declaration: true`, so any referenced external types must be resolvable and may appear in `.d.ts`.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[1-32]
- src/cache/RedisQueryResultCache.ts[23-31]
- src/cache/RedisQueryResultCache.ts[212-227]
### Suggested approach
- Remove `import Redis, { Cluster } from &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;ioredis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;` and `import { RedisClientType } from &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;redis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;`.
- Change `protected client` back to `any`/`unknown`, **or** define a small local structural interface that covers only the methods TypeORM calls (`connect`, `quit`, `get`, `set`, `del`, `flushDb/flushdb`).
- Keep using `PlatformTools.load()` for runtime module loading.
- Ensure generated `.d.ts` does not reference `ioredis`/`redis` types.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Optional deps imported eagerly~~ ☑ 🐞 Bug ⛯ Reliability

Description

RedisQueryResultCache now imports types from optional peer dependencies (and ioredis as a value
import), which forces TypeScript to resolve those modules during build and leaks those types into
generated .d.ts. Because DataSource imports the cache factory, this can break TypeORM
compilation/consumption for users who do not have redis/ioredis installed (even if cache is
disabled).

Code

src/cache/RedisQueryResultCache.ts[R7-8]
+import Redis, { Cluster } from "ioredis"
+import { RedisClientType } from "redis"

Evidence
RedisQueryResultCache adds top-level imports from optional packages and uses them in an exported
class property type. TypeORM emits declarations (tsconfig.json), so the compiler must resolve these
modules/types, and consumers may also need these deps just to typecheck. This violates the existing
optional-peer-dependency/dynamic-load pattern (PlatformTools.load) and is reachable via common
entrypoints because DataSource imports QueryResultCacheFactory which imports RedisQueryResultCache.
src/cache/RedisQueryResultCache.ts[1-32]
src/cache/RedisQueryResultCache.ts[212-227]
src/platform/PlatformTools.ts[106-114]
src/cache/QueryResultCacheFactory.ts[1-45]
src/data-source/DataSource.ts[31-35]
src/data-source/DataSource.ts[141-156]
package.json[109-168]
package.json[169-185]
package.json[186-222]
tsconfig.json[1-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache` introduces top-level imports from `ioredis` and `redis` types, which breaks TypeORM’s optional peer dependency model and can cause `tsc`/declaration generation to fail when `ioredis` isn’t installed. It also leaks those types into the exported class declaration, forcing downstream typecheckers to have those packages installed.
### Issue Context
- TypeORM uses `PlatformTools.load()` to dynamically `require()` optional dependencies.
- `DataSource` imports `QueryResultCacheFactory`, which imports `RedisQueryResultCache`, so any eager import in that file is effectively part of the common entry path.
- `tsconfig.json` enables `declaration: true`, so any referenced external types must be resolvable and may appear in `.d.ts`.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[1-32]
- src/cache/RedisQueryResultCache.ts[23-31]
- src/cache/RedisQueryResultCache.ts[212-227]
### Suggested approach
- Remove `import Redis, { Cluster } from &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;ioredis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;` and `import { RedisClientType } from &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;redis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;`.
- Change `protected client` back to `any`/`unknown`, **or** define a small local structural interface that covers only the methods TypeORM calls (`connect`, `quit`, `get`, `set`, `del`, `flushDb/flushdb`).
- Keep using `PlatformTools.load()` for runtime module loading.
- Ensure generated `.d.ts` does not reference `ioredis`/`redis` types.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. ~~Stale cache unit test~~ ☑ 🐞 Bug ✓ Correctness

Description

The unit tests still invoke the removed detectRedisVersion()/redisMajorVersion logic via (cache as
any), which will now throw at runtime and fail CI (tests run from compiled output).

Code

src/cache/RedisQueryResultCache.ts[L313-341]

-    /**
-     * Detects the Redis package version by reading the installed package.json
-     * and sets the appropriate API version (3 for callback-based, 5 for Promise-based).
-     */
-    private detectRedisVersion(): void {
-        if (this.clientType !== "redis") return
-        const version = PlatformTools.readPackageVersion("redis")
-        const major = parseInt(version.split(".")[0], 10)
-        if (isNaN(major)) {
-            throw new TypeORMError(`Invalid Redis version format: ${version}`)
-        }
-        if (major <= 4) {
-            // Redis 3/4 uses callback-based API
-            this.redisMajorVersion = 3
-        } else {
-            // Redis 5+ uses Promise-based API
-            this.redisMajorVersion = 5
-        }
-    }
-
-    /**
-     * Checks if Redis version is 5.x or higher
-     */
-    private isRedis5OrHigher(): boolean {
-        if (this.clientType !== "redis") return false
-        return (
-            this.redisMajorVersion !== undefined && this.redisMajorVersion >= 5
-        )
-    }

Evidence

The test suite explicitly calls detectRedisVersion() and asserts redisMajorVersion. The updated
RedisQueryResultCache no longer defines those members (class ends after loadRedis). Since mocha runs
compiled tests, this will fail at runtime with `TypeError: cache.detectRedisVersion is not a
function`.

test/unit/cache/redis-query-result-cache.test.ts[8-106]
src/cache/RedisQueryResultCache.ts[200-228]
.mocharc.json[6-9]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Unit tests still rely on `detectRedisVersion()` and `redisMajorVersion`, which were removed. These tests will now fail at runtime.
### Issue Context
`RedisQueryResultCache` was simplified and no longer has version detection. The existing unit test suite is now out of sync.
### Fix Focus Areas
- test/unit/cache/redis-query-result-cache.test.ts[8-106]
- src/cache/RedisQueryResultCache.ts[1-228]
### Suggested approach
- Remove the `detectRedisVersion` describe block entirely, or rewrite it to test the new behavior.
- Add targeted tests for:
- `loadRedis()` throwing a helpful error when the selected client type isn’t installed (stubbing `PlatformTools.load` to throw).
- `connect()` path for `clientType === &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;redis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;` calling `.connect()` on the created client.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

10. ~~Stale cache unit test~~ ☑ 🐞 Bug ✓ Correctness

Description

The unit tests still invoke the removed detectRedisVersion()/redisMajorVersion logic via (cache as
any), which will now throw at runtime and fail CI (tests run from compiled output).

Code

src/cache/RedisQueryResultCache.ts[L313-341]

-    /**
-     * Detects the Redis package version by reading the installed package.json
-     * and sets the appropriate API version (3 for callback-based, 5 for Promise-based).
-     */
-    private detectRedisVersion(): void {
-        if (this.clientType !== "redis") return
-        const version = PlatformTools.readPackageVersion("redis")
-        const major = parseInt(version.split(".")[0], 10)
-        if (isNaN(major)) {
-            throw new TypeORMError(`Invalid Redis version format: ${version}`)
-        }
-        if (major <= 4) {
-            // Redis 3/4 uses callback-based API
-            this.redisMajorVersion = 3
-        } else {
-            // Redis 5+ uses Promise-based API
-            this.redisMajorVersion = 5
-        }
-    }
-
-    /**
-     * Checks if Redis version is 5.x or higher
-     */
-    private isRedis5OrHigher(): boolean {
-        if (this.clientType !== "redis") return false
-        return (
-            this.redisMajorVersion !== undefined && this.redisMajorVersion >= 5
-        )
-    }

Evidence

The test suite explicitly calls detectRedisVersion() and asserts redisMajorVersion. The updated
RedisQueryResultCache no longer defines those members (class ends after loadRedis). Since mocha runs
compiled tests, this will fail at runtime with `TypeError: cache.detectRedisVersion is not a
function`.

test/unit/cache/redis-query-result-cache.test.ts[8-106]
src/cache/RedisQueryResultCache.ts[200-228]
.mocharc.json[6-9]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Unit tests still rely on `detectRedisVersion()` and `redisMajorVersion`, which were removed. These tests will now fail at runtime.
### Issue Context
`RedisQueryResultCache` was simplified and no longer has version detection. The existing unit test suite is now out of sync.
### Fix Focus Areas
- test/unit/cache/redis-query-result-cache.test.ts[8-106]
- src/cache/RedisQueryResultCache.ts[1-228]
### Suggested approach
- Remove the `detectRedisVersion` describe block entirely, or rewrite it to test the new behavior.
- Add targeted tests for:
- `loadRedis()` throwing a helpful error when the selected client type isn’t installed (stubbing `PlatformTools.load` to throw).
- `connect()` path for `clientType === &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;redis&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;` calling `.connect()` on the created client.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. Docs missing redis v3/v4 removal 📘 Rule violation ✓ Correctness

Description

This PR removes legacy Redis v3/v4 client support, which is a user-facing behavior/API compatibility
change. The PR indicates documentation was not updated, violating the docs-update requirement.

Code

src/cache/RedisQueryResultCache.ts[R56-60]

...cacheOptions?.options,
}
-            // Create initial client to test Redis version
-            let tempClient = this.redis.createClient(clientOptions)
-            const isRedis4Plus = typeof tempClient.connect === "function"
-
-            if (isRedis4Plus) {
-                // Redis 4+ detected, recreate with legacyMode for Redis 4.x
-                // (Redis 5 will ignore legacyMode if not needed)
-                clientOptions.legacyMode = true
-                tempClient = this.redis.createClient(clientOptions)
-            }
-
-            // Set as the main client
-            this.client = tempClient
+            this.client = this.redis.createClient(clientOptions)

Evidence

The compliance checklist requires documentation updates for user-facing changes. The PR description
explicitly states legacy redis client versions are being removed and the docs checklist item is left
unchecked, while the code change removes the legacy/compat handling.

Rule 2: Docs updated for user-facing changes
src/cache/RedisQueryResultCache.ts[56-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR removes support for legacy Redis v3/v4 clients, but documentation was not updated to reflect the new minimum supported Redis client/version and any migration notes.
## Issue Context
The PR itself indicates a user-facing change (dropping Redis v3/v4) and the documentation checklist item is unchecked.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[56-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

12. ~~getFromCache introduces any~~ ☑ 📘 Rule violation ✓ Correctness

Description

New code adds result: any in getFromCache, which weakens type safety and resembles the
prohibited "any-casts to bypass types" style. This violates the requirement to avoid AI-like
type-bypassing patterns.

Code

src/cache/RedisQueryResultCache.ts[R137-139]

+        return this.client.get(key).then((result: any) => {
+            return result ? JSON.parse(result) : undefined
})

Evidence
The checklist prohibits introducing type-bypassing any usage. The added callback explicitly
annotates result as any in newly changed lines.
Rule 4: Remove AI-generated noise
src/cache/RedisQueryResultCache.ts[137-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getFromCache` introduces `result: any`, which bypasses type checking and violates the &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;no any-casts to bypass types&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot; requirement.
## Issue Context
`this.client.get(key)` should have a known return type for the chosen redis client types (commonly `string | null`), and JSON parsing should be done with that type.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[137-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

13. ~~Hard peer-dep imports~~ ☑ 🐞 Bug ⛯ Reliability

Description

RedisQueryResultCache now has top-level (value) imports of redis and ioredis, which are optional
peer dependencies. Because DataSource imports the cache factory at module load time, apps can crash
on import (and/or TS type resolution) even when cache is not used.

Code

src/cache/RedisQueryResultCache.ts[R7-8]
+import Redis, { Cluster } from "ioredis"
+import { RedisClientType } from "redis"

Evidence
DataSource imports QueryResultCacheFactory, which imports RedisQueryResultCache at top-level,
so the new static imports are evaluated during normal TypeORM module loading. Since redis and
ioredis are optional peer dependencies, they are not guaranteed to be installed, making these
imports a startup-breaking change.
src/cache/RedisQueryResultCache.ts[1-32]
src/cache/QueryResultCacheFactory.ts[1-6]
src/data-source/DataSource.ts[24-35]
package.json[169-199]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache` now statically imports `redis` and `ioredis`. Since these are optional *peer* dependencies, this can break consumers at runtime/compile-time even if they never enable cache.
### Issue Context
`DataSource` imports `QueryResultCacheFactory` which imports `RedisQueryResultCache` at module scope, so the static imports execute during normal TypeORM initialization.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[1-32]
- src/cache/RedisQueryResultCache.ts[23-32]
### What to change
- Remove value imports from `redis` and `ioredis`.
- Avoid referencing peer-dep types in exported declarations (e.g., keep `protected client: any` or a local structural interface).
- Keep runtime loading via `PlatformTools.load(...)` only, preserving optional peer dependency behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

14. ~~Wrong SET signature~~ ☑ 🐞 Bug ✓ Correctness

Description

storeInCache uses a legacy positional SET signature ("PX", duration) regardless of client
type. With redis peer dependency pinned to v5, this can throw or mis-set TTL for node-redis
clients.

Code

src/cache/RedisQueryResultCache.ts[167]
+        await this.client.set(key, value, "PX", duration)

Evidence

The implementation unconditionally calls this.client.set(key, value, "PX", duration) while
elsewhere (clear) it already acknowledges redis-vs-ioredis API differences. Given the repo
declares redis v5 as the peer dependency, this call shape is very likely incompatible with the
intended supported client API.

src/cache/RedisQueryResultCache.ts[156-168]
src/cache/RedisQueryResultCache.ts[174-181]
package.json[169-185]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`storeInCache()` calls `SET` using a legacy positional signature (`&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;PX&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;, duration`) for all client types, which is risky/incompatible with the supported node-redis v5 client.
### Issue Context
The class already branches per client type in `clear()` (e.g., `flushDb` vs `flushdb`), implying command APIs differ.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[156-168]
### What to change
- Update `storeInCache()` to branch on `this.clientType`.
- For `redis`: call `set(key, value, { PX: duration })` (or equivalent modern options object).
- For `ioredis` / `ioredis/cluster`: keep `set(key, value, &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;PX&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;, duration)` (or the ioredis-supported alternative).
- Add/adjust unit tests to assert the correct signature is used for each client type.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

15. Wrong DEL arguments 🐞 Bug ✓ Correctness

Description

deleteKey calls del([key]), passing an array instead of a key string/variadic keys. This is
likely incompatible with the node-redis v5 API and can cause runtime errors when removing cache
keys.

Code

src/cache/RedisQueryResultCache.ts[209]
+        await this.client.del([key])

Evidence
deleteKey unconditionally passes an array to del. Given the supported redis peer dependency is
v5, this call shape is risky and should be aligned with the expected client API (and/or branched by
clientType, similar to clear).
src/cache/RedisQueryResultCache.ts[204-210]
package.json[169-185]
src/cache/RedisQueryResultCache.ts[174-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`deleteKey()` currently calls `del([key])`. Passing an array is likely not compatible with the supported node-redis v5 API.
### Issue Context
The class already branches per client type for other commands (e.g., flushDb/flushdb).
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[208-210]
- src/cache/RedisQueryResultCache.ts[189-198]
### What to change
- Update `deleteKey(key)` to call `del(key)` (or the correct per-client signature).
- (Optional) Update `remove(identifiers)` to delete multiple keys in a single `del(...)` call when supported, instead of looping `deleteKey`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

16. Docs missing redis v3/v4 removal 📘 Rule violation ✓ Correctness

Description

This PR removes legacy Redis v3/v4 client support, which is a user-facing behavior/API compatibility
change. The PR indicates documentation was not updated, violating the docs-update requirement.

Code

src/cache/RedisQueryResultCache.ts[R56-60]

 ...cacheOptions?.options,
}
-            // Create initial client to test Redis version
-            let tempClient = this.redis.createClient(clientOptions)
-            const isRedis4Plus = typeof tempClient.connect === "function"
-
-            if (isRedis4Plus) {
-                // Redis 4+ detected, recreate with legacyMode for Redis 4.x
-                // (Redis 5 will ignore legacyMode if not needed)
-                clientOptions.legacyMode = true
-                tempClient = this.redis.createClient(clientOptions)
-            }
-
-            // Set as the main client
-            this.client = tempClient
+            this.client = this.redis.createClient(clientOptions)

Evidence

The compliance checklist requires documentation updates for user-facing changes. The PR description
explicitly states legacy redis client versions are being removed and the docs checklist item is left
unchecked, while the code change removes the legacy/compat handling.

Rule 2: Docs updated for user-facing changes
src/cache/RedisQueryResultCache.ts[56-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR removes support for legacy Redis v3/v4 clients, but documentation was not updated to reflect the new minimum supported Redis client/version and any migration notes.
## Issue Context
The PR itself indicates a user-facing change (dropping Redis v3/v4) and the documentation checklist item is unchecked.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[56-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

17. ~~getFromCache introduces any~~ ☑ 📘 Rule violation ✓ Correctness

Description

New code adds result: any in getFromCache, which weakens type safety and resembles the
prohibited "any-casts to bypass types" style. This violates the requirement to avoid AI-like
type-bypassing patterns.

Code

src/cache/RedisQueryResultCache.ts[R137-139]

+        return this.client.get(key).then((result: any) => {
+            return result ? JSON.parse(result) : undefined
})

Evidence
The checklist prohibits introducing type-bypassing any usage. The added callback explicitly
annotates result as any in newly changed lines.
Rule 4: Remove AI-generated noise
src/cache/RedisQueryResultCache.ts[137-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getFromCache` introduces `result: any`, which bypasses type checking and violates the &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;no any-casts to bypass types&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot; requirement.
## Issue Context
`this.client.get(key)` should have a known return type for the chosen redis client types (commonly `string | null`), and JSON parsing should be done with that type.
## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[137-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

18. ~~Hard peer-dep imports~~ ☑ 🐞 Bug ⛯ Reliability

Description

RedisQueryResultCache now has top-level (value) imports of redis and ioredis, which are optional
peer dependencies. Because DataSource imports the cache factory at module load time, apps can crash
on import (and/or TS type resolution) even when cache is not used.

Code

src/cache/RedisQueryResultCache.ts[R7-8]
+import Redis, { Cluster } from "ioredis"
+import { RedisClientType } from "redis"

Evidence
DataSource imports QueryResultCacheFactory, which imports RedisQueryResultCache at top-level,
so the new static imports are evaluated during normal TypeORM module loading. Since redis and
ioredis are optional peer dependencies, they are not guaranteed to be installed, making these
imports a startup-breaking change.
src/cache/RedisQueryResultCache.ts[1-32]
src/cache/QueryResultCacheFactory.ts[1-6]
src/data-source/DataSource.ts[24-35]
package.json[169-199]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedisQueryResultCache` now statically imports `redis` and `ioredis`. Since these are optional *peer* dependencies, this can break consumers at runtime/compile-time even if they never enable cache.
### Issue Context
`DataSource` imports `QueryResultCacheFactory` which imports `RedisQueryResultCache` at module scope, so the static imports execute during normal TypeORM initialization.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[1-32]
- src/cache/RedisQueryResultCache.ts[23-32]
### What to change
- Remove value imports from `redis` and `ioredis`.
- Avoid referencing peer-dep types in exported declarations (e.g., keep `protected client: any` or a local structural interface).
- Keep runtime loading via `PlatformTools.load(...)` only, preserving optional peer dependency behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

19. ~~Wrong SET signature~~ ☑ 🐞 Bug ✓ Correctness

Description

storeInCache uses a legacy positional SET signature ("PX", duration) regardless of client
type. With redis peer dependency pinned to v5, this can throw or mis-set TTL for node-redis
clients.

Code

src/cache/RedisQueryResultCache.ts[167]
+        await this.client.set(key, value, "PX", duration)

Evidence

The implementation unconditionally calls this.client.set(key, value, "PX", duration) while
elsewhere (clear) it already acknowledges redis-vs-ioredis API differences. Given the repo
declares redis v5 as the peer dependency, this call shape is very likely incompatible with the
intended supported client API.

src/cache/RedisQueryResultCache.ts[156-168]
src/cache/RedisQueryResultCache.ts[174-181]
package.json[169-185]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`storeInCache()` calls `SET` using a legacy positional signature (`&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;PX&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;, duration`) for all client types, which is risky/incompatible with the supported node-redis v5 client.
### Issue Context
The class already branches per client type in `clear()` (e.g., `flushDb` vs `flushdb`), implying command APIs differ.
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[156-168]
### What to change
- Update `storeInCache()` to branch on `this.clientType`.
- For `redis`: call `set(key, value, { PX: duration })` (or equivalent modern options object).
- For `ioredis` / `ioredis/cluster`: keep `set(key, value, &amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;PX&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;, duration)` (or the ioredis-supported alternative).
- Add/adjust unit tests to assert the correct signature is used for each client type.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

20. Wrong DEL arguments 🐞 Bug ✓ Correctness

Description

deleteKey calls del([key]), passing an array instead of a key string/variadic keys. This is
likely incompatible with the node-redis v5 API and can cause runtime errors when removing cache
keys.

Code

src/cache/RedisQueryResultCache.ts[209]
+        await this.client.del([key])

Evidence
deleteKey unconditionally passes an array to del. Given the supported redis peer dependency is
v5, this call shape is risky and should be aligned with the expected client API (and/or branched by
clientType, similar to clear).
src/cache/RedisQueryResultCache.ts[204-210]
package.json[169-185]
src/cache/RedisQueryResultCache.ts[174-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`deleteKey()` currently calls `del([key])`. Passing an array is likely not compatible with the supported node-redis v5 API.
### Issue Context
The class already branches per client type for other commands (e.g., flushDb/flushdb).
### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[208-210]
- src/cache/RedisQueryResultCache.ts[189-198]
### What to change
- Update `deleteKey(key)` to call `del(key)` (or the correct per-client signature).
- (Optional) Update `remove(identifiers)` to delete multiple keys in a single `del(...)` call when supported, instead of looping `deleteKey`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

21. Docs missing redis v3/v4 removal 📘 Rule violation ✓ Correctness

Description

This PR removes legacy Redis v3/v4 client support, which is a user-facing behavior/API compatibility
change. The PR indicates documentation was not updated, violating the docs-update requirement.

Details

test/unit/cache/redis-query-result-cache.test.ts

qodo-free-for-open-source-projects · 2026-02-28T12:35:57Z

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 📎 Requirement gaps (0)

1. Docs missing redis v3/v4 removal 📘 Rule violation ✓ Correctness

Description

This PR removes legacy Redis v3/v4 client support, which is a user-facing behavior/API compatibility
change. The PR indicates documentation was not updated, violating the docs-update requirement.

Code

src/cache/RedisQueryResultCache.ts[R56-60]

                ...cacheOptions?.options,
            }

-            // Create initial client to test Redis version
-            let tempClient = this.redis.createClient(clientOptions)
-            const isRedis4Plus = typeof tempClient.connect === "function"
-
-            if (isRedis4Plus) {
-                // Redis 4+ detected, recreate with legacyMode for Redis 4.x
-                // (Redis 5 will ignore legacyMode if not needed)
-                clientOptions.legacyMode = true
-                tempClient = this.redis.createClient(clientOptions)
-            }
-
-            // Set as the main client
-            this.client = tempClient
+            this.client = this.redis.createClient(clientOptions)

Evidence

The compliance checklist requires documentation updates for user-facing changes. The PR description
explicitly states legacy redis client versions are being removed and the docs checklist item is left
unchecked, while the code change removes the legacy/compat handling.

Rule 2: Docs updated for user-facing changes
src/cache/RedisQueryResultCache.ts[56-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR removes support for legacy Redis v3/v4 clients, but documentation was not updated to reflect the new minimum supported Redis client/version and any migration notes.

## Issue Context
The PR itself indicates a user-facing change (dropping Redis v3/v4) and the documentation checklist item is unchecked.

## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[56-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. getFromCache introduces any 📘 Rule violation ✓ Correctness

Description

New code adds result: any in getFromCache, which weakens type safety and resembles the
prohibited "any-casts to bypass types" style. This violates the requirement to avoid AI-like
type-bypassing patterns.

Code

src/cache/RedisQueryResultCache.ts[R137-139]

+        return this.client.get(key).then((result: any) => {
+            return result ? JSON.parse(result) : undefined
        })

Evidence
The checklist prohibits introducing type-bypassing any usage. The added callback explicitly
annotates result as any in newly changed lines.
Rule 4: Remove AI-generated noise
src/cache/RedisQueryResultCache.ts[137-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getFromCache` introduces `result: any`, which bypasses type checking and violates the &quot;no any-casts to bypass types&quot; requirement.

## Issue Context
`this.client.get(key)` should have a known return type for the chosen redis client types (commonly `string | null`), and JSON parsing should be done with that type.

## Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[137-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Hard peer-dep imports 🐞 Bug ⛯ Reliability

Description

RedisQueryResultCache now has top-level (value) imports of redis and ioredis, which are optional
peer dependencies. Because DataSource imports the cache factory at module load time, apps can crash
on import (and/or TS type resolution) even when cache is not used.

Code

src/cache/RedisQueryResultCache.ts[R7-8]
+import Redis, { Cluster } from "ioredis"
+import { RedisClientType } from "redis"

Evidence
DataSource imports QueryResultCacheFactory, which imports RedisQueryResultCache at top-level,
so the new static imports are evaluated during normal TypeORM module loading. Since redis and
ioredis are optional peer dependencies, they are not guaranteed to be installed, making these
imports a startup-breaking change.
src/cache/RedisQueryResultCache.ts[1-32]
src/cache/QueryResultCacheFactory.ts[1-6]
src/data-source/DataSource.ts[24-35]
package.json[169-199]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RedisQueryResultCache` now statically imports `redis` and `ioredis`. Since these are optional *peer* dependencies, this can break consumers at runtime/compile-time even if they never enable cache.

### Issue Context
`DataSource` imports `QueryResultCacheFactory` which imports `RedisQueryResultCache` at module scope, so the static imports execute during normal TypeORM initialization.

### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[1-32]
- src/cache/RedisQueryResultCache.ts[23-32]

### What to change
- Remove value imports from `redis` and `ioredis`.
- Avoid referencing peer-dep types in exported declarations (e.g., keep `protected client: any` or a local structural interface).
- Keep runtime loading via `PlatformTools.load(...)` only, preserving optional peer dependency behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (2)

4. Wrong SET signature 🐞 Bug ✓ Correctness

Description

storeInCache uses a legacy positional SET signature ("PX", duration) regardless of client
type. With redis peer dependency pinned to v5, this can throw or mis-set TTL for node-redis
clients.

Code

src/cache/RedisQueryResultCache.ts[167]
+        await this.client.set(key, value, "PX", duration)

Evidence

The implementation unconditionally calls this.client.set(key, value, "PX", duration) while
elsewhere (clear) it already acknowledges redis-vs-ioredis API differences. Given the repo
declares redis v5 as the peer dependency, this call shape is very likely incompatible with the
intended supported client API.

src/cache/RedisQueryResultCache.ts[156-168]
src/cache/RedisQueryResultCache.ts[174-181]
package.json[169-185]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`storeInCache()` calls `SET` using a legacy positional signature (`&quot;PX&quot;, duration`) for all client types, which is risky/incompatible with the supported node-redis v5 client.

### Issue Context
The class already branches per client type in `clear()` (e.g., `flushDb` vs `flushdb`), implying command APIs differ.

### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[156-168]

### What to change
- Update `storeInCache()` to branch on `this.clientType`.
 - For `redis`: call `set(key, value, { PX: duration })` (or equivalent modern options object).
 - For `ioredis` / `ioredis/cluster`: keep `set(key, value, &quot;PX&quot;, duration)` (or the ioredis-supported alternative).
- Add/adjust unit tests to assert the correct signature is used for each client type.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Wrong DEL arguments 🐞 Bug ✓ Correctness

Description

deleteKey calls del([key]), passing an array instead of a key string/variadic keys. This is
likely incompatible with the node-redis v5 API and can cause runtime errors when removing cache
keys.

Code

src/cache/RedisQueryResultCache.ts[209]
+        await this.client.del([key])

Evidence
deleteKey unconditionally passes an array to del. Given the supported redis peer dependency is
v5, this call shape is risky and should be aligned with the expected client API (and/or branched by
clientType, similar to clear).
src/cache/RedisQueryResultCache.ts[204-210]
package.json[169-185]
src/cache/RedisQueryResultCache.ts[174-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`deleteKey()` currently calls `del([key])`. Passing an array is likely not compatible with the supported node-redis v5 API.

### Issue Context
The class already branches per client type for other commands (e.g., flushDb/flushdb).

### Fix Focus Areas
- src/cache/RedisQueryResultCache.ts[208-210]
- src/cache/RedisQueryResultCache.ts[189-198]

### What to change
- Update `deleteKey(key)` to call `del(key)` (or the correct per-client signature).
- (Optional) Update `remove(identifiers)` to delete multiple keys in a single `del(...)` call when supported, instead of looping `deleteKey`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. Tests skipped 🐞 Bug ⛯ Reliability

Description

The RedisQueryResultCache unit tests are now skipped entirely, removing coverage for cache behavior
during a risky refactor. This increases the likelihood of shipping runtime regressions in
redis/ioredis integrations.

Code

test/unit/cache/redis-query-result-cache.test.ts[8]
+describe.skip("RedisQueryResultCache", () => {

Evidence

The test suite is explicitly disabled via describe.skip, so CI will not exercise any
RedisQueryResultCache behavior or validate the refactor.

test/unit/cache/redis-query-result-cache.test.ts[1-10]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The RedisQueryResultCache unit suite is fully skipped, and the remaining tests target a removed method (`detectRedisVersion`). This leaves the refactor untested.

### Issue Context
This PR changes command invocation patterns and dependency loading behavior; tests should validate client-specific method signatures and optional dependency behavior.

### Fix Focus Areas
- test/unit/cache/redis-query-result-cache.test.ts[1-107]
- src/cache/RedisQueryResultCache.ts[52-108]
- src/cache/RedisQueryResultCache.ts[156-168]
- src/cache/RedisQueryResultCache.ts[208-210]

### What to change
- Replace `describe.skip(...)` with `describe(...)`.
- Remove/update the `detectRedisVersion` tests.
- Add unit tests that stub the underlying client and assert:
 - For `clientType: &quot;redis&quot;`, `storeInCache` uses the redis-v5-compatible `set` signature.
 - For `clientType: &quot;ioredis&quot;` / `&quot;ioredis/cluster&quot;`, `storeInCache` uses the expected ioredis signature.
 - `deleteKey` calls `del` with the correct argument shape.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

G0maa · 2026-02-28T12:47:57Z

(will fix Qodo comments)

G0maa · 2026-02-28T12:51:05Z

maybe I should add tests too, since it's likely storeInCache should have failed 😓

coveralls · 2026-02-28T13:01:23Z

coverage: 74.896% (+0.09%) from 74.802%
when pulling 5c73674 on feat/deprecate-legacy-redis
into c9916c9 on master.

qodo-free-for-open-source-projects · 2026-02-28T13:12:02Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

src/cache/RedisQueryResultCache.ts

qodo-free-for-open-source-projects · 2026-03-07T08:38:42Z

Persistent review updated to latest commit e82d496

qodo-free-for-open-source-projects · 2026-03-07T08:43:00Z

Persistent review updated to latest commit e82d496

qodo-free-for-open-source-projects · 2026-03-07T08:49:29Z

Persistent review updated to latest commit 096bca9

qodo-free-for-open-source-projects · 2026-03-07T08:53:06Z

Persistent review updated to latest commit 096bca9

qodo-free-for-open-source-projects · 2026-03-07T08:56:13Z

Persistent review updated to latest commit cd328c3

qodo-free-for-open-source-projects · 2026-03-07T09:00:12Z

Persistent review updated to latest commit cd328c3

test/functional/cache/redis-cache-integration.test.ts

G0maa · 2026-03-08T15:31:40Z

It seems testcontainers are not supported on Windows, I'll skip the test for Windows platfrom.

G0maa · 2026-03-08T15:33:48Z

Tests are being skipped 😅, any idea to fix this? @gioboa @alumni

qodo-free-for-open-source-projects · 2026-03-08T15:35:15Z

Persistent review updated to latest commit 4930950

pkuczynski · 2026-03-08T15:35:52Z

It seems testcontainers are not supported on Windows, I'll skip the test for Windows platfrom.

I don't think we should spin of container from the test. We should rather assume the test environment has everything we need. So I would rather rely on the side containers in GHA.

pkuczynski · 2026-03-08T16:23:42Z

Tests are being skipped 😅, any idea to fix this? @gioboa @alumni

This should fix it: #12145

qodo-free-for-open-source-projects · 2026-03-08T23:04:07Z

Persistent review updated to latest commit 6efc8b1

qodo-free-for-open-source-projects · 2026-03-09T15:08:42Z

Persistent review updated to latest commit 1b0b51f

qodo-free-for-open-source-projects · 2026-03-09T15:15:27Z

Persistent review updated to latest commit 04a6f13

qodo-free-for-open-source-projects · 2026-03-10T08:16:16Z

Persistent review updated to latest commit 8ee849d

sonarqubecloud · 2026-03-11T08:23:08Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

qodo-free-for-open-source-projects · 2026-03-11T08:29:56Z

Persistent review updated to latest commit 5c73674

feat(redis): deprecate legacy redis

a404fde

fix(redis): temporarily skip test

2326fe3

gioboa reviewed Feb 28, 2026

View reviewed changes

test/unit/cache/redis-query-result-cache.test.ts Outdated Show resolved Hide resolved

refactor(redis): better handling of ioredis

69ffb7b

G0maa commented Mar 1, 2026

View reviewed changes

src/cache/RedisQueryResultCache.ts Outdated Show resolved Hide resolved

alumni reviewed Mar 2, 2026

View reviewed changes

src/cache/RedisQueryResultCache.ts Outdated Show resolved Hide resolved

alumni reviewed Mar 2, 2026

View reviewed changes

src/cache/RedisQueryResultCache.ts Outdated Show resolved Hide resolved

alumni mentioned this pull request Mar 2, 2026

Release plan for 1.0 #11819

Open

19 tasks

refactor: revert to use of any

e82d496

test(redis): add unit tests

096bca9

G0maa requested review from alumni and gioboa March 7, 2026 08:45

fix: return value directly

cd328c3

tests: add integration tests for RedisQueryCacheResult

2d1c9a2

G0maa commented Mar 7, 2026

View reviewed changes

test/functional/cache/redis-cache-integration.test.ts Show resolved Hide resolved

alumni approved these changes Mar 8, 2026

View reviewed changes

gioboa approved these changes Mar 8, 2026

View reviewed changes

Merge branch 'master' into feat/deprecate-legacy-redis

6efc8b1

G0maa added 2 commits March 9, 2026 17:03

Merge branch 'master' into feat/deprecate-legacy-redis

1b0b51f

fix: lint

04a6f13

Merge branch 'master' into feat/deprecate-legacy-redis

8ee849d

G0maa enabled auto-merge March 10, 2026 08:18

G0maa added this pull request to the merge queue Mar 10, 2026

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 10, 2026

G0maa added this pull request to the merge queue Mar 10, 2026

G0maa removed this pull request from the merge queue due to a manual request Mar 10, 2026

G0maa removed the request for review from pkuczynski March 10, 2026 11:31

G0maa added this pull request to the merge queue Mar 10, 2026

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 10, 2026

Merge branch 'master' into feat/deprecate-legacy-redis

5c73674

G0maa enabled auto-merge (squash) March 11, 2026 08:22

G0maa merged commit 29ad659 into master Mar 11, 2026
39 checks passed

G0maa deleted the feat/deprecate-legacy-redis branch March 11, 2026 08:49

github-actions bot added this to the 1.0 milestone Mar 11, 2026

Uh oh!

Conversation

G0maa commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of change

Pull-Request Checklist

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 28, 2026

User description

Description of change

Pull-Request Checklist

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

pkg-pr-new bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 28, 2026

Code Review by Qodo

Uh oh!

G0maa commented Feb 28, 2026

Uh oh!

G0maa commented Feb 28, 2026

Uh oh!

coveralls commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 28, 2026

Code Review by Qodo

Great, no issues found!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 7, 2026

Uh oh!

Uh oh!

G0maa commented Mar 8, 2026

Uh oh!

G0maa commented Mar 8, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 8, 2026

Uh oh!

pkuczynski commented Mar 8, 2026

Uh oh!

pkuczynski commented Mar 8, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 8, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 9, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 9, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G0maa commented Feb 28, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Feb 28, 2026 •

edited

Loading

pkg-pr-new bot commented Feb 28, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Feb 28, 2026 •

edited

Loading

coveralls commented Feb 28, 2026 •

edited

Loading