diff --git a/README.md b/README.md index 4bdc2a2..f991bf6 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,6 @@ CachedResource accepts the following options as a hash: | `:cache_key_prefix` | A prefix to be added to the cache keys. | `nil` | | `:collection_arguments` | The arguments that identify the principal collection request. | `[: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. | `false` | -| `:concurrent_write` | Set to true to make concurrent writes to cache after successful API response.
Requires the [concurrent-ruby](https://rubygems.org/gems/concurrent-ruby) gem | `false` | | `:logger` | The logger to which CachedResource messages should be written. | The `Rails.logger` if available, or an `ActiveSupport::Logger` | | `: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). | `86400` | | `:ttl_randomization_scale` | A Range from which a random value will be selected to scale the ttl. | `1..2` | diff --git a/lib/cached_resource/cached_resource.rb b/lib/cached_resource/cached_resource.rb index 1733f84..3e89b1a 100644 --- a/lib/cached_resource/cached_resource.rb +++ b/lib/cached_resource/cached_resource.rb @@ -18,16 +18,6 @@ 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 - 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 90ab6da..0878c12 100644 --- a/lib/cached_resource/caching.rb +++ b/lib/cached_resource/caching.rb @@ -26,11 +26,7 @@ def find_with_cache(*arguments) # Clear the cache. def clear_cache(options = nil) - if cached_resource.concurrent_write - Concurrent::Promise.execute { cache_clear(options) } - else - cache_clear(options) - end + cache_clear(options) true end @@ -130,14 +126,6 @@ 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 0dd0495..ae381da 100644 --- a/lib/cached_resource/configuration.rb +++ b/lib/cached_resource/configuration.rb @@ -23,7 +23,6 @@ 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({ cache: defined?(Rails.cache) && Rails.cache || CACHE, @@ -31,7 +30,6 @@ def initialize(options = {}) cache_key_prefix: nil, collection_arguments: [:all], collection_synchronize: false, - concurrent_write: false, enabled: true, logger: defined?(Rails.logger) && Rails.logger || LOGGER, race_condition_ttl: 86400, diff --git a/spec/cached_resource/cached_resource_spec.rb b/spec/cached_resource/cached_resource_spec.rb index 7fb5b27..5dec298 100644 --- a/spec/cached_resource/cached_resource_spec.rb +++ b/spec/cached_resource/cached_resource_spec.rb @@ -44,31 +44,6 @@ expect(dummy_class.instance_variable_get(:@cached_resource)).to be_a(CachedResource::Configuration) expect(CachedResource::Configuration).to have_received(:new).with(options) end - - context "when concurrent_write is enabled" do - before do - allow(CachedResource::Configuration).to receive(:new).and_return( - double(concurrent_write: true, logger: logger) - ) - end - - it "requires concurrent/promise" do - expect { dummy_class.setup_cached_resource!(concurrent_write: true) }.not_to raise_error - end - - context 'When "concurrent/promise" is not installed' do - before do - allow(dummy_class).to receive(:require).with("concurrent/promise").and_raise(LoadError) - end - - it "requires concurrent/promise" do - expect(logger).to receive(:error).with( - "`concurrent_write` option is enabled, but `concurrent-ruby` is not an installed dependency" - ).once - expect { dummy_class.setup_cached_resource!(concurrent_write: true) }.to raise_error(LoadError) - end - end - end end describe ".inherited" do diff --git a/spec/cached_resource/caching_spec.rb b/spec/cached_resource/caching_spec.rb index c2ac3bd..c254c9f 100644 --- a/spec/cached_resource/caching_spec.rb +++ b/spec/cached_resource/caching_spec.rb @@ -39,7 +39,6 @@ def read_from_cache(key, model = Thing) cache: CACHE, collection_arguments: [:all], collection_synchronize: false, - concurrent_write: false, enabled: true, generate_ttl: 604800, logger: double(:thing_logger, info: nil, error: nil), @@ -56,7 +55,6 @@ def read_from_cache(key, model = Thing) cache: CACHE, collection_arguments: [:all], collection_synchronize: false, - concurrent_write: false, enabled: true, generate_ttl: 604800, logger: double(:not_the_thing_logger, info: nil, error: nil), @@ -227,42 +225,6 @@ def read_from_cache(key, model = Thing) end end - context "when concurrency is turned on" do - before do - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/5.json", {}, {thing: {id: 1, name: ("x" * 1_000_000)}}.to_json - end - allow(thing_cached_resource).to receive(:concurrent_write).and_return(true) - end - - it "caches a response asynchronously when on" do - result = Thing.find(5) - expect(read_from_cache("thing/5")).to be_nil - expect { read_from_cache("thing/5") }.to eventually(eq(result)) - end - - it "clears cache concurrently" do - result = Thing.find(5) - expect { read_from_cache("thing/5") }.to eventually(eq(result)) - expect { Thing.clear_cache }.to not_change { read_from_cache("thing/5") } - expect { read_from_cache("thing/5") }.to eventually(be_nil) - end - end - - context "when concurrency is turned off" do - before do - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/5.json", {}, {thing: {id: 1, name: ("x" * 1_000_000)}}.to_json - end - allow(thing_cached_resource).to receive(:concurrent_write).and_return(false) - end - - it "caches a response synchronously when off" do - result = Thing.find(5) - expect(read_from_cache("thing/5")).to eq(result) - end - end - context "when cache prefix is set" do context "cache_key_prefix is a string" do before { allow(thing_cached_resource).to receive(:cache_key_prefix).and_return("prefix123") } @@ -386,31 +348,10 @@ def read_from_cache(key, model = Thing) end describe ".clear_cache" do - context "when concurrent_write is false" do - it "clears the cache immediately" do - expect(thing_cached_resource.cache).to receive(:delete_matched) - expect(Thing.clear_cache).to be true - expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR/) - end - end - - context "when concurrent_write is true" do - before do - allow(thing_cached_resource).to receive(:concurrent_write).and_return(true) - end - - it "clears the cache asynchronously" do - promise = instance_double(Concurrent::Promise) - allow(Concurrent::Promise).to receive(:execute).and_return(promise) - expect(Concurrent::Promise).to receive(:execute) - Thing.clear_cache - end - - it "logs the cache clear action" do - allow(Concurrent::Promise).to receive(:execute).and_yield - Thing.clear_cache - expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR/) - end + it "clears the cache immediately" do + expect(thing_cached_resource.cache).to receive(:delete_matched) + expect(Thing.clear_cache).to be true + expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR/) end context "when options are provided" do