Make attribute last_scan_on sql-friendly

app/models/ems_cluster.rb

@@ -26,7 +26,6 @@ class EmsCluster < ApplicationRecord
   virtual_column :v_cpu_vr_ratio,      :type => :float,   :uses => [:aggregate_cpu_total_cores, :aggregate_vm_cpus]
   virtual_column :v_parent_datacenter, :type => :string,  :uses => :all_relationships
   virtual_column :v_qualified_desc,    :type => :string,  :uses => :all_relationships
-  virtual_column :last_scan_on,        :type => :time,    :uses => :last_drift_state_timestamp
   virtual_total  :total_vms,               :vms
   virtual_total  :total_miq_templates,     :miq_templates
   virtual_total  :total_vms_and_templates, :vms_and_templates
@@ -44,7 +43,7 @@ class EmsCluster < ApplicationRecord
   include FilterableMixin
 
   include DriftStateMixin
-  alias_method :last_scan_on, :last_drift_state_timestamp
+  virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
 
   include RelationshipMixin
   self.default_relationship_type = "ems_metadata"

app/models/host.rb

@@ -136,7 +136,6 @@ class Host < ApplicationRecord
   virtual_column :enabled_run_level_4_services, :type => :string_set,  :uses => :host_services
   virtual_column :enabled_run_level_5_services, :type => :string_set,  :uses => :host_services
   virtual_column :enabled_run_level_6_services, :type => :string_set,  :uses => :host_services
-  virtual_column :last_scan_on,                 :type => :time,        :uses => :last_drift_state_timestamp
   virtual_delegate :annotation, :to => :hardware, :prefix => "v", :allow_nil => true
   virtual_column :vmm_vendor_display,           :type => :string
   virtual_column :ipmi_enabled,                 :type => :boolean
@@ -161,7 +160,7 @@ class Host < ApplicationRecord
   self.default_relationship_type = "ems_metadata"
 
   include DriftStateMixin
-  alias_method :last_scan_on, :last_drift_state_timestamp
+  virtual_delegate :last_scan_on, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
 
   include UuidMixin
   include MiqPolicyMixin

app/models/mixins/drift_state_mixin.rb

@@ -9,22 +9,14 @@ module DriftStateMixin
     has_one  :first_drift_state_timestamp_rec, -> { select("id, timestamp, resource_type, resource_id").order("timestamp") }, :as => :resource, :class_name => 'DriftState'
     has_one :last_drift_state_timestamp_rec, -> { order("timestamp DESC").select("id, timestamp, resource_type, resource_id") }, :as => :resource, :class_name => 'DriftState'
 
-    virtual_column :first_drift_state_timestamp, :type => :time, :uses => :first_drift_state_timestamp_rec
-    virtual_column :last_drift_state_timestamp,  :type => :time, :uses => :last_drift_state_timestamp_rec
+    virtual_delegate :first_drift_state_timestamp, :to => "first_drift_state_timestamp_rec.timestamp", :allow_nil => true
+    virtual_delegate :last_drift_state_timestamp, :to => "last_drift_state_timestamp_rec.timestamp", :allow_nil => true
   end
 
   def drift_state_timestamps
     drift_states.order(:timestamp).pluck(:timestamp)
   end
 
-  def first_drift_state_timestamp
-    first_drift_state_timestamp_rec.try(:timestamp)
-  end
-
-  def last_drift_state_timestamp
-    last_drift_state_timestamp_rec.try(:timestamp)
-  end
-
   def save_drift_state
     drift_states.create!(:timestamp => Time.now.utc, :data => to_model_hash)
   end

lib/extensions/ar_virtual.rb

@@ -298,7 +298,7 @@ def self.select_from_alias(to_ref, col, to_model_col_name, src_model_id)
     to_table    = select_from_alias_table(to_ref.klass, src_model_id.relation)
     to_model_id = to_ref.klass.arel_attribute(to_model_col_name, to_table)
     to_column   = to_ref.klass.arel_attribute(col, to_table)
-    arel        = query.select(to_column).arel
+    arel        = query.except(:select).select(to_column).arel
                        .from(to_table)
                        .where(to_model_id.eq(src_model_id))
 

spec/lib/extensions/ar_virtual_spec.rb

@@ -612,6 +612,22 @@ def col2
         end
       end
 
