RM_CreateCommand should not set CMD_KEY_VARIABLE_FLAGS automatically (#11320)

The original idea behind auto-setting the default (first,last,step) spec was to use
the most "open" flags when the user didn't provide any key-spec flags information.

While the above idea is a good approach, it really makes no sense to set
CMD_KEY_VARIABLE_FLAGS if the user didn't provide the getkeys-api flag:
in this case there's not way to retrieve these variable flags, so what's the point?

Internally in redis there was code to ignore this already, so this fix doesn't change
redis's behavior, it only affects the output of COMMAND command.

2 files changed, 29 insertions, 2 deletions

diff --git a/tests/modules/keyspecs.c b/tests/modules/keyspecs.c
index d2ae9fd6c..0a70de814 100644
--- a/tests/modules/keyspecs.c
+++ b/tests/modules/keyspecs.c
@@ -7,6 +7,15 @@
 int kspec_impl(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     UNUSED(argv);
     UNUSED(argc);
+
+    /* Handle getkeys-api introspection (for "kspec.nonewithgetkeys")  */
+    if (RedisModule_IsKeysPositionRequest(ctx)) {
+        for (int i = 1; i < argc; i += 2)
+            RedisModule_KeyAtPosWithFlags(ctx, i, REDISMODULE_CMD_KEY_RO | REDISMODULE_CMD_KEY_ACCESS);
+
+        return REDISMODULE_OK;
+    }
+
     RedisModule_ReplyWithSimpleString(ctx, "OK");
     return REDISMODULE_OK;
 }
diff --git a/tests/unit/moduleapi/keyspecs.tcl b/tests/unit/moduleapi/keyspecs.tcl
index ef5b92334..8491bc19e 100644
--- a/tests/unit/moduleapi/keyspecs.tcl
+++ b/tests/unit/moduleapi/keyspecs.tcl
@@ -13,10 +13,24 @@ start_server {tags {"modules"}} {
         # Verify key-spec auto-generated from the legacy triple
         set keyspecs [lindex $reply 8]
         assert_equal [llength $keyspecs] 1
-        assert_equal [lindex $keyspecs 0] {flags {RW access update variable_flags} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey -1 keystep 2 limit 0}}}
+        assert_equal [lindex $keyspecs 0] {flags {RW access update} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey -1 keystep 2 limit 0}}}
         assert_equal [r command getkeys kspec.none key1 val1 key2 val2] {key1 key2}
     }
 
+    test "Module key specs: No spec, only legacy triple with getkeys-api" {
+        set reply [lindex [r command info kspec.nonewithgetkeys] 0]
+        # Verify (first, last, step) and movablekeys
+        assert_equal [lindex $reply 2] {module movablekeys}
+        assert_equal [lindex $reply 3] 1
+        assert_equal [lindex $reply 4] -1
+        assert_equal [lindex $reply 5] 2
+        # Verify key-spec auto-generated from the legacy triple
+        set keyspecs [lindex $reply 8]
+        assert_equal [llength $keyspecs] 1
+        assert_equal [lindex $keyspecs 0] {flags {RW access update variable_flags} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey -1 keystep 2 limit 0}}}
+        assert_equal [r command getkeys kspec.nonewithgetkeys key1 val1 key2 val2] {key1 key2}
+    }
+
     test "Module key specs: Two ranges" {
         set reply [lindex [r command info kspec.tworanges] 0]
         # Verify (first, last, step) and not movablekeys
@@ -99,7 +113,11 @@ start_server {tags {"modules"}} {
 
     test {COMMAND GETKEYSANDFLAGS correctly reports module key-spec without flags} {
         r command getkeysandflags kspec.none key1 val1 key2 val2
-    } {{key1 {RW access update variable_flags}} {key2 {RW access update variable_flags}}}
+    } {{key1 {RW access update}} {key2 {RW access update}}}
+
+    test {COMMAND GETKEYSANDFLAGS correctly reports module key-spec with flags} {
+        r command getkeysandflags kspec.nonewithgetkeys key1 val1 key2 val2
+    } {{key1 {RO access}} {key2 {RO access}}}
 
     test {COMMAND GETKEYSANDFLAGS correctly reports module key-spec flags} {
         r command getkeysandflags kspec.keyword keys key1 key2 key3