Support adding custom commands at compile time

README.md

@@ -124,6 +124,17 @@ as described in the CMake docs; a specific path can be set using a flag like:
 See `examples/using_cmake_separate/build.sh` or
 `examples/using_cmake_externalproject/build.sh` for alternative CMake builds.
 
+### Extend the list of supported commands
+
+The list of commands and the position of the first key in the command line is
+defined in `cmddef.h` which is included in this repo. It has been generated
+using the JSON files describing the syntax of each command in the Redis
+repository, which makes sure hiredis-cluster supports all commands in Redis, at
+least in terms of cluster routing. To add support for custom commands defined in
+Redis modules, you can regenerate `cmddef.h` using the script `gencommands.py`.
+Use the JSON files from Redis and any additional files on the same format as
+arguments to the script. For details, see the comments inside `gencommands.py`.
+
 ### Alternative build using Makefile directly
 
 When a simpler build setup is preferred a provided Makefile can be used directly

cmddef.h

@@ -98,9 +98,9 @@ COMMAND(DISCARD, "DISCARD", NULL, 1, NONE, 0)
 COMMAND(DUMP, "DUMP", NULL, 2, INDEX, 1)
 COMMAND(ECHO, "ECHO", NULL, 2, NONE, 0)
 COMMAND(EVAL, "EVAL", NULL, -3, KEYNUM, 2)
-COMMAND(EVAL_RO, "EVAL_RO", NULL, -3, KEYNUM, 2)
 COMMAND(EVALSHA, "EVALSHA", NULL, -3, KEYNUM, 2)
 COMMAND(EVALSHA_RO, "EVALSHA_RO", NULL, -3, KEYNUM, 2)
+COMMAND(EVAL_RO, "EVAL_RO", NULL, -3, KEYNUM, 2)
 COMMAND(EXEC, "EXEC", NULL, 1, NONE, 0)
 COMMAND(EXISTS, "EXISTS", NULL, -2, INDEX, 1)
 COMMAND(EXPIRE, "EXPIRE", NULL, -3, INDEX, 1)
@@ -125,9 +125,9 @@ COMMAND(GEODIST, "GEODIST", NULL, -4, INDEX, 1)
 COMMAND(GEOHASH, "GEOHASH", NULL, -2, INDEX, 1)
 COMMAND(GEOPOS, "GEOPOS", NULL, -2, INDEX, 1)
 COMMAND(GEORADIUS, "GEORADIUS", NULL, -6, INDEX, 1)
-COMMAND(GEORADIUS_RO, "GEORADIUS_RO", NULL, -6, INDEX, 1)
 COMMAND(GEORADIUSBYMEMBER, "GEORADIUSBYMEMBER", NULL, -5, INDEX, 1)
 COMMAND(GEORADIUSBYMEMBER_RO, "GEORADIUSBYMEMBER_RO", NULL, -5, INDEX, 1)
+COMMAND(GEORADIUS_RO, "GEORADIUS_RO", NULL, -6, INDEX, 1)
 COMMAND(GEOSEARCH, "GEOSEARCH", NULL, -7, INDEX, 1)
 COMMAND(GEOSEARCHSTORE, "GEOSEARCHSTORE", NULL, -8, INDEX, 1)
 COMMAND(GET, "GET", NULL, 2, INDEX, 1)
@@ -235,8 +235,8 @@ COMMAND(RENAMENX, "RENAMENX", NULL, 3, INDEX, 1)
 COMMAND(REPLCONF, "REPLCONF", NULL, -1, NONE, 0)
 COMMAND(REPLICAOF, "REPLICAOF", NULL, 3, NONE, 0)
 COMMAND(RESET, "RESET", NULL, 1, NONE, 0)
-COMMAND(RESTORE_ASKING, "RESTORE-ASKING", NULL, -4, INDEX, 1)
 COMMAND(RESTORE, "RESTORE", NULL, -4, INDEX, 1)
+COMMAND(RESTORE_ASKING, "RESTORE-ASKING", NULL, -4, INDEX, 1)
 COMMAND(ROLE, "ROLE", NULL, 1, NONE, 0)
 COMMAND(RPOP, "RPOP", NULL, -2, INDEX, 1)
 COMMAND(RPOPLPUSH, "RPOPLPUSH", NULL, 3, INDEX, 1)

command.c

@@ -32,7 +32,10 @@
 #include <errno.h>
 #include <hiredis/alloc.h>
 #ifndef _WIN32
+#include <alloca.h>
 #include <strings.h>
+#else
+#include <malloc.h>
 #endif
 #include <string.h>
 
@@ -75,20 +78,34 @@ static cmddef redis_commands[] = {
 #undef COMMAND
 };
 
