since the moduleGetCommandKeysViaAPI api doesn't support flags, and modules can soon declare key-specs, we should probably re-order the checks in getKeysFromCommandWithSpecs and prefer key-specs over moduleGetCommandKeysViaAPI (only if there are no key-specs or something).

Alternatively, additionally add a variant for RedisModule_KeyAtPos that sets key-spec flags.

Comment From: zuiderkwast

If COMMAND GETKEYS starts using getKeysFromCommandWithSpecs, then modules don't need to implement the getkeys-api as long as they provide key specs without VARIABLE_FLAGS. This can be seen from the implementation of getKeysFromCommandWithSpecs: https://github.com/redis/redis/blob/7.0-rc1/src/db.c#L1829

TL;DR if "getkeys-api" is provided, it's used. Otherwise, the keyspecs are used. A keyspec is auto-generated from the legacy triple, so this is covered too if no key-specs are provided.

Cluster should probably use the same logic to find keys and shard channels (i.e. treating shard channels as keys).

Then, the next question is whether COMMAND GETKEYS should follow the cluster example and treat shard channels as keys too. If COMMAND GETKEYS is used by cluster clients to figure out the shard, then I think it should. (I suspect @madolson may disagree.)

Comment From: oranagra

I think key-specs should be preferred over getkeys-api, at least in all places that require flags (e.g. ACL). it should then fallback to getkeys-api if the key-spec is INCOMPLETE or VARIABLE_FLAGS. since key-specs are new, and getkeys-api is old, key-specs are likely to have proper flags, and getkeys-api doesn't.

the only problem is a key-spec implicitly created from the command creation triple, maybe it should be flagged with VARIABLE_FLAGS. however an old module that's only using the command creation triple, even if using the getkeys-api, is likely not providing any flags anyway. so the priority between these two doesn't really matters, we just need to make sure that missing module flags are treated as requiring full key-permissions.

i already have much of this implemented (waiting to merge the key-spec registration PR before i can publish it)