refactor init_default_in_multi to not use multi if default not defined

Author: tleish

lib/kredis/types/counter.rb

@@ -4,17 +4,11 @@ class Kredis::Types::Counter < Kredis::Types::Proxying
   attr_accessor :expires_in
 
   def increment(by: 1)
-    multi do
-      initialize_with_default
-      incrby by
-    end[-1]
+    init_default_in_multi { incrby by }
   end
 
   def decrement(by: 1)
-    multi do
-      initialize_with_default
-      decrby by
-    end[-1]
+    init_default_in_multi { decrby by }
   end
 
   def value
@@ -26,7 +20,11 @@ def reset
   end
 
   private
-    def initialize_with_default
-      set default.to_i, ex: expires_in, nx: true
+    def set_default(value)
+      set value.to_i, ex: expires_in, nx: true
+    end
+
+    def default
+      super.to_i
     end
 end

lib/kredis/types/enum.rb

@@ -1,7 +1,7 @@
 require "active_support/core_ext/object/inclusion"
 
 class Kredis::Types::Enum < Kredis::Types::Proxying
-  proxying :multi, :set, :get, :del, :exists?
+  proxying :set, :get, :del, :exists?
 
   attr_accessor :values
 
@@ -17,10 +17,7 @@ def value=(value)
   end
 
   def value
-    multi do
-      initialize_with_default
-      get
-    end[-1]
+    get || default.presence_in(values)
   end
 
   def reset
@@ -34,10 +31,4 @@ def define_predicates_for_values
         define_singleton_method("#{defined_value}!") { self.value = defined_value }
       end
     end
-
-    def set_default(value)
-      if validated_choice = value.presence_in(values)
-        set validated_choice, nx: true
-      end
-    end
 end

lib/kredis/types/hash.rb

@@ -1,6 +1,8 @@
 require "active_support/core_ext/hash"
 
 class Kredis::Types::Hash < Kredis::Types::Proxying
+  ZERO_FIELDS_ADDED = 0
+
   proxying :hset, :hdel, :hgetall, :del, :exists?, :multi, :callnx
 
   attr_accessor :typed
@@ -15,9 +17,10 @@ def []=(key, value)
 
 
   def update(**entries)
-    multi do
-      initialize_with_default
-      hset entries.transform_values{ |val| type_to_string(val, typed) } if entries.flatten.any?
+    return ZERO_FIELDS_ADDED if entries.flatten.blank?
+
+    init_default_in_multi do
+      hset entries.transform_values{ |val| type_to_string(val, typed) }
     end
   end
 
@@ -26,10 +29,9 @@ def values_at(*keys)
   end
 
   def delete(*keys)
-    multi do
-      initialize_with_default
-      hdel keys if keys.flatten.any?
-    end
+    return ZERO_FIELDS_ADDED if keys.flatten.blank?
+
+    init_default_in_multi { hdel keys }
   end
 
   def remove

lib/kredis/types/list.rb

@@ -4,38 +4,32 @@ class Kredis::Types::List < Kredis::Types::Proxying
   attr_accessor :typed
 
   def elements
-    values = multi do
-      initialize_with_default
-      lrange(0, -1)
-    end[-1]
+    values = init_default_in_multi { lrange(0, -1) }
     strings_to_types(values || [], typed)
   end
   alias to_a elements
 
   def remove(*elements)
-    return if elements.empty?
+    return [] if elements.flatten.blank?
 
-    multi do
-      initialize_with_default
+    init_default_in_multi do
       types_to_strings(elements, typed).each { |element| lrem 0, element }
     end
   end
 
   def prepend(*elements)
-    return if elements.empty?
+    return self.elements.count if elements.flatten.blank?
 
-    multi do
-      initialize_with_default
-      lpush types_to_strings(elements, typed) if elements.flatten.any?
+    init_default_in_multi do
+      lpush types_to_strings(elements, typed)
     end
   end
 
   def append(*elements)
-    return if elements.empty?
+    return self.elements.count if elements.flatten.blank?
 
-    multi do
-      initialize_with_default
-      rpush types_to_strings(elements, typed) if elements.flatten.any?
+    init_default_in_multi do
+      rpush types_to_strings(elements, typed)
     end
   end
   alias << append

lib/kredis/types/proxying.rb

@@ -29,12 +29,19 @@ def default
       end
     end
 
-    def initialize_with_default
-      default_value = default
-      set_default(default_value) if default_value.present?
+    def init_default_in_multi(&block)
+      if (default_value = default).blank?
+        block.call
+      else
+        multi_results = multi do
+          set_default(default_value)
+          block.call
+        end
+        Array(multi_results)[-1] # convert to array in case in the middle of nested multi
+      end
     end
 
     def set_default(value)
-      set type_to_string(value, typed), ex: expires_in, nx: true
+      raise NotImplementedError, "kredis type needs to define #set_default"
     end
 end

lib/kredis/types/scalar.rb

@@ -8,11 +8,7 @@ def value=(value)
   end
 
   def value
-    get_value = multi do
-      initialize_with_default
-      get
-    end[-1]
-    string_to_type(get_value, typed)
+    string_to_type(init_default_in_multi{ get }, typed)
   end
 
   def to_s
@@ -34,4 +30,9 @@ def expire_in(seconds)
   def expire_at(datetime)
     expireat datetime.to_i
   end
