From 4e8c567d1175de31e2371a9dad308a94fcb5627f Mon Sep 17 00:00:00 2001 From: Zihua Li Date: Sat, 12 Feb 2022 23:03:48 +0800 Subject: [PATCH] fix: improve typing for auto pipelining --- lib/autoPipelining.ts | 7 ++-- lib/command.ts | 10 ++++-- lib/utils/Commander.ts | 33 ++++++++--------- lib/utils/index.ts | 68 +++++------------------------------ test/unit/utils.ts | 82 ++++++++++++++++++++---------------------- 5 files changed, 75 insertions(+), 125 deletions(-) diff --git a/lib/autoPipelining.ts b/lib/autoPipelining.ts index 59d062d1..dfe658ea 100644 --- a/lib/autoPipelining.ts +++ b/lib/autoPipelining.ts @@ -1,6 +1,7 @@ import { flatten, isArguments, noop } from "./utils/lodash"; import * as calculateSlot from "cluster-key-slot"; import asCallback from "standard-as-callback"; +import { ArgumentType } from "./command"; export const kExec = Symbol("exec"); export const kCallbacks = Symbol("callbacks"); @@ -92,7 +93,7 @@ export function shouldUseAutoPipelining( * @private */ export function getFirstValueInFlattenedArray( - args: (string | string[])[] + args: ArgumentType[] ): string | undefined { for (let i = 0; i < args.length; i++) { const arg = args[i]; @@ -106,7 +107,7 @@ export function getFirstValueInFlattenedArray( } const flattened = flatten([arg]); if (flattened.length > 0) { - return flattened[0]; + return String(flattened[0]); } } return undefined; @@ -116,7 +117,7 @@ export function executeWithAutoPipelining( client, functionName: string, commandName: string, - args: (string | string[])[], + args: ArgumentType[], callback ) { // On cluster mode let's wait for slots to be available diff --git a/lib/command.ts b/lib/command.ts index 91ac2389..1912c419 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -11,6 +11,12 @@ import { import { flatten } from "./utils/lodash"; import { CallbackFunction, ICommand, CommandParameter } from "./types"; +export type ArgumentType = + | string + | Buffer + | number + | (string | Buffer | number | any[])[]; + interface ICommandOptions { /** * Set the encoding of the reply, by default buffer will be returned. @@ -179,9 +185,7 @@ export default class Command implements ICommand { */ constructor( public name: string, - args: Array< - string | Buffer | number | Array - > = [], + args: Array = [], options: ICommandOptions = {}, callback?: CallbackFunction ) { diff --git a/lib/utils/Commander.ts b/lib/utils/Commander.ts index 747ab8f6..34795fd1 100644 --- a/lib/utils/Commander.ts +++ b/lib/utils/Commander.ts @@ -3,9 +3,9 @@ import { executeWithAutoPipelining, shouldUseAutoPipelining, } from "../autoPipelining"; -import Command from "../command"; +import Command, { ArgumentType } from "../command"; import Script from "../script"; -import { NetStream } from "../types"; +import { CallbackFunction, NetStream } from "../types"; export interface CommanderOptions { keyPrefix?: string; @@ -33,8 +33,6 @@ class Commander { /** * Return supported builtin commands - * - * @return {string[]} command list */ getBuiltinCommands() { return commands.slice(0); @@ -42,9 +40,6 @@ class Commander { /** * Create a builtin command - * - * @param {string} commandName - command name - * @return {object} functions */ createBuiltinCommand(commandName: string) { return { @@ -69,14 +64,16 @@ class Commander { /** * Define a custom command using lua script * - * @param {string} name - the command name * @param {object} definition * @param {string} definition.lua - the lua code * @param {number} [definition.numberOfKeys=null] - the number of keys. * @param {boolean} [definition.readOnly=false] - force this script to be readonly so it executes on slaves as well. * If omit, you have to pass the number of keys as the first argument every time you invoke the command */ - defineCommand(name, definition) { + defineCommand( + name: string, + definition: { lua: string; numberOfKeys?: number; readOnly?: boolean } + ) { const script = new Script( definition.lua, definition.numberOfKeys, @@ -139,8 +136,10 @@ function generateFunction( _commandName = null; } - return function (...args) { - const commandName = _commandName || args.shift(); + return function ( + ...args: ArgumentType[] | [...ArgumentType[], CallbackFunction] + ) { + const commandName = (_commandName || args.shift()) as string; let callback = args[args.length - 1]; if (typeof callback === "function") { @@ -158,13 +157,14 @@ function generateFunction( if (this.options.dropBufferSupport && !_encoding) { return asCallback( Promise.reject(new Error(DROP_BUFFER_SUPPORT_ERROR)), - callback + callback as CallbackFunction | undefined ); } // No auto pipeline, use regular command sending if (!shouldUseAutoPipelining(this, functionName, commandName)) { return this.sendCommand( + // @ts-expect-error new Command(commandName, args, options, callback) ); } @@ -174,6 +174,7 @@ function generateFunction( this, functionName, commandName, + // @ts-expect-error args, callback ); @@ -181,10 +182,10 @@ function generateFunction( } function generateScriptingFunction( - functionName, - commandName, - script, - encoding + functionName: string, + commandName: string, + script: Script, + encoding: unknown ) { return function () { let length = arguments.length; diff --git a/lib/utils/index.ts b/lib/utils/index.ts index 853cd788..6e642f9e 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -5,44 +5,15 @@ import Debug from "./debug"; import TLSProfiles from "../constants/TLSProfiles"; -/** - * Test if two buffers are equal - * - * @export - * @param {Buffer} a - * @param {Buffer} b - * @returns {boolean} Whether the two buffers are equal - */ -export function bufferEqual(a: Buffer, b: Buffer): boolean { - if (typeof a.equals === "function") { - return a.equals(b); - } - - if (a.length !== b.length) { - return false; - } - - for (let i = 0; i < a.length; ++i) { - if (a[i] !== b[i]) { - return false; - } - } - return true; -} - /** * Convert a buffer to string, supports buffer array * - * @param {*} value - The input value - * @param {string} encoding - string encoding - * @return {*} The result * @example * ```js - * var input = [Buffer.from('foo'), [Buffer.from('bar')]] - * var res = convertBufferToString(input, 'utf8') + * const input = [Buffer.from('foo'), [Buffer.from('bar')]] + * const res = convertBufferToString(input, 'utf8') * expect(res).to.eql(['foo', ['bar']]) * ``` - * @private */ export function convertBufferToString(value: any, encoding?: string) { if (value instanceof Buffer) { @@ -65,17 +36,14 @@ export function convertBufferToString(value: any, encoding?: string) { /** * Convert a list of results to node-style * - * @param {Array} arr - The input value - * @return {Array} The output value * @example * ```js - * var input = ['a', 'b', new Error('c'), 'd'] - * var output = exports.wrapMultiResult(input) + * const input = ['a', 'b', new Error('c'), 'd'] + * const output = exports.wrapMultiResult(input) * expect(output).to.eql([[null, 'a'], [null, 'b'], [new Error('c')], [null, 'd']) * ``` - * @private */ -export function wrapMultiResult(arr: any[] | null): any[][] { +export function wrapMultiResult(arr: unknown[] | null): unknown[][] { // When using WATCH/EXEC transactions, the EXEC will return // a null instead of an array if (!arr) { @@ -96,9 +64,6 @@ export function wrapMultiResult(arr: any[] | null): any[][] { /** * Detect if the argument is a int - * - * @param {string} value - * @return {boolean} Whether the value is a int * @example * ```js * > isInt('123') @@ -122,8 +87,6 @@ export function isInt(value: any): value is string { /** * Pack an array to an Object * - * @param {array} array - * @return {object} * @example * ```js * > packObject(['a', 'b', 'c', 'd']) @@ -143,10 +106,6 @@ export function packObject(array: any[]): Record { /** * Return a callback with timeout - * - * @param {function} callback - * @param {number} timeout - * @return {function} */ export function timeout(callback: CallbackFunction, timeout: number) { let timer: NodeJS.Timeout; @@ -163,9 +122,6 @@ export function timeout(callback: CallbackFunction, timeout: number) { /** * Convert an object to an array - * - * @param {object} obj - * @return {array} * @example * ```js * > convertObjectToArray({ a: '1' }) @@ -174,7 +130,7 @@ export function timeout(callback: CallbackFunction, timeout: number) { */ export function convertObjectToArray( obj: Record -): Array { +): (string | T)[] { const result = []; const keys = Object.keys(obj); // Object.entries requires node 7+ @@ -186,16 +142,13 @@ export function convertObjectToArray( /** * Convert a map to an array - * - * @param {Map} map - * @return {array} * @example * ```js * > convertMapToArray(new Map([[1, '2']])) * [1, '2'] * ``` */ -export function convertMapToArray(map: Map): Array { +export function convertMapToArray(map: Map): (K | V)[] { const result: Array = []; let pos = 0; map.forEach(function (value, key) { @@ -208,9 +161,6 @@ export function convertMapToArray(map: Map): Array { /** * Convert a non-string arg to a string - * - * @param {*} arg - * @return {string} */ export function toArg(arg: any): string { if (arg === null || typeof arg === "undefined") { @@ -298,7 +248,7 @@ export function parseURL(url: string) { return result; } -interface ITLSOptions { +interface TLSOptions { port: number; host: string; [key: string]: any; @@ -310,7 +260,7 @@ interface ITLSOptions { * @param {Object} options - the redis connection options * @return {Object} */ -export function resolveTLSProfile(options: ITLSOptions): ITLSOptions { +export function resolveTLSProfile(options: TLSOptions): TLSOptions { let tls = options?.tls; if (typeof tls === "string") tls = { profile: tls }; diff --git a/test/unit/utils.ts b/test/unit/utils.ts index 728be0dd..d015bb19 100644 --- a/test/unit/utils.ts +++ b/test/unit/utils.ts @@ -4,37 +4,6 @@ import * as utils from "../../lib/utils"; import TLSProfiles from "../../lib/constants/TLSProfiles"; describe("utils", function () { - describe(".bufferEqual", function () { - it("should return correctly", function () { - expect(utils.bufferEqual(Buffer.from("123"), Buffer.from("abc"))).to.eql( - false - ); - expect(utils.bufferEqual(Buffer.from("abc"), Buffer.from("abc"))).to.eql( - true - ); - expect(utils.bufferEqual(Buffer.from("bc"), Buffer.from("abc"))).to.eql( - false - ); - expect(utils.bufferEqual(Buffer.from(""), Buffer.from(""))).to.eql(true); - }); - - it("should work when Buffer#equals not exists", function () { - const equals = Buffer.prototype.equals; - delete Buffer.prototype.equals; - expect(utils.bufferEqual(Buffer.from("123"), Buffer.from("abc"))).to.eql( - false - ); - expect(utils.bufferEqual(Buffer.from("abc"), Buffer.from("abc"))).to.eql( - true - ); - expect(utils.bufferEqual(Buffer.from("bc"), Buffer.from("abc"))).to.eql( - false - ); - expect(utils.bufferEqual(Buffer.from(""), Buffer.from(""))).to.eql(true); - Buffer.prototype.equals = equals; - }); - }); - describe(".convertBufferToString", function () { it("should return correctly", function () { expect(utils.convertBufferToString(Buffer.from("123"))).to.eql("123"); @@ -283,13 +252,13 @@ describe("utils", function () { describe(".resolveTLSProfile", function () { it("should leave options alone when no tls profile is set", function () { [ - {}, - { tls: true }, - { tls: false }, - { tls: "foo" }, - { tls: {} }, - { tls: { ca: "foo" } }, - { tls: { profile: "foo" } }, + { host: "localhost", port: 6379 }, + { host: "localhost", port: 6379, tls: true }, + { host: "localhost", port: 6379, tls: false }, + { host: "localhost", port: 6379, tls: "foo" }, + { host: "localhost", port: 6379, tls: {} }, + { host: "localhost", port: 6379, tls: { ca: "foo" } }, + { host: "localhost", port: 6379, tls: { profile: "foo" } }, ].forEach((options) => { expect(utils.resolveTLSProfile(options)).to.eql(options); }); @@ -301,23 +270,48 @@ describe("utils", function () { }); it("should read profile from options.tls.profile", function () { - const input = { tls: { profile: "RedisCloudFixed" } }; - const expected = { tls: TLSProfiles.RedisCloudFixed }; + const input = { + host: "localhost", + port: 6379, + tls: { profile: "RedisCloudFixed" }, + }; + const expected = { + host: "localhost", + port: 6379, + tls: TLSProfiles.RedisCloudFixed, + }; expect(utils.resolveTLSProfile(input)).to.eql(expected); }); it("should read profile from options.tls", function () { - const input = { tls: "RedisCloudFixed" }; - const expected = { tls: TLSProfiles.RedisCloudFixed }; + const input = { + host: "localhost", + port: 6379, + tls: "RedisCloudFixed", + }; + const expected = { + host: "localhost", + port: 6379, + tls: TLSProfiles.RedisCloudFixed, + }; expect(utils.resolveTLSProfile(input)).to.eql(expected); }); it("supports extra options when using options.tls.profile", function () { - const input = { tls: { profile: "RedisCloudFixed", key: "foo" } }; + const input = { + host: "localhost", + port: 6379, + tls: { profile: "RedisCloudFixed", key: "foo" }, + }; const expected = { - tls: { ...TLSProfiles.RedisCloudFixed, key: "foo" }, + tls: { + host: "localhost", + port: 6379, + ...TLSProfiles.RedisCloudFixed, + key: "foo", + }, }; expect(utils.resolveTLSProfile(input)).to.eql(expected);