From c11dfe5e7ef615425e7d87b0bc6c04792520645d Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Tue, 3 Nov 2020 18:57:00 -0700 Subject: [PATCH 01/13] added find_by(attribute: value) methods --- active_remote-cached.gemspec | 2 + lib/active_remote/cached.rb | 20 ++ .../cached_find_by_methods_spec.rb | 179 ++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 spec/active_remote/cached_find_by_methods_spec.rb diff --git a/active_remote-cached.gemspec b/active_remote-cached.gemspec index 1e86d93..3a5aba7 100644 --- a/active_remote-cached.gemspec +++ b/active_remote-cached.gemspec @@ -24,4 +24,6 @@ Gem::Specification.new do |gem| gem.add_development_dependency "mocha" gem.add_development_dependency "pry" gem.add_development_dependency "rake" + gem.add_development_dependency "rspec" + gem.add_development_dependency "byebug" end diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 4940e46..7a1997f 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -33,6 +33,26 @@ def self.default_options(options = nil) end module ClassMethods + def cached_find_by(**args) + options = args.delete(:options) || {} + find_method_name = _cached_search_method_name(args.keys) + raise "no such finder #{find_method_name}" unless respond_to?(find_method_name.to_sym) + send(find_method_name, args.values, options).first + end + + def cached_find_by!(**args) + result = self.cached_find_by(args) + raise ::ActiveRemote::RemoteRecordNotFound, self if result.nil? + result + end + + def cached_exist_find_by?(**options) + options = args.delete(:options) || {} + exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased down below. + raise "#{exist_by_method_name} does not exist for #{self}" unless respond_to?(exist_by_method_name) + send(exist_by_method_name, args.values, options) + end + def cached_finders_for(*cached_finder_keys) options = cached_finder_keys.extract_options! diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb new file mode 100644 index 0000000..b307639 --- /dev/null +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -0,0 +1,179 @@ +require 'spec_helper' +require 'byebug' + +class FindByMethodClass + include ::ActiveRemote::Cached + + def self.find(**args) + byebug + end + + def self.search; nil; end; + + cached_finders_for :foo, :expires_in => 500 + cached_finders_for :guid + cached_finders_for :guid, :user_guid + cached_finders_for [:user_guid, :client_guid] + cached_finders_for [:derp, :user_guid, :client_guid] +end + +describe FindByMethodClass do + describe "API" do + it "creates 'cached_find_by'" do + expect(FindByMethodClass).to respond_to(:cached_find_by) + end + + it "creates 'cached_find_by!'" do + expect(FindByMethodClass).to respond_to(:cached_find_by!) + end + + it "creates 'cached_exist_find_by?'" do + expect(FindByMethodClass).to respond_to(:cached_exist_find_by?) + end + end + + describe "#cached_find_by(:attribute => value)" do + context "guid" do + it "calls cached_find_by_guid and returns 1" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([1]) + expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to eq(1) + end + + it "calls cached_find_by_guid and nothing found" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to be nil + end + + it "calls cached_find_by_guid and nothing found does not raise an error" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect do + FindByMethodClass.cached_find_by(:guid => "foobar") + end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) + end + end + + context "foo" do + + end + + context "user_guid, client_guid" do + it "calls cached_find_by_guid and returns 1" do + expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], {}).and_return([1]) + expect(FindByMethodClass.cached_find_by(:user_guid => "foo", :client_guid => "bar")).to eq(1) + end + + it "calls cached_search_by_user_guid_and_client_guid with options returns 1" do + expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], {:expires_in => 1}).and_return([1]) + expect(FindByMethodClass.cached_find_by(:user_guid => "foo", :client_guid => "bar", :options => { :expires_in => 1 })).to eq(1) + end + + it "calls cached_find_by_guid and nothing found" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to be nil + end + + it "calls cached_find_by_guid and nothing found does not raise an error" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect do + FindByMethodClass.cached_find_by(:guid => "foobar") + end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) + end + end + end + + describe "#cached_find_by!(:attribute => value)" do + context "guid" do + it "calls cached_find_by_guid! and returns 1" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([1]) + expect(FindByMethodClass.cached_find_by!(:guid => "foobar")).to eq(1) + end + + it "calls cached_find_by_guid! and nothing found to raise an error" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect do + FindByMethodClass.cached_find_by!(:guid => "foobar") + end.to raise_error(::ActiveRemote::RemoteRecordNotFound) + end + end + end + + # describe "#cached_find_by_guid" do + # before do + # ::ActiveRemote::Cached.cache(HashCache.new) + # ::ActiveRemote::Cached.default_options(:expires_in => 100) + # end + + # after do + # ::ActiveRemote::Cached.default_options({}) + # end + + # it "executes find_by_guid when cached_find with guid called" do + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find(:guid => :guid).must_equal(:hello) + # end + # end + + # it "executes the fetch block if not present in cache" do + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_guid(:guid).must_equal(:hello) + # end + # end + + # it "merges the default options in for the fetch call" do + # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "guid"], :expires_in => 100).returns(:hello) + + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_guid(:guid).must_equal(:hello) + # end + # end + + # it "overrides the default options with local options for the fetch call" do + # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "guid"], :expires_in => 200).returns(:hello) + + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_guid(:guid, :expires_in => 200).must_equal(:hello) + # end + # end + + # describe "namespaced cache" do + # before do + # ::ActiveRemote::Cached.default_options(:expires_in => 100, :namespace => "MyApp") + # end + + # it "uses the namespace as a prefix to the cache key" do + # ::ActiveRemote::Cached.cache.expects(:fetch).with(["MyApp", FindMethodClass.name, "#find", "guid"], :expires_in => 100).returns(:hello) + + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_guid(:guid) + # end + # end + # end + # end + + # describe "#cached_find_by_foo" do + # before do + # ::ActiveRemote::Cached.cache(HashCache.new) + # ::ActiveRemote::Cached.default_options(:expires_in => 100) + # end + + # after do + # ::ActiveRemote::Cached.default_options({}) + # end + + # it "overrides the default options with cached_finder options for the fetch call" do + # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "foo"], :expires_in => 500).returns(:hello) + + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_foo(:foo).must_equal(:hello) + # end + # end + + # it "overrides the cached_finder options with local options for the fetch call" do + # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "foo"], :expires_in => 200).returns(:hello) + + # FindMethodClass.stub(:find, :hello) do + # FindMethodClass.cached_find_by_foo(:foo, :expires_in => 200).must_equal(:hello) + # end + # end + # end +end From b7ec249fafb958474fcd0a4fedcfddbd7e857532 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 11:55:49 -0700 Subject: [PATCH 02/13] added more specs --- lib/active_remote/cached.rb | 9 +- .../cached_find_by_methods_spec.rb | 120 ++++-------------- 2 files changed, 28 insertions(+), 101 deletions(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 7a1997f..67d1912 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -34,9 +34,9 @@ def self.default_options(options = nil) module ClassMethods def cached_find_by(**args) - options = args.delete(:options) || {} + options = args.delete(:cache_options) || {} find_method_name = _cached_search_method_name(args.keys) - raise "no such finder #{find_method_name}" unless respond_to?(find_method_name.to_sym) + raise "no such finder #{find_method_name}" unless respond_to?(find_method_name) send(find_method_name, args.values, options).first end @@ -46,8 +46,8 @@ def cached_find_by!(**args) result end - def cached_exist_find_by?(**options) - options = args.delete(:options) || {} + def cached_exist_find_by?(**args) + options = args.delete(:cache_options) || {} exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased down below. raise "#{exist_by_method_name} does not exist for #{self}" unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, options) @@ -55,7 +55,6 @@ def cached_exist_find_by?(**options) def cached_finders_for(*cached_finder_keys) options = cached_finder_keys.extract_options! - cached_finder_keys.each do |cached_finder_key| _create_cached_finder_for(cached_finder_key, options) end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index b307639..ad83bc0 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -4,11 +4,8 @@ class FindByMethodClass include ::ActiveRemote::Cached - def self.find(**args) - byebug - end - - def self.search; nil; end; + def self.find(**args); nil; end; + def self.search(**args); nil; end; cached_finders_for :foo, :expires_in => 500 cached_finders_for :guid @@ -35,27 +32,23 @@ def self.search; nil; end; describe "#cached_find_by(:attribute => value)" do context "guid" do it "calls cached_find_by_guid and returns 1" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([1]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([1]) expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to eq(1) end it "calls cached_find_by_guid and nothing found" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to be nil end it "calls cached_find_by_guid and nothing found does not raise an error" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect do FindByMethodClass.cached_find_by(:guid => "foobar") end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) end end - context "foo" do - - end - context "user_guid, client_guid" do it "calls cached_find_by_guid and returns 1" do expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], {}).and_return([1]) @@ -64,32 +57,46 @@ def self.search; nil; end; it "calls cached_search_by_user_guid_and_client_guid with options returns 1" do expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], {:expires_in => 1}).and_return([1]) - expect(FindByMethodClass.cached_find_by(:user_guid => "foo", :client_guid => "bar", :options => { :expires_in => 1 })).to eq(1) + expect(FindByMethodClass.cached_find_by(:user_guid => "foo", :client_guid => "bar", :cache_options => { :expires_in => 1 })).to eq(1) + end + + it "calls cached_search_by_user_guid_and_client_guid with out of order attribute keys" do + # cached_search_by_client_guid_and_user_guid is inverse of `cached_finders_for [:user_guid, :client_guid]` because it permutes + # all possible combinations of the attributes + expect(FindByMethodClass).to receive(:cached_search_by_client_guid_and_user_guid).with(["bar", "foo"], {}).and_return([1]) + expect(FindByMethodClass.cached_find_by(:client_guid => "bar", :user_guid => "foo")).to eq(1) end it "calls cached_find_by_guid and nothing found" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect(FindByMethodClass.cached_find_by(:guid => "foobar")).to be nil end it "calls cached_find_by_guid and nothing found does not raise an error" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect do FindByMethodClass.cached_find_by(:guid => "foobar") end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) end end + + context "derp, user_guid, client_guid" do + it "calls cached_search_by_user_guid_and_client_guid with out of order attribute keys" do + expect(FindByMethodClass).to receive(:cached_search_by_derp_and_user_guid_and_client_guid).with(["foo", "bar", "baz"], {}).and_return([1]) + expect(FindByMethodClass.cached_find_by(:derp => "foo", :user_guid => "bar", :client_guid => "baz")).to eq(1) + end + end end describe "#cached_find_by!(:attribute => value)" do context "guid" do it "calls cached_find_by_guid! and returns 1" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([1]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([1]) expect(FindByMethodClass.cached_find_by!(:guid => "foobar")).to eq(1) end it "calls cached_find_by_guid! and nothing found to raise an error" do - expect(FindByMethodClass).to receive(:cached_search_by_guid).with("foobar", {}).and_return([]) + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect do FindByMethodClass.cached_find_by!(:guid => "foobar") end.to raise_error(::ActiveRemote::RemoteRecordNotFound) @@ -97,83 +104,4 @@ def self.search; nil; end; end end - # describe "#cached_find_by_guid" do - # before do - # ::ActiveRemote::Cached.cache(HashCache.new) - # ::ActiveRemote::Cached.default_options(:expires_in => 100) - # end - - # after do - # ::ActiveRemote::Cached.default_options({}) - # end - - # it "executes find_by_guid when cached_find with guid called" do - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find(:guid => :guid).must_equal(:hello) - # end - # end - - # it "executes the fetch block if not present in cache" do - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_guid(:guid).must_equal(:hello) - # end - # end - - # it "merges the default options in for the fetch call" do - # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "guid"], :expires_in => 100).returns(:hello) - - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_guid(:guid).must_equal(:hello) - # end - # end - - # it "overrides the default options with local options for the fetch call" do - # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "guid"], :expires_in => 200).returns(:hello) - - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_guid(:guid, :expires_in => 200).must_equal(:hello) - # end - # end - - # describe "namespaced cache" do - # before do - # ::ActiveRemote::Cached.default_options(:expires_in => 100, :namespace => "MyApp") - # end - - # it "uses the namespace as a prefix to the cache key" do - # ::ActiveRemote::Cached.cache.expects(:fetch).with(["MyApp", FindMethodClass.name, "#find", "guid"], :expires_in => 100).returns(:hello) - - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_guid(:guid) - # end - # end - # end - # end - - # describe "#cached_find_by_foo" do - # before do - # ::ActiveRemote::Cached.cache(HashCache.new) - # ::ActiveRemote::Cached.default_options(:expires_in => 100) - # end - - # after do - # ::ActiveRemote::Cached.default_options({}) - # end - - # it "overrides the default options with cached_finder options for the fetch call" do - # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "foo"], :expires_in => 500).returns(:hello) - - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_foo(:foo).must_equal(:hello) - # end - # end - - # it "overrides the cached_finder options with local options for the fetch call" do - # ::ActiveRemote::Cached.cache.expects(:fetch).with([FindMethodClass.name, "#find", "foo"], :expires_in => 200).returns(:hello) - - # FindMethodClass.stub(:find, :hello) do - # FindMethodClass.cached_find_by_foo(:foo, :expires_in => 200).must_equal(:hello) - # end - # end - # end end From 4dbb4ad37a86df902c2779ff082a2e9ca6d57071 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 11:56:18 -0700 Subject: [PATCH 03/13] removed extraneous from gemspec --- active_remote-cached.gemspec | 2 -- 1 file changed, 2 deletions(-) diff --git a/active_remote-cached.gemspec b/active_remote-cached.gemspec index 3a5aba7..1e86d93 100644 --- a/active_remote-cached.gemspec +++ b/active_remote-cached.gemspec @@ -24,6 +24,4 @@ Gem::Specification.new do |gem| gem.add_development_dependency "mocha" gem.add_development_dependency "pry" gem.add_development_dependency "rake" - gem.add_development_dependency "rspec" - gem.add_development_dependency "byebug" end From 0afa08ea60d35669862fdd1e6fd4876ca6188293 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 11:57:20 -0700 Subject: [PATCH 04/13] removed byebug, reverted deleted blank line --- lib/active_remote/cached.rb | 1 + spec/active_remote/cached_find_by_methods_spec.rb | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 67d1912..905046e 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -55,6 +55,7 @@ def cached_exist_find_by?(**args) def cached_finders_for(*cached_finder_keys) options = cached_finder_keys.extract_options! + cached_finder_keys.each do |cached_finder_key| _create_cached_finder_for(cached_finder_key, options) end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index ad83bc0..b73cd8b 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'byebug' class FindByMethodClass include ::ActiveRemote::Cached From 8bec8f91efa0c63b4350f568fe05524fb52caceb Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:03:00 -0700 Subject: [PATCH 05/13] added rspec dependency --- active_remote-cached.gemspec | 1 + spec/active_remote/cached_find_by_methods_spec.rb | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/active_remote-cached.gemspec b/active_remote-cached.gemspec index 1e86d93..8dac338 100644 --- a/active_remote-cached.gemspec +++ b/active_remote-cached.gemspec @@ -24,4 +24,5 @@ Gem::Specification.new do |gem| gem.add_development_dependency "mocha" gem.add_development_dependency "pry" gem.add_development_dependency "rake" + gem.add_development_dependency "rspec" end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index b73cd8b..c6d53a3 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -46,6 +46,11 @@ def self.search(**args); nil; end; FindByMethodClass.cached_find_by(:guid => "foobar") end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) end + + it "calls cached_find_by_guid with cache options" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], { :expires_in => 1 }).and_return([1]) + expect(FindByMethodClass.cached_find_by(:guid => "foobar", :cache_options => { :expires_in => 1 })).to eq(1) + end end context "user_guid, client_guid" do @@ -55,7 +60,7 @@ def self.search(**args); nil; end; end it "calls cached_search_by_user_guid_and_client_guid with options returns 1" do - expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], {:expires_in => 1}).and_return([1]) + expect(FindByMethodClass).to receive(:cached_search_by_user_guid_and_client_guid).with(["foo", "bar"], { :expires_in => 1 }).and_return([1]) expect(FindByMethodClass.cached_find_by(:user_guid => "foo", :client_guid => "bar", :cache_options => { :expires_in => 1 })).to eq(1) end From e933bd03046fd3d477ae0552ddd13a81a77d601a Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:04:56 -0700 Subject: [PATCH 06/13] added ! options spec --- lib/active_remote/cached.rb | 8 ++++---- spec/active_remote/cached_find_by_methods_spec.rb | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 905046e..7e60dd8 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -34,10 +34,10 @@ def self.default_options(options = nil) module ClassMethods def cached_find_by(**args) - options = args.delete(:cache_options) || {} + cache_options = args.delete(:cache_options) || {} find_method_name = _cached_search_method_name(args.keys) raise "no such finder #{find_method_name}" unless respond_to?(find_method_name) - send(find_method_name, args.values, options).first + send(find_method_name, args.values, cache_options).first end def cached_find_by!(**args) @@ -47,10 +47,10 @@ def cached_find_by!(**args) end def cached_exist_find_by?(**args) - options = args.delete(:cache_options) || {} + cache_options = args.delete(:cache_options) || {} exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased down below. raise "#{exist_by_method_name} does not exist for #{self}" unless respond_to?(exist_by_method_name) - send(exist_by_method_name, args.values, options) + send(exist_by_method_name, args.values, cache_options) end def cached_finders_for(*cached_finder_keys) diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index c6d53a3..e058da6 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -105,6 +105,11 @@ def self.search(**args); nil; end; FindByMethodClass.cached_find_by!(:guid => "foobar") end.to raise_error(::ActiveRemote::RemoteRecordNotFound) end + + it "calls cached_find_by_guid! with cache options" do + expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {:expires_in => 500}).and_return([1]) + FindByMethodClass.cached_find_by!(:guid => "foobar", :cache_options => {:expires_in => 500}) + end end end From ad2b7128ebac1a1e85ff961d1ded8b59afb7a413 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:05:23 -0700 Subject: [PATCH 07/13] comment --- lib/active_remote/cached.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 7e60dd8..d880470 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -48,7 +48,7 @@ def cached_find_by!(**args) def cached_exist_find_by?(**args) cache_options = args.delete(:cache_options) || {} - exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased down below. + exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased raise "#{exist_by_method_name} does not exist for #{self}" unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, cache_options) end From 1a17206b8e6aea039f9905e2e17e6ada03687eae Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:27:29 -0700 Subject: [PATCH 08/13] added error message when cached_finders_for is not defined for a particular attribute --- lib/active_remote/cached.rb | 4 ++-- spec/active_remote/cached_find_by_methods_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index d880470..62a632c 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -36,7 +36,7 @@ module ClassMethods def cached_find_by(**args) cache_options = args.delete(:cache_options) || {} find_method_name = _cached_search_method_name(args.keys) - raise "no such finder #{find_method_name}" unless respond_to?(find_method_name) + raise "cached_finders_for #{args.keys.map(&:to_sym)} is not declared. Add it to call this method." unless respond_to?(find_method_name) send(find_method_name, args.values, cache_options).first end @@ -49,7 +49,7 @@ def cached_find_by!(**args) def cached_exist_find_by?(**args) cache_options = args.delete(:cache_options) || {} exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased - raise "#{exist_by_method_name} does not exist for #{self}" unless respond_to?(exist_by_method_name) + raise "cached_finders_for #{args.keys.map(&:to_sym).join(',')} is not declared. Add it to call this method." unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, cache_options) end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index e058da6..25f3e80 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -26,6 +26,18 @@ def self.search(**args); nil; end; it "creates 'cached_exist_find_by?'" do expect(FindByMethodClass).to respond_to(:cached_exist_find_by?) end + + it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by" do + expect do + FindByMethodClass.cached_find_by(not_an_attribute: "foo") + end.to raise_error("cached_finders_for [:not_an_attribute] is not declared. Add it to call this method.") + end + + it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_find_by" do + expect do + FindByMethodClass.cached_find_by(not_an_attribute: "foo", other_not_an_attribute: "bar") + end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] is not declared. Add it to call this method.") + end end describe "#cached_find_by(:attribute => value)" do From 5092cd7c3701580e390db2b7ecfa02eb53de59b2 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:28:28 -0700 Subject: [PATCH 09/13] changed error message for missing finders --- lib/active_remote/cached.rb | 4 ++-- spec/active_remote/cached_find_by_methods_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 62a632c..58c1e77 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -36,7 +36,7 @@ module ClassMethods def cached_find_by(**args) cache_options = args.delete(:cache_options) || {} find_method_name = _cached_search_method_name(args.keys) - raise "cached_finders_for #{args.keys.map(&:to_sym)} is not declared. Add it to call this method." unless respond_to?(find_method_name) + raise "cached_finders_for #{args.keys.map(&:to_sym)} not included in class definition" unless respond_to?(find_method_name) send(find_method_name, args.values, cache_options).first end @@ -49,7 +49,7 @@ def cached_find_by!(**args) def cached_exist_find_by?(**args) cache_options = args.delete(:cache_options) || {} exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased - raise "cached_finders_for #{args.keys.map(&:to_sym).join(',')} is not declared. Add it to call this method." unless respond_to?(exist_by_method_name) + raise "cached_finders_for #{args.keys.map(&:to_sym).join(',')} not included in class definition" unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, cache_options) end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index 25f3e80..c7cb85d 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -30,13 +30,13 @@ def self.search(**args); nil; end; it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by" do expect do FindByMethodClass.cached_find_by(not_an_attribute: "foo") - end.to raise_error("cached_finders_for [:not_an_attribute] is not declared. Add it to call this method.") + end.to raise_error("cached_finders_for [:not_an_attribute] not included in class definition") end it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_find_by" do expect do FindByMethodClass.cached_find_by(not_an_attribute: "foo", other_not_an_attribute: "bar") - end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] is not declared. Add it to call this method.") + end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] not included in class definition") end end From 26481037239e57cb779f018a5175bc3e31db5a71 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:30:33 -0700 Subject: [PATCH 10/13] added spec for making sure we hit HB --- lib/active_remote/cached.rb | 2 +- .../cached_find_by_methods_spec.rb | 38 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index 58c1e77..c2caa81 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -49,7 +49,7 @@ def cached_find_by!(**args) def cached_exist_find_by?(**args) cache_options = args.delete(:cache_options) || {} exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased - raise "cached_finders_for #{args.keys.map(&:to_sym).join(',')} not included in class definition" unless respond_to?(exist_by_method_name) + raise "cached_finders_for #{args.keys.map(&:to_sym)} not included in class definition" unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, cache_options) end diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index c7cb85d..e842d12 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -23,20 +23,36 @@ def self.search(**args); nil; end; expect(FindByMethodClass).to respond_to(:cached_find_by!) end - it "creates 'cached_exist_find_by?'" do - expect(FindByMethodClass).to respond_to(:cached_exist_find_by?) - end + context "cached_find_by" do + it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by" do + expect do + FindByMethodClass.cached_find_by(not_an_attribute: "foo") + end.to raise_error("cached_finders_for [:not_an_attribute] not included in class definition") + end - it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by" do - expect do - FindByMethodClass.cached_find_by(not_an_attribute: "foo") - end.to raise_error("cached_finders_for [:not_an_attribute] not included in class definition") + it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_find_by" do + expect do + FindByMethodClass.cached_find_by(not_an_attribute: "foo", other_not_an_attribute: "bar") + end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] not included in class definition") + end end - it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_find_by" do - expect do - FindByMethodClass.cached_find_by(not_an_attribute: "foo", other_not_an_attribute: "bar") - end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] not included in class definition") + context "cached_exist_find_by?" do + it "creates 'cached_exist_find_by?'" do + expect(FindByMethodClass).to respond_to(:cached_exist_find_by?) + end + + it "raises an error if the cached_finders_for does not exist for a single attribute cached_exist_find_by" do + expect do + FindByMethodClass.cached_exist_find_by?(not_an_attribute: "foo") + end.to raise_error("cached_finders_for [:not_an_attribute] not included in class definition") + end + + it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_exist_find_by?" do + expect do + FindByMethodClass.cached_exist_find_by?(not_an_attribute: "foo", other_not_an_attribute: "bar") + end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] not included in class definition") + end end end From a1144642912e389831e8285250176b9028018969 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:31:37 -0700 Subject: [PATCH 11/13] added more specs for other cached_find_by* methods --- .../cached_find_by_methods_spec.rb | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index e842d12..afecd7f 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -15,15 +15,12 @@ def self.search(**args); nil; end; describe FindByMethodClass do describe "API" do - it "creates 'cached_find_by'" do - expect(FindByMethodClass).to respond_to(:cached_find_by) - end - - it "creates 'cached_find_by!'" do - expect(FindByMethodClass).to respond_to(:cached_find_by!) - end context "cached_find_by" do + it "creates 'cached_find_by'" do + expect(FindByMethodClass).to respond_to(:cached_find_by) + end + it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by" do expect do FindByMethodClass.cached_find_by(not_an_attribute: "foo") @@ -37,6 +34,25 @@ def self.search(**args); nil; end; end end + context "cached_find_by" do + it "creates 'cached_find_by!'" do + expect(FindByMethodClass).to respond_to(:cached_find_by!) + end + + it "raises an error if the cached_finders_for does not exist for a single attribute cached_find_by!" do + expect do + FindByMethodClass.cached_find_by!(not_an_attribute: "foo") + end.to raise_error("cached_finders_for [:not_an_attribute] not included in class definition") + end + + it "raises an error if the cached_finders_for does not exist for a multiple attribute cached_find_by!" do + expect do + FindByMethodClass.cached_find_by!(not_an_attribute: "foo", other_not_an_attribute: "bar") + end.to raise_error("cached_finders_for [:not_an_attribute, :other_not_an_attribute] not included in class definition") + end + end + + context "cached_exist_find_by?" do it "creates 'cached_exist_find_by?'" do expect(FindByMethodClass).to respond_to(:cached_exist_find_by?) From 7134701290856439ca68db3590d573b025b88c43 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 12:32:02 -0700 Subject: [PATCH 12/13] comment change --- lib/active_remote/cached.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_remote/cached.rb b/lib/active_remote/cached.rb index c2caa81..0064b88 100644 --- a/lib/active_remote/cached.rb +++ b/lib/active_remote/cached.rb @@ -48,7 +48,7 @@ def cached_find_by!(**args) def cached_exist_find_by?(**args) cache_options = args.delete(:cache_options) || {} - exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as it's aliased + exist_by_method_name = _cached_exist_search_method_name(args.keys) # ? is not necessary as the method is aliased both ways raise "cached_finders_for #{args.keys.map(&:to_sym)} not included in class definition" unless respond_to?(exist_by_method_name) send(exist_by_method_name, args.values, cache_options) end From 5c5f2003435428049008addab858057c1343b8d1 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Wed, 4 Nov 2020 13:33:45 -0700 Subject: [PATCH 13/13] removed code smell of raise_error --- spec/active_remote/cached_find_by_methods_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/active_remote/cached_find_by_methods_spec.rb b/spec/active_remote/cached_find_by_methods_spec.rb index afecd7f..a22c6a5 100644 --- a/spec/active_remote/cached_find_by_methods_spec.rb +++ b/spec/active_remote/cached_find_by_methods_spec.rb @@ -88,7 +88,7 @@ def self.search(**args); nil; end; expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect do FindByMethodClass.cached_find_by(:guid => "foobar") - end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) + end.not_to raise_error end it "calls cached_find_by_guid with cache options" do @@ -124,7 +124,7 @@ def self.search(**args); nil; end; expect(FindByMethodClass).to receive(:cached_search_by_guid).with(["foobar"], {}).and_return([]) expect do FindByMethodClass.cached_find_by(:guid => "foobar") - end.not_to raise_error(::ActiveRemote::RemoteRecordNotFound) + end.not_to raise_error end end