Merge pull request #22589 from kbrock/capture_vim_performance_fetch

VimPerformanceState do not bring back archived records

app/models/mixins/archived_mixin.rb

@@ -4,6 +4,9 @@ module ArchivedMixin
   included do
     scope :archived, -> { where.not(:deleted_on => nil) }
     scope :active, -> { where(:deleted_on => nil) }
+    scope :not_archived_before, ->(timestamp) {
+      unscope(:where => :deleted_on).where(arel_table[:deleted_on].eq(nil).or(arel_table[:deleted_on].gteq(timestamp)))
+    }
   end
 
   def archived?

app/models/vim_performance_state.rb

@@ -141,17 +141,17 @@ def container_nodes
 
   def container_groups
     ids = get_assoc(:container_groups)
-    ids.empty? ? ContainerGroup.none : ContainerGroup.where(:id => ids).order(:id)
+    ids.empty? ? ContainerGroup.none : ContainerGroup.where(:id => ids).order(:id).not_archived_before(timestamp)
   end
 
   def containers
     ids = get_assoc(:containers)
-    ids.empty? ? Container.none : Container.where(:id => ids).order(:id)
+    ids.empty? ? Container.none : Container.where(:id => ids).order(:id).not_archived_before(timestamp)
   end
 
   def all_container_groups
     ids = get_assoc(:all_container_groups)
-    ids.empty? ? [] : ContainerGroup.where(:id => ids).order(:id).to_a
+    ids.empty? ? ContainerGroup.none : ContainerGroup.where(:id => ids).order(:id)
   end
 
   def get_assoc(relat, mode = nil)

spec/models/mixins/archived_mixin_spec.rb

@@ -0,0 +1,90 @@
+RSpec.describe ArchivedMixin do
+  let(:model_name) { :container }
+  let(:klass) { Container }
+
+  let(:timestamp) { Time.now.utc }
+  let(:active_model) { FactoryBot.create(model_name) }
+  let(:archived_model) { FactoryBot.create(model_name, :deleted_on => timestamp) }
+
+  describe ".archived" do
+    it "fetches archived records" do
+      active_model
+      archived_model
+
+      expect(klass.archived).to eq([archived_model])
+    end
+  end
+
+  describe ".active" do
+    it "fetches active records" do
+      active_model
+      archived_model
+
+      expect(klass.active).to eq([active_model])
+    end
+  end
+
+  describe ".not_archived_before" do
+    it "detects active are in the range" do
+      active_model
+      expect(klass.not_archived_before(2.years.ago)).to eq([active_model])
+    end
+
+    it "detects not yet deleted records are in the range" do
+      archived_model
+      expect(klass.not_archived_before(2.years.ago)).to eq([archived_model])
+    end
+
+    it "detects already deleted records are not in the range" do
+      archived_model
+      expect(klass.not_archived_before(1.year.from_now)).to be_empty
+    end
+
+    # some associations have active as a default scope e.g.: ContainerProject#container_groups
+    # this makes sure not_archived_before will override the scope
+    it "overrides active scope" do
+      archived_model
+      expect(klass.active.not_archived_before(2.years.ago)).to eq([archived_model])
+    end
+  end
+
+  describe "#archived?", "#archived" do
+    it "detects not archived" do
+      expect(active_model.archived?).to be false
+    end
+
+    it "detects archived" do
+      expect(archived_model.archived?).to be true
+    end
+  end
+
+  describe "#active", "#active?" do
+    it "detects not active" do
+      expect(archived_model.active?).to be false
+    end
+
+    it "detects active" do
+      expect(active_model.active?).to be true
+    end
+  end
+
+  describe "#archive!" do
+    let(:model) { active_model }
+    it "makes archived" do
+      expect(model.archived?).to be false
+      model.archive!
+      expect(model.archived?).to be true
+      expect(model.deleted_on).not_to be_nil
+    end
+  end
+
+  describe "#unarchive!" do
+    let(:model) { archived_model }
+    it "makes unarchived" do
+      expect(model.archived?).to be true
+      model.unarchive!
+      expect(model.archived?).to be false
+      expect(model.deleted_on).to be_nil
+    end
+  end
+end

spec/models/vim_performance_state_spec.rb

@@ -9,6 +9,51 @@
     end
   end
 
+  describe "#containers" do
+    let(:capture_time) { 5.hours.ago.utc.beginning_of_hour }
+
+    it "doesn't return records that are archived before the timestamp" do
+      container_image = FactoryBot.create(:container_image)
+      containers, other_containers = create_past_present_future_both(:containers, {:container_image => container_image})
+
+      state_data = {:assoc_ids => {:containers => {:on => (containers + other_containers).map(&:id)}}}
+      actual = described_class.new(:timestamp => capture_time, :state_data => state_data)
+      expect(actual.containers).to match_array(containers)
+    end
+
+    it "handles pruned ids" do
+      container_image = FactoryBot.create(:container_image)
+      containers = FactoryBot.create_list(:container, 1, :container_image => container_image)
+
+      state_data = {:assoc_ids => {:containers => {:on => (containers.map(&:id) + deleted_record_ids(containers))}}}
+      actual = described_class.new(:timestamp => capture_time, :state_data => state_data)
+      expect(actual.containers).to match_array(containers)
+    end
+  end
+
+  describe "#container_groups" do
+    # picking a date in the past to make sure active scope doesn't mess things up
+    let(:capture_time) { 5.hours.ago.utc.beginning_of_hour }
+
+    it "doesn't return records that are archived before the timestamp" do
+      container_project = FactoryBot.create(:container_project)
+      container_groups, other_container_groups = create_past_present_future_both(:container_group, :container_project => container_project)
+
+      state_data = {:assoc_ids => {:container_groups => {:on => (container_groups + other_container_groups).map(&:id)}}}
+      actual = described_class.new(:timestamp => capture_time, :state_data => state_data)
+      expect(actual.container_groups).to match_array(container_groups)
+    end
+
+    it "handles pruned ids" do
+      container_project = FactoryBot.create(:container_project)
+      container_groups = FactoryBot.create_list(:container_group, 1, :container_project => container_project)
+
+      state_data = {:assoc_ids => {:container_groups => {:on => (container_groups.map(&:id) + deleted_record_ids(container_groups))}}}
+      actual = described_class.new(:timestamp => capture_time, :state_data => state_data)
+      expect(actual.container_groups).to match_array(container_groups)
+    end
+  end
+
   describe "#vm_count_total" do
     it "will return the total vms regardless of mode" do
       state_data = {:assoc_ids => {:vms => {:on => [1], :off => [2]}}}
@@ -76,4 +121,27 @@
       end
     end
   end
+
+  private
+
+  def create_past_present_future(factory, params)
+    create_past_present_future_both(factory, params).first
+  end
+
+  def create_past_present_future_both(factory, params)
+    # not deleted is captured
+    ret = FactoryBot.create_list(factory, 1, params)
+    # deleted before is not captured
+    old = FactoryBot.create_list(factory, 1, params.merge(:deleted_on => (capture_time - 2.hours)))
+    # deleted later is captured
+    ret << FactoryBot.create(factory, params.merge(:deleted_on => (capture_time + 2.hours)))
+
+    [ret, old]
+  end
+
+  # these represent records that have been pruned from the database
+  # so a find(id) will not be found
+  def deleted_record_ids(objs)
+    [objs.last.id + 1]
+  end
 end