From 56bd0e5d4ce5bd1bf2617d9ecff96d0b473f074f Mon Sep 17 00:00:00 2001 From: Jean Luis Urena Date: Thu, 11 Jul 2024 09:16:57 -0400 Subject: [PATCH] Add/fix another set of spec and logic. Update standard.yml --- .standard.yml | 2 +- cached_resource.gemspec | 4 +- lib/cached_resource/caching.rb | 4 +- spec/cached_resource/caching_spec.rb | 148 +++++++++++++++++++++------ 4 files changed, 121 insertions(+), 37 deletions(-) diff --git a/.standard.yml b/.standard.yml index ed3d205..8e65e46 100644 --- a/.standard.yml +++ b/.standard.yml @@ -1,3 +1,3 @@ -fix: true +fix: false parallel: true ruby_version: 3.0 \ No newline at end of file diff --git a/cached_resource.gemspec b/cached_resource.gemspec index a7371ff..96950bc 100644 --- a/cached_resource.gemspec +++ b/cached_resource.gemspec @@ -21,11 +21,11 @@ Gem::Specification.new do |s| s.add_runtime_dependency "activesupport", ">= 4.0" s.add_runtime_dependency "nilio", ">= 1.0" - s.add_development_dependency "concurrent-ruby", "~> 1.2", ">= 1.2.3" + s.add_development_dependency "concurrent-ruby" s.add_development_dependency "pry-byebug" s.add_development_dependency "rake" s.add_development_dependency "rspec" s.add_development_dependency "simplecov", "~> 0.22.0" - s.add_development_dependency "timecop", "~> 0.9.10" s.add_development_dependency "standard", "~> 1.39", ">= 1.39.1" + s.add_development_dependency "timecop", "~> 0.9.10" end diff --git a/lib/cached_resource/caching.rb b/lib/cached_resource/caching.rb index 648e7a4..470f197 100644 --- a/lib/cached_resource/caching.rb +++ b/lib/cached_resource/caching.rb @@ -147,10 +147,8 @@ def _cache_write(key, object, *arguments) result end - # Clear the cache. def cache_clear(options = nil) - # Memcache doesn't support delete_matched, which can also be computationally expensive - if (Object.const_defined?(:Dalli) && cached_resource.cache.instance_of?(ActiveSupport::Cache::MemCacheStore)) || options.try(:fetch, :all) + if !cached_resource.cache.respond_to?(:delete_matched) || options.try(:fetch, :all) cached_resource.cache.clear.tap do |result| cached_resource.logger.info("#{CachedResource::Configuration::LOGGER_PREFIX} CLEAR ALL") end diff --git a/spec/cached_resource/caching_spec.rb b/spec/cached_resource/caching_spec.rb index b0d58bf..48f7fe3 100644 --- a/spec/cached_resource/caching_spec.rb +++ b/spec/cached_resource/caching_spec.rb @@ -1,5 +1,7 @@ require "spec_helper" +CACHE = ActiveSupport::Cache::MemoryStore.new + class Thing < ActiveResource::Base self.site = "http://api.thing.com" cached_resource @@ -30,8 +32,44 @@ def read_from_cache(key, model = Thing) let(:not_the_thing) { {not_the_thing: {id: 1, name: "Not"}} } let(:not_the_thing_collection) { [{not_the_thing: {id: 1, name: "Not"}}] } + let(:thing_cached_resource) do + double(:thing_cached_resource, + cache_collections: true, + cache_key_prefix: nil, + cache: CACHE, + collection_arguments: [:all], + collection_synchronize: false, + concurrent_write: false, + enabled: true, + generate_ttl: 604800, + logger: double(:thing_logger, info: nil, error: nil), + race_condition_ttl: 86400, + ttl_randomization_scale: 1..2, + ttl_randomization: false, + ttl: 604800) + end + + let(:not_the_thing_cached_resource) do + double(:not_the_thing_cached_resource, + cache_collections: true, + cache_key_prefix: nil, + 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), + race_condition_ttl: 86400, + ttl_randomization_scale: 1..2, + ttl_randomization: false, + ttl: 604800) + end + before do - CachedResource::Configuration::CACHE.clear + CACHE.clear + allow(Thing).to receive(:cached_resource).and_return(thing_cached_resource) + allow(NotTheThing).to receive(:cached_resource).and_return(not_the_thing_cached_resource) ActiveResource::HttpMock.reset! ActiveResource::HttpMock.respond_to do |mock| mock.get "/things/1.json", {}, thing.to_json @@ -47,6 +85,11 @@ def read_from_cache(key, model = Thing) end context "when caching is enabled" do + before do + allow(thing_cached_resource).to receive(:enabled).and_return(true) + allow(not_the_thing_cached_resource).to receive(:enabled).and_return(true) + end + context "Caching single resource" do it "caches a response" do result = Thing.find(1) @@ -73,7 +116,7 @@ def read_from_cache(key, model = Thing) ) end - it "empties all the cache when clear_cache is called on Thing with :all option set" do + it "empties all shared cache when clear_cache is called on Thing with :all option set" do Thing.find(1) NotTheThing.find(1) expect { Thing.clear_cache(all: true) }.to change { read_from_cache("thing/1") }.to(nil).and( @@ -144,11 +187,7 @@ def read_from_cache(key, model = Thing) context "Caching collection is turned off" do before do - Thing.cached_resource.cache_collections = false - end - - after do - Thing.cached_resource.cache_collections = true + allow(thing_cached_resource).to receive(:cache_collections).and_return(false) end it "always remakes a request" do @@ -158,11 +197,7 @@ def read_from_cache(key, model = Thing) context "custom collection arguments" do before do - Thing.cached_resource.collection_arguments = [:all, params: {name: 42}] - end - - after do - Thing.cached_resource.collection_arguments = [:all] + allow(thing_cached_resource).to receive(:collection_arguments).and_return([:all, params: {name: 42}]) end it "checks for custom collection arguments" do @@ -174,20 +209,20 @@ def read_from_cache(key, model = Thing) context "TTL" do let(:now) { Time.new(1999, 12, 31, 12, 0, 0) } + let(:travel_seconds) { 1000 } before do Timecop.freeze(now) - Thing.cached_resource.ttl = 1 + allow(thing_cached_resource).to receive(:generate_ttl).and_return(travel_seconds) end after do - Thing.cached_resource.ttl = 604800 Timecop.return end it "remakes the request when the ttl expires" do expect { Thing.find(1) }.to change { ActiveResource::HttpMock.requests.length }.from(0).to(1) - Timecop.travel(now + 2) + Timecop.travel(now + travel_seconds) expect { Thing.find(1) }.to change { ActiveResource::HttpMock.requests.length }.from(1).to(2) end end @@ -197,11 +232,7 @@ def read_from_cache(key, model = Thing) ActiveResource::HttpMock.respond_to do |mock| mock.get "/things/5.json", {}, {thing: {id: 1, name: ("x" * 1_000_000)}}.to_json end - Thing.cached_resource.concurrent_write = true - end - - after do - Thing.cached_resource.concurrent_write = false + allow(thing_cached_resource).to receive(:concurrent_write).and_return(true) end it "caches a response asynchronously when on" do @@ -218,12 +249,12 @@ def read_from_cache(key, model = Thing) end end - context "when concurrency is turned on" do + 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 - Thing.cached_resource.concurrent_write = false + allow(thing_cached_resource).to receive(:concurrent_write).and_return(false) end it "caches a response synchronously when off" do @@ -235,12 +266,11 @@ def read_from_cache(key, model = Thing) context "when cache prefix is set" do before do Thing.instance_variable_set(:@name_key, nil) # Remove memoization - Thing.cached_resource.cache_key_prefix = "prefix123" + allow(thing_cached_resource).to receive(:cache_key_prefix).and_return("prefix123") end after do Thing.instance_variable_set(:@name_key, nil) # Remove memoization - Thing.cached_resource.cache_key_prefix = nil end it "caches with the cache_key_prefix" do @@ -268,8 +298,7 @@ def read_from_cache(key, model = Thing) context "cache_collection_synchronize" do before do - Thing.cached_resource.cache.clear - Thing.cached_resource.collection_synchronize = true + allow(thing_cached_resource).to receive(:collection_synchronize).and_return(true) ActiveResource::HttpMock.respond_to do |mock| mock.get "/things/1.json", {}, thing.to_json mock.get "/things.json", {}, [thing2[:thing], other_string_thing[:thing]].to_json @@ -299,9 +328,9 @@ def read_from_cache(key, model = Thing) end context "when caching is disabled" do - before(:context) do - Thing.cached_resource.off! - NotTheThing.cached_resource.off! + before do + allow(thing_cached_resource).to receive(:enabled).and_return(false) + allow(not_the_thing_cached_resource).to receive(:enabled).and_return(false) end it "does not cache a response" do @@ -320,7 +349,7 @@ def read_from_cache(key, model = Thing) end end - describe "#cache_key_delete_pattern" do + describe ".cache_key_delete_pattern" do let(:cache_class) { "Redis" } before do @@ -335,7 +364,7 @@ def read_from_cache(key, model = Thing) end context "with cache ActiveSupport::Cache::FileStore" do - let(:cache_class) { ActiveSupport::Cache::FileStore.new('tmp/') } + let(:cache_class) { ActiveSupport::Cache::FileStore.new("tmp/") } it do expect(Thing.send(:cache_key_delete_pattern)).to eq(/^thing\//) end @@ -347,4 +376,61 @@ def read_from_cache(key, model = Thing) end end 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 + end + + context "when options are provided" do + it "clears all cache if options[:all] is true" do + options = {all: true} + expect(thing_cached_resource.cache).to receive(:clear).and_call_original + Thing.clear_cache(options) + end + + it "deletes matched cache keys if options[:all] is not provided" do + options = {all: false} + allow(thing_cached_resource.cache).to receive(:delete_matched) + Thing.clear_cache(options) + expect(thing_cached_resource.cache).to have_received(:delete_matched) + end + end + + context 'when using a cache store that does not support "delete_matched"' do + before do + mem_cache_store = double(:cache_store_without_delete_matched, clear: nil) + allow(thing_cached_resource).to receive(:cache).and_return(mem_cache_store) + end + + it "clears the cache if Dalli is defined" do + expect(thing_cached_resource.cache).to receive(:clear) + Thing.clear_cache + expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR ALL/) + end + end + end end