diff --git a/app/models/metric/ci_mixin/state_finders.rb b/app/models/metric/ci_mixin/state_finders.rb index 670708e31f28..d54c94ded61d 100644 --- a/app/models/metric/ci_mixin/state_finders.rb +++ b/app/models/metric/ci_mixin/state_finders.rb @@ -36,16 +36,26 @@ def preload_vim_performance_state_for_ts_iso8601(conditions = {}) # using @last_vps_ts to ensure we don't load at one time and then use at another def vim_performance_state_association(ts, assoc) if assoc.to_s == "miq_regions" - return respond_to?(:miq_regions) ? miq_regions : [] + return respond_to?(:miq_regions) ? miq_regions : MiqRegion.none + end + + # this is a virtual reflection, just return the value + if ! self.class.reflect_on_association(assoc) + return vim_performance_state_for_ts(ts).public_send(assoc) end if !defined?(@last_vps_ts) || @last_vps_ts != ts @last_vps_ts = ts - public_send(assoc).reset - - MiqPreloader.preload(self, assoc, vim_performance_state_for_ts(ts).public_send(assoc).load) + # we are using a different timestamp + # clear out relevant associations + (VimPerformanceState::ASSOCIATIONS & self.class.reflections.keys.map(&:to_sym)).each do |vps_assoc| + association(vps_assoc).reset + end end + if !association(assoc.to_sym).loaded? + MiqPreloader.preload_from_array(self, assoc, vim_performance_state_for_ts(ts).public_send(assoc)) + end public_send(assoc) end end diff --git a/spec/models/metric/ci_mixin/state_finders_spec.rb b/spec/models/metric/ci_mixin/state_finders_spec.rb index c5d560c4b35b..cffd40119455 100644 --- a/spec/models/metric/ci_mixin/state_finders_spec.rb +++ b/spec/models/metric/ci_mixin/state_finders_spec.rb @@ -2,12 +2,69 @@ let(:image) { FactoryBot.create(:container_image) } let(:container1) { FactoryBot.create(:container, :container_image => image) } let(:container2) { FactoryBot.create(:container, :container_image => image) } - let(:node1) { FactoryBot.create(:container_node) } - let(:node2) { FactoryBot.create(:container_node) } + + # region is currently the only class that has multiple rollups + let(:region) { MiqRegion.my_region || MiqRegion.seed } + let(:ems1) { FactoryBot.create(:ext_management_system) } # implied :region => region + let(:ems2) { FactoryBot.create(:ext_management_system) } + let(:storage1) { FactoryBot.create(:storage) } + let(:storage2) { FactoryBot.create(:storage) } let(:ts_now) { Time.now.utc.beginning_of_hour.to_s } let(:timestamp) { 2.hours.ago.utc.beginning_of_hour.to_s } + describe "#vim_performance_state_association" do + let(:c_vps_now) { create_vps(image, ts_now, :containers => [container1, container2]) } + let(:c_vps) { create_vps(image, timestamp, :containers => [container1]) } + + let(:r_vps_now) { create_vps(region, ts_now, :ext_management_systems => [ems1, ems2], :storages => [storage1, storage2]) } + let(:r_vps) { create_vps(region, timestamp, :ext_management_systems => [ems1], :storages => [storage1]) } + + it "performs a single query when looking up an association multiple times" do + c_vps + + expect do + expect(image.vim_performance_state_association(timestamp, :containers)).to eq([container1]) + end.to make_database_queries(:count => 2) + + expect do + expect(image.vim_performance_state_association(timestamp, :containers)).to eq([container1]) + end.to make_database_queries(:count => 0) + end + + it "supports virtual associations" do + r_vps + + expect do + expect(region.vim_performance_state_association(timestamp, :ext_management_systems)).to eq([ems1]) + end.to make_database_queries(:count => 2) + + expect do + expect(region.vim_performance_state_association(timestamp, :ext_management_systems)).to eq([ems1]) + end.to make_database_queries(:count => 2) # TODO: vps caching (another PR) will change to 1 + end + + it "fetches a second timestamp" do + c_vps + c_vps_now + expect(image.vim_performance_state_association(timestamp, :containers)).to match_array([container1]) + + expect do + expect(image.vim_performance_state_association(ts_now, :containers)).to match_array([container1, container2]) + end.to make_database_queries(:count => 2) + end + + it "assigns reverse association" do + c_vps + expect(image.vim_performance_state_association(timestamp, :containers)).to match_array([container1]) + + expect do + c = image.vim_performance_state_association(timestamp, :containers).first + expect(c.container_image).to eq(image) + end.to make_database_queries(:count => 0) + end + end + # NOTE: in these specs, we could let perf_capture_state be called # but using this reduces the queries describe "#vim_performance_state_for_ts" do @@ -114,16 +171,15 @@ private - def create_vps(image, timestamp, containers = [], nodes = []) + def create_vps(parent, timestamp, association = {}) FactoryBot.create( :vim_performance_state, - :resource => image, + :resource => parent, :timestamp => timestamp, :state_data => { - :assoc_ids => { - :containers => {:on => containers.map(&:id), :off => []}, - :container_nodes => {:on => nodes.map(&:id), :off => []}, - } + :assoc_ids => association.each_with_object({}) do |(name, values), hash| + hash[name] = {:on => values.map(&:id), :off => []} + end } ) end