From f6a75b037839f3fe1ba37c9e8d06d3b5ad583dd3 Mon Sep 17 00:00:00 2001
From: Nick LaMuro <nicklamuro@gmail.com>
Date: Wed, 23 May 2018 18:03:48 -0500
Subject: [PATCH] Convert ComplianceMixin to has_one/virtual_delegates

This converts the functionality defined in the virtual_has_one,
virtual_columns, and associated methods to a has_one and a pair of
virtual_delegates.

This allows these columns to also be used in SQL, and removes some of
the boilerplate code necessary for the methods that are defined.
---
 app/models/mixins/compliance_mixin.rb       | 37 ++++++---------
 spec/models/mixins/compliance_mixin_spec.rb | 50 +++++++++++++++++++++
 2 files changed, 63 insertions(+), 24 deletions(-)
 create mode 100644 spec/models/mixins/compliance_mixin_spec.rb

diff --git a/app/models/mixins/compliance_mixin.rb b/app/models/mixins/compliance_mixin.rb
index 17d6da9ab7b..21262d79d0d 100644
--- a/app/models/mixins/compliance_mixin.rb
+++ b/app/models/mixins/compliance_mixin.rb
@@ -3,30 +3,19 @@ module ComplianceMixin
 
   included do
     has_many :compliances, :as => :resource, :dependent => :destroy
-
-    virtual_has_one :last_compliance, :class_name => "Compliance"
-
-    virtual_column  :last_compliance_status,    :type => :boolean,  :uses => :last_compliance
-    virtual_column  :last_compliance_timestamp, :type => :datetime, :uses => :last_compliance
-  end
-
-  def last_compliance
-    return @last_compliance unless @last_compliance.nil?
-    @last_compliance = if @association_cache.include?(:compliances)
-                         compliances.max_by(&:timestamp)
-                       else
-                         compliances.order("timestamp DESC").first
-                       end
-  end
-
-  def last_compliance_status
-    lc = last_compliance
-    lc.try(:compliant)
-  end
-
-  def last_compliance_timestamp
-    lc = last_compliance
-    lc.try(:timestamp)
+    has_one  :last_compliance,
+             -> { order('"compliances"."timestamp" DESC') },
+             :as         => :resource,
+             :inverse_of => :resource,
+             :class_name => "Compliance"
+
+    virtual_delegate :last_compliance_status,
+                     :to        => "last_compliance.compliant",
+                     :allow_nil => true
+    virtual_delegate :timestamp,
+                     :to        => :last_compliance,
+                     :allow_nil => true,
+                     :prefix    => true
   end
 
   def check_compliance
diff --git a/spec/models/mixins/compliance_mixin_spec.rb b/spec/models/mixins/compliance_mixin_spec.rb
new file mode 100644
index 00000000000..1acc26a23b4
--- /dev/null
+++ b/spec/models/mixins/compliance_mixin_spec.rb
@@ -0,0 +1,50 @@
+describe ComplianceMixin do
+  include Spec::Support::ArelHelper
+
+  let(:host)           { FactoryGirl.create(:host) }
+  let(:new_timestamp)  { 2.months.ago.change(:usec => 0) }
+  let(:old_timestamp)  { 4.months.ago.change(:usec => 0) }
+  let(:new_compliance) { FactoryGirl.create(:compliance, :resource => host, :timestamp => new_timestamp, :compliant => false) }
+  let(:old_compliance) { FactoryGirl.create(:compliance, :resource => host, :timestamp => old_timestamp) }
+  let(:compliances)    { [old_compliance, new_compliance] }
+
+  describe "#last_compliance" do
+    it "uses the most recent value" do
+      compliances
+      expect(host.last_compliance.timestamp).to eq(new_timestamp)
+    end
+  end
+
+  describe "#last_compliance_status" do
+    context "with no compliances" do
+      it "is nil with sql" do
+        host
+        expect(virtual_column_sql_value(Host, "last_compliance_status")).to be_nil
+      end
+
+      it "is nil with ruby" do
+        expect(host.last_compliance_status).to be_nil
+      end
+    end
+
+    context "with compliances" do
+      before { compliances }
+
+      it "has the most recent timestamp with sql" do
+        h = Host.select(:id, :last_compliance_status).first
+
+        expect do
+          expect(h.last_compliance_status).to eq(false)
+        end.to match_query_limit_of(0)
+        expect(h.association(:last_compliance)).not_to be_loaded
+      end
+
+      it "has the most recent timestamp with ruby" do
+        h = Host.first # clean host record
+
+        expect(h.last_compliance_status).to eq(false)
+        expect(h.association(:last_compliance)).to be_loaded
+      end
+    end
+  end
+end