+
+  private
+    def set_default(value)
+      set type_to_string(value, typed), ex: expires_in, nx: true
+    end
 end

lib/kredis/types/set.rb

@@ -4,55 +4,47 @@ class Kredis::Types::Set < Kredis::Types::Proxying
   attr_accessor :typed
 
   def members
-    values = multi do
-      initialize_with_default
-      smembers
-    end[-1]
+    values = init_default_in_multi { smembers }
     strings_to_types(values || [], typed).sort
   end
   alias to_a members
 
   def add(*members)
-    multi do
-      initialize_with_default
-      sadd types_to_strings(members, typed) if members.flatten.any?
+    return size if members.flatten.blank?
+
+    init_default_in_multi do
+      sadd types_to_strings(members, typed)
     end
   end
   alias << add
 
   def remove(*members)
-    multi do
-      initialize_with_default
-      srem types_to_strings(members, typed) if members.flatten.any?
+    return size if members.flatten.blank?
+
+    init_default_in_multi do
+      srem types_to_strings(members, typed)
     end
   end
 
   def replace(*members)
+    return size if members.flatten.blank?
+
     multi do
       del
       add members
-    end
+    end[-1]
   end
 
   def include?(member)
-    multi do
-      initialize_with_default
-      sismember type_to_string(member, typed)
-    end[-1]
+    init_default_in_multi { sismember type_to_string(member, typed) }
   end
 
   def size
-    multi do
-      initialize_with_default
-      scard
-    end[-1].to_i
+    init_default_in_multi { scard }.to_i
   end
 
   def take
-    multi do
-      initialize_with_default
-      spop
-    end[-1]
+    init_default_in_multi { spop }
   end
 
   def clear

test/types/enum_test.rb

@@ -13,8 +13,10 @@ class EnumTest < ActiveSupport::TestCase
   end
 
   test "does not set default for invalid option" do
-    @enum = Kredis.enum "myenum", values: %w[ one two three ], default: ->() { "four" }
-    assert_nil @enum.value
+    enum = Kredis.enum "myenum1", values: [ 1, 2, 3 ], default: ->() { nil }
+    assert_nil enum.value
+    enum = Kredis.enum "myenum2", values: [ 1, 2, 3 ], default: ->() { 4 }
+    assert_nil enum.value
   end
 
   test "predicates" do

test/types/hash_test.rb

@@ -18,7 +18,15 @@ class HashTest < ActiveSupport::TestCase
   end
 
   test "update" do
-    @hash.update(key: :value)
+    update_count = @hash.update(key: :value)
+    assert_equal(1, update_count)
+
+    update_count = @hash.update(key: :value)
+    assert_equal(0, update_count)
+
+    update_count = @hash.update
+    assert_equal(0, update_count)
+
     @hash.update("key2" => "value2", "key3" => "value3")
     assert_equal({ "key" => "value", "key2" => "value2", "key3" => "value3" }, @hash.to_h)
   end
@@ -33,12 +41,17 @@ class HashTest < ActiveSupport::TestCase
     @hash.update("key2" => "value2", "key3" => "value3")
     assert_equal({ "key" => "value", "key2" => "value2", "key3" => "value3" }, @hash.to_h)
 
-    @hash.delete("key")
+    delete_count = @hash.delete("key")
+    assert_equal(1, delete_count)
     assert_equal({ "key2" => "value2", "key3" => "value3" }, @hash.to_h)
 
-    @hash.delete("bogus")
+    delete_count = @hash.delete("bogus")
+    assert_equal(0, delete_count)
     assert_equal({ "key2" => "value2", "key3" => "value3" }, @hash.to_h)
 
+    delete_count = @hash.delete
+    assert_equal(0, delete_count)
+
     @hash.delete("key2", "key3")
     assert_equal({}, @hash.to_h)
   end

test/types/list_test.rb

@@ -11,8 +11,10 @@ class ListTest < ActiveSupport::TestCase
   end
 
   test "append nothing" do
-    @list.append(%w[ 1 2 3 ])
-    @list.append([])
+    list_length = @list.append(%w[ 1 2 3 ])
+    assert_equal 3, list_length
+    list_length = @list.append([])
+    assert_equal 3, list_length
     assert_equal %w[ 1 2 3 ], @list.to_a
   end
 
@@ -23,8 +25,10 @@ class ListTest < ActiveSupport::TestCase
   end
 
   test "prepend nothing" do
-    @list.prepend("1", "2", "3")
-    @list.prepend([])
+    list_length = @list.prepend("1", "2", "3")
+    assert_equal 3, list_length
+    list_length = @list.prepend([])
+    assert_equal 3, list_length
     assert_equal %w[ 3 2 1 ], @list.elements
   end
 
@@ -35,6 +39,16 @@ class ListTest < ActiveSupport::TestCase
     assert_equal %w[ 4 ], @list.elements
   end
 
+  test "remove nothing" do
+    @list.append(%w[ 1 2 3 4 ])
+    @list.remove(%w[ 1 2 ])
+    removed_items = @list.remove(3)
+    assert_equal %w[ 3 ], removed_items
+    removed_items = @list.remove
+    assert_equal [], removed_items
+    assert_equal %w[ 4 ], @list.elements
+  end
+
   test "clear" do
     @list.append(%w[ 1 2 3 4 ])
     @list.clear