diff --git a/.gitignore b/.gitignore index 83f5ebf..dad0222 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ pkg/* *DS_Store .ruby-version .tool-versions +coverage diff --git a/cached_resource.gemspec b/cached_resource.gemspec index 3db4d19..db629ad 100644 --- a/cached_resource.gemspec +++ b/cached_resource.gemspec @@ -10,20 +10,24 @@ Gem::Specification.new do |s| s.homepage = "https://github.com/mhgbrown/cached_resource" s.summary = %q{Caching for ActiveResource} s.description = %q{Enables request-based caching for ActiveResource} - s.licenses = ['MIT'] + s.licenses = ["MIT"] s.files = `git ls-files`.split("\n") s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n") s.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) } s.require_paths = ["lib"] - s.required_ruby_version = '>= 1.9.0' + s.required_ruby_version = ">= 1.9.0" s.add_runtime_dependency "activeresource", ">= 4.0" 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 "pry-byebug" s.add_development_dependency "rake" s.add_development_dependency "rspec" - s.add_development_dependency "concurrent-ruby", '~> 1.2', '>= 1.2.3' + s.add_development_dependency "simplecov", "~> 0.22.0" + s.add_development_dependency "timecop", "~> 0.9.10" + end diff --git a/lib/cached_resource/cached_resource.rb b/lib/cached_resource/cached_resource.rb index 0ff1750..99302fc 100644 --- a/lib/cached_resource/cached_resource.rb +++ b/lib/cached_resource/cached_resource.rb @@ -20,7 +20,7 @@ def setup_cached_resource!(options) @cached_resource = CachedResource::Configuration.new(options) if @cached_resource.concurrent_write begin - send :require, 'concurrent/promise' + require 'concurrent/promise' rescue LoadError @cached_resource.logger.error( "`concurrent_write` option is enabled, but `concurrent-ruby` is not an installed dependency" diff --git a/lib/cached_resource/caching.rb b/lib/cached_resource/caching.rb index 803b534..99207e4 100644 --- a/lib/cached_resource/caching.rb +++ b/lib/cached_resource/caching.rb @@ -26,7 +26,13 @@ def find_with_cache(*arguments) # Clear the cache. def clear_cache(options=nil) - cache_clear(options) + if cached_resource.concurrent_write + Concurrent::Promise.execute { cache_clear(options) } + else + cache_clear(options) + end + + true end private @@ -205,8 +211,6 @@ def object_to_json(object, prefix_options, query_options) :prefix_options => prefix_options, :original_params => query_options }.to_json - elsif object.nil? - nil.to_json else { :resource => { :object => object, :persistence => object.persisted? }, diff --git a/lib/cached_resource/configuration.rb b/lib/cached_resource/configuration.rb index 2c8655a..8f080b7 100644 --- a/lib/cached_resource/configuration.rb +++ b/lib/cached_resource/configuration.rb @@ -50,6 +50,8 @@ def generate_ttl end # Enables caching. + # NOTE: Disabled coverage because it is not being picked up by simplecov + # :nocov: def on! self.enabled = true end @@ -58,6 +60,7 @@ def on! def off! self.enabled = false end + # :nocov: private diff --git a/spec/cached_resource/cached_resource_spec.rb b/spec/cached_resource/cached_resource_spec.rb index 970a777..666e126 100644 --- a/spec/cached_resource/cached_resource_spec.rb +++ b/spec/cached_resource/cached_resource_spec.rb @@ -1,29 +1,92 @@ require 'spec_helper' -RSpec.describe CachedResource do - before do - class BaseThing < ActiveResource::Base +RSpec.describe CachedResource::Model do + let(:logger) { double(:Logger, error: nil)} + let(:dummy_class) do + Class.new(ActiveResource::Base) do + include CachedResource::Model end + end + + describe '.cached_resource' do + context 'when cached resource is not set up' do + it 'initializes and returns a new cached resource configuration' do + options = { logger: logger } + allow(CachedResource::Configuration).to receive(:new).with(options).and_call_original - class FirstChildThing < BaseThing - self.site = 'http://api.first-child-thing.com' - cached_resource + cached_resource = dummy_class.cached_resource(options) + + expect(cached_resource).to be_a(CachedResource::Configuration) + expect(dummy_class.instance_variable_get(:@cached_resource)).to eq(cached_resource) + expect(CachedResource::Configuration).to have_received(:new).with(options) + end end - class SecondChildThing < BaseThing - self.site = 'http://api.second-child-thing.com' + context 'when cached resource is already set up' do + it 'returns the existing cached resource configuration' do + existing_config = CachedResource::Configuration.new + dummy_class.instance_variable_set(:@cached_resource, existing_config) + + cached_resource = dummy_class.cached_resource + + expect(cached_resource).to eq(existing_config) + end end end - after do - [:BaseThing, :FirstChildThing, :SecondChildThing].each do |klass| - Object.send(:remove_const, klass) + describe '.setup_cached_resource!' do + it 'creates a new cached resource configuration' do + options = { logger: logger } + allow(CachedResource::Configuration).to receive(:new).with(options).and_call_original + + dummy_class.setup_cached_resource!(options) + + 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(LoadError) + 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 - it 'should include descendants when calling .descendants' do - BaseThing.descendants.sort_by { |klass| klass.name }.should == [FirstChildThing, SecondChildThing] + let(:child_class) { Class.new(dummy_class) } + + context 'when cached resource is defined in superclass' do + it 'copies the cached resource configuration to the subclass' do + existing_config = CachedResource::Configuration.new + dummy_class.instance_variable_set(:@cached_resource, existing_config) + + expect(child_class.cached_resource).to eq(existing_config) + end + end + + context 'when cached resource is not defined in superclass' do + it 'does not set cached resource configuration in the subclass' do + expect(child_class.instance_variable_defined?(:@cached_resource)).to be_falsey + end end end end diff --git a/spec/cached_resource/caching_spec.rb b/spec/cached_resource/caching_spec.rb index 7869e3e..640a8fe 100644 --- a/spec/cached_resource/caching_spec.rb +++ b/spec/cached_resource/caching_spec.rb @@ -1,666 +1,343 @@ require 'spec_helper' -describe CachedResource do +class Thing < ActiveResource::Base + self.site = "http://api.thing.com" + cached_resource +end - def read_from_cache(key) - Thing.send(:cache_read, key) - end +class NotTheThing < ActiveResource::Base + self.site = "http://api.notthething.com" + cached_resource +end - before(:each) do - class Thing < ActiveResource::Base - self.site = "http://api.thing.com" - cached_resource - end +def read_from_cache(key, model = Thing) + model.send(:cache_read, key) +end - class NotTheThing < ActiveResource::Base - self.site = "http://api.notthething.com" - cached_resource +describe CachedResource::Caching do + let(:thing) { {thing: {id: 1, name: "Ada"}} } + let(:thing_collection) { [{id: 1, name: "Ada"}, {id: 2, name: "Ada", major: 'CS'}] } + let(:thing_collection2) { [{id: 2, name: "Ada", major: 'CS'}] } + let(:other_thing) { {thing: {id: 1, name: "Ari"}} } + let(:thing2) { {thing: {id: 2, name: "Joe"}} } + let(:other_thing2) { {thing: {id: 2, name: "Jeb"}} } + let(:thing3) { {thing: {id: 3, name: "Stu"}} } + let(:string_thing) { {thing: {id: "fded", name: "Lev"}} } + let(:other_string_thing) { {thing: {id: "fded", name: "Lon"}} } + let(:date_thing) { {thing: {id: 4, created_at: DateTime.new(2020)}} } + let(:nil_thing) { nil.to_json } + let(:empty_array_thing) { [].to_json } + let(:not_the_thing) { {not_the_thing: {id: 1, name: "Not"}} } + let(:not_the_thing_collection) { [{not_the_thing: {id: 1, name: "Not"}}] } + + before do + CachedResource::Configuration::CACHE.clear + ActiveResource::HttpMock.reset! + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/things/1.json", {}, thing.to_json + mock.get "/things/1.json?foo=bar", {}, thing.to_json + mock.get "/things/fded.json", {}, string_thing.to_json + mock.get "/things.json?name=42", {}, nil_thing, 404 + mock.get "/things.json?name=43", {}, empty_array_thing + mock.get "/things/4.json", {}, date_thing.to_json + mock.get "/not_the_things/1.json", {}, not_the_thing.to_json + mock.get "/things.json", {}, thing_collection.to_json + mock.get "/not_the_things.json", {}, not_the_thing_collection.to_json end - - @thing = {:thing => {:id => 1, :name => "Ada"}} - @thing_collection = [{:id => 1, :name => "Ada"}, {:id => 2, :name => "Ada", :major => 'CS'}] - @thing_collection2 = [{:id => 2, :name => "Ada", :major => 'CS'}] - @other_thing = {:thing => {:id => 1, :name => "Ari"}} - @thing2 = {:thing => {:id => 2, :name => "Joe"}} - @other_thing2 = {:thing => {:id => 2, :name => "Jeb"}} - @thing3 = {:thing => {:id => 3, :name => "Stu"}} - @string_thing = {:thing => {:id => "fded", :name => "Lev"}} - @other_string_thing = {:thing => {:id => "fded", :name => "Lon"}} - @date_thing = {:thing => {:id => 4, :created_at => DateTime.new(2020)}} - @thing_json = @thing.to_json - @other_thing_json = @other_thing.to_json - @string_thing_json = @string_thing.to_json - @other_string_thing_json = @other_string_thing.to_json - @date_thing_json = @date_thing.to_json - @nil_thing = nil.to_json - @empty_array_thing = [].to_json - @not_the_thing = {:not_the_thing => {:id => 1, :name => "Not"}} - @not_the_thing_json = @not_the_thing.to_json end - after(:each) do - Thing.cached_resource.cache.clear - Object.send(:remove_const, :Thing) - NotTheThing.cached_resource.cache.clear - Object.send(:remove_const, :NotTheThing) - end - - describe "when enabled" do - before(:each) do - # it's on by default, but lets call the method - # to make sure it works - Thing.cached_resource.cache.clear - Thing.cached_resource.on! - NotTheThing.cached_resource.cache.clear - NotTheThing.cached_resource.on! - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/1.json?foo=bar", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json?name=42", {}, @nil_thing, 404 - mock.get "/things.json?name=43", {}, @empty_array_thing - mock.get "/things/4.json", {}, @date_thing_json - mock.get "/not_the_things/1.json", {}, @not_the_thing_json - end - end - - shared_examples "caching" do - it "should cache a response" do + context "when caching is enabled" do + context "Caching single resource" do + it "caches a response" do result = Thing.find(1) - read_from_cache("thing/1").should == result + Thing.find(1) + expect(read_from_cache("thing/1")).to eq(result) + expect(ActiveResource::HttpMock.requests.length).to eq(1) end - it "shouldn't cache nil response" do - Thing.find(:all, :params => { :name => '42' }) - read_from_cache("thing/all/name/42").should == nil + it "caches without whitespace in keys" do + result = Thing.find(1, from: 'path', params: { foo: 'bar' }) + expect(read_from_cache('thing/1/{:from=>"path",:params=>{:foo=>"bar"}}')).to eq(result) end - it "shouldn't cache blank response" do - Thing.find(:all, :params => { :name => '43' }) - read_from_cache("thing/all/name/43").should == nil + it "empties the cache when clear_cache is called" do + Thing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.from(kind_of(Thing)).to(nil) end - end - include_examples "caching" - - # NOTE: Possibly flaky, but best easy/cheap way to test concurrency. - if ActiveResource::VERSION::MAJOR >= 5 - context 'when concurrency is turned on' do - before do - # it's on by default, but lets call the method - # to make sure it works - Thing.cached_resource.cache.clear - Thing.cached_resource.on! - NotTheThing.cached_resource.cache.clear - NotTheThing.cached_resource.on! - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/5.json", {}, {:thing => {:id => 1, :name => ("x" * 1_000_000) }}.to_json - end - end - - after do - Thing.cached_resource.concurrent_write = false - end - - it "should cache a response asynchronusly when on" do - Thing.cached_resource.concurrent_write = true - result = Thing.find(5) - read_from_cache("thing/5").should == nil - loops = 0 - begin - Timeout.timeout(5) do - loop do - loops += 1 - if read_from_cache("thing/5") == result - break - else - sleep 0.5 - end - end - end - rescue Timeout::Error - RSpec::Expectations.fail_with("Concurrency failed: Cache was not populated within the expected time") - end - - loops.should > 0 - read_from_cache("thing/5").should == result - end - - it "will cache a response synchronously when off" do - Thing.cached_resource.concurrent_write = false - result = Thing.find(5) - read_from_cache("thing/5").should == result - end + it "does not empty cache of other ActiveResource objects" do + Thing.find(1) + NotTheThing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.from(kind_of(Thing)).to(nil).and( + not_change { read_from_cache("notthething/1", NotTheThing) } + ) end - end - context "When there is a cache prefix" do - before do - Thing.cached_resource.cache_key_prefix = "prefix123" + it "empties all the 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( + change { read_from_cache("notthething/1", NotTheThing) }.to(nil) + ) end - after do - Thing.cached_resource.cache_key_prefix = nil + it "caches a response with the same persistence" do + result1 = Thing.find(1) + expect(result1.persisted?).to be true + result2 = Thing.find(1) + expect(result2.persisted?).to eq(result1.persisted?) end - it "caches with the cache_key_prefix" do - result = Thing.find(1) - read_from_cache("prefix123/thing/1").should == result + it "remakes a request when reloaded" do + Thing.find(1) + expect { Thing.find(1, reload: true) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end - end - - it "should cache a response for a string primary key" do - result = Thing.find("fded") - read_from_cache("thing/fded").should == result - end - - it "should cache without whitespace in keys" do - result = Thing.find(1, :from => 'path', :params => { :foo => 'bar' }) - read_from_cache('thing/1/{:from=>"path",:params=>{:foo=>"bar"}}').should == result - end - - it "should empty the cache when clear_cache is called" do - result = Thing.find(1) - Thing.clear_cache - read_from_cache("thing/1").should == nil - end - - it "should not empty the cache of NotTheThing when clear_cache is called on the Thing" do - result1 = Thing.find(1) - result2 = NotTheThing.find(1) - Thing.clear_cache - NotTheThing.send(:cache_read, 'notthething/1').should == result2 - end - it "should empty all the cache when clear_cache is called on the Thing with :all option set" do - result1 = Thing.find(1) - result2 = NotTheThing.find(1) - Thing.clear_cache(all: true) - NotTheThing.send(:cache_read, 'notthething/1').should == nil - end - - it "should cache a response with the same persistence" do - result1 = Thing.find(1) - result1.persisted?.should be true - result2 = Thing.find(1) - result1.persisted?.should == result2.persisted? - end - - it "should read a response when the request is made again" do - # make a request - Thing.find(1) - # make the same request - Thing.find(1) - # only one request should have happened - ActiveResource::HttpMock.requests.length.should == 1 - end + it "rewrites the cache when the request is reloaded" do + Thing.find(1) + old_result = read_from_cache("thing/1").dup - it "should read a response when the request is made again for a string primary key" do - # make a request - Thing.find("fded") - # make the same request - Thing.find("fded") - # only one request should have happened - ActiveResource::HttpMock.requests.length.should == 1 - end + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/things/1.json", {}, other_thing.to_json + end - it "should remake a request when reloaded" do - # make a request - Thing.find(1) - # make the same request, but reload it - Thing.find(1, :reload => true) - # we should get two requests - ActiveResource::HttpMock.requests.length.should == 2 - end + expect { Thing.find(1, reload: true) }.to change { read_from_cache("thing/1").name }.from(old_result.name) + end - it "should remake a request when reloaded for a string primary key" do - # make a request - Thing.find("fded") - # make the same request, but reload it - Thing.find("fded", :reload => true) - # we should get two requests - ActiveResource::HttpMock.requests.length.should == 2 + it "does not return a frozen object on first request, or subsequent" do + result1 = Thing.find(1) + expect(result1).not_to be_frozen + expect(Thing.find(1)).not_to be_frozen + expect(result1).not_to be_frozen + end end - it "should rewrite the cache when the request is reloaded" do - # make a request - Thing.find(1) - # get the cached result of the request - old_result = read_from_cache("thing/1") - - # change the response - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json + context "Caching a collection" do + it "does not cache an empty array response" do + Thing.find(:all, params: { name: '43' }) + expect(read_from_cache("thing/all/name/43")).to be_nil end - Thing.find(1, :reload => true) - new_result = read_from_cache("thing/1") - # since active resources are equal if and only if they - # are the same object or an instance of the same class, - # not new?, and have the same id. - new_result.name.should_not == old_result.name - end - - it "should remake the request when the ttl expires" do - # set cache time to live to 1 second - Thing.cached_resource.ttl = 1 - # make a request - Thing.find(1) - # wait for the cache to expire - sleep(1.5) - # make the same request - Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 2 - end + it "does not cache a nil response" do + Thing.find(:all, params: { name: '42' }) + expect(read_from_cache("thing/all/name/42")).to be_nil + end - it "should not return a frozen object on first request" do - result1 = Thing.find(1) - result1.should_not be_frozen - end + it "empties the cache when clear_cache is called" do + Thing.all + expect { Thing.clear_cache }.to change { read_from_cache("thing/all") }.from(kind_of(Enumerable)).to(nil) + end - it "should not return frozen object on a subsequent request" do - result1 = Thing.find(1) - result2 = Thing.find(1) - result2.should_not be_frozen - end + it "does not empty cache of other ActiveResource objects" do + Thing.all + NotTheThing.all + expect { Thing.clear_cache }.to change { read_from_cache("thing/all") }.from(kind_of(Enumerable)).to(nil).and( + not_change { read_from_cache("notthething/all", NotTheThing) } + ) + end - it "should not freeze first requested object on a subsequent request" do - result1 = Thing.find(1) - result2 = Thing.find(1) - result1.should_not be_frozen + it "remakes a request when reloaded" do + Thing.all + expect { Thing.all(reload: true) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) + end end - describe "when ActiveSupport.parse_json_times is enabled" do - before(:all) do - Time.zone = 'UTC' - ActiveSupport.parse_json_times = true + context "Caching collection is turned off" do + before do + Thing.cached_resource.cache_collections = false end - it "should convert date times to objects when reading from cache" do - Thing.find(4) + after do + Thing.cached_resource.cache_collections = true + end - read_from_cache("thing/4").created_at.should == @date_thing[:thing][:created_at] + it "always remakes a request" do + Thing.all + expect { Thing.all }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end - end - shared_examples "collection_return_type" do - if ActiveResource::VERSION::MAJOR >= 4 - it "should return an ActiveResource::Collection" do - cached = read_from_cache("thing/all") - cached.should be_instance_of(ActiveResource::Collection) + context "custom collection arguments" do + before do + Thing.cached_resource.collection_arguments = [:all, params: {name: 42}] end - it "should return a chainable instance of the collection_parser" do - Thing.cached_resource.cache.clear - class CustomCollection < ActiveResource::Collection; end - Thing.collection_parser = CustomCollection - - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json?name=ada", {}, @thing_collection.to_json - mock.get "/things.json?major=CS&name=ada", {}, @thing_collection2.to_json - end - - non_cached = Thing.where(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.resource_class.should == Thing - cached.map(&:id).should == @thing_collection.map { |h| h[:id]} - - if ActiveResource::VERSION::MAJOR < 5 - non_cached = cached.resource_class.where(cached.original_params.merge(major: 'CS')) - else - non_cached = cached.where(major: 'CS') - end - - 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.resource_class.should == Thing - cached.map(&:id).should == @thing_collection2.map { |h| h[:id]} + after do + Thing.cached_resource.collection_arguments = [:all] end - else - it "should return an Array" do - cached = read_from_cache("thing/all") - cached.should be_instance_of(Array) + + it "checks for custom collection arguments" do + Thing.all + expect { Thing.find(:all, params: {name: 42}) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end end end - shared_examples "collection_freezing" do - it "should not return a frozen collection on first request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection1.should_not be_frozen - end + context "TTL" do + let(:now) { Time.new(1999,12,31, 12, 0, 0) } - it "should not return a frozen collection on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection2 = Thing.all - collection2.should_not be_frozen + before do + Timecop.freeze(now) + Thing.cached_resource.ttl = 1 end - it "should not freeze first requested collection on a subsequent request" do - Thing.cached_resource.cache.clear - result1 = Thing.all - result2 = Thing.all - result1.should_not be_frozen + after do + Thing.cached_resource.ttl = 604800 + Timecop.return end - it "should not return frozen members on first request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection1.first.should_not be_frozen + 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 ) + expect { Thing.find(1) }.to change { ActiveResource::HttpMock.requests.length }.from(1).to(2) end + end - it "should not return frozen members on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection2 = Thing.all - collection2.first.should_not be_frozen + 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 + Thing.cached_resource.concurrent_write = true end - it "should not freeze members on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - member1 = Thing.find(1) - collection1.first.should_not be_frozen + after do + Thing.cached_resource.concurrent_write = false end - end - - shared_examples "collection_cache_clearing" do - it "should empty the cache when clear_cache is called" do - Thing.clear_cache - read_from_cache("thing/all").should == nil - read_from_cache("thing/1").should == nil + 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 - describe "when collection synchronize is enabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.collection_synchronize = true - - ActiveResource::HttpMock.reset! + context "when concurrency is turned on" do + before do ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json", {}, [@thing[:thing], @string_thing[:thing]].to_json(:root => :thing) + mock.get "/things/5.json", {}, { thing: { id: 1, name: ("x" * 1_000_000) } }.to_json end - - # make a request for all things - Thing.all + Thing.cached_resource.concurrent_write = false end - include_examples "collection_return_type" - - include_examples "collection_freezing" + it "caches a response synchronously when off" do + result = Thing.find(5) + expect(read_from_cache("thing/5")).to eq(result) + end + end - it "should write cache entries for its members" do - result = Thing.find(1) - string_result = Thing.find("fded") - # only the all request should have been made - ActiveResource::HttpMock.requests.length.should == 1 - # the result should be cached with the appropriate key - read_from_cache("thing/1").should == result - read_from_cache("thing/fded").should == string_result + 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" end - include_examples "collection_cache_clearing" + after do + Thing.instance_variable_set(:@name_key, nil) # Remove memoization + Thing.cached_resource.cache_key_prefix = nil + end - it "should rewrite cache entries for its members when reloaded" do - # get the soon to be stale result so that we have a cache entry - old_result = Thing.find(1) - old_string_result = Thing.find("fded") - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) - end - # reload the collection - Thing.all(:reload => true) - # get the updated result, read from the cache + it "caches with the cache_key_prefix" do result = Thing.find(1) - read_from_cache("thing/all")[0].should == result - read_from_cache("thing/all")[0].name.should == result.name - string_result = Thing.find("fded") - read_from_cache("thing/all")[1].should == string_result - read_from_cache("thing/all")[1].name.should == string_result.name + expect(read_from_cache("prefix123/thing/1")).to eq(result) end + end - it "should update the collection when an individual request is reloaded" do - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) - end - - # reload the individual - result = Thing.find(1, :reload => true) - read_from_cache("thing/all")[0].should == result - read_from_cache("thing/all")[0].name.should == result.name - string_result = Thing.find("fded", :reload => true) - read_from_cache("thing/all")[1].should == string_result - read_from_cache("thing/all")[1].name.should == string_result.name + context "when ActiveSupport.parse_json_times is enabled" do + before(:all) do + Time.zone = 'UTC' + ActiveSupport.parse_json_times = true end - it "should update both the collection and the member cache entries when a subset of the collection is retrieved" do - # create cache entries for 1 and all - old_individual = Thing.find(1) - old_collection = Thing.all - - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json?name=Ari", {}, [@other_thing[:thing]].to_json(:root => :thing) - mock.get "/things.json?name=Lon", {}, [@other_string_thing[:thing]].to_json(:root => :thing) - end - - # make a request for a subset of the "mother" collection - result = Thing.find(:all, :params => {:name => "Ari"}) - # the collection should be updated to reflect the server change - read_from_cache("thing/all")[0].should == result[0] - read_from_cache("thing/all")[0].name.should == result[0].name - # the individual should be updated to reflect the server change - read_from_cache("thing/1").should == result[0] - read_from_cache("thing/1").name.should == result[0].name - - # make a request for a subset of the "mother" collection - result = Thing.find(:all, :params => {:name => "Lon"}) - # the collection should be updated to reflect the server change - read_from_cache("thing/all")[1].should == result[0] - read_from_cache("thing/all")[1].name.should == result[0].name - # the individual should be updated to reflect the server change - read_from_cache("thing/fded").should == result[0] - read_from_cache("thing/fded").name.should == result[0].name - end - - it "should maintain the order of the collection when updating it" do - # change the server to return a longer collection - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json", {}, [@thing[:thing], @thing3[:thing], @thing2[:thing], @string_thing[:thing]].to_json(:root => :thing) - end - - # create cache entry for the collection (we reload because in before block we make an all request) - old_collection = Thing.all(:reload => true) - - # change the server's response for the thing with id 2 - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/2.json", {}, @other_thing2.to_json(:root => :thing) - mock.get "/things/fded.json", {}, @other_string_thing.to_json(:root => :thing) - end - - # get thing 2, thereby updating the collection - result = Thing.find(2, :reload => true) - # get the updated collection from the cache - updated_collection = Thing.all - # name should have changed to "Jeb" - updated_collection[2].name.should == result.name - # the updated collection should have the elements in the same order - old_collection.each_with_index do |thing, i| - updated_collection[i].id.should == thing.id - end + after(:all) do + ActiveSupport.parse_json_times = false + Time.zone = nil + end - # get string thing, thereby updating the collection - string_result = Thing.find("fded", :reload => true) - # get the updated collection from the cache - updated_collection = Thing.all - # name should have changed to "Lon" - updated_collection[3].name.should == string_result.name - # the updated collection should have the elements in the same order - old_collection.each_with_index do |thing, i| - updated_collection[i].id.should == thing.id - end + it "returns a time object when a time attribute is present in the response" do + result = Thing.find(4) + expect(result.created_at).to be_a(Time) end end - describe "when collection synchronize is disabled" do - before(:each) do + context "cache_collection_synchronize" do + before do Thing.cached_resource.cache.clear - Thing.cached_resource.collection_synchronize = false - - ActiveResource::HttpMock.reset! + Thing.cached_resource.collection_synchronize = true ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json", {}, [@thing[:thing], @string_thing[:thing]].to_json(:root => :thing) + mock.get "/things/1.json", {}, thing.to_json + mock.get "/things.json", {}, [thing2[:thing], other_string_thing[:thing]].to_json end - - # make a request for all things - Thing.all end - include_examples "collection_return_type" - - include_examples "collection_freezing" - - it "should not write cache entries for its members" do - result = Thing.find(1) - result = Thing.find("fded") - # both the all in the before each and this request should have been made - ActiveResource::HttpMock.requests.length.should == 3 - end - - include_examples "collection_cache_clearing" - - it "should not update the collection when an individual request is reloaded" do - # change the server + it "should rewrite cache entries for its members when reloaded" do + old_results = Thing.all(:reload => true) + # Update response ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) + mock.get "/things/1.json", {}, other_thing.to_json end - # reload the individual - result = Thing.find(1, :reload => true) - # the ids are the same, but the names should be different - read_from_cache("thing/all")[0].name.should_not == result.name + new_result = Thing.find(1, reload: true) - # reload the individual - string_result = Thing.find("fded", :reload => true) - # the ids are the same, but the names should be different - read_from_cache("thing/all")[1].name.should_not == string_result.name + expect(old_results).not_to include(new_result) + expect(read_from_cache("thing/all")).to include(new_result) end end - describe "when ttl randomization is enabled" do - before(:each) do - @ttl = 1 - Thing.cached_resource.ttl = @ttl - Thing.cached_resource.ttl_randomization = true - Thing.cached_resource.send(:sample_range, 1..2, @ttl) - # next ttl 1.72032449344216 - end - - it "should generate a random ttl" do - Thing.cached_resource.cache.should_receive(:write) - Thing.cached_resource.cache.stub(:write) do |key, value, options| - # we know the ttl should not be the same as the set ttl - options[:expires_in].should_not == @ttl - end - + context "Cache clearing" do + it "clears the cache" do Thing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.to(nil) end - end end - describe "when disabled" do - before(:each) do - Thing.cached_resource.cache.clear + context "when caching is disabled" do + before(:context) do Thing.cached_resource.off! - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - end + NotTheThing.cached_resource.off! end - it "should not cache a response" do - result = Thing.find(1) - read_from_cache("thing/1").should be_nil - end - - it "should always remake the request" do - Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 + it "does not cache a response" do Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 2 + expect(read_from_cache("thing/1")).to be_nil end - it "should always remake the request for a string primary key" do - Thing.find("fded") - ActiveResource::HttpMock.requests.length.should == 1 - Thing.find("fded") - ActiveResource::HttpMock.requests.length.should == 2 + it "does not cache a nil response" do + Thing.find(:all, params: { name: '42' }) + expect(read_from_cache("thing/all/name/42")).to be_nil end - end - describe "when cache_collections is disabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.cache_collections = false - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json", {}, [@thing[:thing],@string_thing[:thing]].to_json(:root => :thing) - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - end + it "does not cache an empty array response" do + Thing.find(:all, params: { name: '43' }) + expect(read_from_cache("thing/all/name/43")).to be_nil end + end - it "should cache a response" do - result = Thing.find(1) - read_from_cache("thing/1").should == result + describe "#cache_key_delete_pattern" do + let(:cache_class_name) { "Redis" } + before do + allow(Thing.cached_resource).to receive(:cache).and_return(double(class: cache_class_name)) end - it "should not remake a single request" do - result = Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 - result = Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 + ["ActiveSupport::Cache::MemoryStore", "ActiveSupport::Cache::FileStore"].each do |cache_name| + context "with cache #{cache_name}" do + let(:cache_class_name) { cache_name } + it do + expect(Thing.send(:cache_key_delete_pattern)).to eq(/^thing\//) + end + end end - - it "should always remake the request for collections" do - Thing.all - ActiveResource::HttpMock.requests.length.should == 1 - Thing.all - ActiveResource::HttpMock.requests.length.should == 2 + context "default" do + it do + expect(Thing.send(:cache_key_delete_pattern)).to eq("thing/*") + end end end end diff --git a/spec/cached_resource/configuration_spec.rb b/spec/cached_resource/configuration_spec.rb index 445364b..2e1b157 100644 --- a/spec/cached_resource/configuration_spec.rb +++ b/spec/cached_resource/configuration_spec.rb @@ -1,25 +1,67 @@ require 'spec_helper' -describe "CachedResource::Configuration" do - - let(:configuration) { CachedResource::Configuration.new } +class Foo < ActiveResource::Base + cached_resource +end + +class Bar < ActiveResource::Base + cached_resource :ttl => 1, + :race_condition_ttl => 5, + :cache => "cache", + :logger => "logger", + :enabled => false, + :collection_synchronize => true, + :collection_arguments => [:every], + :custom => "irrelevant", + :cache_collections => true +end + +class Bar2 < Bar; end +class Bar3 < Bar + # override the superclasses configuration + self.cached_resource = CachedResource::Configuration.new(:ttl => 60) +end + +describe CachedResource::Configuration do + + let(:configuration) { described_class.new } let(:default_logger) { defined?(ActiveSupport::Logger) ? ActiveSupport::Logger : ActiveSupport::BufferedLogger } - describe "by default" do + describe "#off!" do + subject { configuration } + before { subject.on! } + after { subject.off! } + + it "sets cache off" do + expect { subject.off! }.to change { subject.enabled }.from(true).to(false) + end + end + + describe "#on!" do + subject { configuration } + before { subject.off! } + after { subject.off! } + + it "sets cache off" do + expect { subject.on! }.to change { subject.enabled }.from(false).to(true) + end + end + + context "defaults" do it "should be enabled" do - configuration.enabled.should == true + expect(configuration.enabled).to eq(true) end it "should have a cache expiry of 1 week" do - configuration.ttl.should == 604800 + expect(configuration.ttl).to eq(604800) end it "should disable collection synchronization" do - configuration.collection_synchronize.should == false + expect(configuration.collection_synchronize).to eq(false) end it "should default to :all for collection arguments" do - configuration.collection_arguments.should == [:all] + expect(configuration.collection_arguments).to eq([:all]) end it "should cache collections" do @@ -28,17 +70,17 @@ describe "outside a Rails environment" do it "should be logging to a buffered logger attached to a NilIO" do - configuration.logger.class.should == default_logger + expect(configuration.logger.class).to eq(default_logger) # ActiveSupport switched around the log destination variables # Check if either are what we expect to be compatible old_as = configuration.logger.instance_variable_get(:@log).class == NilIO new_as = configuration.logger.instance_variable_get(:@log_dest).class == NilIO newer_as = configuration.logger.instance_variable_get(:@logdev).instance_variable_get(:@dev).class == NilIO - (old_as || new_as || newer_as).should == true + expect(old_as || new_as || newer_as).to eq(true) end it "should cache responses in a memory store" do - configuration.cache.class.should == ActiveSupport::Cache::MemoryStore + expect(configuration.cache.class).to eq(ActiveSupport::Cache::MemoryStore) end end @@ -57,144 +99,55 @@ end it "should be logging to the rails logger" do - configuration.logger.should == "logger" + expect(configuration.logger).to eq("logger") end it "should cache responses in a memory store" do - configuration.cache.should == "cache" + expect(configuration.cache).to eq("cache") end end end - describe "when initialized through cached resource" do - before(:each) do - class Foo < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - end - + context "when initialized through cached resource" do it "should relfect the specified options" do - cr = Foo.cached_resource - cr.ttl.should == 1 + cr = Bar.cached_resource + expect(cr.ttl).to eq(1) expect(cr.race_condition_ttl).to eq(5) - cr.cache.should == "cache" - cr.logger.should == "logger" - cr.enabled.should == false - cr.collection_synchronize.should == true - cr.collection_arguments.should == [:every] - cr.custom.should == "irrelevant" - cr.cache_collections.should == true + expect(cr.cache).to eq("cache") + expect(cr.logger).to eq("logger") + expect(cr.enabled).to eq(false) + expect(cr.collection_synchronize).to eq(true) + expect(cr.collection_arguments).to eq([:every]) + expect(cr.custom).to eq("irrelevant") + expect(cr.cache_collections).to eq(true) end end - # re-evaluate - describe "when multiple are initialized through cached resource" do - before(:each) do - class Foo < ActiveResource::Base - cached_resource - end - - class Bar < ActiveResource::Base - cached_resource - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - + context "when multiple are initialized through cached resource" do it "they should have different configuration objects" do - Foo.cached_resource.object_id.should_not == Bar.cached_resource.object_id + expect(Foo.cached_resource.object_id).not_to eq(Bar.cached_resource.object_id) end - - it "they should have the same attributes" do - Foo.cached_resource.instance_variable_get(:@table).should == Bar.cached_resource.instance_variable_get(:@table) - end - end - describe "when cached resource is inherited" do - before(:each) do - class Bar < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - - class Foo < Bar - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - + context "when cached resource is inherited" do it "it should make sure each subclass has the same configuration" do - Bar.cached_resource.object_id.should == Foo.cached_resource.object_id + expect(Bar.cached_resource.object_id).to eq(Bar2.cached_resource.object_id) end - end - describe "when cached resource is inherited and then overriden" do - before(:each) do - class Bar < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - - class Foo < Bar - # override the superclasses configuration - self.cached_resource = CachedResource::Configuration.new(:ttl => 60) - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - - it "should have the specified options" do - Foo.cached_resource.ttl.should == 60 - end - - it "should have the default options for anything unspecified" do - cr = Foo.cached_resource - cr.cache.class.should == ActiveSupport::Cache::MemoryStore - cr.logger.class.should == default_logger - cr.enabled.should == true - cr.collection_synchronize.should == false - cr.collection_arguments.should == [:all] - cr.custom.should == nil - cr.ttl_randomization.should == false - cr.ttl_randomization_scale.should == (1..2) - cr.cache_collections.should == true + context "when cached resource is inherited and then overriden" do + it "only overwrites explicitly set options" do + cr = Bar3.cached_resource + expect(Bar3.cached_resource.ttl).to eq(60) + expect(cr.cache.class).to eq(ActiveSupport::Cache::MemoryStore) + expect(cr.logger.class).to eq(default_logger) + expect(cr.enabled).to eq(true) + expect(cr.collection_synchronize).to eq(false) + expect(cr.collection_arguments).to eq([:all]) + expect(cr.custom).to eq(nil) + expect(cr.ttl_randomization).to eq(false) + expect(cr.ttl_randomization_scale).to eq(1..2) + expect(cr.cache_collections).to eq(true) expect(cr.race_condition_ttl).to eq(86400) end @@ -202,7 +155,7 @@ class Foo < Bar # At the moment, not too keen on implementing some fancy # randomness validator. - describe "when ttl randomization is enabled" do + context "when ttl randomization is enabled" do before(:each) do @ttl = 1 configuration.ttl = @ttl @@ -213,8 +166,8 @@ class Foo < Bar it "it should produce a random ttl between ttl and ttl * 2" do generated_ttl = configuration.generate_ttl - generated_ttl.should_not == 10 - (@ttl..(2 * @ttl)).should include(generated_ttl) + expect(generated_ttl).not_to eq(10) + expect(@ttl..(2 * @ttl)).to include(generated_ttl) end describe "when a ttl randomization scale is set" do @@ -228,8 +181,8 @@ class Foo < Bar it "should produce a random ttl between ttl * lower bound and ttl * upper bound" do lower = @ttl * @lower upper = @ttl * @upper - (lower..upper).should include(configuration.generate_ttl) + expect(lower..upper).to include(configuration.generate_ttl) end end end -end \ No newline at end of file +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cde1699..86a757b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,13 +1,22 @@ -require 'rubygems' -require 'bundler/setup' -require 'active_resource' -require 'active_support' -require 'active_support/time' +require "simplecov" +SimpleCov.start do + add_filter "/spec/" + add_group "Lib", "lib/" + minimum_coverage 100 + refuse_coverage_drop +end -$:.unshift(File.dirname(__FILE__) + '/../lib') -require 'cached_resource' +require "rubygems" +require "bundler/setup" +require "active_resource" +require "active_support" +require "active_support/time" +require "cached_resource" +require "support/matchers" +require "pry-byebug" +require "timecop" +RSpec::Matchers.define_negated_matcher :not_change, :change RSpec.configure do |config| # nada end - diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb new file mode 100644 index 0000000..d4f6d03 --- /dev/null +++ b/spec/support/matchers.rb @@ -0,0 +1,36 @@ +require 'timeout' + +RSpec::Matchers.define :eventually do |expected_matcher| + supports_block_expectations + + chain :within do |timeout_duration| + @timeout_duration = timeout_duration + end + + match do |actual| + begin + Timeout.timeout(@timeout_duration || 5) do + until expected_matcher.matches?(actual.call) + sleep 0.5 + end + true + end + rescue Timeout::Error + false + end + end + + description do + description_text = "eventually #{expected_matcher.description}" + description_text += " within #{@timeout_duration} seconds" if @timeout_duration + description_text + end + + failure_message do |actual| + "expected #{actual.call} to #{description}" + end + + failure_message_when_negated do |actual| + "expected #{actual.call} not to #{description}" + end +end