[ISSUE-26] Async writes using concurrent-ruby (#61)

* [ISSUE-26] Async writes using concurrent-ruby

* [ISSUE-26] Keep original_params keytype consistent

Additionally, remove an unnecessary read from cache immediately after writing to it to prevent an empty read incase of a race condition where the parallel write does not do it first.

* [ISSUE-26] Updated README with new config option

README.md

@@ -55,15 +55,17 @@ end
 CachedResource accepts the following options:
 
 * `:enabled` Default: `true`
-* `:ttl` The time in seconds until the cache should expire. Default: `604800`
-* `:race_condition_ttl` The race condition ttl, to prevent [dog pile effect](https://en.wikipedia.org/wiki/Cache_stampede) or [cache stampede](https://en.wikipedia.org/wiki/Cache_stampede). Default: 86400
-* `:ttl_randomization` Enable ttl randomization. Default: `false`
-* `:ttl_randomization_scale` A Range from which a random value will be selected to scale the ttl. Default: `1..2`
-* `:collection_synchronize` Use collections to generate cache entries for individuals.  Update the existing cached principal collection when retrieving subsets of the principal collection or individuals.  Default: `false`
+* `:cache_collections` Set to false to always remake a request for collections. Default: `true`
+* `:cache` The cache store that CacheResource should use. Default: The `Rails.cache` if available, or an `ActiveSupport::Cache::MemoryStore`
 * `:collection_arguments` The arguments that identify the principal collection request. Default: `[:all]`
+* `:collection_synchronize` Use collections to generate cache entries for individuals.  Update the existing cached principal collection when retrieving subsets of the principal collection or individuals.  Default: `false`
+* `:concurrent_write` Set to true to make concurrent writes to cache after successful API response. Default: `false`
+    * Requires the [concurrent-ruby](https://rubygems.org/gems/concurrent-ruby) gem
 * `:logger` The logger to which CachedResource messages should be written. Default: The `Rails.logger` if available, or an `ActiveSupport::Logger`
-* `:cache` The cache store that CacheResource should use. Default: The `Rails.cache` if available, or an `ActiveSupport::Cache::MemoryStore`
-* `:cache_collections` Set to false to always remake a request for collections. Default: `true`
+* `:race_condition_ttl` The race condition ttl, to prevent [dog pile effect](https://en.wikipedia.org/wiki/Cache_stampede) or [cache stampede](https://en.wikipedia.org/wiki/Cache_stampede). Default: 86400
+* `:ttl_randomization_scale` A Range from which a random value will be selected to scale the ttl. Default: `1..2`
+* `:ttl_randomization` Enable ttl randomization. Default: `false`
+* `:ttl` The time in seconds until the cache should expire. Default: `604800`
 
 You can set them like this:
 

cached_resource.gemspec

@@ -25,4 +25,5 @@ Gem::Specification.new do |s|
 
   s.add_development_dependency "rake"
   s.add_development_dependency "rspec"
+  s.add_development_dependency "concurrent-ruby", '~> 1.2', '>= 1.2.3'
 end

lib/cached_resource/cached_resource.rb

@@ -18,6 +18,16 @@ def cached_resource(options={})
         # and establishing the necessary methods.
         def setup_cached_resource!(options)
           @cached_resource = CachedResource::Configuration.new(options)
+          if @cached_resource.concurrent_write
+            begin
+              send :require, 'concurrent/promise'
+            rescue LoadError
+              @cached_resource.logger.error(
+                "`concurrent_write` option is enabled, but `concurrent-ruby` is not an installed dependency"
+              )
+              raise
+            end
+          end
           send :include, CachedResource::Caching
           @cached_resource
         end

lib/cached_resource/caching.rb

@@ -45,7 +45,7 @@ def find_via_reload(key, *arguments)
         cache_collection_synchronize(object, *arguments) if cached_resource.collection_synchronize
         return object if !cached_resource.cache_collections && is_any_collection?(*arguments)
         cache_write(key, object, *arguments)
-        cache_read(key)
+        object
       end
 
       # If this is a pure, unadulterated "all" request
@@ -82,7 +82,7 @@ def update_collection_cache(updates, *arguments)
       # Avoid cache nil or [] objects
       def should_cache?(object)
         return false unless cached_resource.enabled
-        object.respond_to?(:empty?) ? !object.empty? : !!object
+        object.present?
       end
 
       # Determine if the given arguments represent
@@ -110,7 +110,7 @@ def cache_read(key)
               next restored unless respond_to?(:collection_parser)
               collection_parser.new(restored).tap do |parser|
                 parser.resource_class = self
-                parser.original_params = json['original_params']
+                parser.original_params = json['original_params'].deep_symbolize_keys
               end
             else
               full_dup(cache)
@@ -123,6 +123,14 @@ def cache_read(key)
 
       # Write an entry to the cache for the given key and value.
       def cache_write(key, object, *arguments)
+        if cached_resource.concurrent_write
+          Concurrent::Promise.execute { _cache_write(key, object, *arguments) } && true
+        else
+          _cache_write(key, object, *arguments)
+        end
+      end
+
+      def _cache_write(key, object, *arguments)
         options = arguments[1] || {}
         params = options[:params]
         prefix_options, query_options = split_options(params)

lib/cached_resource/configuration.rb

@@ -24,18 +24,20 @@ class Configuration < OpenStruct
     # :cache, default: Rails.cache or ActiveSupport::Cache::MemoryStore.new,
     # :logger, default: Rails.logger or ActiveSupport::Logger.new(NilIO.new),
     # :cache_collections, default: true
+    # :concurrent_write, default: false
     def initialize(options={})
       super({
         :enabled => true,
+        :cache => defined?(Rails.cache)  && Rails.cache || CACHE,
+        :cache_collections => true,
+        :collection_arguments => [:all],
+        :collection_synchronize => false,
+        :concurrent_write => false,
+        :logger => defined?(Rails.logger) && Rails.logger || LOGGER,
         :race_condition_ttl => 86400,
         :ttl => 604800,
         :ttl_randomization => false,
-        :ttl_randomization_scale => 1..2,
-        :collection_synchronize => false,
-        :collection_arguments => [:all],
-        :cache => defined?(Rails.cache)  && Rails.cache || CACHE,
-        :logger => defined?(Rails.logger) && Rails.logger || LOGGER,
-        :cache_collections => true
+        :ttl_randomization_scale => 1..2
       }.merge(options))
     end
 
@@ -72,4 +74,4 @@ def sample_range(range, seed=nil)
     end
 
   end
-end
+end

spec/cached_resource/caching_spec.rb

@@ -66,19 +66,27 @@ class NotTheThing < ActiveResource::Base
       end
     end
 
-    it "should cache a response" do
-      result = Thing.find(1)
-      read_from_cache("thing/1").should == result
-    end
+    shared_examples "caching" do
+      it "should cache a response" do
+        result = Thing.find(1)
+        read_from_cache("thing/1").should == result
+      end
+
+      it "shouldn't cache nil response" do
+        Thing.find(:all, :params => { :name => '42' })
+        read_from_cache("thing/all/name/42").should == nil
+      end
 
-    it "shouldn't cache nil response" do
-      Thing.find(:all, :params => { :name => '42' })
-      read_from_cache("thing/all/name/42").should == nil
+      it "shouldn't cache blank response" do
+        Thing.find(:all, :params => { :name => '43' })
+        read_from_cache("thing/all/name/43").should == nil
+      end
     end
 
-    it "shouldn't cache [] response" do
-      Thing.find(:all, :params => { :name => '43' })
-      read_from_cache("thing/all/name/43").should == nil
+    include_examples "caching"
+
+    context 'when concurrency is turned on' do
+      include_examples "caching"
     end
 
     it "should cache a response for a string primary key" do
@@ -234,12 +242,12 @@ class CustomCollection < ActiveResource::Collection; end
           end
 
           non_cached = Thing.where(name: 'ada')
-          non_cached.original_params.should == { 'name' => 'ada' }
+          non_cached.original_params.should == { :name => 'ada' }
           non_cached.map(&:id).should == @thing_collection.map { |h| h[:id]}
 
           cached = read_from_cache('thing/all/{:params=>{:name=>"ada"}}')
           cached.should be_instance_of(CustomCollection)
-          cached.original_params.should == { 'name' => 'ada' }
+          cached.original_params.should == { :name => 'ada' }
           cached.resource_class.should == Thing
           cached.map(&:id).should == @thing_collection.map { |h| h[:id]}
 
@@ -249,11 +257,11 @@ class CustomCollection < ActiveResource::Collection; end
             non_cached = cached.where(major: 'CS')
           end
 
-          non_cached.original_params.should == { 'name' => 'ada', 'major' => 'CS' }
+          non_cached.original_params.should == { :name => 'ada', :major => 'CS' }
           non_cached.resource_class.should == Thing
           non_cached.map(&:id).should == @thing_collection2.map { |h| h[:id]}
-          cached = read_from_cache('thing/all/{:params=>{"name"=>"ada",:major=>"cs"}}')
-          cached.original_params.should == { 'name' => 'ada', 'major' => 'CS' }
+          cached = read_from_cache('thing/all/{:params=>{:name=>"ada",:major=>"cs"}}')
+          cached.original_params.should == { :name => 'ada', :major => 'CS' }
           cached.resource_class.should == Thing
           cached.map(&:id).should == @thing_collection2.map { |h| h[:id]}
         end