Make sadd and srem always return an Integer. Add sadd? and srem?

Fix: #820

Also review all variadic commands to have consistent handling

Author: byroot

CHANGELOG.md

@@ -3,6 +3,8 @@
 # Unreleased 5.0.0
 
 - Use `MD5` for hashing server nodes in `Redis::Distributed`. This should improve keys distribution among servers. See #1089.
+- Changed `sadd` and `srem` to now always return an Integer.
+- Added `sadd?` and `srem?` which always return a Boolean.
 - Cluster support has been moved to a `redis_cluster` companion gem.
 - `select` no longer record the current database. If the client has to reconnect after `select` was used, it will reconnect to the original database.
 - Better support Float timeout in blocking commands. See #977.

lib/redis/commands.rb

@@ -40,14 +40,7 @@ module Commands
     # where the method call will return nil. Propagate the nil instead of falsely
     # returning false.
     Boolify = lambda { |value|
-      case value
-      when 1
-        true
-      when 0
-        false
-      else
-        value
-      end
+      value != 0 unless value.nil?
     }
 
     BoolifySet = lambda { |value|

lib/redis/commands/bitmaps.rb

@@ -39,7 +39,10 @@ def bitcount(key, start = 0, stop = -1)
       # @param [String, Array<String>] keys one or more source keys to perform `operation`
       # @return [Integer] the length of the string stored in `destkey`
       def bitop(operation, destkey, *keys)
-        send_command([:bitop, operation, destkey, *keys])
+        keys.flatten!(1)
+        command = [:bitop, operation, destkey]
+        command.concat(keys)
+        send_command(command)
       end
 
       # Return the position of the first bit set to 1 or 0 in a string.

lib/redis/commands/hashes.rb

@@ -87,7 +87,8 @@ def hget(key, field)
       #
       # @see #mapped_hmget
       def hmget(key, *fields, &blk)
-        send_command([:hmget, key] + fields, &blk)
+        fields.flatten!(1)
+        send_command([:hmget, key].concat(fields), &blk)
       end
 
       # Get the values of all the given hash fields.
@@ -102,7 +103,8 @@ def hmget(key, *fields, &blk)
       #
       # @see #hmget
       def mapped_hmget(key, *fields)
-        hmget(key, *fields) do |reply|
+        fields.flatten!(1)
+        hmget(key, fields) do |reply|
           if reply.is_a?(Array)
             Hash[fields.zip(reply)]
           else
@@ -152,7 +154,8 @@ def hrandfield(key, count = nil, withvalues: false, with_values: withvalues)
       # @param [String, Array<String>] field
       # @return [Integer] the number of fields that were removed from the hash
       def hdel(key, *fields)
-        send_command([:hdel, key, *fields])
+        fields.flatten!(1)
+        send_command([:hdel, key].concat(fields))
       end
 
       # Determine if a hash field exists.

lib/redis/commands/lists.rb

@@ -260,8 +260,8 @@ def _bpop(cmd, args, &blk)
           raise ArgumentError, "timeout must be an Integer or Float, got: #{timeout.class}"
         end
 
-        command = [cmd]
-        command.concat(args.flatten)
+        args.flatten!(1)
+        command = [cmd].concat(args)
         command << timeout
         send_blocking_command(command, timeout, &blk)
       end

lib/redis/commands/sets.rb

@@ -15,40 +15,40 @@ def scard(key)
       #
       # @param [String] key
       # @param [String, Array<String>] member one member, or array of members
-      # @return [Boolean, Integer] `Boolean` when a single member is specified,
-      #   holding whether or not adding the member succeeded, or `Integer` when an
-      #   array of members is specified, holding the number of members that were
-      #   successfully added
-      def sadd(key, member)
-        send_command([:sadd, key, member]) do |reply|
-          if member.is_a? Array
-            # Variadic: return integer
-            reply
-          else
-            # Single argument: return boolean
-            Boolify.call(reply)
-          end
-        end
+      # @return [Integer] The number of members that were successfully added
+      def sadd(key, *members)
+        members.flatten!(1)
+        send_command([:sadd, key].concat(members))
+      end
+
+      # Add one or more members to a set.
+      #
+      # @param [String] key
+      # @param [String, Array<String>] member one member, or array of members
+      # @return [Boolean] Wether at least one member was successfully added.
+      def sadd?(key, *members)
+        members.flatten!(1)
+        send_command([:sadd, key].concat(members), &Boolify)
       end
 
       # Remove one or more members from a set.
       #
       # @param [String] key
       # @param [String, Array<String>] member one member, or array of members
-      # @return [Boolean, Integer] `Boolean` when a single member is specified,
-      #   holding whether or not removing the member succeeded, or `Integer` when an
-      #   array of members is specified, holding the number of members that were
-      #   successfully removed
-      def srem(key, member)
-        send_command([:srem, key, member]) do |reply|
-          if member.is_a? Array
-            # Variadic: return integer
-            reply
-          else
-            # Single argument: return boolean
-            Boolify.call(reply)
-          end
-        end
+      # @return [Integer] The number of members that were successfully removed
+      def srem(key, *members)
+        members.flatten!(1)
+        send_command([:srem, key].concat(members))
+      end
+
+      # Remove one or more members from a set.
+      #
+      # @param [String] key
+      # @param [String, Array<String>] member one member, or array of members
+      # @return [Boolean] Wether at least one member was successfully removed.
+      def srem?(key, *members)
+        members.flatten!(1)
+        send_command([:srem, key].concat(members), &Boolify)
       end
 
       # Remove and return one or more random member from a set.
@@ -102,7 +102,8 @@ def sismember(key, member)
       # @param [String, Array<String>] members
       # @return [Array<Boolean>]
       def smismember(key, *members)
-        send_command([:smismember, key, *members]) do |reply|
+        members.flatten!(1)
+        send_command([:smismember, key].concat(members)) do |reply|
           reply.map(&Boolify)
         end
       end
@@ -120,7 +121,8 @@ def smembers(key)
       # @param [String, Array<String>] keys keys pointing to sets to subtract
       # @return [Array<String>] members in the difference
       def sdiff(*keys)
-        send_command([:sdiff, *keys])
+        keys.flatten!(1)
+        send_command([:sdiff].concat(keys))
       end
 
       # Subtract multiple sets and store the resulting set in a key.
@@ -129,15 +131,17 @@ def sdiff(*keys)
       # @param [String, Array<String>] keys keys pointing to sets to subtract
       # @return [Integer] number of elements in the resulting set
       def sdiffstore(destination, *keys)
-        send_command([:sdiffstore, destination, *keys])
+        keys.flatten!(1)
+        send_command([:sdiffstore, destination].concat(keys))
       end
 
       # Intersect multiple sets.
       #
       # @param [String, Array<String>] keys keys pointing to sets to intersect
       # @return [Array<String>] members in the intersection
       def sinter(*keys)
-        send_command([:sinter, *keys])
+        keys.flatten!(1)
+        send_command([:sinter].concat(keys))
       end
 
       # Intersect multiple sets and store the resulting set in a key.
@@ -146,15 +150,17 @@ def sinter(*keys)
       # @param [String, Array<String>] keys keys pointing to sets to intersect
       # @return [Integer] number of elements in the resulting set
       def sinterstore(destination, *keys)
-        send_command([:sinterstore, destination, *keys])
+        keys.flatten!(1)
+        send_command([:sinterstore, destination].concat(keys))
       end
 
       # Add multiple sets.
       #
       # @param [String, Array<String>] keys keys pointing to sets to unify
       # @return [Array<String>] members in the union
       def sunion(*keys)
-        send_command([:sunion, *keys])
+        keys.flatten!(1)
+        send_command([:sunion].concat(keys))
       end
 
       # Add multiple sets and store the resulting set in a key.
@@ -163,7 +169,8 @@ def sunion(*keys)
       # @param [String, Array<String>] keys keys pointing to sets to unify
       # @return [Integer] number of elements in the resulting set
       def sunionstore(destination, *keys)
-        send_command([:sunionstore, destination, *keys])
+        keys.flatten!(1)
+        send_command([:sunionstore, destination].concat(keys))
       end
 
       # Scan a set

lib/redis/commands/sorted_sets.rb

@@ -778,7 +778,8 @@ def zscan_each(key, **options, &block)
       private
 
       def _zsets_operation(cmd, *keys, weights: nil, aggregate: nil, with_scores: false)
-        command = [cmd, keys.size, *keys]
+        keys.flatten!(1)
+        command = [cmd, keys.size].concat(keys)
 
         if weights
           command << "WEIGHTS"
@@ -796,7 +797,8 @@ def _zsets_operation(cmd, *keys, weights: nil, aggregate: nil, with_scores: fals
       end
 
       def _zsets_operation_store(cmd, destination, keys, weights: nil, aggregate: nil)
-        command = [cmd, destination, keys.size, *keys]
+        keys.flatten!(1)
+        command = [cmd, destination, keys.size].concat(keys)
 
         if weights
           command << "WEIGHTS"

lib/redis/commands/strings.rb

@@ -202,6 +202,7 @@ def get(key)
       #
       # @see #mapped_mget
       def mget(*keys, &blk)
+        keys.flatten!(1)
         send_command([:mget, *keys], &blk)
       end