stefanwille / crystal-redis

Full featured Redis client for Crystal
MIT License
381 stars 61 forks source link

Error: no overload matches 'Redis#namespaced' with type Array(Redis::RedisValue) #100

Closed kostya closed 3 years ago

kostya commented 4 years ago

After update to crystal-redid to 2.5.3 i got unexpected error in this code, for me this code looks ok:

redis = Redis.new
redis.keys("*").each { |key| redis.del(key) }
Showing last frame. Use --error-trace for full trace.

In src/redis/commands.cr:156:39

 156 | integer_command(concat(["DEL"], namespaced(keys)))
                                       ^---------
Error: no overload matches 'Redis#namespaced' with type Array(Redis::RedisValue)

Overloads are:
 - Redis::Commands#namespaced(keys : Array(String) | Tuple(String, String))
 - Redis::Commands#namespaced(key : String | Symbol | Int32)
stefanwille commented 4 years ago

The problem seems to be that #keys returns an Array(RedisValue), and #namespaced wants an Array(String). A possible fix is to allow the latter to accept an Array(RedisValue) instead and then convert to String.

A workaround would also be to perform the type cast yourself, as shown here:

https://github.com/stefanwille/crystal-redis/pull/101/files

rodrigopinto commented 4 years ago

Hi @stefanwille, I was updating the lib on https://github.com/defense-cr/defense as it is a dependency and faced the same issue.

Debugging a bit the problem, I don't think the proposal workaround is a good way to proceed, neither the late suggestion of changing the method signature, but I explain below.

The method #keys is currently returning(Array(RedisValue)) an Array with the union type of RedisValue. By implementation redis keys are binary-safe strings, so, from my understanding, the keys will always be a string. Considering it, I believe Redis#keys("*") should return Array(String) instead of Array(RedisValue).

The current issue happens because of the private method without_namespace that cast all values to RedisValue as seen below. https://github.com/stefanwille/crystal-redis/blob/009e35fa40bbd518798afcc32c397402c6c6acb2/src/redis/commands.cr#L562-L573

What do you think?