new "transform arguments" API for better key and metadata extraction (#2733)

* Parser support with all commands

* remove "dist" from all imports for consistency

* address most of my review comments

* small tweak to multi type mapping handling

* tweak multi commands / fix addScript cases

* nits

* addressed all in person review comments

* revert addCommand/addScript changes to multi-commands

addCommand needs to be there for sendCommand like ability within a multi.

If its there, it might as well be used by createCommand() et al, to avoid repeating code.

addScript is there (even though only used once), but now made private to keep the logic for bookkeeping near each other.

packages/client/lib/sentinel/commands/SENTINEL_MASTER.ts

@@ -1,9 +1,10 @@
 import { RedisArgument, MapReply, BlobStringReply, Command } from '../../RESP/types';
+import { CommandParser } from '../../client/parser';
 import { transformTuplesReply } from '../../commands/generic-transformers';
 
 export default {
-  transformArguments(dbname: RedisArgument) {
-    return ['SENTINEL', 'MASTER', dbname];
+  parseCommand(parser: CommandParser, dbname: RedisArgument) {
+    parser.push('SENTINEL', 'MASTER', dbname);
   },
   transformReply: {
     2: transformTuplesReply<BlobStringReply>,

packages/client/lib/sentinel/commands/SENTINEL_MONITOR.ts

@@ -1,8 +1,9 @@
+import { CommandParser } from '../../client/parser';
 import { RedisArgument, SimpleStringReply, Command } from '../../RESP/types';
 
 export default {
-  transformArguments(dbname: RedisArgument, host: RedisArgument, port: RedisArgument, quorum: RedisArgument) {
-    return ['SENTINEL', 'MONITOR', dbname, host, port, quorum];
+  parseCommand(parser: CommandParser, dbname: RedisArgument, host: RedisArgument, port: RedisArgument, quorum: RedisArgument) {
+    parser.push('SENTINEL', 'MONITOR', dbname, host, port, quorum);
   },
   transformReply: undefined as unknown as () => SimpleStringReply<'OK'> 
 } as const satisfies Command;

packages/client/lib/sentinel/commands/SENTINEL_REPLICAS.ts

@@ -1,9 +1,10 @@
+import { CommandParser } from '../../client/parser';
 import { RedisArgument, ArrayReply, BlobStringReply, MapReply, Command, TypeMapping, UnwrapReply } from '../../RESP/types';
 import { transformTuplesReply } from '../../commands/generic-transformers';
 
 export default {
-  transformArguments(dbname: RedisArgument) {
-    return ['SENTINEL', 'REPLICAS', dbname];
+  parseCommand(parser: CommandParser, dbname: RedisArgument) {
+    parser.push('SENTINEL', 'REPLICAS', dbname);
   },
   transformReply: {
     2: (reply: ArrayReply<ArrayReply<BlobStringReply>>, preserve?: any, typeMapping?: TypeMapping) => {

packages/client/lib/sentinel/commands/SENTINEL_SENTINELS.ts

@@ -1,9 +1,10 @@
+import { CommandParser } from '../../client/parser';
 import { RedisArgument, ArrayReply, MapReply, BlobStringReply, Command, TypeMapping, UnwrapReply } from '../../RESP/types';
 import { transformTuplesReply } from '../../commands/generic-transformers';
 
 export default {
-  transformArguments(dbname: RedisArgument) {
-    return ['SENTINEL', 'SENTINELS', dbname];
+  parseCommand(parser: CommandParser, dbname: RedisArgument) {
+    parser.push('SENTINEL', 'SENTINELS', dbname);
   },
   transformReply: {
     2: (reply: ArrayReply<ArrayReply<BlobStringReply>>, preserve?: any, typeMapping?: TypeMapping) => {

packages/client/lib/sentinel/commands/SENTINEL_SET.ts

@@ -1,3 +1,4 @@
+import { CommandParser } from '../../client/parser';
 import { RedisArgument, SimpleStringReply, Command } from '../../RESP/types';
 
 export type SentinelSetOptions = Array<{
@@ -6,14 +7,12 @@ export type SentinelSetOptions = Array<{
 }>;
 
 export default {
-  transformArguments(dbname: RedisArgument, options: SentinelSetOptions) {
-    const args = ['SENTINEL', 'SET', dbname];
+  parseCommand(parser: CommandParser, dbname: RedisArgument, options: SentinelSetOptions) {
+    parser.push('SENTINEL', 'SET', dbname);
 
     for (const option of options) {
-      args.push(option.option, option.value);
+      parser.push(option.option, option.value);
     }
-
-    return args;
   },
   transformReply: undefined as unknown as () => SimpleStringReply<'OK'> 
 } as const satisfies Command;

packages/client/lib/sentinel/index.spec.ts

@@ -14,21 +14,10 @@ import { defineScript } from '../lua-script';
 import { MATH_FUNCTION } from '../commands/FUNCTION_LOAD.spec';
 import RedisBloomModules from '@redis/bloom';
 import { RedisTcpSocketOptions } from '../client/socket';
+import { SQUARE_SCRIPT } from '../client/index.spec';
 
 const execAsync = promisify(exec);
 
-const SQUARE_SCRIPT = defineScript({
-  SCRIPT:
-    `local number = redis.call('GET', KEYS[1])
-    return number * number`,
-  NUMBER_OF_KEYS: 1,
-  FIRST_KEY_INDEX: 0,
-  transformArguments(key: string) {
-    return [key];
-  },
-  transformReply: undefined as unknown as () => NumberReply
-});
-
 /* used to ensure test environment resets to normal state
    i.e. 
    - all redis nodes are active and are part of the topology

packages/client/lib/sentinel/index.ts

@@ -1,5 +1,5 @@
 import { EventEmitter } from 'node:events';
-import { CommandArguments, RedisArgument, RedisFunctions, RedisModules, RedisScript, RedisScripts, ReplyUnion, RespVersions, TypeMapping } from '../RESP/types';
+import { CommandArguments, RedisFunctions, RedisModules, RedisScripts, ReplyUnion, RespVersions, TypeMapping } from '../RESP/types';
 import RedisClient, { RedisClientOptions, RedisClientType } from '../client';
 import { CommandOptions } from '../client/commands-queue';
 import { attachConfig } from '../commander';
@@ -164,18 +164,6 @@ export class RedisSentinelClient<
     );
   }
 
-  executeScript(
-    script: RedisScript,  
-    isReadonly: boolean | undefined,
-    args: Array<RedisArgument>,
-    options?: CommandOptions
-  ) {
-    return this._execute(
-      isReadonly,
-      client => client.executeScript(script, args, options)
-    );
-  }
-
   /**
    * @internal
    */
@@ -440,18 +428,6 @@ export default class RedisSentinel<
     );
   }
 
-  executeScript(
-    script: RedisScript,
-    isReadonly: boolean | undefined,
-    args: Array<RedisArgument>,
-    options?: CommandOptions
-  ) {
-    return this._execute(
-      isReadonly,
-      client => client.executeScript(script, args, options)
-    );
-  }
-
   /**
    * @internal
    */

packages/client/lib/sentinel/multi-commands.ts

@@ -3,6 +3,8 @@ import RedisMultiCommand, { MULTI_REPLY, MultiReply, MultiReplyType } from '../m
 import { ReplyWithTypeMapping, CommandReply, Command, CommandArguments, CommanderConfig, RedisFunctions, RedisModules, RedisScripts, RespVersions, TransformReply, RedisScript, RedisFunction, TypeMapping } from '../RESP/types';
 import { attachConfig, functionArgumentsPrefix, getTransformReply } from '../commander';
 import { RedisSentinelType } from './types';
+import { BasicCommandParser } from '../client/parser';
+import { Tail } from '../commands/generic-transformers';
 
 type CommandSignature<
   REPLIES extends Array<unknown>,
@@ -12,7 +14,7 @@ type CommandSignature<
   S extends RedisScripts,
   RESP extends RespVersions,
   TYPE_MAPPING extends TypeMapping
-> = (...args: Parameters<C['transformArguments']>) => RedisSentinelMultiCommandType<
+> = (...args: Tail<Parameters<C['parseCommand']>>) => RedisSentinelMultiCommandType<
   [...REPLIES, ReplyWithTypeMapping<CommandReply<C, RESP>, TYPE_MAPPING>],
   M,
   F,
@@ -87,8 +89,14 @@ export type RedisSentinelMultiCommandType<
 export default class RedisSentinelMultiCommand<REPLIES = []> {
   private static _createCommand(command: Command, resp: RespVersions) {
     const transformReply = getTransformReply(command, resp);
+
     return function (this: RedisSentinelMultiCommand, ...args: Array<unknown>) {
-      const redisArgs = command.transformArguments(...args);
+      const parser = new BasicCommandParser();
+      command.parseCommand(parser, ...args);
+
+      const redisArgs: CommandArguments = parser.redisArgs;
+      redisArgs.preserve = parser.preserve;
+
       return this.addCommand(
         command.IS_READ_ONLY,
         redisArgs,
@@ -99,8 +107,14 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
 
   private static _createModuleCommand(command: Command, resp: RespVersions) {
     const transformReply = getTransformReply(command, resp);
+
     return function (this: { _self: RedisSentinelMultiCommand }, ...args: Array<unknown>) {
-      const redisArgs = command.transformArguments(...args);
+      const parser = new BasicCommandParser();
+      command.parseCommand(parser, ...args);
+
+      const redisArgs: CommandArguments = parser.redisArgs;
+      redisArgs.preserve = parser.preserve;
+
       return this._self.addCommand(
         command.IS_READ_ONLY,
         redisArgs,
@@ -110,12 +124,17 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
   }
 
   private static _createFunctionCommand(name: string, fn: RedisFunction, resp: RespVersions) {
-    const prefix = functionArgumentsPrefix(name, fn),
-      transformReply = getTransformReply(fn, resp);
+    const prefix = functionArgumentsPrefix(name, fn);
+    const transformReply = getTransformReply(fn, resp);
+
     return function (this: { _self: RedisSentinelMultiCommand }, ...args: Array<unknown>) {
-      const fnArgs = fn.transformArguments(...args);
-      const redisArgs: CommandArguments = prefix.concat(fnArgs);
-      redisArgs.preserve = fnArgs.preserve;
+      const parser = new BasicCommandParser();
+      parser.push(...prefix);
+      fn.parseCommand(parser, ...args);
+
+      const redisArgs: CommandArguments = parser.redisArgs;
+      redisArgs.preserve = parser.preserve;
+
       return this._self.addCommand(
         fn.IS_READ_ONLY,
         redisArgs,
@@ -126,17 +145,20 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
 
   private static _createScriptCommand(script: RedisScript, resp: RespVersions) {
     const transformReply = getTransformReply(script, resp);
+
     return function (this: RedisSentinelMultiCommand, ...args: Array<unknown>) {
-      const scriptArgs = script.transformArguments(...args);
-      this._setState(
-        script.IS_READ_ONLY
-      );
-      this._multi.addScript(
+      const parser = new BasicCommandParser();
+      script.parseCommand(parser, ...args);
+
+      const scriptArgs: CommandArguments = parser.redisArgs;
+      scriptArgs.preserve = parser.preserve;
+
+      return this.#addScript(
+        script.IS_READ_ONLY,
         script,
         scriptArgs,
         transformReply
       );
-      return this;
     };
   }
 
@@ -157,20 +179,19 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
     });
   }
 
-  private readonly _multi = new RedisMultiCommand();
-  private readonly _sentinel: RedisSentinelType
-  private _isReadonly: boolean | undefined = true;
-  private readonly _typeMapping?: TypeMapping;
+  readonly #multi = new RedisMultiCommand();
+  readonly #sentinel: RedisSentinelType
+  #isReadonly: boolean | undefined = true;
 
   constructor(sentinel: RedisSentinelType, typeMapping: TypeMapping) {
-    this._sentinel = sentinel;
-    this._typeMapping = typeMapping;
+    this.#multi = new RedisMultiCommand(typeMapping);
+    this.#sentinel = sentinel;
   }
 
-  private _setState(
+  #setState(
     isReadonly: boolean | undefined,
   ) {
-    this._isReadonly &&= isReadonly;
+    this.#isReadonly &&= isReadonly;
   }
 
   addCommand(
@@ -178,20 +199,31 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
     args: CommandArguments,
     transformReply?: TransformReply
   ) {
-    this._setState(isReadonly);
-    this._multi.addCommand(args, transformReply);
+    this.#setState(isReadonly);
+    this.#multi.addCommand(args, transformReply);
+    return this;
+  }
+
+  #addScript(
+    isReadonly: boolean | undefined,
+    script: RedisScript,
+    args: CommandArguments,
+    transformReply?: TransformReply
+  ) {
+    this.#setState(isReadonly);
+    this.#multi.addScript(script, args, transformReply);
+
     return this;
   }
 
   async exec<T extends MultiReply = MULTI_REPLY['GENERIC']>(execAsPipeline = false) {
     if (execAsPipeline) return this.execAsPipeline<T>();
 
-    return this._multi.transformReplies(
-      await this._sentinel._executeMulti(
-        this._isReadonly,
-        this._multi.queue
-      ),
-      this._typeMapping
+    return this.#multi.transformReplies(
+      await this.#sentinel._executeMulti(
+        this.#isReadonly,
+        this.#multi.queue
+      )
     ) as MultiReplyType<T, REPLIES>;
   }
 
@@ -202,14 +234,13 @@ export default class RedisSentinelMultiCommand<REPLIES = []> {
   }
 
   async execAsPipeline<T extends MultiReply = MULTI_REPLY['GENERIC']>() {
-    if (this._multi.queue.length === 0) return [] as MultiReplyType<T, REPLIES>;
+    if (this.#multi.queue.length === 0) return [] as MultiReplyType<T, REPLIES>;
 
-    return this._multi.transformReplies(
-      await this._sentinel._executePipeline(
-        this._isReadonly,
-        this._multi.queue
-      ),
-      this._typeMapping
+    return this.#multi.transformReplies(
+      await this.#sentinel._executePipeline(
+        this.#isReadonly,
+        this.#multi.queue
+      )
     ) as MultiReplyType<T, REPLIES>;
   }
 

packages/client/lib/sentinel/utils.ts

@@ -1,3 +1,4 @@
+import { BasicCommandParser } from '../client/parser';
 import { ArrayReply, Command, RedisFunction, RedisScript, RespVersions, UnwrapReply } from '../RESP/types';
 import { RedisSocketOptions, RedisTcpSocketOptions } from '../client/socket';
 import { functionArgumentsPrefix, getTransformReply, scriptArgumentsPrefix } from '../commander';
@@ -38,77 +39,60 @@ export function clientSocketToNode(socket: RedisSocketOptions): RedisNode {
 
 export function createCommand<T extends ProxySentinel | ProxySentinelClient>(command: Command, resp: RespVersions) {
   const transformReply = getTransformReply(command, resp);
+
   return async function (this: T, ...args: Array<unknown>) {
-    const redisArgs = command.transformArguments(...args);
-    const typeMapping = this._self.commandOptions?.typeMapping;
+    const parser = new BasicCommandParser();
+    command.parseCommand(parser, ...args);
 
-    const reply = await this._self.sendCommand(
+    return this._self._execute(
       command.IS_READ_ONLY,
-      redisArgs,
-      this._self.commandOptions
+      client => client._executeCommand(command, parser, this.commandOptions, transformReply)
     );
-
-    return transformReply ?
-      transformReply(reply, redisArgs.preserve, typeMapping) :
-      reply;
   };
 }
 
 export function createFunctionCommand<T extends NamespaceProxySentinel | NamespaceProxySentinelClient>(name: string, fn: RedisFunction, resp: RespVersions) {
-  const prefix = functionArgumentsPrefix(name, fn),
-  transformReply = getTransformReply(fn, resp);
+  const prefix = functionArgumentsPrefix(name, fn);
+  const transformReply = getTransformReply(fn, resp);
+
   return async function (this: T, ...args: Array<unknown>) {
-    const fnArgs = fn.transformArguments(...args);
-    const redisArgs = prefix.concat(fnArgs);
-    const typeMapping = this._self._self.commandOptions?.typeMapping;
+    const parser = new BasicCommandParser();
+    parser.push(...prefix);
+    fn.parseCommand(parser, ...args);
 
-    const reply = await this._self._self.sendCommand(
+    return this._self._execute(
       fn.IS_READ_ONLY,
-      redisArgs,
-      this._self._self.commandOptions
+      client => client._executeCommand(fn, parser, this._self.commandOptions, transformReply)
     );
-
-    return transformReply ?
-      transformReply(reply, fnArgs.preserve, typeMapping) :
-      reply;
   }
 };
 
 export function createModuleCommand<T extends NamespaceProxySentinel | NamespaceProxySentinelClient>(command: Command, resp: RespVersions) {
   const transformReply = getTransformReply(command, resp);
+
   return async function (this: T, ...args: Array<unknown>) {
-    const redisArgs = command.transformArguments(...args);
-    const typeMapping = this._self._self.commandOptions?.typeMapping;
+    const parser = new BasicCommandParser();
+    command.parseCommand(parser, ...args);
 
-    const reply = await this._self._self.sendCommand(
+    return this._self._execute(
       command.IS_READ_ONLY,
-      redisArgs,
-      this._self._self.commandOptions
+      client => client._executeCommand(command, parser, this._self.commandOptions, transformReply)
     );
-
-    return transformReply ?
-      transformReply(reply, redisArgs.preserve, typeMapping) :
-      reply;
   }
 };
 
 export function createScriptCommand<T extends ProxySentinel | ProxySentinelClient>(script: RedisScript, resp: RespVersions) {
-  const prefix = scriptArgumentsPrefix(script),
-    transformReply = getTransformReply(script, resp);
+  const prefix = scriptArgumentsPrefix(script);
+  const transformReply = getTransformReply(script, resp);
+
   return async function (this: T, ...args: Array<unknown>) {
-    const scriptArgs = script.transformArguments(...args);
-    const redisArgs = prefix.concat(scriptArgs);
-    const typeMapping = this._self.commandOptions?.typeMapping;
+    const parser = new BasicCommandParser();
+    parser.push(...prefix);
+    script.parseCommand(parser, ...args);
 
-    const reply = await this._self.executeScript(
-      script,
+    return this._self._execute(
       script.IS_READ_ONLY,
-      redisArgs,
-      this._self.commandOptions
+      client => client._executeScript(script, parser, this.commandOptions, transformReply)
     );
-
-    return transformReply ?
-      transformReply(reply, scriptArgs.preserve, typeMapping) :
-      reply;
   };
 }