+static inline void to_upper(char *dst, const char *src, uint32_t len) {
+    uint32_t i;
+    for (i = 0; i < len; i++) {
+        if (src[i] >= 'a' && src[i] <= 'z')
+            dst[i] = src[i] - ('a' - 'A');
+        else
+            dst[i] = src[i];
+    }
+}
+
 /* Looks up a command or subcommand in the command table. Arg0 and arg1 are used
  * to lookup the command. The function returns CMD_UNKNOWN on failure. On
  * success, the command type is returned and *firstkey and *arity are
  * populated. */
 cmddef *redis_lookup_cmd(const char *arg0, uint32_t arg0_len, const char *arg1,
                          uint32_t arg1_len) {
     int num_commands = sizeof(redis_commands) / sizeof(cmddef);
+    /* Compare command name in uppercase. */
+    char *cmd = alloca(arg0_len);
+    to_upper(cmd, arg0, arg0_len);
+    char *subcmd = NULL; /* Alloca later on demand. */
     /* Find the command using binary search. */
     int left = 0, right = num_commands - 1;
     while (left <= right) {
         int i = (left + right) / 2;
         cmddef *c = &redis_commands[i];
 
-        int cmp = strncasecmp(c->name, arg0, arg0_len);
+        int cmp = strncmp(c->name, cmd, arg0_len);
         if (cmp == 0 && strlen(c->name) > arg0_len)
             cmp = 1; /* "HGETALL" vs "HGET" */
 
@@ -97,11 +114,14 @@ cmddef *redis_lookup_cmd(const char *arg0, uint32_t arg0_len, const char *arg1,
             if (arg1 == NULL) {
                 /* Command has subcommands, but none given. */
                 return NULL;
-            } else {
-                cmp = strncasecmp(c->subname, arg1, arg1_len);
-                if (cmp == 0 && strlen(c->subname) > arg1_len)
-                    cmp = 1;
             }
+            if (subcmd == NULL) {
+                subcmd = alloca(arg1_len);
+                to_upper(subcmd, arg1, arg1_len);
+            }
+            cmp = strncmp(c->subname, subcmd, arg1_len);
+            if (cmp == 0 && strlen(c->subname) > arg1_len)
+                cmp = 1;
         }
 
         if (cmp < 0) {

gencommands.py

@@ -7,12 +7,29 @@
 # describing the commands. This is done manually when commands have been added
 # to Redis.
 #
-# Usage: ./gencommands.py path/to/redis > cmddef.h
+# Usage: ./gencommands.py path/to/redis/src/commands/*.json > cmddef.h
+#
+# Additional JSON files can be added to define custom commands. The JSON file
+# format is not fully documented but hopefully the format can be understood from
+# reading the existing JSON files. Alternatively, you can read the source code
+# of this script to see what it does.
+#
+# The key specifications part is documented here:
+# https://redis.io/docs/reference/key-specs/
+#
+# The discussion where this JSON format was added in Redis is here:
+# https://github.com/redis/redis/issues/9359
+#
+# For convenience, files on the output format like cmddef.h can also be used as
+# input files to this script. It can be used for adding more commands to the
+# existing set of commands, but please do not abuse it. Do not to write commands
+# information directly in this format.
 
 import glob
 import json
 import os
 import sys
+import re
 
 # Returns a tuple (method, index) where method is one of the following:
 #
@@ -25,9 +42,35 @@
 #                     number of keys is zero in which case there are no
 #                     keys (example EVAL)
 def firstkey(props):
-    if not "key_specs" in props or len(props["key_specs"]) == 0:
-        # No keys
+    if not "key_specs" in props:
+        # Key specs missing. Best-effort fallback to "arguments" for modules. To
+        # avoid returning UNKNOWN instead of NONE for official Redis commands
+        # without keys, we check for "arity" which is always defined in Redis
+        # but not in the Redis Stack modules which also lack key specs.
+        if "arguments" in props and "arity" not in props:
+            args = props["arguments"]
+            for i in range(1, len(args)):
+                arg = args[i - 1]
+                if not "type" in arg:
+                    return ("NONE", 0)
+                if arg["type"] == "key":
+                    return ("INDEX", i)
+                elif arg["type"] == "string":
+                    if "name" in arg and arg["name"] == "key":
+                        # add-hoc case for RediSearch
+                        return ("INDEX", i)
+                    if "optional" in arg and arg["optional"]:
+                        return ("UNKNOWN", 0)
+                    if "multiple" in arg and arg["multiple"]:
+                        return ("UNKNOWN", 0)
+                else:
+                    # Too complex for this fallback.
+                    return ("UNKNOWN", 0)
         return ("NONE", 0)
+
+    if len(props["key_specs"]) == 0:
+        return ("NONE", 0)
+
     # We detect the first key spec and only if the begin_search is by index.
     # Otherwise we return -1 for unknown (for example if the first key is
     # indicated by a keyword like KEYS or STREAMS).
@@ -50,32 +93,71 @@ def firstkey(props):
 def extract_command_info(name, props):
     (firstkeymethod, firstkeypos) = firstkey(props)
     container = props.get("container", "")
+    name = name.upper()
     subcommand = None
     if container != "":
         subcommand = name
-        name = container
-    return (name, subcommand, props["arity"], firstkeymethod, firstkeypos);
-
-# Checks that the filename matches the command. We need this because we rely on
-# the alphabetic order.
-def check_filename(filename, cmd):
-    if cmd[1] is None:
-        expect = "%s" % cmd[0]
+        name = container.upper()
     else:
-        expect = "%s-%s" % (cmd[0], cmd[1])
-    expect = expect.lower() + ".json"
-    assert os.path.basename(filename) == expect
+        # Ad-hoc handling of command and subcommand in the same string,
+        # sepatated by a space. This form is used in e.g. RediSearch's JSON file
+        # in commands like "FT.CONFIG GET".
+        tokens = name.split(maxsplit=1)
+        if len(tokens) > 1:
+            name, subcommand = tokens
+            if firstkeypos > 0:
+                firstkeypos += 1
+
+    arity = props["arity"] if "arity" in props else -1
+    return (name, subcommand, arity, firstkeymethod, firstkeypos);
+
+# Parses a file with lines like
+# COMMAND(identifier, cmd, subcmd, arity, firstkeymethod, firstkeypos)
+def collect_command_from_cmddef_h(f, commands):
+   for line in f:
+       m = re.match(r'^COMMAND\(\S+, *"(\S+)", NULL, *(-?\d+), *(\w+), *(\d+)\)', line)
+       if m:
+           commands[m.group(1)] = (m.group(1), None, int(m.group(2)), m.group(3), int(m.group(4)))
+           continue
+       m = re.match(r'^COMMAND\(\S+, *"(\S+)", *"(\S+)", *(-?\d+), *(\w+), *(\d)\)', line)
+       if m:
+           key = m.group(1) + "_" + m.group(2)
+           commands[key] = (m.group(1), m.group(2), int(m.group(3)), m.group(4), int(m.group(5)))
+           continue
+       if re.match(r'^(?:/\*.*\*/)?\s*$', line):
+           # Comment or blank line
+           continue
+       else:
+           print("Error processing line: %s" % (line))
+           exit(1)
 
 def collect_commands_from_files(filenames):
-    commands = []
+    # The keys in the dicts are "command" or "command_subcommand".
+    commands = dict()
+    commands_that_have_subcommands = set()
     for filename in filenames:
         with open(filename, "r") as f:
+            if filename.endswith(".h"):
+                collect_command_from_cmddef_h(f, commands)
+                continue
             try:
                 d = json.load(f)
                 for name, props in d.items():
                     cmd = extract_command_info(name, props)
-                    check_filename(filename, cmd)
-                    commands.append(cmd)
+                    (name, subcmd, _, _, _) = cmd
+
+                    # For commands with subcommands, we want only the
+                    # command-subcommand pairs, not the container command alone
+                    if subcmd is not None:
+                        commands_that_have_subcommands.add(name)
+                        if name in commands:
+                            del commands[name]
+                        name += "_" + subcmd
+                    elif name in commands_that_have_subcommands:
+                        continue
+
+                    commands[name] = cmd
+
             except json.decoder.JSONDecodeError as err:
                 print("Error processing %s: %s" % (filename, err))
                 exit(1)
@@ -85,34 +167,38 @@ def generate_c_code(commands):
     print("/* This file was generated using gencommands.py */")
     print("")
     print("/* clang-format off */")
-    commands_that_have_subcommands = set()
-    for (name, subcmd, arity, firstkeymethod, firstkeypos) in commands:
+    for key in sorted(commands):
+        (name, subcmd, arity, firstkeymethod, firstkeypos) = commands[key]
+        # Make valid C identifier (macro name)
+        key = re.sub(r'\W', '_', key)
         if subcmd is None:
-            if name in commands_that_have_subcommands:
-                continue # only include the command with its subcommands
             print("COMMAND(%s, \"%s\", NULL, %d, %s, %d)" %
-                  (name.replace("-", "_"), name, arity, firstkeymethod, firstkeypos))
+                  (key, name, arity, firstkeymethod, firstkeypos))
         else:
-            commands_that_have_subcommands.add(name)
-            print("COMMAND(%s_%s, \"%s\", \"%s\", %d, %s, %d)" %
-                  (name.replace("-", "_"), subcmd.replace("-", "_"),
-                   name, subcmd, arity, firstkeymethod, firstkeypos))
+            print("COMMAND(%s, \"%s\", \"%s\", %d, %s, %d)" %
+                  (key, name, subcmd, arity, firstkeymethod, firstkeypos))
 
 # MAIN
 
 if len(sys.argv) < 2 or sys.argv[1] == "--help":
-    print("Usage: %s REDIS-DIR > cmddef.h" % sys.argv[0])
+    print("Usage: %s path/to/redis/src/commands/*.json > cmddef.h" % sys.argv[0])
     exit(1)
 
-redisdir = sys.argv[1]
-jsondir = os.path.join(redisdir, "src", "commands")
-if not os.path.isdir(jsondir):
-    print("The path %s doesn't point to a Redis source directory." % redisdir)
-    exit(1)
+# Find all JSON files
+filenames = []
+for filename in sys.argv[1:]:
+    if os.path.isdir(filename):
+        # A redis repo root dir (accepted for backward compatibility)
+        jsondir = os.path.join(filename, "src", "commands")
+        if not os.path.isdir(jsondir):
+            print("The directory %s is not a Redis source directory." % filename)
+            exit(1)
+
+        filenames += glob.glob(os.path.join(jsondir, "*.json"))
+    else:
+        filenames.append(filename)
 
 # Collect all command info
-filenames = glob.glob(os.path.join(jsondir, "*.json"))
-filenames.sort()
 commands = collect_commands_from_files(filenames)
 
 # Print C code

tests/ut_parse_cmd.c

@@ -13,6 +13,11 @@
 /* Helper for the macro ASSERT_KEYS below. */
 void check_keys(char **keys, int numkeys, struct cmd *command, char *file,
                 int line) {
+    if (command->result != CMD_PARSE_OK) {
+        fprintf(stderr, "%s:%d: Command parsing failed: %s\n", file, line,
+                command->errstr);
+        assert(0);
+    }
     int actual_numkeys = (int)hiarray_n(command->keys);
     if (actual_numkeys != numkeys) {
         fprintf(stderr, "%s:%d: Expected %d keys but got %d\n", file, line,
@@ -155,6 +160,42 @@ void test_redis_parse_cmd_xread_ok(void) {
     command_destroy(c);
 }
 
+void test_redis_parse_cmd_restore_ok(void) {
+    /* The ordering of RESTORE and RESTORE-ASKING in the lookup-table was wrong
+     * in a previous version, leading to the command not being found. */
+    struct cmd *c = command_get();
+    int len = redisFormatCommand(&c->cmd, "restore k 0 xxx");
+    ASSERT_MSG(len >= 0, "Format command error");
+    c->clen = len;
+    redis_parse_cmd(c);
+    ASSERT_KEYS(c, "k");
+    command_destroy(c);
+}
+
+void test_redis_parse_cmd_restore_asking_ok(void) {
+    /* The ordering of RESTORE and RESTORE-ASKING in the lookup-table was wrong
+     * in a previous version, leading to the command not being found. */
+    struct cmd *c = command_get();
+    int len = redisFormatCommand(&c->cmd, "restore-asking k 0 xxx");
+    ASSERT_MSG(len >= 0, "Format command error");
+    c->clen = len;
+    redis_parse_cmd(c);
+    ASSERT_KEYS(c, "k");
+    command_destroy(c);
+}
+
+void test_redis_parse_cmd_georadius_ro_ok(void) {
+    /* The position of GEORADIUS_RO was wrong in a previous version of the
+     * lookup-table, leading to the command not being found. */
+    struct cmd *c = command_get();
+    int len = redisFormatCommand(&c->cmd, "georadius_ro k 0 0 0 km");
+    ASSERT_MSG(len >= 0, "Format command error");
+    c->clen = len;
+    redis_parse_cmd(c);
+    ASSERT_KEYS(c, "k");
+    command_destroy(c);
+}
+
 int main(void) {
     test_redis_parse_error_nonresp();
     test_redis_parse_cmd_get();
@@ -166,5 +207,8 @@ int main(void) {
     test_redis_parse_cmd_xgroup_destroy_ok();
     test_redis_parse_cmd_xreadgroup_ok();
     test_redis_parse_cmd_xread_ok();
+    test_redis_parse_cmd_restore_ok();
+    test_redis_parse_cmd_restore_asking_ok();
+    test_redis_parse_cmd_georadius_ro_ok();
     return 0;
 }