redis-namespace
redis-namespace copied to clipboard
nil key should also be namespaced
currently nil
key is treated as empty-string key without namespace. which is confusing.
redis = Redis.new
namespaced = Redis::Namespace.new(:ns)
namespaced.set nil, 'foo'
namespaced.set '', 'bar'
redis.keys # => ["", "ns:"]
The redis ruby adapter's documentation explicitly calls out that keys should be strings, so I'm very wary of defining behaviour here where it is undefined upstream:
# Set the string value of a key. # # @param [String] key # @param [String] value # @param [Hash] options # - `:ex => Fixnum`: Set the specified expire time, in seconds. # - `:px => Fixnum`: Set the specified expire time, in milliseconds. # - `:nx => true`: Only set the key if it does not already exist. # - `:xx => true`: Only set the key if it already exist. # @return [String, Boolean] `"OK"` or true, false if `:nx => true` or `:xx => true` def set(key, value, options = {})
-- https://github.com/redis/redis-rb/blob/v3.2.1/lib/redis.rb#L654
I believe this is the line where redis-rb convert the key from nil
to empty string. (it basically converts every command to string before sending to Redis because that's the only thing it recognize)
https://github.com/redis/redis-rb/blob/v3.2.1/lib/redis/connection/command_helper.rb#L18
# @param [String] key
could also mean give me anything, I'll try to convert it to String, and if I can't, I'll raise an exception (that's quite a common behaviour for a dynamic typing language like Ruby)
for example, # @param [String] value
says the value
should be a String as well, and within that method, to_s
is called on value
.
https://github.com/redis/redis-rb/blob/v3.2.1/lib/redis.rb#L681
As I said earlier, I'm wary of defining behaviour based on the observed (but undefined) behaviour of private api's, but this behaviour is also surprising, which is why I left this ticket open. I'll circle back to it in the coming days, when I have time to consider all of the potential ramifications of your proposed fix (#107) and/or any alternates.
That said, I'd like to address a couple of your arguments:
# @param [String] key
could also mean give me anything
No, it doesn't; in yardoc (which is how the redis ruby adapter gem documents itself), the way to require an object that responds to to_s
would be # @param [#to_s] key
.
for example, # @param [String] value says the value should be a String as well, and within that method, to_s is called on value. https://github.com/redis/redis-rb/blob/v3.2.1/lib/redis.rb#L681
The documentation declares the contract of the public API for a library; in Semantic Versioning, the implementation details are free to change without a major version release (and thus an obvious warning to the consumers of a library), so long as the method behaves in the same manner from an external standpoint. The fact that version 3.2.1 of the redis ruby adapter sends #to_s
to the value argument that its documentation already requires be a String
is immaterial, and since the redis ruby adapter has no tests to validate the behaviour when a non-String
key is given, the behaviour is undefined and can change out of under us without warning.
ah I see! Thanks a lot for such a great explanation! Very clear! :+1: