read multi vs namespace/normalized/escaped keys

lib/active_support/cache/libmemcached_local_store.rb

@@ -34,26 +34,23 @@ def read(*args)
       end
 
       # make read multi hit local cache
-      def read_multi(*names)
+      def read_multi_entry(keys, options)
         return super unless cache = local_cache
 
-        options = names.extract_options!
-
-        missing_names = []
+        missing_keys = []
 
         # We write raw values to the local cache, unlike rails MemcachedStore, so we cannot use local_cache.read_multi.
         # Once read_multi_entry is available we can switch to that.
-        results = names.each_with_object({}) do |name, results|
-          value = local_cache.fetch_entry(name) do
-            missing_names << name
+        results = keys.each_with_object({}) do |key, results|
+          value = local_cache.fetch_entry(key) do
+            missing_keys << key
             nil
           end
-          results[name] = value unless value.nil?
+          results[key] = value unless value.nil?
         end
 
-        if missing_names.any?
-          missing_names << options
-          missing = super(*missing_names)
+        if missing_keys.any?
+          missing = super(missing_keys, options)
           missing.each { |k,v| cache.write_entry(k, v, nil) }
           results.merge!(missing)
         end
@@ -94,9 +91,11 @@ def read_entry(key, options)
 
       # for memcached writing raw means writing a string and not the actual value
       def write_entry(key, entry, options) # :nodoc:
+        return super unless cache = local_cache
+
         written = super
-        if options && options[:raw] && local_cache && written
-          local_cache.write_entry(key, Entry.new(entry.to_s), options)
+        if options && options[:raw] && written
+          cache.write_entry(key, Entry.new(entry.to_s), options)
         end
         written
       end

lib/active_support/cache/libmemcached_store.rb

@@ -91,7 +91,7 @@ def fetch(key, options = nil, &block)
           if options && options[:race_condition_ttl] && options[:expires_in]
             fetch_with_race_condition_ttl(key, options, &block)
           else
-            key = expanded_key(key)
+            key = normalize_key(key)
             unless options && options[:force]
               entry = instrument(:read, key, options) do |payload|
                 read_entry(key, options).tap do |result|
@@ -120,7 +120,7 @@ def fetch(key, options = nil, &block)
       end
 
       def read(key, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:read, key, options) do |payload|
           entry = read_entry(key, options)
           payload[:hit] = !!entry if payload
@@ -129,24 +129,24 @@ def read(key, options = nil)
       end
 
       def write(key, value, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:write, key, options) do |payload|
           write_entry(key, value, options)
         end
       end
 
       def delete(key, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:delete, key) do |payload|
           delete_entry(key, options)
         end
       end
 
       def exist?(key, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:exist?, key) do |payload|
           if @cache.respond_to?(:exist)
-            @cache.exist(escape_and_normalize(key))
+            @cache.exist(key)
             true
           else
             read_entry(key, options) != nil
@@ -157,9 +157,9 @@ def exist?(key, options = nil)
       end
 
       def increment(key, amount = 1, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:increment, key, amount: amount) do
-          @cache.incr(escape_and_normalize(key), amount)
+          @cache.incr(key, amount)
         end
       rescue Memcached::NotFound
         nil
@@ -169,9 +169,9 @@ def increment(key, amount = 1, options = nil)
       end
 
       def decrement(key, amount = 1, options = nil)
-        key = expanded_key(key)
+        key = normalize_key(key)
         instrument(:decrement, key, amount: amount) do
-          @cache.decr(escape_and_normalize(key), amount)
+          @cache.decr(key, amount)
         end
       rescue Memcached::NotFound
         nil
@@ -186,17 +186,15 @@ def read_multi(*names)
 
         return {} if names.empty?
 
-        mapping = Hash[names.map {|name| [escape_and_normalize(expanded_key(name)), name] }]
+        mapping = Hash[names.map {|name| [normalize_key(name), name] }]
         keys = mapping.keys
-        raw_values, flags = instrument(:read_multi, keys, options) do
-          @cache.get(keys, false, true)
-        end
-
-        values = {}
-        raw_values.each do |key, value|
-          values[mapping[key]] = convert_race_condition_entry(deserialize(value, options[:raw], flags[key]))
+        instrument(:read_multi, keys, options) do
+          values = {}
+          read_multi_entry(keys, options).each do |key, value|
+            values[mapping[key]] = value
+          end
+          values
         end
-        values
       rescue Memcached::Error => e
         log_error(e)
         {}
@@ -214,9 +212,18 @@ def stats
 
       protected
 
+      def read_multi_entry(keys, options)
+        raw_values, flags = @cache.get(keys, false, true)
+        values = {}
+        raw_values.each do |key, value|
+          values[key] = convert_race_condition_entry(deserialize(value, options[:raw], flags[key]))
+        end
+        values
+      end
+
       def read_entry(key, options = nil)
         options ||= {}
-        raw_value, flags = @cache.get(escape_and_normalize(key), false, true)
+        raw_value, flags = @cache.get(key, false, true)
         value = deserialize(raw_value, options[:raw], flags)
         convert_race_condition_entry(value, options)
       rescue Memcached::NotFound
@@ -237,15 +244,15 @@ def write_entry(key, entry, options = nil)
           flags |= FLAG_COMPRESSED
         end
 
-        @cache.send(method, escape_and_normalize(key), entry, options[:expires_in].to_i, false, flags)
+        @cache.send(method, key, entry, options[:expires_in].to_i, false, flags)
         true
       rescue Memcached::Error => e
         log_error(e)
         false
       end
 
       def delete_entry(key, options = nil)
-        @cache.delete(escape_and_normalize(key))
+        @cache.delete(key)
         true
       rescue Memcached::NotFound
         false
@@ -296,6 +303,10 @@ def deserialize(value, raw = false, flags = 0)
         value
       end
 
+      def normalize_key(key)
+        escape_and_normalize(expanded_key(key))
+      end
+
       def escape_and_normalize(key)
         key = key.to_s.force_encoding("BINARY").gsub(ESCAPE_KEY_CHARS) { |match| "%#{match.getbyte(0).to_s(16).upcase}" }
         key_length = key.length

test/active_support/libmemcached_local_store_test.rb

@@ -127,5 +127,14 @@ def self.it_wawo(description, &block)
       assert_equal expected, store.read_multi('a', 'b', 'c')
       assert_equal({}, store.read_multi)
     end
+
+    it "can read interchangeable keys with read_multi" do
+      @cache.with_local_cache do
+        @cache.write 'x', 1 # write plain key
+        @cache.read_multi('x').must_equal('x' => 1)
+        @memcache.set 'x', 2
+        @cache.read_multi(['x']).must_equal('x' => 1) # reads normalized key
+      end
+    end
   end
 end