Set expiration atomically

fixes #225
fixes #159

lib/redis/base_object.rb

@@ -22,18 +22,27 @@ def redis
 
     def set_expiration
       if !@options[:expiration].nil?
-        redis.expire(@key, @options[:expiration]) if redis.ttl(@key) < 0
+        redis.expire(@key, @options[:expiration])
       elsif !@options[:expireat].nil?
         expireat = @options[:expireat]
         at = expireat.respond_to?(:call) ? expireat.call.to_i : expireat.to_i
-        redis.expireat(@key, at) if redis.ttl(@key) < 0
+        redis.expireat(@key, at)
       end
     end
 
-    def allow_expiration(&block)
-      result = block.call
-      set_expiration
-      result
+    def allow_expiration
+      expiration_set = false
+      result =
+        redis.multi do
+          yield
+          expiration_set = set_expiration
+        end
+      # Nested calls to multi/pipelined return `nil`,
+      # return value should be handled by outer call to multi/pipelined.
+      return if result.nil?
+
+      result.pop if expiration_set
+      result.size == 1 ? result.first : result
     end
 
     def to_json(*args)

lib/redis/counter.rb

@@ -21,10 +21,8 @@ def initialize(key, *args)
     # with a parent and starting over (for example, restarting a game and
     # disconnecting all players).
     def reset(to=options[:start])
-      allow_expiration do
-        redis.set key, to.to_i
-        true  # hack for redis-rb regression
-      end
+      allow_expiration { redis.set(key, to.to_i) }
+      true  # hack for redis-rb regression
     end
 
     # Reset the counter to its starting value, and return previous value.
@@ -45,13 +43,9 @@ def value
     alias_method :get, :value
 
     def value=(val)
-      allow_expiration do
-        if val.nil?
-          delete
-        else
-          redis.set key, val
-        end
-      end
+      return delete if val.nil?
+
+      allow_expiration { redis.set(key, val) }
     end
     alias_method :set, :value=
 
@@ -66,10 +60,8 @@ def to_f
     # counter will automatically be decremented to its previous value.  This
     # method is aliased as incr() for brevity.
     def increment(by=1, &block)
-      allow_expiration do
-        val = redis.incrby(key, by).to_i
-        block_given? ? rewindable_block(:decrement, by, val, &block) : val
-      end
+      val = allow_expiration { redis.incrby(key, by) }.to_i
+      block_given? ? rewindable_block(:decrement, by, val, &block) : val
     end
     alias_method :incr, :increment
     alias_method :incrby, :increment
@@ -80,30 +72,24 @@ def increment(by=1, &block)
     # counter will automatically be incremented to its previous value.  This
     # method is aliased as decr() for brevity.
     def decrement(by=1, &block)
-      allow_expiration do
-        val = redis.decrby(key, by).to_i
-        block_given? ? rewindable_block(:increment, by, val, &block) : val
-      end
+      val = allow_expiration { redis.decrby(key, by) }.to_i
+      block_given? ? rewindable_block(:increment, by, val, &block) : val
     end
     alias_method :decr, :decrement
     alias_method :decrby, :decrement
 
     # Increment a floating point counter atomically.
     # Redis uses separate API's to interact with integers vs floats.
     def incrbyfloat(by=1.0, &block)
-      allow_expiration do
-        val = redis.incrbyfloat(key, by).to_f
-        block_given? ? rewindable_block(:decrbyfloat, by, val, &block) : val
-      end
+      val = allow_expiration { redis.incrbyfloat(key, by) }.to_f
+      block_given? ? rewindable_block(:decrbyfloat, by, val, &block) : val
     end
 
     # Decrement a floating point counter atomically.
     # Redis uses separate API's to interact with integers vs floats.
     def decrbyfloat(by=1.0, &block)
-      allow_expiration do
-        val = redis.incrbyfloat(key, -by).to_f
-        block_given? ? rewindable_block(:incrbyfloat, -by, val, &block) : val
-      end
+      val = allow_expiration { redis.incrbyfloat(key, -by) }.to_f
+      block_given? ? rewindable_block(:incrbyfloat, -by, val, &block) : val
     end
 
     ##

lib/redis/hash_key.rb

@@ -12,15 +12,13 @@ def initialize(key, *args)
 
     # Redis: HSET
     def store(field, value)
-      allow_expiration do
-        redis.hset(key, field, marshal(value, options[:marshal_keys][field]))
-      end
+      allow_expiration { redis.hset(key, field, marshal(value, options[:marshal_keys][field])) }
     end
     alias_method :[]=, :store
 
     # Redis: HGET
     def hget(field)
