From 1596721d7af6faf54df75512598f49cc8f656ac4 Mon Sep 17 00:00:00 2001 From: Jean Luis Urena <15808412+jlurena@users.noreply.github.com> Date: Sun, 5 May 2024 12:12:28 -0400 Subject: [PATCH] [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 | 16 ++++++----- cached_resource.gemspec | 1 + lib/cached_resource/cached_resource.rb | 10 +++++++ lib/cached_resource/caching.rb | 14 ++++++++-- lib/cached_resource/configuration.rb | 16 ++++++----- spec/cached_resource/caching_spec.rb | 38 ++++++++++++++++---------- 6 files changed, 63 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 61f9a25..ea584b0 100644 --- a/README.md +++ b/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: diff --git a/cached_resource.gemspec b/cached_resource.gemspec index 3d8bcd6..3db4d19 100644 --- a/cached_resource.gemspec +++ b/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 diff --git a/lib/cached_resource/cached_resource.rb b/lib/cached_resource/cached_resource.rb index 9c840a0..0ff1750 100644 --- a/lib/cached_resource/cached_resource.rb +++ b/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 diff --git a/lib/cached_resource/caching.rb b/lib/cached_resource/caching.rb index 441a2a6..e2b465a 100644 --- a/lib/cached_resource/caching.rb +++ b/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) diff --git a/lib/cached_resource/configuration.rb b/lib/cached_resource/configuration.rb index e90d8f1..7374ed6 100644 --- a/lib/cached_resource/configuration.rb +++ b/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 \ No newline at end of file +end diff --git a/spec/cached_resource/caching_spec.rb b/spec/cached_resource/caching_spec.rb index 14c2f48..6d144b7 100644 --- a/spec/cached_resource/caching_spec.rb +++ b/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