+      context "with has_many and select" do
+        before do
+          TestClass.has_one :ref2, -> { select(:col1) }, :class_name => 'TestClass', :foreign_key => :col1
+        end
+        # child.col1 will be getting parent's (aka tc's) id
+        let(:child) { TestClass.create(:id => 1) }
+
+        # ensure virtual attribute referencing a relation with a select()
+        # does not throw an exception due to multi-column select
+        it "properly generates sub select" do
+          TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+          TestClass.create(:id => 2, :ref2 => child)
+          expect { TestClass.all.select(:id, :col1, :child_col1).to_a }.to_not raise_error
+        end
+      end
+
       context "with relation in foreign table" do
         before do
           class TestOtherClass < ActiveRecord::Base

spec/models/mixins/drift_state_mixin_spec.rb

@@ -0,0 +1,100 @@
+describe DriftStateMixin do
+  include Spec::Support::ArelHelper
+
+  let(:host) { FactoryGirl.create(:host) }
+
+  let(:drift_states) do
+    [
+      FactoryGirl.create(:drift_state, :resource => host, :timestamp => recent_timestamp, :data => "bogus"),
+      FactoryGirl.create(:drift_state, :resource => host, :timestamp => old_timestamp, :data => "bogus")
+    ]
+  end
+
+  let(:recent_timestamp) { 2.months.ago.change(:usec => 0) }
+  let(:old_timestamp) { 4.months.ago.change(:usec => 0) }
+
+  describe "#first_drift_state" do
+    it "uses the most recent value" do
+      drift_states
+      expect(host.last_drift_state.timestamp).to eq(recent_timestamp)
+    end
+  end
+
+  describe "#last_drift_state" do
+    it "uses the least recent value" do
+      drift_states
+      expect(host.first_drift_state.timestamp).to eq(old_timestamp)
+    end
+  end
+
+  describe "#last_drift_state_timestamp" do
+    context "with no drift_state records" do
+      before { host }
+
+      it "has a nil value with sql" do
+        expect(virtual_column_sql_value(Host, "last_drift_state_timestamp")).to be_nil
+      end
+
+      it "has a nil value with ruby" do
+        expect(host.last_drift_state_timestamp).to be_nil
+      end
+    end
+
+    context "with drift_state records" do
+      before { drift_states }
+
+      it "has the most recent timestamp with sql" do
+        h = Host.select(:id, :last_drift_state_timestamp).first
+        expect do
+          expect(h.last_drift_state_timestamp).to eq(recent_timestamp)
+        end.to match_query_limit_of(0)
+        expect(h.association(:last_drift_state)).not_to be_loaded
+        expect(h.association(:last_drift_state_timestamp_rec)).not_to be_loaded
+      end
+
+      it "has the most recent timestamp with ruby" do
+        h = Host.first # want a clean host record
+        expect(h.last_drift_state_timestamp).to eq(recent_timestamp)
+        expect(h.association(:last_drift_state)).not_to be_loaded
+        expect(h.association(:last_drift_state_timestamp_rec)).to be_loaded
+      end
+    end
+  end
+
+  # ems_cluster and host specific
+  describe "#last_scan_on" do
+    context "with no drift_state records" do
+      before { host }
+
+      it "has a nil value with sql" do
+        expect(virtual_column_sql_value(Host, "last_scan_on")).to be_nil
+      end
+
+      it "has a nil value with ruby" do
+        expect(host.last_scan_on).to be_nil
+      end
+    end
+
+    context "with drift_state records" do
+      before { drift_states }
+
+      it "has the most recent timestamp with sql" do
+        h = Host.select(:id, :last_scan_on).first
+        expect do
+          expect(h.last_scan_on).to eq(recent_timestamp)
+        end.to match_query_limit_of(0)
+        expect(h.association(:last_drift_state)).not_to be_loaded
+        expect(h.association(:last_drift_state_timestamp_rec)).not_to be_loaded
+      end
+
+      it "has the most recent timestamp with ruby" do
+        h = Host.first # want a clean host record
+        expect do
+          expect(h.last_scan_on).to eq(recent_timestamp)
+        end.to match_query_limit_of(1)
+        expect(h.association(:last_drift_state)).not_to be_loaded
+        expect(h.association(:last_drift_state_timestamp_rec)).to be_loaded
+      end
+    end
+  end
+end