-      unmarshal redis.hget(key, field), options[:marshal_keys][field]
+      unmarshal(redis.hget(key, field), options[:marshal_keys][field])
     end
     alias_method :get, :hget
     alias_method :[],  :hget
@@ -134,14 +132,7 @@ def bulk_values(*keys)
 
     # Increment value by integer at field. Redis: HINCRBY
     def incrby(field, by=1)
-      allow_expiration do
-        ret = redis.hincrby(key, field, by)
-        unless ret.is_a? Array
-          ret.to_i
-        else
-          nil
-        end
-      end
+      allow_expiration { redis.hincrby(key, field, by) }.to_i
     end
     alias_method :incr, :incrby
 
@@ -153,14 +144,7 @@ def decrby(field, by=1)
 
     # Increment value by float at field. Redis: HINCRBYFLOAT
     def incrbyfloat(field, by=1.0)
-      allow_expiration do
-        ret = redis.hincrbyfloat(key, field, by)
-        unless ret.is_a? Array
-          ret.to_f
-        else
-          nil
-        end
-      end
+      allow_expiration { redis.hincrbyfloat(key, field, by) }.to_f
     end
 
     # Decrement value by float at field. Redis: HINCRBYFLOAT

lib/redis/list.rb

@@ -13,32 +13,30 @@ def <<(value)
     end
 
     # Add a member before or after pivot in the list. Redis: LINSERT
-    def insert(where,pivot,value)
-      allow_expiration do
-        redis.linsert(key,where,marshal(pivot),marshal(value))
-      end
+    def insert(where, pivot, value)
+      allow_expiration { redis.linsert(key, where, marshal(pivot), marshal(value)) }
     end
 
     # Add a member to the end of the list. Redis: RPUSH
     def push(*values)
-      allow_expiration do
-        count = redis.rpush(key, values.map{|v| marshal(v) })
-        redis.ltrim(key, -options[:maxlength], -1) if options[:maxlength]
-        count
-      end
+      count, =
+        allow_expiration do
+          redis.rpush(key, values.map { |v| marshal(v) })
+          redis.ltrim(key, -options[:maxlength], -1) if options[:maxlength]
+        end
+      count
     end
 
     # Remove a member from the end of the list. Redis: RPOP
     def pop(n=nil)
-      if n
-        result, = redis.multi do
+      return unmarshal(redis.rpop(key)) unless n
+
+      result, =
+        redis.multi do
           redis.lrange(key, -n, -1)
           redis.ltrim(key, 0, -n - 1)
         end
-        unmarshal result
-      else
-        unmarshal redis.rpop(key)
-      end
+      unmarshal(result)
     end
 
     # Atomically pops a value from one list, pushes to another and returns the
@@ -55,24 +53,26 @@ def rpoplpush(destination)
 
     # Add a member to the start of the list. Redis: LPUSH
     def unshift(*values)
-      allow_expiration do
-        count = redis.lpush(key, values.map{|v| marshal(v) })
-        redis.ltrim(key, 0, options[:maxlength] - 1) if options[:maxlength]
-        count
-      end
+      count, =
+        allow_expiration do
+          redis.multi do
+            redis.lpush(key, values.map { |v| marshal(v) })
+            redis.ltrim(key, 0, options[:maxlength] - 1) if options[:maxlength]
+          end
+        end
+      count
     end
 
     # Remove a member from the start of the list. Redis: LPOP
     def shift(n=nil)
-      if n
-        result, = redis.multi do
+      return unmarshal(redis.lpop(key)) unless n
+
+      result, =
+        redis.multi do
           redis.lrange(key, 0, n - 1)
           redis.ltrim(key, n, -1)
         end
-        unmarshal result
-      else
-        unmarshal redis.lpop(key)
-      end
+      unmarshal(result)
     end
 
     # Return all values in the list. Redis: LRANGE(0,-1)
@@ -103,9 +103,7 @@ def [](index, length=nil)
 
     # Same functionality as Ruby arrays.
     def []=(index, value)
-      allow_expiration do
-        redis.lset(key, index, marshal(value))
-      end
+      allow_expiration { redis.lset(key, index, marshal(value)) }
     end
 
     # Delete the element(s) from the list that match name. If count is specified,

lib/redis/set.rb

@@ -14,9 +14,9 @@ def <<(value)
     # Add the specified value to the set only if it does not exist already.
     # Redis: SADD
     def add(value)
