From 7f7a53a1b1fce9d5fcb42a20f0decdcd1100e431 Mon Sep 17 00:00:00 2001 From: Leibale Date: Thu, 7 Dec 2023 09:42:04 -0500 Subject: [PATCH] uncomment some tests, fix bugs with `_selectedDB` --- packages/client/lib/client/index.spec.ts | 172 +++++++------------- packages/client/lib/client/index.ts | 21 ++- packages/client/lib/client/multi-command.ts | 9 +- 3 files changed, 78 insertions(+), 124 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 1aaf767b0d..d8ac7487cc 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -1,17 +1,13 @@ import { strict as assert } from 'node:assert'; import testUtils, { GLOBAL, waitTillBeenCalled } from '../test-utils'; import RedisClient, { RedisClientType } from '.'; -// import { RedisClientMultiCommandType } from './multi-command'; -// import { RedisCommandRawReply, RedisModules, RedisFunctions, RedisScripts } from '../commands'; import { AbortError, ClientClosedError, ClientOfflineError, ConnectionTimeoutError, DisconnectsClientError, ErrorReply, MultiErrorReply, SocketClosedUnexpectedlyError, WatchError } from '../errors'; import { defineScript } from '../lua-script'; import { spy } from 'sinon'; import { once } from 'node:events'; -// import { ClientKillFilters } from '../commands/CLIENT_KILL'; -// import { promisify } from 'node:util'; import { MATH_FUNCTION, loadMathFunction } from '../commands/FUNCTION_LOAD.spec'; import { RESP_TYPES } from '../RESP/decoder'; -import { NumberReply } from '../RESP/types'; +import { BlobStringReply, NumberReply } from '../RESP/types'; import { SortedSetMember } from '../commands/generic-transformers'; export const SQUARE_SCRIPT = defineScript({ @@ -232,24 +228,26 @@ describe('Client', () => { } }); - // testUtils.testWithClient('WatchError', async client => { - // await client.watch('key'); + testUtils.testWithClient('WatchError', async client => { + await client.watch('key'); - // await client.set( - // RedisClient.commandOptions({ - // isolated: true - // }), - // 'key', - // '1' - // ); + const duplicate = await client.duplicate().connect(); + try { + await client.set( + 'key', + '1' + ); + } finally { + duplicate.destroy(); + } - // await assert.rejects( - // client.multi() - // .decr('key') - // .exec(), - // WatchError - // ); - // }, GLOBAL.SERVERS.OPEN); + await assert.rejects( + client.multi() + .decr('key') + .exec(), + WatchError + ); + }, GLOBAL.SERVERS.OPEN); describe('execAsPipeline', () => { testUtils.testWithClient('exec(true)', async client => { @@ -269,20 +267,19 @@ describe('Client', () => { }, GLOBAL.SERVERS.OPEN); }); - // testUtils.testWithClient('should remember selected db', async client => { - // await client.multi() - // .select(1) - // .exec(); - // await killClient(client); - // assert.equal( - // (await client.clientInfo()).db, - // 1 - // ); - // }, { - // ...GLOBAL.SERVERS.OPEN, - // minimumDockerVersion: [6, 2] // CLIENT INFO - // }); - + testUtils.testWithClient('should remember selected db', async client => { + await client.multi() + .select(1) + .exec(); + await killClient(client); + assert.equal( + (await client.clientInfo()).db, + 1 + ); + }, { + ...GLOBAL.SERVERS.OPEN, + minimumDockerVersion: [6, 2] // CLIENT INFO + }); testUtils.testWithClient('should handle error replies (#2665)', async client => { await assert.rejects( @@ -320,12 +317,10 @@ describe('Client', () => { const module = { echo: { - transformArguments(message: string): Array { + transformArguments(message: string) { return ['ECHO', message]; }, - transformReply(reply: string): string { - return reply; - } + transformReply: undefined as unknown as () => BlobStringReply } }; @@ -386,81 +381,34 @@ describe('Client', () => { disableClientSetup: true, }); -// describe('isolationPool', () => { -// testUtils.testWithClient('executeIsolated', async client => { -// const id = await client.clientId(), -// isolatedId = await client.executeIsolated(isolatedClient => isolatedClient.clientId()); -// assert.ok(id !== isolatedId); -// }, GLOBAL.SERVERS.OPEN); + async function killClient( + client: RedisClientType, + errorClient: RedisClientType = client + ): Promise { + const onceErrorPromise = once(errorClient, 'error'); + await client.sendCommand(['QUIT']); + await Promise.all([ + onceErrorPromise, + assert.rejects(client.ping(), SocketClosedUnexpectedlyError) + ]); + } -// testUtils.testWithClient('should be able to use pool even before connect', async client => { -// await client.executeIsolated(() => Promise.resolve()); -// // make sure to destroy isolation pool -// await client.connect(); -// await client.disconnect(); -// }, { -// ...GLOBAL.SERVERS.OPEN, -// disableClientSetup: true -// }); + testUtils.testWithClient('should reconnect when socket disconnects', async client => { + await killClient(client); + await assert.doesNotReject(client.ping()); + }, GLOBAL.SERVERS.OPEN); -// testUtils.testWithClient('should work after reconnect (#2406)', async client => { -// await client.disconnect(); -// await client.connect(); -// await client.executeIsolated(() => Promise.resolve()); -// }, GLOBAL.SERVERS.OPEN); - -// testUtils.testWithClient('should throw ClientClosedError after disconnect', async client => { -// await client.connect(); -// await client.disconnect(); -// await assert.rejects( -// client.executeIsolated(() => Promise.resolve()), -// ClientClosedError -// ); -// }, { -// ...GLOBAL.SERVERS.OPEN, -// disableClientSetup: true -// }); -// }); - -// async function killClient< -// M extends RedisModules, -// F extends RedisFunctions, -// S extends RedisScripts -// >( -// client: RedisClientType, -// errorClient: RedisClientType = client -// ): Promise { -// const onceErrorPromise = once(errorClient, 'error'); -// await client.sendCommand(['QUIT']); -// await Promise.all([ -// onceErrorPromise, -// assert.rejects(client.ping(), SocketClosedUnexpectedlyError) -// ]); -// } - -// testUtils.testWithClient('should reconnect when socket disconnects', async client => { -// await killClient(client); -// await assert.doesNotReject(client.ping()); -// }, GLOBAL.SERVERS.OPEN); - -// testUtils.testWithClient('should remember selected db', async client => { -// await client.select(1); -// await killClient(client); -// assert.equal( -// (await client.clientInfo()).db, -// 1 -// ); -// }, { -// ...GLOBAL.SERVERS.OPEN, -// minimumDockerVersion: [6, 2] // CLIENT INFO -// }); - -// testUtils.testWithClient('should propagated errors from "isolated" clients', client => { -// client.on('error', () => { -// // ignore errors -// }); -// return client.executeIsolated(isolated => killClient(isolated, client)); -// }, GLOBAL.SERVERS.OPEN); + testUtils.testWithClient('should remember selected db', async client => { + await client.select(1); + await killClient(client); + assert.equal( + (await client.clientInfo()).db, + 1 + ); + }, { + ...GLOBAL.SERVERS.OPEN, + minimumDockerVersion: [6, 2] // CLIENT INFO + }); testUtils.testWithClient('scanIterator', async client => { const entries: Array = [], diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index e47a500420..1f33ca4483 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -315,7 +315,7 @@ export default class RedisClient< } if (options?.database) { - this._selectedDB = options.database; + this.self._selectedDB = options.database; } if (options?.commandOptions) { @@ -584,7 +584,7 @@ export default class RedisClient< async SELECT(db: number): Promise { await this.sendCommand(['SELECT', db.toString()]); - this._selectedDB = db; + this.self._selectedDB = db; } select = this.SELECT; @@ -742,7 +742,10 @@ export default class RedisClient< /** * @internal */ - _executePipeline(commands: Array) { + async _executePipeline( + commands: Array, + selectedDB?: number + ) { if (!this._socket.isOpen) { return Promise.reject(new ClientClosedError()); } @@ -753,7 +756,13 @@ export default class RedisClient< })) ); this._scheduleWrite(); - return promise; + const result = await promise; + + if (selectedDB !== undefined) { + this.self._selectedDB = selectedDB; + } + + return result; } /** @@ -764,7 +773,7 @@ export default class RedisClient< selectedDB?: number ) { if (!this._socket.isOpen) { - return Promise.reject(new ClientClosedError()); + throw new ClientClosedError(); } const typeMapping = this._commandOptions?.typeMapping, @@ -796,7 +805,7 @@ export default class RedisClient< } if (selectedDB !== undefined) { - this._selectedDB = selectedDB; + this.self._selectedDB = selectedDB; } return execResult as Array; diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index 5f7679d2b1..7daf4b5d80 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -85,8 +85,6 @@ export type RedisClientMultiCommandType< type ExecuteMulti = (commands: Array, selectedDB?: number) => Promise>; -type ExecutePipeline = (commands: Array) => Promise>; - export default class RedisClientMultiCommand { private static _createCommand(command: Command, resp: RespVersions) { const transformReply = getTransformReply(command, resp); @@ -153,13 +151,12 @@ export default class RedisClientMultiCommand { private readonly _multi = new RedisMultiCommand(); private readonly _executeMulti: ExecuteMulti; - private readonly _executePipeline: ExecutePipeline; + private readonly _executePipeline: ExecuteMulti; private _selectedDB?: number; - constructor(executeMulti: ExecuteMulti, executePipeline: ExecutePipeline) { + constructor(executeMulti: ExecuteMulti, executePipeline: ExecuteMulti) { this._executeMulti = executeMulti; this._executePipeline = executePipeline; - // this._client = client; } SELECT(db: number, transformReply?: TransformReply): this { @@ -193,7 +190,7 @@ export default class RedisClientMultiCommand { if (this._multi.queue.length === 0) return [] as MultiReplyType; return this._multi.transformReplies( - await this._executePipeline(this._multi.queue) + await this._executePipeline(this._multi.queue, this._selectedDB) ) as MultiReplyType; }