From 046169bd9b7249eaffe97f6b3f9fa0a9134d43b7 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Fri, 24 Apr 2026 04:14:23 +0900 Subject: [PATCH 1/4] feat(redis): implement COMMAND for client capability probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the Redis `COMMAND` handler (COUNT / LIST / INFO / DOCS / GETKEYS subcommands) so modern clients (go-redis, redis-py, ioredis) stop polluting the unsupported-command metric at connect time. Metadata lives in a single static table in adapter/redis_command_info.go so adding a new command is a one-line row addition. - Route `COMMAND` in RedisServer.route and register `-1` in argsLen. - Extend monitoring.redisCommandSet so the metric shows the real name. - Return 6-element INFO arrays (name, arity, flags, first/last/step) — enough for every client that consumes COMMAND output. - GETKEYS walks the per-command first_key/last_key/step positionally. - GETKEYSANDFLAGS and LIST FILTERBY variants are explicitly rejected. - Routed-but-missing-from-table commands emit a one-shot warning and a zero-metadata fallback row so the omission is discoverable in prod; TestCommand_RouteMatchesTable is the hard CI gate. --- adapter/redis.go | 3 + adapter/redis_command_info.go | 249 ++++++++++++++++++++++ adapter/redis_command_test.go | 354 +++++++++++++++++++++++++++++++ adapter/redis_compat_commands.go | 166 +++++++++++++++ monitoring/redis.go | 1 + 5 files changed, 773 insertions(+) create mode 100644 adapter/redis_command_info.go create mode 100644 adapter/redis_command_test.go diff --git a/adapter/redis.go b/adapter/redis.go index f21aa8c3..33b00f7a 100644 --- a/adapter/redis.go +++ b/adapter/redis.go @@ -30,6 +30,7 @@ import ( const ( cmdBZPopMin = "BZPOPMIN" cmdClient = "CLIENT" + cmdCommand = "COMMAND" cmdDBSize = "DBSIZE" cmdDel = "DEL" cmdDiscard = "DISCARD" @@ -167,6 +168,7 @@ var txnApplyHandlers = map[string]txnCommandHandler{ var argsLen = map[string]int{ cmdBZPopMin: -3, cmdClient: -2, + cmdCommand: -1, cmdDBSize: 1, cmdDel: -2, cmdDiscard: 1, @@ -417,6 +419,7 @@ func NewRedisServer(listen net.Listener, redisAddr string, store store.MVCCStore r.route = map[string]func(conn redcon.Conn, cmd redcon.Command){ cmdBZPopMin: r.bzpopmin, cmdClient: r.client, + cmdCommand: r.command, cmdDBSize: r.dbsize, cmdDel: r.del, cmdDiscard: r.discard, diff --git a/adapter/redis_command_info.go b/adapter/redis_command_info.go new file mode 100644 index 00000000..79739885 --- /dev/null +++ b/adapter/redis_command_info.go @@ -0,0 +1,249 @@ +package adapter + +// redis_command_info.go holds the static metadata table consumed by the +// Redis `COMMAND` handler. It is intentionally a single, grep-able file so +// that adding a new command is a one-liner: +// +// 1) Register the new handler in RedisServer.route (redis.go). +// 2) Add an argsLen entry (redis.go). +// 3) Add a row below. Forgetting step 3 is NOT fatal — the COMMAND +// handler falls back to a zero-metadata entry and emits one warning +// log per command name so the omission is discoverable — but you +// should do step 3 anyway. +// +// The table is the source of truth for `COMMAND`, `COMMAND INFO`, +// `COMMAND COUNT`, `COMMAND LIST`, `COMMAND DOCS`, and `COMMAND GETKEYS`. +// +// Shape notes (Redis reference): +// - arity: exact positive arity, or negative meaning "at least |arity|" +// - flags: one of "readonly" | "write" | "admin". We do NOT currently +// emit "denyoom" / "pubsub" / "loading" / "stale" / "fast" etc. — +// real Redis clients only consume this field for coarse routing. +// - first_key / last_key / step describe the key positions inside the +// argv. first_key=0 means the command operates on zero keys (pure +// connection / server commands). last_key=-1 means "all remaining +// args are keys" (MSET-shaped). step=1 means keys are consecutive; +// step=2 is used by MSET-like key/value pairs. + +import ( + "log" + "sort" + "strings" + "sync" +) + +// redisCommandFlag values are string constants so the raw strings are not +// duplicated across the table. +const ( + redisCmdFlagReadonly = "readonly" + redisCmdFlagWrite = "write" + redisCmdFlagAdmin = "admin" +) + +// redisCommandMeta is a single row in the COMMAND table. +type redisCommandMeta struct { + // Name is the lowercase command name as reported by Redis. Keyed in the + // table by uppercase for dispatch-time lookup; the lowercase form is + // what goes onto the wire in COMMAND INFO. + Name string + Arity int + Flags []string + FirstKey int + LastKey int + Step int +} + +// redisCommandTable maps UPPERCASE command name -> metadata. Every entry +// routed by RedisServer.route should appear here. Entries are listed in +// alphabetical order to keep diffs small when adding new commands. +// +//nolint:mnd // magic numbers here are literal Redis metadata (arity, key positions) +var redisCommandTable = map[string]redisCommandMeta{ + "BZPOPMIN": {Name: "bzpopmin", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -2, Step: 1}, + "CLIENT": {Name: "client", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "COMMAND": {Name: "command", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "DBSIZE": {Name: "dbsize", Arity: 1, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + "DEL": {Name: "del", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -1, Step: 1}, + "DISCARD": {Name: "discard", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "EVAL": {Name: "eval", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + "EVALSHA": {Name: "evalsha", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + "EXEC": {Name: "exec", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "EXISTS": {Name: "exists", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, + "EXPIRE": {Name: "expire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "FLUSHALL": {Name: "flushall", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + "FLUSHDB": {Name: "flushdb", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + // FLUSHLEGACY is an elastickv-internal alias; mirror FLUSHDB metadata. + "FLUSHLEGACY": {Name: "flushlegacy", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + "GET": {Name: "get", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "GETDEL": {Name: "getdel", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "HDEL": {Name: "hdel", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "HEXISTS": {Name: "hexists", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "HGET": {Name: "hget", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "HGETALL": {Name: "hgetall", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "HINCRBY": {Name: "hincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "HLEN": {Name: "hlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "HMGET": {Name: "hmget", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "HMSET": {Name: "hmset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "HSET": {Name: "hset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "INCR": {Name: "incr", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "INFO": {Name: "info", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "KEYS": {Name: "keys", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + "LINDEX": {Name: "lindex", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "LLEN": {Name: "llen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "LPOP": {Name: "lpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "LPOS": {Name: "lpos", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "LPUSH": {Name: "lpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "LRANGE": {Name: "lrange", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "LREM": {Name: "lrem", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "LSET": {Name: "lset", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "LTRIM": {Name: "ltrim", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "MULTI": {Name: "multi", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "PEXPIRE": {Name: "pexpire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "PFADD": {Name: "pfadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "PFCOUNT": {Name: "pfcount", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, + "PING": {Name: "ping", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "PTTL": {Name: "pttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "PUBLISH": {Name: "publish", Arity: 3, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "PUBSUB": {Name: "pubsub", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "QUIT": {Name: "quit", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "RENAME": {Name: "rename", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, + "RPOP": {Name: "rpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "RPOPLPUSH": {Name: "rpoplpush", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, + "RPUSH": {Name: "rpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SADD": {Name: "sadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SCAN": {Name: "scan", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + "SCARD": {Name: "scard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "SELECT": {Name: "select", Arity: 2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "SET": {Name: "set", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SETEX": {Name: "setex", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SETNX": {Name: "setnx", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SISMEMBER": {Name: "sismember", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "SMEMBERS": {Name: "smembers", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "SREM": {Name: "srem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "SUBSCRIBE": {Name: "subscribe", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + "TTL": {Name: "ttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "TYPE": {Name: "type", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "XADD": {Name: "xadd", Arity: -5, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "XLEN": {Name: "xlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "XRANGE": {Name: "xrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "XREAD": {Name: "xread", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + "XREVRANGE": {Name: "xrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "XTRIM": {Name: "xtrim", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZADD": {Name: "zadd", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZCARD": {Name: "zcard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZCOUNT": {Name: "zcount", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZINCRBY": {Name: "zincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZPOPMIN": {Name: "zpopmin", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZRANGE": {Name: "zrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZRANGEBYSCORE": {Name: "zrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZREM": {Name: "zrem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZREMRANGEBYRANK": {Name: "zremrangebyrank", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZREMRANGEBYSCORE": {Name: "zremrangebyscore", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZREVRANGE": {Name: "zrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZREVRANGEBYSCORE": {Name: "zrevrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + "ZSCORE": {Name: "zscore", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, +} + +// redisCommandFallbackWarnedOnce deduplicates the "missing metadata" log so +// that a hostile or buggy client probing the same unknown-but-routed name +// cannot generate unbounded log spam. The fallback is a safety net for +// commands that get added to the route but where the table row is +// forgotten; the unit test `TestCommand_RouteMatchesTable` is the hard +// gate, but in production we prefer a degraded reply + one log line over +// a silently-missing command. +var ( + redisCommandFallbackWarnedOnceMu sync.Mutex + redisCommandFallbackWarnedOnce = map[string]struct{}{} +) + +// routedRedisCommandMetas returns the metadata rows for every command +// currently routed (keyed via argsLen, which is populated 1:1 with the +// route map — see redis.go). Rows are returned in sorted UPPER-case order +// so wire output is deterministic. Names present in redisCommandTable +// produce their real row; names absent from the table but routed produce +// a zero-metadata row and a one-shot log warning. This is the source of +// truth for `COMMAND` (no args) and `COMMAND LIST`; `COMMAND INFO ` +// goes through redisCommandTable directly so unknowns produce the nil +// reply required by Redis semantics. +func routedRedisCommandMetas() []redisCommandMeta { + names := make([]string, 0, len(argsLen)) + for name := range argsLen { + names = append(names, strings.ToUpper(name)) + } + sort.Strings(names) + metas := make([]redisCommandMeta, 0, len(names)) + for _, name := range names { + if meta, ok := redisCommandTable[name]; ok { + metas = append(metas, meta) + continue + } + warnMissingRedisCommandMeta(name) + metas = append(metas, redisCommandMeta{ + Name: strings.ToLower(name), + Arity: -1, + Flags: nil, + FirstKey: 0, + LastKey: 0, + Step: 0, + }) + } + return metas +} + +// warnMissingRedisCommandMeta emits a one-shot warning when a routed +// command has no entry in redisCommandTable. Subsequent calls for the +// same name are silent so a hot dispatch path does not produce log spam. +func warnMissingRedisCommandMeta(upper string) { + redisCommandFallbackWarnedOnceMu.Lock() + _, warned := redisCommandFallbackWarnedOnce[upper] + if !warned { + redisCommandFallbackWarnedOnce[upper] = struct{}{} + } + redisCommandFallbackWarnedOnceMu.Unlock() + if !warned { + log.Printf("redis-command: routed command %q has no entry in redisCommandTable; emitting zero-metadata fallback. Add a row to adapter/redis_command_info.go.", upper) + } +} + +// redisCommandGetKeys extracts the key positions from a full command-form +// argv (argv[0] is the command name, argv[1:] are its arguments). +// Returns an error when the command is unknown; returns an empty slice when +// the command is routed but has no keys. +// +// Semantics mirror Redis's own COMMAND GETKEYS: +// - first_key=0: no keys (empty slice). +// - last_key=-1: "all args after first_key are keys". step controls spacing +// (step=1 → every arg; step=2 → every other arg, as in MSET). +// - otherwise: args in [first_key .. last_key] at `step` stride. +// +// This is *positional*. It does not understand option prefixes (e.g. the +// `EX`/`PX` flags of SET); clients that need option-aware parsing would +// look at Redis 7's key-specs shape, which we explicitly do not emit. For +// the commands elastickv supports the naive positional scheme is correct. +func redisCommandGetKeys(meta redisCommandMeta, argv [][]byte) [][]byte { + if meta.FirstKey <= 0 { + return nil + } + if meta.Step <= 0 { + return nil + } + if meta.FirstKey >= len(argv) { + return nil + } + last := meta.LastKey + if last < 0 { + // "to end": last arg index is len(argv)-1. + last = len(argv) - 1 + } + if last >= len(argv) { + last = len(argv) - 1 + } + if last < meta.FirstKey { + return nil + } + keys := make([][]byte, 0, (last-meta.FirstKey)/meta.Step+1) + for i := meta.FirstKey; i <= last; i += meta.Step { + keys = append(keys, argv[i]) + } + return keys +} diff --git a/adapter/redis_command_test.go b/adapter/redis_command_test.go new file mode 100644 index 00000000..37114631 --- /dev/null +++ b/adapter/redis_command_test.go @@ -0,0 +1,354 @@ +package adapter + +import ( + "net" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tidwall/redcon" +) + +// commandRecorder captures the sequence of writes performed by a handler +// so array-structured replies (like COMMAND INFO) can be asserted +// element-by-element rather than via the flat `recordingConn.bulk` field, +// which only remembers the last write. +// +// Each entry in `writes` is one of: +// +// {op: "array", n: } +// {op: "bulk", s: } +// {op: "int", i: } +// {op: "string", s: } +// {op: "null"} +// {op: "error", s: } +// +// Tests walk `writes` with a small cursor-based helper rather than trying +// to reconstruct a tree — reconstruction is more code than the tests it +// enables, and a flat trace is trivially greppable. +type commandRecorderEntry struct { + op string + s string + i int64 + n int +} + +type commandRecorder struct { + writes []commandRecorderEntry + ctx any +} + +func (c *commandRecorder) RemoteAddr() string { return "" } +func (c *commandRecorder) Close() error { return nil } +func (c *commandRecorder) WriteError(msg string) { + c.writes = append(c.writes, commandRecorderEntry{op: "error", s: msg}) +} +func (c *commandRecorder) WriteString(str string) { + c.writes = append(c.writes, commandRecorderEntry{op: "string", s: str}) +} +func (c *commandRecorder) WriteBulk(b []byte) { + c.writes = append(c.writes, commandRecorderEntry{op: "bulk", s: string(b)}) +} +func (c *commandRecorder) WriteBulkString(s string) { + c.writes = append(c.writes, commandRecorderEntry{op: "bulk", s: s}) +} +func (c *commandRecorder) WriteInt(num int) { + c.writes = append(c.writes, commandRecorderEntry{op: "int", i: int64(num)}) +} +func (c *commandRecorder) WriteInt64(num int64) { + c.writes = append(c.writes, commandRecorderEntry{op: "int", i: num}) +} +func (c *commandRecorder) WriteUint64(num uint64) { + c.writes = append(c.writes, commandRecorderEntry{op: "int", i: int64(num)}) //nolint:gosec +} +func (c *commandRecorder) WriteArray(count int) { + c.writes = append(c.writes, commandRecorderEntry{op: "array", n: count}) +} +func (c *commandRecorder) WriteNull() { + c.writes = append(c.writes, commandRecorderEntry{op: "null"}) +} +func (c *commandRecorder) WriteRaw([]byte) {} +func (c *commandRecorder) WriteAny(any) {} +func (c *commandRecorder) Context() any { return c.ctx } +func (c *commandRecorder) SetContext(v any) { c.ctx = v } +func (c *commandRecorder) SetReadBuffer(int) {} +func (c *commandRecorder) Detach() redcon.DetachedConn { return nil } +func (c *commandRecorder) ReadPipeline() []redcon.Command { return nil } +func (c *commandRecorder) PeekPipeline() []redcon.Command { return nil } +func (c *commandRecorder) NetConn() net.Conn { return nil } + +// helper: skip `count` entries starting from writes[start], treating the +// current entry as an array header. Returns (start+1, size). Unused for +// now — kept simple by manually walking indices in individual tests. + +// newCommandTestServer returns a RedisServer sufficient for COMMAND tests. +// COMMAND has no dependency on coordinator / store, so all fields stay +// zero. We use a literal construction rather than NewRedisServer because +// the latter requires a real listener + coordinator. +func newCommandTestServer() *RedisServer { + return &RedisServer{} +} + +func runCommand(t *testing.T, args ...string) *commandRecorder { + t.Helper() + srv := newCommandTestServer() + conn := &commandRecorder{} + cmdArgs := make([][]byte, 0, len(args)) + for _, a := range args { + cmdArgs = append(cmdArgs, []byte(a)) + } + srv.command(conn, redcon.Command{Args: cmdArgs}) + return conn +} + +func TestCommandCount_MatchesTableSize(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "COUNT") + require.Len(t, conn.writes, 1) + require.Equal(t, "int", conn.writes[0].op) + // COUNT reports the size of the routed set (argsLen); the table must + // have the same size by the route-matches-table invariant. + require.Equal(t, int64(len(argsLen)), conn.writes[0].i) + require.Equal(t, len(argsLen), len(redisCommandTable)) +} + +func TestCommandNoArgs_ReturnsAllEntries(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND") + require.NotEmpty(t, conn.writes) + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, len(argsLen), conn.writes[0].n) +} + +func TestCommandInfo_Get(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "INFO", "GET") + // outer array of 1 + 6-element inner. + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 1, conn.writes[0].n) + require.Equal(t, "array", conn.writes[1].op) + require.Equal(t, 6, conn.writes[1].n) + require.Equal(t, "bulk", conn.writes[2].op) + require.Equal(t, "get", conn.writes[2].s) + require.Equal(t, "int", conn.writes[3].op) + require.Equal(t, int64(2), conn.writes[3].i) + // flags array + require.Equal(t, "array", conn.writes[4].op) + flagN := conn.writes[4].n + flags := make([]string, 0, flagN) + for i := 0; i < flagN; i++ { + flags = append(flags, conn.writes[5+i].s) + } + require.Contains(t, flags, "readonly") + // first / last / step + require.Equal(t, int64(1), conn.writes[5+flagN].i) + require.Equal(t, int64(1), conn.writes[6+flagN].i) + require.Equal(t, int64(1), conn.writes[7+flagN].i) +} + +func TestCommandInfo_Set(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "INFO", "SET") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 1, conn.writes[0].n) + require.Equal(t, "array", conn.writes[1].op) + require.Equal(t, 6, conn.writes[1].n) + require.Equal(t, "set", conn.writes[2].s) + require.Equal(t, int64(-3), conn.writes[3].i) + require.Equal(t, "array", conn.writes[4].op) + flagN := conn.writes[4].n + flags := make([]string, 0, flagN) + for i := 0; i < flagN; i++ { + flags = append(flags, conn.writes[5+i].s) + } + require.Contains(t, flags, "write") + require.Equal(t, int64(1), conn.writes[5+flagN].i) + require.Equal(t, int64(1), conn.writes[6+flagN].i) + require.Equal(t, int64(1), conn.writes[7+flagN].i) +} + +func TestCommandInfo_UnknownReturnsNil(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "INFO", "nosuchcommand") + // outer array of 1, then a nil entry. + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 1, conn.writes[0].n) + require.Equal(t, "null", conn.writes[1].op) +} + +func TestCommandInfo_MixedKnownUnknown(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "INFO", "GET", "NOSUCH", "SET") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 3, conn.writes[0].n) + // first entry: 6-element GET array header. + require.Equal(t, "array", conn.writes[1].op) + require.Equal(t, 6, conn.writes[1].n) + // Walk past GET: outer-array-header(1) + inner-array-header(1) + name(1) + // + arity(1) + flags-header(1) + flagN flag strings + 3 ints. + flagN := conn.writes[4].n + cursor := 5 + flagN + 3 + // cursor now at NOSUCH → must be nil. + require.Equal(t, "null", conn.writes[cursor].op) + cursor++ + // SET entry: 6-element header then its fields. + require.Equal(t, "array", conn.writes[cursor].op) + require.Equal(t, 6, conn.writes[cursor].n) + require.Equal(t, "set", conn.writes[cursor+1].s) +} + +func TestCommandGetKeys_SingleKey(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "GETKEYS", "SET", "foo", "bar") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 1, conn.writes[0].n) + require.Equal(t, "bulk", conn.writes[1].op) + require.Equal(t, "foo", conn.writes[1].s) +} + +func TestCommandGetKeys_MSETLike_DEL(t *testing.T) { + t.Parallel() + // DEL has first_key=1, last_key=-1, step=1 (every arg after DEL). + conn := runCommand(t, "COMMAND", "GETKEYS", "DEL", "k1", "k2", "k3") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 3, conn.writes[0].n) + require.Equal(t, "k1", conn.writes[1].s) + require.Equal(t, "k2", conn.writes[2].s) + require.Equal(t, "k3", conn.writes[3].s) +} + +// MSET is not in the table (elastickv doesn't route it), so we verify the +// sibling shape via MSET-like args using another multi-key command we DO +// support — HMSET has first_key=1, last_key=1, step=1 (single key). The +// spec-provided MSET example in the ticket is checked against the raw +// redisCommandGetKeys helper below rather than the handler. +func TestRedisCommandGetKeysHelper_MSET(t *testing.T) { + t.Parallel() + // Synthetic MSET-shaped metadata: first=1, last=-1, step=2. + meta := redisCommandMeta{FirstKey: 1, LastKey: -1, Step: 2} + argv := [][]byte{[]byte("MSET"), []byte("k1"), []byte("v1"), []byte("k2"), []byte("v2")} + keys := redisCommandGetKeys(meta, argv) + got := make([]string, 0, len(keys)) + for _, k := range keys { + got = append(got, string(k)) + } + require.Equal(t, []string{"k1", "k2"}, got) +} + +func TestCommandGetKeys_UnknownCommand(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "GETKEYS", "NOSUCH") + require.Len(t, conn.writes, 1) + require.Equal(t, "error", conn.writes[0].op) + require.Contains(t, conn.writes[0].s, "Invalid command specified") +} + +func TestCommandList_ReturnsAllNames(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "LIST") + require.Equal(t, "array", conn.writes[0].op) + // The outer length equals the number of routed commands (argsLen is + // the 1:1 route-keyed set; redisCommandTable mirrors it because of + // TestCommand_RouteMatchesTable's invariant). + require.Equal(t, len(argsLen), conn.writes[0].n) + names := make([]string, 0, conn.writes[0].n) + for i := 1; i <= conn.writes[0].n; i++ { + require.Equal(t, "bulk", conn.writes[i].op) + names = append(names, conn.writes[i].s) + } + require.Contains(t, names, "get") + require.Contains(t, names, "set") + require.Contains(t, names, "command") + // Names must be sorted by UPPERCASE key. Reconstruct the expected + // ordering from the table rather than a separate sort helper so this + // remains a single source of truth. + upper := make([]string, 0, len(names)) + for _, n := range names { + upper = append(upper, strings.ToUpper(n)) + } + require.True(t, slices.IsSorted(upper), "names must be emitted in sorted UPPER order, got %v", upper) +} + +func TestCommandList_RejectsFilterBy(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "LIST", "FILTERBY", "MODULE", "foo") + require.Len(t, conn.writes, 1) + require.Equal(t, "error", conn.writes[0].op) + require.Contains(t, conn.writes[0].s, "unsupported") +} + +func TestCommandDocs_Get(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "DOCS", "GET") + // outer array of 1, then 4-element map-shaped array. + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 1, conn.writes[0].n) + require.Equal(t, "array", conn.writes[1].op) + require.Equal(t, 4, conn.writes[1].n) + require.Equal(t, "summary", conn.writes[2].s) + require.Equal(t, "", conn.writes[3].s) + require.Equal(t, "arguments", conn.writes[4].s) + require.Equal(t, "array", conn.writes[5].op) + require.Equal(t, 0, conn.writes[5].n) +} + +func TestCommand_UnknownSubcommand(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "BADSUB") + require.Len(t, conn.writes, 1) + require.Equal(t, "error", conn.writes[0].op) + require.Contains(t, conn.writes[0].s, "Unknown COMMAND subcommand") +} + +func TestCommand_GetKeysAndFlagsRejected(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "GETKEYSANDFLAGS", "GET", "k") + require.Len(t, conn.writes, 1) + require.Equal(t, "error", conn.writes[0].op) + require.Contains(t, conn.writes[0].s, "unsupported") +} + +func TestCommand_RoutedViaServer(t *testing.T) { + t.Parallel() + // Verify the route wiring: a COMMAND request reaching RedisServer.route + // dispatches to r.command. We cannot exercise redcon.Serve in a unit + // test without a network listener, so we instead reach into the route + // map as the live server would. + srv := &RedisServer{} + srv.route = map[string]func(conn redcon.Conn, cmd redcon.Command){ + cmdCommand: srv.command, + } + handler, ok := srv.route[cmdCommand] + require.True(t, ok, "COMMAND must be routed") + conn := &commandRecorder{} + handler(conn, redcon.Command{Args: [][]byte{[]byte("COMMAND"), []byte("COUNT")}}) + require.Len(t, conn.writes, 1) + require.Equal(t, "int", conn.writes[0].op) +} + +// TestCommand_ArgsLenAllowsBare ensures the validateCmd arity check +// accepts bare `COMMAND` (no subcommand), matching real Redis where +// `COMMAND` with no args is the canonical "dump all" call. +func TestCommand_ArgsLenAllowsBare(t *testing.T) { + t.Parallel() + require.Equal(t, -1, argsLen[cmdCommand]) +} + +// TestCommand_RouteMatchesTable asserts that every command currently +// routed has a metadata row in redisCommandTable. Catches the common +// mistake of adding a new handler without extending the table — the +// runtime path would fall back to zero-metadata + a log warning, but we +// want hard failure in CI. +func TestCommand_RouteMatchesTable(t *testing.T) { + t.Parallel() + srv := &RedisServer{} + // Rebuild the route map via NewRedisServer would need a listener; we + // instead iterate argsLen (which every routed command has an entry + // in — see redis.go) as the closest proxy. + for name := range argsLen { + _, ok := redisCommandTable[name] + require.Truef(t, ok, "command %q routed but missing from redisCommandTable", name) + } + _ = srv +} diff --git a/adapter/redis_compat_commands.go b/adapter/redis_compat_commands.go index 59a94712..bf98e6ca 100644 --- a/adapter/redis_compat_commands.go +++ b/adapter/redis_compat_commands.go @@ -220,6 +220,172 @@ func (r *RedisServer) client(conn redcon.Conn, cmd redcon.Command) { } } +// command implements the Redis `COMMAND` family used by clients for +// capability probing at connect time (go-redis, redis-py, ioredis, …). +// Subcommand matrix: +// +// COMMAND -> array of per-command info +// COMMAND COUNT -> integer +// COMMAND LIST -> array of names (FILTERBY rejected) +// COMMAND INFO [name ...] -> array of per-command info (nil per unknown) +// COMMAND DOCS [name ...] -> minimal map-shaped doc entries +// COMMAND GETKEYS cmd args -> array of extracted keys +// COMMAND GETKEYSANDFLAGS -> ERR unsupported +func (r *RedisServer) command(conn redcon.Conn, cmd redcon.Command) { + if len(cmd.Args) == 1 { + r.writeCommandInfoAll(conn) + return + } + sub := strings.ToUpper(string(cmd.Args[1])) + switch sub { + case "COUNT": + // COUNT must match the cardinality of COMMAND / COMMAND LIST — + // which iterate argsLen (= routed set). The table has the same + // size by invariant, but driving COUNT off argsLen keeps the + // three subcommands wire-consistent even during the brief + // window when a new route has been added but the table row is + // still pending. + conn.WriteInt(len(argsLen)) + case "LIST": + // `COMMAND LIST` takes no args (bare list) or `FILTERBY …` which we + // reject below. Anything past the subcommand slot is a filter. + const commandListArgFixed = 2 + if len(cmd.Args) > commandListArgFixed { + // We explicitly do not support FILTERBY MODULE|ACLCAT|PATTERN + // — elastickv has no modules and no ACL categories. Rejecting + // here is consistent with how real Redis would behave when a + // filter resolves to an empty universe; clients that see this + // fall back to COMMAND (no args), which we support. + conn.WriteError("ERR unsupported COMMAND LIST filter") + return + } + r.writeCommandList(conn) + case "INFO": + r.writeCommandInfo(conn, cmd.Args[2:]) + case "DOCS": + r.writeCommandDocs(conn, cmd.Args[2:]) + case "GETKEYS": + r.writeCommandGetKeys(conn, cmd.Args[2:]) + case "GETKEYSANDFLAGS": + conn.WriteError("ERR unsupported COMMAND subcommand 'GETKEYSANDFLAGS'") + default: + conn.WriteError("ERR Unknown COMMAND subcommand '" + sub + "'") + } +} + +// writeCommandInfoEntry emits the 6-element per-command info array for a +// single command. Redis 7 extends this to 10 elements; we deliberately +// stop at 6 because every client we care about parses the first 6 fields +// and ignores trailing elements. +func writeCommandInfoEntry(conn redcon.Conn, meta redisCommandMeta) { + const infoArity = 6 + conn.WriteArray(infoArity) + conn.WriteBulkString(meta.Name) + conn.WriteInt(meta.Arity) + conn.WriteArray(len(meta.Flags)) + for _, f := range meta.Flags { + conn.WriteBulkString(f) + } + conn.WriteInt(meta.FirstKey) + conn.WriteInt(meta.LastKey) + conn.WriteInt(meta.Step) +} + +func (r *RedisServer) writeCommandInfoAll(conn redcon.Conn) { + metas := routedRedisCommandMetas() + conn.WriteArray(len(metas)) + for _, meta := range metas { + writeCommandInfoEntry(conn, meta) + } +} + +func (r *RedisServer) writeCommandList(conn redcon.Conn) { + metas := routedRedisCommandMetas() + conn.WriteArray(len(metas)) + for _, meta := range metas { + conn.WriteBulkString(meta.Name) + } +} + +func (r *RedisServer) writeCommandInfo(conn redcon.Conn, requested [][]byte) { + // `COMMAND INFO` with no names is equivalent to `COMMAND` (no args): + // return info for every known command. This is what real Redis does + // and what go-redis relies on when it issues bare `COMMAND INFO`. + if len(requested) == 0 { + r.writeCommandInfoAll(conn) + return + } + conn.WriteArray(len(requested)) + for _, raw := range requested { + meta, ok := redisCommandTable[strings.ToUpper(string(raw))] + if !ok { + conn.WriteNull() + continue + } + writeCommandInfoEntry(conn, meta) + } +} + +// writeCommandDocs emits the minimum shape real clients check for: +// a 4-element map-shaped array per requested command with "summary" and +// "arguments" keys. We do not maintain per-command docs, so the summary +// is empty and the arguments array is empty. Unknown commands produce a +// nil entry, matching Redis semantics. +func (r *RedisServer) writeCommandDocs(conn redcon.Conn, requested [][]byte) { + const docEntryLen = 4 + conn.WriteArray(len(requested)) + for _, raw := range requested { + if _, ok := redisCommandTable[strings.ToUpper(string(raw))]; !ok { + conn.WriteNull() + continue + } + conn.WriteArray(docEntryLen) + conn.WriteBulkString("summary") + conn.WriteBulkString("") + conn.WriteBulkString("arguments") + conn.WriteArray(0) + } +} + +// writeCommandGetKeys dispatches COMMAND GETKEYS for a given subcommand +// plus its arguments. Real Redis requires at least one arg after GETKEYS +// (the command name itself); we enforce that here rather than lean on +// argsLen which only validates the outer COMMAND call. +func (r *RedisServer) writeCommandGetKeys(conn redcon.Conn, argv [][]byte) { + if len(argv) == 0 { + conn.WriteError("ERR wrong number of arguments for 'command|getkeys' command") + return + } + meta, ok := redisCommandTable[strings.ToUpper(string(argv[0]))] + if !ok { + conn.WriteError("ERR Invalid command specified") + return + } + // validate arity of the nested command so we match Redis behaviour of + // refusing to compute keys for obviously malformed commands (a common + // source of confusion in client test suites). + switch { + case meta.Arity > 0 && len(argv) != meta.Arity: + conn.WriteError("ERR Invalid arguments specified for populating the array of keys") + return + case meta.Arity < 0 && len(argv) < -meta.Arity: + conn.WriteError("ERR Invalid arguments specified for populating the array of keys") + return + } + keys := redisCommandGetKeys(meta, argv) + if len(keys) == 0 { + // `The command has no key arguments` — real Redis returns an error + // in this case rather than an empty array, and go-redis's test + // suite expects the error form. + conn.WriteError("ERR The command has no key arguments") + return + } + conn.WriteArray(len(keys)) + for _, k := range keys { + conn.WriteBulk(k) + } +} + func (r *RedisServer) selectDB(conn redcon.Conn, cmd redcon.Command) { if _, err := strconv.Atoi(string(cmd.Args[1])); err != nil { conn.WriteError("ERR invalid DB index") diff --git a/monitoring/redis.go b/monitoring/redis.go index 8454da3e..287e5322 100644 --- a/monitoring/redis.go +++ b/monitoring/redis.go @@ -50,6 +50,7 @@ const ( var redisCommandSet = map[string]struct{}{ "BZPOPMIN": {}, "CLIENT": {}, + "COMMAND": {}, "DBSIZE": {}, "DEL": {}, "DISCARD": {}, From 9a90b7edd43cdbb82125f7f9b8490dbf9cf9f9d4 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Sat, 25 Apr 2026 06:14:12 +0900 Subject: [PATCH 2/4] fix(redis): interpret negative COMMAND GETKEYS last_key as end-offset Codex P1: redisCommandGetKeys collapsed every negative LastKey to "to end" (len(argv)-1), but the command table includes entries like BZPOPMIN with LastKey=-2 to EXCLUDE the trailing blocking timeout arg. With the previous logic, COMMAND GETKEYS BZPOPMIN k1 k2 0 returned 0 as a key, which can misdirect client key-routing decisions. Negative LastKey is now treated as an offset from the end: -1 is the final arg, -2 is the second-to-last, and so on (len(argv)+last). This matches the Redis key-specs semantics. Tests pin the BZPOPMIN case end-to-end via the handler and a helper- level test for synthetic -2 metadata, so a future change that reverts to the "every negative means to end" shortcut trips CI. --- adapter/redis_command_info.go | 11 +++++++++-- adapter/redis_command_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/adapter/redis_command_info.go b/adapter/redis_command_info.go index 79739885..7f05b473 100644 --- a/adapter/redis_command_info.go +++ b/adapter/redis_command_info.go @@ -214,6 +214,10 @@ func warnMissingRedisCommandMeta(upper string) { // - first_key=0: no keys (empty slice). // - last_key=-1: "all args after first_key are keys". step controls spacing // (step=1 → every arg; step=2 → every other arg, as in MSET). +// - last_key=-N (N>1): last key index is len(argv)-N. Commands like +// BZPOPMIN use -2 to exclude a trailing timeout arg that is NOT a key; +// treating every negative as "to end" would wrongly expose the timeout +// via COMMAND GETKEYS and break client key-routing decisions. // - otherwise: args in [first_key .. last_key] at `step` stride. // // This is *positional*. It does not understand option prefixes (e.g. the @@ -232,8 +236,11 @@ func redisCommandGetKeys(meta redisCommandMeta, argv [][]byte) [][]byte { } last := meta.LastKey if last < 0 { - // "to end": last arg index is len(argv)-1. - last = len(argv) - 1 + // Negative last_key is an offset from the end: -1 means the + // final arg, -2 means the second-to-last, and so on. Use + // len(argv)+last so BZPOPMIN (-2) excludes its trailing + // timeout argument instead of claiming the timeout as a key. + last = len(argv) + last } if last >= len(argv) { last = len(argv) - 1 diff --git a/adapter/redis_command_test.go b/adapter/redis_command_test.go index 37114631..d44d71ef 100644 --- a/adapter/redis_command_test.go +++ b/adapter/redis_command_test.go @@ -244,6 +244,36 @@ func TestCommandGetKeys_UnknownCommand(t *testing.T) { require.Contains(t, conn.writes[0].s, "Invalid command specified") } +// BZPOPMIN declares last_key=-2: the final argv entry is a blocking timeout +// that must NOT be reported as a key. This pins the negative-offset +// semantics so a future change that collapses all negatives to "to end" +// would trip the test instead of silently mis-routing client writes. +func TestCommandGetKeys_BZPOPMIN_ExcludesTimeout(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "GETKEYS", "BZPOPMIN", "k1", "k2", "0") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 2, conn.writes[0].n) + require.Equal(t, "bulk", conn.writes[1].op) + require.Equal(t, "k1", conn.writes[1].s) + require.Equal(t, "bulk", conn.writes[2].op) + require.Equal(t, "k2", conn.writes[2].s) +} + +// Pin the helper against synthetic -2 metadata as well, matching the +// MSET helper test above. Guards the len(argv)+last arithmetic directly. +func TestRedisCommandGetKeysHelper_NegativeLastKeyOffset(t *testing.T) { + t.Parallel() + // BZPOPMIN-shaped: first=1, last=-2 (exclude trailing timeout), step=1. + meta := redisCommandMeta{FirstKey: 1, LastKey: -2, Step: 1} + argv := [][]byte{[]byte("BZPOPMIN"), []byte("k1"), []byte("k2"), []byte("0")} + keys := redisCommandGetKeys(meta, argv) + got := make([]string, 0, len(keys)) + for _, k := range keys { + got = append(got, string(k)) + } + require.Equal(t, []string{"k1", "k2"}, got) +} + func TestCommandList_ReturnsAllNames(t *testing.T) { t.Parallel() conn := runCommand(t, "COMMAND", "LIST") From fb540a29b4db721ebe253e3ff1040f960177b9e0 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Sat, 25 Apr 2026 06:33:55 +0900 Subject: [PATCH 3/4] fix(redis): emit name keys + full-docs default for COMMAND DOCS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 (x2) on COMMAND DOCS wire shape: 1. Bare COMMAND DOCS (no command names) previously returned an empty array — the implementation sized the reply from len(requested) and iterated only the requested slice. Real Redis returns docs for every command in that case (same pattern as bare COMMAND and bare COMMAND INFO), which is what redis-cli --docs and capability- probing clients rely on. Bare DOCS now iterates the routed set via routedRedisCommandMetas(). 2. The RESP2 form of COMMAND DOCS is a flattened map: alternating command-name keys and 4-element doc-map values. Previously we wrote only the value slots (summary / arguments) and never the command- name key, so any client decoding the response as a name -> docs map saw malformed data. Now each entry is a (name, docs) pair; unknown commands emit (name, nil) preserving the flat-map layout. Updated the existing TestCommandDocs_Get to assert the new shape (outer array length 2, name bulk, then doc-map). Added TestCommandDocs_BareReturnsAllDocs pinning the full-set default and TestCommandDocs_UnknownReturnsNamedNil pinning the (name, nil) shape for unknown commands. --- adapter/redis_command_test.go | 59 +++++++++++++++++++++++++++----- adapter/redis_compat_commands.go | 48 ++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 16 deletions(-) diff --git a/adapter/redis_command_test.go b/adapter/redis_command_test.go index d44d71ef..b542bc58 100644 --- a/adapter/redis_command_test.go +++ b/adapter/redis_command_test.go @@ -311,16 +311,57 @@ func TestCommandList_RejectsFilterBy(t *testing.T) { func TestCommandDocs_Get(t *testing.T) { t.Parallel() conn := runCommand(t, "COMMAND", "DOCS", "GET") - // outer array of 1, then 4-element map-shaped array. + // RESP2 flat-map shape: outer array of 2 (name + doc-map pair), + // then a name bulk string, then a 4-element map-shaped doc-map. require.Equal(t, "array", conn.writes[0].op) - require.Equal(t, 1, conn.writes[0].n) - require.Equal(t, "array", conn.writes[1].op) - require.Equal(t, 4, conn.writes[1].n) - require.Equal(t, "summary", conn.writes[2].s) - require.Equal(t, "", conn.writes[3].s) - require.Equal(t, "arguments", conn.writes[4].s) - require.Equal(t, "array", conn.writes[5].op) - require.Equal(t, 0, conn.writes[5].n) + require.Equal(t, 2, conn.writes[0].n, "outer array must be (name, docs) pair") + require.Equal(t, "bulk", conn.writes[1].op) + require.Equal(t, "get", conn.writes[1].s) + require.Equal(t, "array", conn.writes[2].op) + require.Equal(t, 4, conn.writes[2].n) + require.Equal(t, "summary", conn.writes[3].s) + require.Equal(t, "", conn.writes[4].s) + require.Equal(t, "arguments", conn.writes[5].s) + require.Equal(t, "array", conn.writes[6].op) + require.Equal(t, 0, conn.writes[6].n) +} + +// TestCommandDocs_BareReturnsAllDocs pins that bare COMMAND DOCS +// (no command names) returns docs for every routed command, matching +// real Redis semantics. Previously bare DOCS returned an empty +// array, which broke redis-cli --docs and any client that relied on +// the default full-docs behaviour. +func TestCommandDocs_BareReturnsAllDocs(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "DOCS") + require.Equal(t, "array", conn.writes[0].op) + // Outer array is 2 × routed-command-count (each command contributes + // a name key and a doc-map value slot). + wantOuterLen := len(routedRedisCommandMetas()) * 2 + require.Equal(t, wantOuterLen, conn.writes[0].n, + "bare COMMAND DOCS must emit (name, docs) for every routed command") + // First pair: name bulk, then 4-element doc map. + require.Equal(t, "bulk", conn.writes[1].op) + require.NotEmpty(t, conn.writes[1].s) + require.Equal(t, "array", conn.writes[2].op) + require.Equal(t, 4, conn.writes[2].n) + require.Equal(t, "summary", conn.writes[3].s) +} + +// TestCommandDocs_UnknownReturnsNamedNil pins the Redis "unknown +// command" shape for DOCS: the requested name key is still written +// (preserving the flat-map layout) but its value is nil. A client +// decoding the response as a map then sees `"FOO" -> nil`, which is +// the canonical "we acknowledged your question, we just have no +// entry" reply. +func TestCommandDocs_UnknownReturnsNamedNil(t *testing.T) { + t.Parallel() + conn := runCommand(t, "COMMAND", "DOCS", "NOSUCH") + require.Equal(t, "array", conn.writes[0].op) + require.Equal(t, 2, conn.writes[0].n) + require.Equal(t, "bulk", conn.writes[1].op) + require.Equal(t, "NOSUCH", conn.writes[1].s) + require.Equal(t, "null", conn.writes[2].op) } func TestCommand_UnknownSubcommand(t *testing.T) { diff --git a/adapter/redis_compat_commands.go b/adapter/redis_compat_commands.go index bf98e6ca..b9b90b8e 100644 --- a/adapter/redis_compat_commands.go +++ b/adapter/redis_compat_commands.go @@ -326,19 +326,53 @@ func (r *RedisServer) writeCommandInfo(conn redcon.Conn, requested [][]byte) { } } -// writeCommandDocs emits the minimum shape real clients check for: -// a 4-element map-shaped array per requested command with "summary" and -// "arguments" keys. We do not maintain per-command docs, so the summary -// is empty and the arguments array is empty. Unknown commands produce a -// nil entry, matching Redis semantics. +// writeCommandDocs emits the RESP2 flat-map form of COMMAND DOCS: +// alternating command-name keys and 4-element doc-maps with "summary" +// and "arguments" fields. Two compliance-critical behaviours: +// +// 1. Bare `COMMAND DOCS` (no names) returns docs for ALL routed +// commands, identical to how `COMMAND INFO` and bare `COMMAND` +// behave. Clients/tools like redis-cli --docs rely on this. +// 2. Every requested entry writes BOTH the command-name key AND the +// doc map value. Clients decode the top-level array as a map of +// name -> docs, so skipping the name key makes the reply +// unparseable. Unknown commands emit the requested name followed +// by nil (Redis semantics). +// +// We do not maintain per-command docs, so summary is "" and arguments +// is empty. The wire-shape is what clients care about at connect time. func (r *RedisServer) writeCommandDocs(conn redcon.Conn, requested [][]byte) { const docEntryLen = 4 - conn.WriteArray(len(requested)) + // Bare DOCS (no command names): iterate the routed set so the + // reply mirrors `COMMAND` / `COMMAND INFO` / `COMMAND LIST`. + if len(requested) == 0 { + metas := routedRedisCommandMetas() + // Two wire slots per command (name + doc map). + conn.WriteArray(len(metas) * 2) //nolint:mnd // 2 = (name, docs) pair + for _, meta := range metas { + conn.WriteBulkString(meta.Name) + conn.WriteArray(docEntryLen) + conn.WriteBulkString("summary") + conn.WriteBulkString("") + conn.WriteBulkString("arguments") + conn.WriteArray(0) + } + return + } + // Explicit names: preserve the caller-supplied order so a client + // that expects its own request ordering back (e.g. for building a + // lookup table) is not surprised. Each pair is (name, docs) or + // (name, nil) for unknowns. + conn.WriteArray(len(requested) * 2) //nolint:mnd // 2 = (name, docs) pair for _, raw := range requested { - if _, ok := redisCommandTable[strings.ToUpper(string(raw))]; !ok { + name := string(raw) + meta, ok := redisCommandTable[strings.ToUpper(name)] + if !ok { + conn.WriteBulkString(name) conn.WriteNull() continue } + conn.WriteBulkString(meta.Name) conn.WriteArray(docEntryLen) conn.WriteBulkString("summary") conn.WriteBulkString("") From ccde86891709c75829a31abb3f28cad27f4de783 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Sat, 25 Apr 2026 21:27:09 +0900 Subject: [PATCH 4/4] refactor(redis): unify command registry to eliminate route/argsLen/table drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-cause fix for the post-merge CI failure on this branch (TestCommand _RouteMatchesTable / TestCommandCount_MatchesTableSize) and for the recurring Codex P2 (HELLO missing from redisCommandTable). Both symptoms shared the same underlying cause: command registration was spread across THREE separate maps (r.route in redis.go, argsLen in redis.go, redisCommandTable in redis_command_info.go) and adding a new command required updating all three by hand. HELLO was added to the first two on main but missed the third — silently breaking COMMAND INFO HELLO and tripping the route-vs-table mirror test. This refactor collapses the three views to a single source of truth. Single-row registration (adapter/redis_command_specs.go): - redisCommandSpecs is the canonical slice. Each row carries Constant / Name / Arity / Flags / FirstKey / LastKey / Step. - argsLen is derived from the slice once at init (function-call package var initializer); same for redisCommandTable. - buildRouteMap binds handlers per RedisServer and panics at server construction if any spec lacks a handler or any handler lacks a spec, so the surviving 2-way sync between specs and handlers is enforced at startup rather than at first request. Why handlers stay separate from the spec slice: - Embedding method values directly in the spec literal triggered a Go init cycle (redisCommandSpecs → method body → argsLen → redisCommandSpecs) because Go's init-order analysis follows function bodies. The buildRouteMap layout sidesteps the cycle while keeping the structural invariant under test. Tests added: - TestRedisCommandSpecs_NoDuplicateConstants pins the "every Constant appears exactly once" invariant — without it a duplicate spec would silently shadow earlier rows in the derived maps. - TestRedisCommandSpecs_HELLOPresent is the explicit regression guard: HELLO must have a redisCommandTable row (the exact gap Codex P2 + TestCommand_RouteMatchesTable both flagged). - The pre-existing TestCommand_RouteMatchesTable and TestCommandCount_MatchesTableSize now pass by construction; they remain as the perimeter check for "anything routed must be tabled". Build / vet / lint clean. CI failures from the previous merge commit (3632cdc7) are eliminated by construction. --- adapter/redis.go | 179 +-------------------- adapter/redis_command_info.go | 111 +------------ adapter/redis_command_specs.go | 275 +++++++++++++++++++++++++++++++++ adapter/redis_command_test.go | 33 ++++ 4 files changed, 323 insertions(+), 275 deletions(-) create mode 100644 adapter/redis_command_specs.go diff --git a/adapter/redis.go b/adapter/redis.go index ba498350..19c05a2a 100644 --- a/adapter/redis.go +++ b/adapter/redis.go @@ -165,92 +165,9 @@ var txnApplyHandlers = map[string]txnCommandHandler{ cmdPExpire: (*txnContext).applyExpireMilliseconds, } -//nolint:mnd -var argsLen = map[string]int{ - cmdBZPopMin: -3, - cmdClient: -2, - cmdCommand: -1, - cmdDBSize: 1, - cmdDel: -2, - cmdDiscard: 1, - cmdEval: -3, - cmdEvalSHA: -3, - cmdExec: 1, - cmdExists: -2, - cmdExpire: -3, - cmdFlushAll: 1, - cmdFlushDB: 1, - cmdFlushLegacy: 1, - cmdGet: 2, - cmdGetDel: 2, - cmdHDel: -3, - cmdHExists: 3, - cmdHGet: 3, - cmdHGetAll: 2, - cmdHIncrBy: 4, - cmdHLen: 2, - cmdHMGet: -3, - cmdHMSet: -4, - cmdHSet: -4, - cmdHello: -1, - cmdInfo: -1, - cmdIncr: 2, - cmdKeys: 2, - cmdLIndex: 3, - cmdLLen: 2, - cmdLPop: 2, - cmdLPush: -3, - cmdLPos: -3, - cmdLRange: 4, - cmdLRem: 4, - cmdLSet: 4, - cmdLTrim: 4, - cmdMulti: 1, - cmdPExpire: -3, - cmdPFAdd: -3, - cmdPFCount: -2, - cmdPing: -1, - cmdPTTL: 2, - cmdPublish: 3, - cmdPubSub: -2, - cmdQuit: 1, - cmdRename: 3, - cmdRPop: 2, - cmdRPopLPush: 3, - cmdRPush: -3, - cmdSAdd: -3, - cmdSCard: 2, - cmdScan: -2, - cmdSelect: 2, - cmdSet: -3, - cmdSetEx: 4, - cmdSetNX: 3, - cmdSIsMember: 3, - cmdSMembers: 2, - cmdSRem: -3, - cmdSubscribe: -2, - cmdTTL: 2, - cmdType: 2, - cmdXAdd: -5, - cmdXLen: 2, - cmdXRead: -4, - cmdXRange: -4, - cmdXRevRange: -4, - cmdXTrim: -4, - cmdZAdd: -4, - cmdZCard: 2, - cmdZCount: 4, - cmdZIncrBy: 4, - cmdZRange: -4, - cmdZRangeByScore: -4, - cmdZRem: -3, - cmdZRemRangeByScore: 4, - cmdZRemRangeByRank: 4, - cmdZPopMin: -2, - cmdZRevRange: -4, - cmdZRevRangeByScore: -4, - cmdZScore: 3, -} +// argsLen is derived from redisCommandSpecs in adapter/redis_command_specs.go. +// See that file for the canonical row list and the rationale for the +// single source of truth. type RedisServer struct { listen net.Listener @@ -434,91 +351,11 @@ func NewRedisServer(listen net.Listener, redisAddr string, store store.MVCCStore } r.relay.Bind(r.publishLocal) - r.route = map[string]func(conn redcon.Conn, cmd redcon.Command){ - cmdBZPopMin: r.bzpopmin, - cmdClient: r.client, - cmdCommand: r.command, - cmdDBSize: r.dbsize, - cmdDel: r.del, - cmdDiscard: r.discard, - cmdEval: r.eval, - cmdEvalSHA: r.evalsha, - cmdExec: r.exec, - cmdExists: r.exists, - cmdExpire: r.expire, - cmdFlushAll: r.flushall, - cmdFlushDB: r.flushdb, - cmdFlushLegacy: r.flushlegacy, - cmdGet: r.get, - cmdGetDel: r.getdel, - cmdHDel: r.hdel, - cmdHExists: r.hexists, - cmdHGet: r.hget, - cmdHGetAll: r.hgetall, - cmdHIncrBy: r.hincrby, - cmdHLen: r.hlen, - cmdHMGet: r.hmget, - cmdHMSet: r.hmset, - cmdHSet: r.hset, - cmdHello: r.hello, - cmdInfo: r.info, - cmdIncr: r.incr, - cmdKeys: r.keys, - cmdLIndex: r.lindex, - cmdLLen: r.llen, - cmdLPop: r.lpop, - cmdLPos: r.lpos, - cmdLPush: r.lpush, - cmdLRange: r.lrange, - cmdLRem: r.lrem, - cmdLSet: r.lset, - cmdLTrim: r.ltrim, - cmdMulti: r.multi, - cmdPExpire: r.pexpire, - cmdPFAdd: r.pfadd, - cmdPFCount: r.pfcount, - cmdPing: r.ping, - cmdPTTL: r.pttl, - cmdPublish: r.publish, - cmdPubSub: r.pubsubCmd, - cmdQuit: r.quit, - cmdRename: r.rename, - cmdRPop: r.rpop, - cmdRPopLPush: r.rpoplpush, - cmdRPush: r.rpush, - cmdSAdd: r.sadd, - cmdSCard: r.scard, - cmdScan: r.scan, - cmdSelect: r.selectDB, - cmdSet: r.set, - cmdSetEx: r.setex, - cmdSetNX: r.setnx, - cmdSIsMember: r.sismember, - cmdSMembers: r.smembers, - cmdSRem: r.srem, - cmdSubscribe: r.subscribe, - cmdTTL: r.ttl, - cmdType: r.typeCmd, - cmdXAdd: r.xadd, - cmdXLen: r.xlen, - cmdXRead: r.xread, - cmdXRange: r.xrange, - cmdXRevRange: r.xrevrange, - cmdXTrim: r.xtrim, - cmdZAdd: r.zadd, - cmdZCard: r.zcard, - cmdZCount: r.zcount, - cmdZIncrBy: r.zincrby, - cmdZRange: r.zrange, - cmdZRangeByScore: r.zrangebyscore, - cmdZRem: r.zrem, - cmdZRemRangeByScore: r.zremrangebyscore, - cmdZRemRangeByRank: r.zremrangebyrank, - cmdZPopMin: r.zpopmin, - cmdZRevRange: r.zrevrange, - cmdZRevRangeByScore: r.zrevrangebyscore, - cmdZScore: r.zscore, - } + // route, argsLen, and redisCommandTable all derive from the single + // redisCommandSpecs slice (adapter/redis_command_specs.go) so adding + // a command is a one-row diff there and the three views can never + // drift. See buildRouteMap for the per-server bind. + r.route = r.buildRouteMap() for _, opt := range opts { if opt != nil { opt(r) diff --git a/adapter/redis_command_info.go b/adapter/redis_command_info.go index 7f05b473..c4078e4e 100644 --- a/adapter/redis_command_info.go +++ b/adapter/redis_command_info.go @@ -1,18 +1,12 @@ package adapter -// redis_command_info.go holds the static metadata table consumed by the -// Redis `COMMAND` handler. It is intentionally a single, grep-able file so -// that adding a new command is a one-liner: -// -// 1) Register the new handler in RedisServer.route (redis.go). -// 2) Add an argsLen entry (redis.go). -// 3) Add a row below. Forgetting step 3 is NOT fatal — the COMMAND -// handler falls back to a zero-metadata entry and emits one warning -// log per command name so the omission is discoverable — but you -// should do step 3 anyway. -// -// The table is the source of truth for `COMMAND`, `COMMAND INFO`, -// `COMMAND COUNT`, `COMMAND LIST`, `COMMAND DOCS`, and `COMMAND GETKEYS`. +// redis_command_info.go holds the COMMAND-family helpers. The metadata +// table itself (redisCommandTable) and the routed-set source of truth +// (argsLen) both live in adapter/redis_command_specs.go and are derived +// from the canonical redisCommandSpecs slice — adding a new command is +// a single row there, with no risk of drifting between r.route / +// argsLen / redisCommandTable the way HELLO did when it was added to +// the route + arity check but missed the metadata table. // // Shape notes (Redis reference): // - arity: exact positive arity, or negative meaning "at least |arity|" @@ -53,97 +47,6 @@ type redisCommandMeta struct { Step int } -// redisCommandTable maps UPPERCASE command name -> metadata. Every entry -// routed by RedisServer.route should appear here. Entries are listed in -// alphabetical order to keep diffs small when adding new commands. -// -//nolint:mnd // magic numbers here are literal Redis metadata (arity, key positions) -var redisCommandTable = map[string]redisCommandMeta{ - "BZPOPMIN": {Name: "bzpopmin", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -2, Step: 1}, - "CLIENT": {Name: "client", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "COMMAND": {Name: "command", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "DBSIZE": {Name: "dbsize", Arity: 1, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, - "DEL": {Name: "del", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -1, Step: 1}, - "DISCARD": {Name: "discard", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "EVAL": {Name: "eval", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, - "EVALSHA": {Name: "evalsha", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, - "EXEC": {Name: "exec", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "EXISTS": {Name: "exists", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, - "EXPIRE": {Name: "expire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "FLUSHALL": {Name: "flushall", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, - "FLUSHDB": {Name: "flushdb", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, - // FLUSHLEGACY is an elastickv-internal alias; mirror FLUSHDB metadata. - "FLUSHLEGACY": {Name: "flushlegacy", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, - "GET": {Name: "get", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "GETDEL": {Name: "getdel", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "HDEL": {Name: "hdel", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "HEXISTS": {Name: "hexists", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "HGET": {Name: "hget", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "HGETALL": {Name: "hgetall", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "HINCRBY": {Name: "hincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "HLEN": {Name: "hlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "HMGET": {Name: "hmget", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "HMSET": {Name: "hmset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "HSET": {Name: "hset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "INCR": {Name: "incr", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "INFO": {Name: "info", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "KEYS": {Name: "keys", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, - "LINDEX": {Name: "lindex", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "LLEN": {Name: "llen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "LPOP": {Name: "lpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "LPOS": {Name: "lpos", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "LPUSH": {Name: "lpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "LRANGE": {Name: "lrange", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "LREM": {Name: "lrem", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "LSET": {Name: "lset", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "LTRIM": {Name: "ltrim", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "MULTI": {Name: "multi", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "PEXPIRE": {Name: "pexpire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "PFADD": {Name: "pfadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "PFCOUNT": {Name: "pfcount", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, - "PING": {Name: "ping", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "PTTL": {Name: "pttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "PUBLISH": {Name: "publish", Arity: 3, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "PUBSUB": {Name: "pubsub", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "QUIT": {Name: "quit", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "RENAME": {Name: "rename", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, - "RPOP": {Name: "rpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "RPOPLPUSH": {Name: "rpoplpush", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, - "RPUSH": {Name: "rpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SADD": {Name: "sadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SCAN": {Name: "scan", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, - "SCARD": {Name: "scard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "SELECT": {Name: "select", Arity: 2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "SET": {Name: "set", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SETEX": {Name: "setex", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SETNX": {Name: "setnx", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SISMEMBER": {Name: "sismember", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "SMEMBERS": {Name: "smembers", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "SREM": {Name: "srem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "SUBSCRIBE": {Name: "subscribe", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, - "TTL": {Name: "ttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "TYPE": {Name: "type", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "XADD": {Name: "xadd", Arity: -5, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "XLEN": {Name: "xlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "XRANGE": {Name: "xrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "XREAD": {Name: "xread", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, - "XREVRANGE": {Name: "xrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "XTRIM": {Name: "xtrim", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZADD": {Name: "zadd", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZCARD": {Name: "zcard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZCOUNT": {Name: "zcount", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZINCRBY": {Name: "zincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZPOPMIN": {Name: "zpopmin", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZRANGE": {Name: "zrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZRANGEBYSCORE": {Name: "zrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZREM": {Name: "zrem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZREMRANGEBYRANK": {Name: "zremrangebyrank", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZREMRANGEBYSCORE": {Name: "zremrangebyscore", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZREVRANGE": {Name: "zrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZREVRANGEBYSCORE": {Name: "zrevrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, - "ZSCORE": {Name: "zscore", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, -} - // redisCommandFallbackWarnedOnce deduplicates the "missing metadata" log so // that a hostile or buggy client probing the same unknown-but-routed name // cannot generate unbounded log spam. The fallback is a safety net for diff --git a/adapter/redis_command_specs.go b/adapter/redis_command_specs.go new file mode 100644 index 00000000..f10176d6 --- /dev/null +++ b/adapter/redis_command_specs.go @@ -0,0 +1,275 @@ +package adapter + +import ( + "fmt" + + "github.com/tidwall/redcon" +) + +// redisCommandSpec is the canonical row for a single Redis command. The +// route map (handler dispatch), argsLen (arity validation), and +// redisCommandTable (COMMAND INFO/COUNT/LIST/DOCS metadata) are all +// derived from redisCommandSpecs below — adding a new command requires +// a single row here plus a handler entry in routeHandlers, and the +// three views can never drift the way HELLO did when it was added to +// r.route + argsLen but missed redisCommandTable. buildRouteMap panics +// at server construction if any spec is missing a handler or vice +// versa, so the surviving 2-way sync is enforced at startup. +// +// We keep handler bindings out of the spec slice itself because Go's +// package-level init dependency analysis would otherwise see +// redisCommandSpecs → method body → argsLen → redisCommandSpecs and +// flag the whole graph as a cycle. +type redisCommandSpec struct { + // Constant is the UPPERCASE wire name (matches the cmd* identifier + // in redis.go) used as the route / argsLen / redisCommandTable key. + Constant string + // Name is the lowercase form Redis emits in COMMAND INFO replies. + Name string + Arity int + Flags []string + FirstKey int + LastKey int + Step int +} + +// redisCommandSpecs is the single source of truth for every routed +// Redis command's metadata. Rows are alphabetised by Constant so diffs +// stay small when adding a command. +// +//nolint:mnd // magic numbers here are literal Redis metadata (arity, key positions) +var redisCommandSpecs = []redisCommandSpec{ + {Constant: cmdBZPopMin, Name: "bzpopmin", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -2, Step: 1}, + {Constant: cmdClient, Name: "client", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdCommand, Name: "command", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdDBSize, Name: "dbsize", Arity: 1, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdDel, Name: "del", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: -1, Step: 1}, + {Constant: cmdDiscard, Name: "discard", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdEval, Name: "eval", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdEvalSHA, Name: "evalsha", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdExec, Name: "exec", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdExists, Name: "exists", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, + {Constant: cmdExpire, Name: "expire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdFlushAll, Name: "flushall", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdFlushDB, Name: "flushdb", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + // FLUSHLEGACY is an elastickv-internal alias; mirror FLUSHDB metadata. + {Constant: cmdFlushLegacy, Name: "flushlegacy", Arity: 1, Flags: []string{redisCmdFlagWrite}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdGet, Name: "get", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdGetDel, Name: "getdel", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHDel, Name: "hdel", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHExists, Name: "hexists", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHGet, Name: "hget", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHGetAll, Name: "hgetall", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHIncrBy, Name: "hincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHLen, Name: "hlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHMGet, Name: "hmget", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHMSet, Name: "hmset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdHSet, Name: "hset", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + // HELLO is a connection-establishment command (RESP2 negotiation, + // AUTH probe, SETNAME). Arity is -1 because the protover and option + // list are all optional. No key arguments. + {Constant: cmdHello, Name: "hello", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdInfo, Name: "info", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdIncr, Name: "incr", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdKeys, Name: "keys", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdLIndex, Name: "lindex", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLLen, Name: "llen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLPop, Name: "lpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLPos, Name: "lpos", Arity: -3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLPush, Name: "lpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLRange, Name: "lrange", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLRem, Name: "lrem", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLSet, Name: "lset", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdLTrim, Name: "ltrim", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdMulti, Name: "multi", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdPExpire, Name: "pexpire", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdPFAdd, Name: "pfadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdPFCount, Name: "pfcount", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: -1, Step: 1}, + {Constant: cmdPing, Name: "ping", Arity: -1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdPTTL, Name: "pttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdPublish, Name: "publish", Arity: 3, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdPubSub, Name: "pubsub", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdQuit, Name: "quit", Arity: 1, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdRename, Name: "rename", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, + {Constant: cmdRPop, Name: "rpop", Arity: 2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdRPopLPush, Name: "rpoplpush", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 2, Step: 1}, + {Constant: cmdRPush, Name: "rpush", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSAdd, Name: "sadd", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdScan, Name: "scan", Arity: -2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdSCard, Name: "scard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSelect, Name: "select", Arity: 2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdSet, Name: "set", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSetEx, Name: "setex", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSetNX, Name: "setnx", Arity: 3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSIsMember, Name: "sismember", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSMembers, Name: "smembers", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSRem, Name: "srem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdSubscribe, Name: "subscribe", Arity: -2, Flags: []string{redisCmdFlagAdmin}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdTTL, Name: "ttl", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdType, Name: "type", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdXAdd, Name: "xadd", Arity: -5, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdXLen, Name: "xlen", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdXRange, Name: "xrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdXRead, Name: "xread", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 0, LastKey: 0, Step: 0}, + {Constant: cmdXRevRange, Name: "xrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdXTrim, Name: "xtrim", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZAdd, Name: "zadd", Arity: -4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZCard, Name: "zcard", Arity: 2, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZCount, Name: "zcount", Arity: 4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZIncrBy, Name: "zincrby", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZPopMin, Name: "zpopmin", Arity: -2, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRange, Name: "zrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRangeByScore, Name: "zrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRem, Name: "zrem", Arity: -3, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRemRangeByRank, Name: "zremrangebyrank", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRemRangeByScore, Name: "zremrangebyscore", Arity: 4, Flags: []string{redisCmdFlagWrite}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRevRange, Name: "zrevrange", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZRevRangeByScore, Name: "zrevrangebyscore", Arity: -4, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, + {Constant: cmdZScore, Name: "zscore", Arity: 3, Flags: []string{redisCmdFlagReadonly}, FirstKey: 1, LastKey: 1, Step: 1}, +} + +// argsLen is derived once at init from redisCommandSpecs so callers +// (RedisServer.dispatchCommand arity check) keep their map shape. +// +//nolint:gochecknoglobals // package-level derived from redisCommandSpecs by design +var argsLen = func() map[string]int { + m := make(map[string]int, len(redisCommandSpecs)) + for _, s := range redisCommandSpecs { + m[s.Constant] = s.Arity + } + return m +}() + +// redisCommandTable is derived once at init from redisCommandSpecs. +// COMMAND INFO/COUNT/LIST/DOCS/GETKEYS read from this map; deriving it +// from the spec list means a routed-but-untabled drift like the HELLO +// regression cannot recur. +// +//nolint:gochecknoglobals // package-level derived from redisCommandSpecs by design +var redisCommandTable = func() map[string]redisCommandMeta { + m := make(map[string]redisCommandMeta, len(redisCommandSpecs)) + for _, s := range redisCommandSpecs { + m[s.Constant] = redisCommandMeta{ + Name: s.Name, + Arity: s.Arity, + Flags: s.Flags, + FirstKey: s.FirstKey, + LastKey: s.LastKey, + Step: s.Step, + } + } + return m +}() + +// buildRouteMap returns the dispatch table for r. The handler binding +// is pulled from a private map built on each call so handler references +// (which transitively read argsLen via RedisServer.dispatchCommand) +// don't pull redisCommandSpecs into a package-init cycle. buildRouteMap +// panics if any spec lacks a handler or vice-versa, surfacing drift at +// server construction rather than at the first request for a missing +// command. +func (r *RedisServer) buildRouteMap() map[string]func(redcon.Conn, redcon.Command) { + handlers := map[string]func(redcon.Conn, redcon.Command){ + cmdBZPopMin: r.bzpopmin, + cmdClient: r.client, + cmdCommand: r.command, + cmdDBSize: r.dbsize, + cmdDel: r.del, + cmdDiscard: r.discard, + cmdEval: r.eval, + cmdEvalSHA: r.evalsha, + cmdExec: r.exec, + cmdExists: r.exists, + cmdExpire: r.expire, + cmdFlushAll: r.flushall, + cmdFlushDB: r.flushdb, + cmdFlushLegacy: r.flushlegacy, + cmdGet: r.get, + cmdGetDel: r.getdel, + cmdHDel: r.hdel, + cmdHExists: r.hexists, + cmdHGet: r.hget, + cmdHGetAll: r.hgetall, + cmdHIncrBy: r.hincrby, + cmdHLen: r.hlen, + cmdHMGet: r.hmget, + cmdHMSet: r.hmset, + cmdHSet: r.hset, + cmdHello: r.hello, + cmdInfo: r.info, + cmdIncr: r.incr, + cmdKeys: r.keys, + cmdLIndex: r.lindex, + cmdLLen: r.llen, + cmdLPop: r.lpop, + cmdLPos: r.lpos, + cmdLPush: r.lpush, + cmdLRange: r.lrange, + cmdLRem: r.lrem, + cmdLSet: r.lset, + cmdLTrim: r.ltrim, + cmdMulti: r.multi, + cmdPExpire: r.pexpire, + cmdPFAdd: r.pfadd, + cmdPFCount: r.pfcount, + cmdPing: r.ping, + cmdPTTL: r.pttl, + cmdPublish: r.publish, + cmdPubSub: r.pubsubCmd, + cmdQuit: r.quit, + cmdRename: r.rename, + cmdRPop: r.rpop, + cmdRPopLPush: r.rpoplpush, + cmdRPush: r.rpush, + cmdSAdd: r.sadd, + cmdScan: r.scan, + cmdSCard: r.scard, + cmdSelect: r.selectDB, + cmdSet: r.set, + cmdSetEx: r.setex, + cmdSetNX: r.setnx, + cmdSIsMember: r.sismember, + cmdSMembers: r.smembers, + cmdSRem: r.srem, + cmdSubscribe: r.subscribe, + cmdTTL: r.ttl, + cmdType: r.typeCmd, + cmdXAdd: r.xadd, + cmdXLen: r.xlen, + cmdXRange: r.xrange, + cmdXRead: r.xread, + cmdXRevRange: r.xrevrange, + cmdXTrim: r.xtrim, + cmdZAdd: r.zadd, + cmdZCard: r.zcard, + cmdZCount: r.zcount, + cmdZIncrBy: r.zincrby, + cmdZPopMin: r.zpopmin, + cmdZRange: r.zrange, + cmdZRangeByScore: r.zrangebyscore, + cmdZRem: r.zrem, + cmdZRemRangeByRank: r.zremrangebyrank, + cmdZRemRangeByScore: r.zremrangebyscore, + cmdZRevRange: r.zrevrange, + cmdZRevRangeByScore: r.zrevrangebyscore, + cmdZScore: r.zscore, + } + m := make(map[string]func(redcon.Conn, redcon.Command), len(redisCommandSpecs)) + for _, s := range redisCommandSpecs { + h, ok := handlers[s.Constant] + if !ok { + panic(fmt.Sprintf("redis: command %q has a spec but no handler in buildRouteMap; add a row", s.Constant)) + } + m[s.Constant] = h + } + if len(handlers) != len(m) { + // At least one handler has no matching spec — surface the + // orphan so the operator knows which row to add. + for name := range handlers { + if _, ok := m[name]; !ok { + panic(fmt.Sprintf("redis: command %q has a handler but no spec in redisCommandSpecs; add a row", name)) + } + } + } + return m +} diff --git a/adapter/redis_command_test.go b/adapter/redis_command_test.go index b542bc58..2d782923 100644 --- a/adapter/redis_command_test.go +++ b/adapter/redis_command_test.go @@ -423,3 +423,36 @@ func TestCommand_RouteMatchesTable(t *testing.T) { } _ = srv } + +// TestRedisCommandSpecs_NoDuplicateConstants guards the structural +// invariant the spec list maintains: every Constant appears exactly +// once. Without this, the derived argsLen / redisCommandTable maps +// would silently shadow earlier entries (whichever spec listed last +// would win), and a subtle regression in Arity / Flags / key positions +// could ride in unnoticed. +func TestRedisCommandSpecs_NoDuplicateConstants(t *testing.T) { + t.Parallel() + seen := make(map[string]int, len(redisCommandSpecs)) + for i, s := range redisCommandSpecs { + if prev, dup := seen[s.Constant]; dup { + t.Fatalf("duplicate spec for %q at indices %d and %d", s.Constant, prev, i) + } + seen[s.Constant] = i + } +} + +// TestRedisCommandSpecs_HELLOPresent is the explicit regression test +// for the round-N CI failure that triggered this refactor: HELLO was +// added to r.route + argsLen on main but missed from +// redisCommandTable, so COMMAND INFO HELLO returned nil and TestCommand +// _RouteMatchesTable fired. With the unified spec list, HELLO must be +// in redisCommandTable by construction; this test pins the invariant +// in case anyone is tempted to delete the row "for symmetry". +func TestRedisCommandSpecs_HELLOPresent(t *testing.T) { + t.Parallel() + hello, ok := redisCommandTable[cmdHello] + require.Truef(t, ok, "HELLO must have a redisCommandTable row (regression #607)") + require.Equal(t, "hello", hello.Name) + require.Equal(t, -1, hello.Arity) + require.Equal(t, 0, hello.FirstKey) +}