-      allow_expiration do
-        redis.sadd(key, marshal(value)) if value.nil? || !Array(value).empty?
-      end
+      return unless value.nil? || !Array(value).empty?
+
+      allow_expiration { redis.sadd(key, marshal(value)) }
     end
 
     # Remove and return a random member.  Redis: SPOP
@@ -32,15 +32,13 @@ def randmember(count = nil)
     # Adds the specified values to the set. Only works on redis > 2.4
     # Redis: SADD
     def merge(*values)
-      allow_expiration do
-        redis.sadd(key, values.flatten.map{|v| marshal(v)})
-      end
+      allow_expiration { redis.sadd(key, values.flatten.map { |v| marshal(v) }) }
     end
 
     # Return all members in the set.  Redis: SMEMBERS
     def members
       vals = redis.smembers(key)
-      vals.nil? ? [] : vals.map{|v| unmarshal(v) }
+      vals.nil? ? [] : vals.map { |v| unmarshal(v) }
     end
     alias_method :get, :members
     alias_method :value, :members

lib/redis/sorted_set.rb

@@ -16,9 +16,7 @@ def []=(member, score)
     # arguments to this are flipped; the member comes first rather than
     # the score, since the member is the unique item (not the score).
     def add(member, score)
-      allow_expiration do
-        redis.zadd(key, score, marshal(member))
-      end
+      allow_expiration { redis.zadd(key, score, marshal(member)) }
     end
 
     # Add a list of members and their corresponding value (or a hash mapping
@@ -27,7 +25,7 @@ def add(member, score)
     # item (not the score).
     def merge(values)
       allow_expiration do
-        vals = values.map{|v,s| [s, marshal(v)] }
+        vals = values.map { |v, s| [s, marshal(v)] }
         redis.zadd(key, vals)
       end
     end
@@ -117,7 +115,7 @@ def rangebyscore(min, max, options={})
                 options[:offset] || options[:limit] || options[:count]
       args[:with_scores] = true if options[:withscores] || options[:with_scores]
 
-      redis.zrangebyscore(key, min, max, args).map{|v| unmarshal(v) }
+      redis.zrangebyscore(key, min, max, args).map { |v| unmarshal(v) }
     end
 
     # Returns all the elements in the sorted set at key with a score between max and min
@@ -153,9 +151,7 @@ def remrangebyscore(min, max)
 
     # Delete the value from the set.  Redis: ZREM
     def delete(value)
-      allow_expiration do
-        redis.zrem(key, marshal(value))
-      end
+      allow_expiration { redis.zrem(key, marshal(value)) }
     end
 
     # Delete element if it matches block
@@ -173,18 +169,14 @@ def delete_if(&block)
     # Increment the rank of that member atomically and return the new value. This
     # method is aliased as incr() for brevity. Redis: ZINCRBY
     def increment(member, by=1)
-      allow_expiration do
-        zincrby(member, by)
-      end
+      allow_expiration { zincrby(member, by) }.to_i
     end
     alias_method :incr, :increment
     alias_method :incrby, :increment
 
     # Convenience to calling increment() with a negative number.
     def decrement(member, by=1)
-      allow_expiration do
-        zincrby(member, -by)
-      end
+      allow_expiration { zincrby(member, -by) }.to_i
     end
     alias_method :decr, :decrement
     alias_method :decrby, :decrement
@@ -206,9 +198,8 @@ def intersection(*sets)
 
       redis.multi do
         interstore(temp_key, *sets)
-        redis.expire(temp_key, 1)
-
         result = redis.zrange(temp_key, 0, -1)
+        redis.del(temp_key)
       end
 
       result.value
@@ -243,9 +234,8 @@ def union(*sets)
 
       redis.multi do
         unionstore(temp_key, *sets)
-        redis.expire(temp_key, 1)
-
         result = redis.zrange(temp_key, 0, -1)
+        redis.del(temp_key)
       end
 
       result.value
@@ -254,7 +244,7 @@ def union(*sets)
     alias_method :+, :union
 
     # Calculate the union and store it in Redis as +name+. Returns the number
-    # of elements in the stored union. Redis: SUNIONSTORE
+    # of elements in the stored union. Redis: ZUNIONSTORE
     def unionstore(name, *sets)
       allow_expiration do
         opts = sets.last.is_a?(Hash) ? sets.pop : {}
@@ -319,7 +309,7 @@ def keys_from_objects(sets)
     end
 
     def zincrby(member, by)
-      redis.zincrby(key, by, marshal(member)).to_i
+      redis.zincrby(key, by, marshal(member))
     end
   end
 end