Unverified Commit 43e736f7 authored by guybe7's avatar guybe7 Committed by GitHub
Browse files

Treat subcommands as commands (#9504)

## Intro

The purpose is to allow having different flags/ACL categories for
subcommands (Example: CONFIG GET is ok-loading but CONFIG SET isn't)

We create a small command table for every command that has subcommands
and each subcommand has its own flags, etc. (same as a "regular" command)

This commit also unites the Redis and the Sentinel command tables

## Affected commands

CONFIG
Used to have "admin ok-loading ok-stale no-script"
Changes:
1. Dropped "ok-loading" in all except GET (this doesn't change behavior since
there were checks in the code doing that)

XINFO
Used to have "read-only random"
Changes:
1. Dropped "random" in all except CONSUMERS

XGROUP
Used to have "write use-memory"
Changes:
1. Dropped "use-memory" in all except CREATE and CREATECONSUMER

COMMAND
No changes.

MEMORY
Used to have "random read-only"
Changes:
1. Dropped "random" in PURGE and USAGE

ACL
Used to have "admin no-script ok-loading ok-stale"
Changes:
1. Dropped "admin" in WHOAMI, GENPASS, and CAT

LATENCY
No changes.

MODULE
No changes.

SLOWLOG
Used to have "admin random ok-loading ok-stale"
Changes:
1. Dropped "random" in RESET

OBJECT
Used to have "read-only random"
Changes:
1. Dropped "random" in ENCODING and REFCOUNT

SCRIPT
Used to have "may-replicate no-script"
Changes:
1. Dropped "may-replicate" in all except FLUSH and LOAD

CLIENT
Used to have "admin no-script random ok-loading ok-stale"
Changes:
1. Dropped "random" in all except INFO and LIST
2. Dropped "admin" in ID, TRACKING, CACHING, GETREDIR, INFO, SETNAME, GETNAME, and REPLY

STRALGO
No changes.

PUBSUB
No changes.

CLUSTER
Changes:
1. Dropped "admin in countkeysinslots, getkeysinslot, info, nodes, keyslot, myid, and slots

SENTINEL
No changes.

(note that DEBUG also fits, but we decided not to convert it since it's for
debugging and anyway undocumented)

## New sub-command
This commit adds another element to the per-command output of COMMAND,
describing the list of subcommands, if any (in the same structure as "regular" commands)
Also, it adds a new subcommand:
```
COMMAND LIST [FILTERBY (MODULE <module-name>|ACLCAT <cat>|PATTERN <pattern>)]
```
which returns a set of all commands (unless filters), but excluding subcommands.

## Module API
A new module API, RM_CreateSubcommand, was added, in order to allow
module writer to define subcommands

## ACL changes:
1. Now, that each subcommand is actually a command, each has its own ACL id.
2. The old mechanism of allowed_subcommands is redundant
(blocking/allowing a subcommand is the same as blocking/allowing a regular command),
but we had to keep it, to support the widespread usage of allowed_subcommands
to block commands with certain args, that aren't subcommands (e.g. "-select +select|0").
3. I have renamed allowed_subcommands to allowed_firstargs to emphasize the difference.
4. Because subcommands are commands in ACL too, you can now use "-" to block subcommands
(e.g. "+client -client|kill"), which wasn't possible in the past.
5. It is also possible to use the allowed_firstargs mechanism with subcommand.
For example: `+config -config|set +config|set|loglevel` will block all CONFIG SET except
for setting the log level.
6. All of the ACL changes above required some amount of refactoring.

## Misc
1. There are two approaches: Either each subcommand has its own function or all
   subcommands use the same function, determining what to do according to argv[0].
   For now, I took the former approaches only with CONFIG and COMMAND,
   while other commands use the latter approach (for smaller blamelog diff).
2. Deleted memoryGetKeys: It is no longer needed because MEMORY USAGE now uses the "range" key spec.
4. Bugfix: GETNAME was missing from CLIENT's help message.
5. Sentinel and Redis now use the same table, with the same function pointer.
   Some commands have a different implementation in Sentinel, so we redirect
   them (these are ROLE, PUBLISH, and INFO).
6. Command stats now show the stats per subcommand (e.g. instead of stats just
   for "config" you will have stats for "config|set", "config|get", etc.)
7. It is now possible to use COMMAND directly on subcommands:
   COMMAND INFO CONFIG|GET (The pipeline syntax was inspired from ACL, and
   can be used in functions lookupCommandBySds and lookupCommandByCString)
8. STRALGO is now a container command (has "help")

## Breaking changes:
1. Command stats now show the stats per subcommand (see (5) above)
parent 4962c552
......@@ -76,4 +76,26 @@ start_server {tags {"introspection"}} {
assert_match {*calls=1,*} [cmdstat expire]
assert_match {*calls=1,*} [cmdstat geoadd]
} {} {needs:config-resetstat}
test {COMMAND GETKEYS GET} {
assert_equal {key} [r command getkeys get key]
}
test {COMMAND GETKEYS MEMORY USAGE} {
assert_equal {key} [r command getkeys memory usage key]
}
test {COMMAND GETKEYS XGROUP} {
assert_equal {key} [r command getkeys xgroup create key groupname $]
}
test "COMMAND LIST FILTERBY ACLCAT" {
set reply [r command list filterby aclcat hyperloglog]
assert_equal [lsort $reply] {pfadd pfcount pfdebug pfmerge pfselftest}
}
test "COMMAND LIST FILTERBY PATTERN" {
set reply [r command list filterby pattern pf*]
assert_equal [lsort $reply] {pfadd pfcount pfdebug pfmerge pfselftest}
}
}
......@@ -74,6 +74,6 @@ start_server {tags {"latency-monitor needs:latency"}} {
test {LATENCY HELP should not have unexpected options} {
catch {r LATENCY help xxx} e
assert_match "*Unknown subcommand or wrong number of arguments*" $e
assert_match "*wrong number of arguments*" $e
}
}
......@@ -5,16 +5,22 @@ start_server {tags {"modules"}} {
test "Module key specs: Legacy" {
set reply [r command info kspec.legacy]
assert_equal $reply {{kspec.legacy -1 {} 1 2 1 {} {{flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}}}}
assert_equal $reply {{kspec.legacy -1 {} 1 2 1 {} {{flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}}
}
test "Module key specs: Complex specs, case 1" {
set reply [r command info kspec.complex1]
assert_equal $reply {{kspec.complex1 -1 movablekeys 1 1 1 {} {{flags {} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type keyword spec {keyword STORE startfrom 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type keyword spec {keyword KEYS startfrom 2}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}}}}
assert_equal $reply {{kspec.complex1 -1 movablekeys 1 1 1 {} {{flags {} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type keyword spec {keyword STORE startfrom 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type keyword spec {keyword KEYS startfrom 2}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}} {}}}
}
test "Module key specs: Complex specs, case 2" {
set reply [r command info kspec.complex2]
assert_equal $reply {{kspec.complex2 -1 movablekeys 1 2 1 {} {{flags write begin_search {type keyword spec {keyword STORE startfrom 5}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type index spec {index 3}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}} {flags write begin_search {type keyword spec {keyword MOREKEYS startfrom 5}} find_keys {type range spec {lastkey -1 keystep 1 limit 0}}}}}}
assert_equal $reply {{kspec.complex2 -1 movablekeys 1 2 1 {} {{flags write begin_search {type keyword spec {keyword STORE startfrom 5}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags read begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}} {flags write begin_search {type index spec {index 3}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}} {flags write begin_search {type keyword spec {keyword MOREKEYS startfrom 5}} find_keys {type range spec {lastkey -1 keystep 1 limit 0}}}} {}}}
}
test "Module command list filtering" {
;# Note: we piggyback this tcl file to test the general functionality of command list filtering
set reply [r command list filterby module keyspecs]
assert_equal [lsort $reply] {kspec.complex1 kspec.complex2 kspec.legacy}
}
}
set testmodule [file normalize tests/modules/subcommands.so]
start_server {tags {"modules"}} {
r module load $testmodule
test "Module subcommands via COMMAND" {
set reply [r command info subcommands.bitarray]
set subcmds [lindex [lindex $reply 0] 8]
assert_equal [lsort $subcmds] {{get -2 {} 1 1 1 {} {{flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}} {set -2 {} 1 1 1 {} {{flags write begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}}
}
test "Module pure-container command fails on arity error" {
catch {r subcommands.bitarray} e
assert_match {*wrong number of arguments*} $e
# Subcommands can be called
assert_equal [r subcommands.bitarray get k1] {OK}
}
}
......@@ -712,11 +712,11 @@ start_server {tags {"stream needs:debug"} overrides {appendonly yes aof-use-rdb-
start_server {tags {"stream"}} {
test {XGROUP HELP should not have unexpected options} {
catch {r XGROUP help xxx} e
assert_match "*Unknown subcommand or wrong number of arguments*" $e
assert_match "*wrong number of arguments*" $e
}
test {XINFO HELP should not have unexpected options} {
catch {r XINFO help xxx} e
assert_match "*Unknown subcommand or wrong number of arguments*" $e
assert_match "*wrong number of arguments*" $e
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment