nil key should also be namespaced

[ghprince]

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:"]

[yaauie]

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

[ghprince]

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

[yaauie]

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.

[ghprince]

ah I see! Thanks a lot for such a great explanation! Very clear! :+1: