diff --git a/app/models/pglogical_subscription.rb b/app/models/pglogical_subscription.rb index 5f990c1c48b..fa3ea9d6465 100644 --- a/app/models/pglogical_subscription.rb +++ b/app/models/pglogical_subscription.rb @@ -16,22 +16,20 @@ class PglogicalSubscription < ActsAsArModel :provider_region_name => :string ) - def self.find(*args) - case args.first + def self.search(mode, args) + case mode when :all then find_all - when :first, :last then find_one(args.first) - else find_id(args.first) + when :first, :last then find_one(mode) + else find_id(mode) end end def self.lookup_by_id(to_find) - find(to_find) + find_id(to_find) rescue ActiveRecord::RecordNotFound nil end - singleton_class.send(:alias_method, :find_by_id, :lookup_by_id) - Vmdb::Deprecation.deprecate_methods(singleton_class, :find_by_id => :lookup_by_id) def save!(reload_failover_monitor = true) assert_different_region! @@ -73,7 +71,7 @@ def delete(reload_failover_monitor_service = true) end def self.delete_all(list = nil) - (list.nil? ? find(:all) : list)&.each { |sub| sub.delete(false) } + (list.nil? ? all : list)&.each { |sub| sub.delete(false) } EvmDatabase.restart_failover_monitor_service_queue nil end @@ -189,7 +187,7 @@ def self.find_id(to_find) subscriptions.each do |s| return new(subscription_to_columns(s)) if s.symbolize_keys[:subscription_name] == to_find end - raise ActiveRecord::RecordNotFound, "Coundn't find subscription with id #{to_find}" + raise ActiveRecord::RecordNotFound, "Couldn't find subscription with id #{to_find}" end private_class_method :find_id diff --git a/lib/acts_as_ar_model.rb b/lib/acts_as_ar_model.rb index bee378212ad..6b00277cda4 100644 --- a/lib/acts_as_ar_model.rb +++ b/lib/acts_as_ar_model.rb @@ -1,3 +1,5 @@ +require 'query_relation' + class ActsAsArModel include Vmdb::Logging include ArVisibleAttribute @@ -121,6 +123,7 @@ def self.set_columns_hash(hash) include ActiveRecord::VirtualAttributes::VirtualFields include AttributeBag + extend QueryRelation::Queryable def self.instances_are_derived? true @@ -146,59 +149,49 @@ def self.reflections # # Find routines # - def self.find(*_args) - raise NotImplementedError, _("find must be implemented in a subclass") + def self.find(mode_or_id, *args) + if %i[all first last].include?(mode_or_id) + Vmdb::Deprecation.warn("find(:#{mode_or_id}) is deprecated (use #{mode_or_id} instead)") + search(mode_or_id, from_legacy_options(args.extract_options!)) + else + lookup_by_id(mode_or_id, *args).tap do |record| + raise ActiveRecord::RecordNotFound, "Couldn't find #{self} with id=#{mode_or_id}" if record.nil? + end + end end - # This method is called by QueryRelation upon executing the query. - # Since it will receive non-legacy search options, we need to convert - # them to handle the legacy find. - def self.search(mode, options = {}) - find(mode, to_legacy_options(options)) + def self.search(mode, options) + raise NotImplementedError, "must be defined in a subclass" end - private_class_method :search - def self.to_legacy_options(options) + def self.from_legacy_options(options) { - :conditions => options[:where], - :include => options[:includes], - :limit => options[:limit], - :order => options[:order], - :offset => options[:offset], - :select => options[:select], - :group => options[:group], + :where => options[:conditions], + :includes => options[:include], + :limit => options[:limit], + :order => options[:order], + :offset => options[:offset], + :select => options[:select], + :group => options[:group], }.delete_blanks end - private_class_method :to_legacy_options - - def self.all(*args) - require 'query_relation' - QueryRelation.new(self, *args) - end - - def self.first(*args) - find(:first, *args) - end - - def self.last(*args) - find(:last, *args) - end - - def self.count(*args) - all(*args).count - end + private_class_method :from_legacy_options def self.find_by_id(*id) options = id.extract_options! - options[:conditions] = {:id => id.first} - first(options) + options[:where] = {:id => id.first} + search(:first, options) end + singleton_class.send(:alias_method, :lookup_by_id, :find_by_id) + Vmdb::Deprecation.deprecate_methods(singleton_class, :find_by_id => :lookup_by_id) + def self.find_all_by_id(*ids) options = ids.extract_options! - options[:conditions] = {:id => ids.flatten} - all(options) + options[:where] = {:id => ids.flatten} + search(:all, options) end + Vmdb::Deprecation.deprecate_methods(singleton_class, :find_all_by_id => "use where(:id => ids) instead") # # Methods pulled from ActiveRecord 2.3.8 diff --git a/spec/lib/acts_as_ar_model_spec.rb b/spec/lib/acts_as_ar_model_spec.rb index 9de8db03201..4113e5af720 100644 --- a/spec/lib/acts_as_ar_model_spec.rb +++ b/spec/lib/acts_as_ar_model_spec.rb @@ -55,35 +55,133 @@ end describe ".all" do - it "chains through active query" do - expect(base_class).to receive(:find).with(:all, {}).and_return([]) + it "comes from QueryRelation" do + expect(base_class).to receive(:search).with(:all, {}).and_return([]) expect(base_class.all.to_a).to eq([]) end - it "supports where (as an example)" do - expect(base_class).to receive(:find).with(:all, {:conditions => {:id => 5}}).and_return([]) + it "supports where from QueryRelation (as an example)" do + expect(base_class).to receive(:search).with(:all, {:where => {:id => 5}}).and_return([]) expect(base_class.all.where(:id => 5).to_a).to eq([]) end end describe ".first" do - it "chains through active query" do - expect(base_class).to receive(:find).with(:first).and_return(nil) + it "comes from QueryRelation" do + expect(base_class).to receive(:search).with(:first, {}).and_return(nil) expect(base_class.first).to eq(nil) end end describe ".last" do - it "chains through active query" do - expect(base_class).to receive(:find).with(:last).and_return(nil) + it "comes from QueryRelation" do + expect(base_class).to receive(:search).with(:last, {}).and_return(nil) expect(base_class.last).to eq(nil) end end describe ".count" do - it "chains through active query" do - expect(base_class).to receive(:find).with(:all, {}).and_return([]) + it "comes from QueryRelation" do + expect(base_class).to receive(:search).with(:all, {}).and_return([]) expect(base_class.count).to eq(0) end end + + describe ".find (deprecated)" do + around do |example| + Vmdb::Deprecation.silence do + example.run + end + end + + describe "find(:all)" do + it "chains through QueryRelation" do + expect(base_class).to receive(:search).with(:all, {}).and_return([]) + expect(base_class.find(:all).to_a).to eq([]) + end + + it "supports :conditions legacy option (as an example)" do + expect(base_class).to receive(:search).with(:all, {:where => {:id => 5}}).and_return([]) + expect(base_class.find(:all, :conditions => {:id => 5}).to_a).to eq([]) + end + end + + describe "find(:first)" do + it "chains through QueryRelation" do + expect(base_class).to receive(:search).with(:first, {}).and_return(nil) + expect(base_class.find(:first)).to eq(nil) + end + end + + describe ".find(:last)" do + it "chains through QueryRelation" do + expect(base_class).to receive(:search).with(:last, {}).and_return(nil) + expect(base_class.find(:last)).to eq(nil) + end + end + end + + describe ".find" do + it "finds a record by id" do + expected = Object.new + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(expected) + expect(base_class.find(5)).to eq(expected) + end + + it "raises when not found" do + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(nil) + expect { base_class.find(5) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + describe ".lookup_by_id" do + it "finds a record by id" do + expected = Object.new + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(expected) + expect(base_class.lookup_by_id(5)).to eq(expected) + end + + it "returns nil when not found" do + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(nil) + expect(base_class.lookup_by_id(5)).to be_nil + end + end + + describe ".find_by_id (deprecated)" do + around do |example| + Vmdb::Deprecation.silence do + example.run + end + end + + it "finds a record by id" do + expected = Object.new + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(expected) + expect(base_class.find_by_id(5)).to eq(expected) + end + + it "returns nil when not found" do + expect(base_class).to receive(:search).with(:first, {:where => {:id => 5}}).and_return(nil) + expect(base_class.find_by_id(5)).to be_nil + end + end + + describe ".find_all_by_id (deprecated)" do + around do |example| + Vmdb::Deprecation.silence do + example.run + end + end + + it "finds record by ids" do + expected = [Object.new, Object.new] + expect(base_class).to receive(:search).with(:all, {:where => {:id => [5, 6]}}).and_return(expected) + expect(base_class.find_all_by_id(5, 6)).to eq(expected) + end + + it "returns empty Array when none found" do + expect(base_class).to receive(:search).with(:all, {:where => {:id => [5, 6]}}).and_return([]) + expect(base_class.find_all_by_id(5, 6)).to eq([]) + end + end end diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index 2c73289f501..8113b5097f0 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -2660,7 +2660,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) end it "support aaarm object" do - expect(VimPerformanceTrend).to receive(:find).with(:all, {:include => {:a => {}}}).and_return([:good]) + expect(VimPerformanceTrend).to receive(:search).with(:all, {:includes => {:a => {}}, :references => {:a => {}}}).and_return([:good]) expect(described_class.filtered(VimPerformanceTrend, :include_for_find => {:a => {}}).to_a).to match_array([:good]) end diff --git a/spec/models/pglogical_subscription_spec.rb b/spec/models/pglogical_subscription_spec.rb index e871e915247..bba0d0b4574 100644 --- a/spec/models/pglogical_subscription_spec.rb +++ b/spec/models/pglogical_subscription_spec.rb @@ -118,42 +118,35 @@ expect(actual_attrs).to match_array(expected_attrs) end - it "supports find(:all) with records" do - with_records - actual_attrs = described_class.find(:all).map(&:attributes) - expect(actual_attrs).to match_array(expected_attrs) - end - it "retrieves no records with no records" do with_no_records expect(described_class.all).to be_empty - expect(described_class.find(:all)).to be_empty end end describe ".first" do it "retrieves the first record with records" do with_records - rec = described_class.find(:first) + rec = described_class.first expect(rec.attributes).to eq(expected_attrs.first) end it "returns nil with no records" do with_no_records - expect(described_class.find(:first)).to be_nil + expect(described_class.first).to be_nil end end describe ".last" do it "retrieves the last record with :last" do with_records - rec = described_class.find(:last) + rec = described_class.last expect(rec.attributes).to eq(expected_attrs.last) end it "returns nil with :last" do with_no_records - expect(described_class.find(:last)).to be_nil + expect(described_class.last).to be_nil end end @@ -264,7 +257,7 @@ with_records allow(MiqRegionRemote).to receive(:with_remote_connection).and_yield(double(:connection)) - sub = described_class.find(:first) + sub = described_class.first sub.host = "other-host.example.com" allow(sub).to receive(:assert_different_region!) @@ -289,7 +282,7 @@ end it "deletes all subscriptions if no parameter passed" do - allow(described_class).to receive(:find).with(:all).and_return([subscription1, subscription2]) + allow(described_class).to receive(:search).with(:all, {}).and_return([subscription1, subscription2]) expect(subscription1).to receive(:delete) expect(subscription2).to receive(:delete) described_class.delete_all @@ -368,7 +361,7 @@ end describe "#delete" do - let(:sub) { described_class.find(:first) } + let(:sub) { described_class.first } it "drops the subscription" do allow(pglogical).to receive(:subscriptions).and_return([subscriptions.first], []) @@ -444,7 +437,7 @@ it "validates existing subscriptions with new parameters" do allow(pglogical).to receive(:subscriptions).and_return([subscriptions.first]) - sub = described_class.find(:first) + sub = described_class.first expect(sub.host).to eq "example.com" expect(sub.port).to be_blank expect(sub.user).to eq "root"