Skip to content

Commit

Permalink
[AuthenticationMixin] authentication_status to virtual_delegate
Browse files Browse the repository at this point in the history
Creates a virtual_delegate for AuthenticationMixin#authentication_status
which allows it to be included in reports and put in as part of the main
query.

The biggest thing of note in this change is how the status_severity_arel
is being used to handle the sorting properly in the SQL query.  It makes
use of a SQL's case statement to handle what would normally be a
`sort_by` on the hash lookup of Authentication::STATUS_SEVERITY.
Instead, we match each key to the status column and use that as the
value for the `ORDER BY exp DESC` in the `has_one`, which allows for the
same affect.

This also means that it can inject itself in the `SELECT` of a
`MiqReport`, and avoid N+1's on things like quadicons and such for pages
that use `MiqReport#paged_view_search`.
  • Loading branch information
NickLaMuro committed Nov 15, 2018
1 parent ed18040 commit 49d854d
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 6 deletions.
16 changes: 16 additions & 0 deletions app/models/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,22 @@ def self.new(*args, &block)
"invalid" => 3,
).freeze

# Builds a case statement that case be used in a sql ORDER BY.
#
# Generated SQL looks like:
#
# CASE
# WHEN LOWER(status) = '' THEN -1
# WHEN LOWER(status) = 'valid' THEN 0
# ...
# ELSE -1
#
STATUS_SEVERITY_AREL = Arel::Nodes::Case.new.tap do |arel_case|
STATUS_SEVERITY.each do |value, sort_weight|
arel_case.when(arel_table[:status].lower.eq(value)).then(sort_weight)
end
end.else(-1)

RETRYABLE_STATUS = %w(error unreachable).freeze

CREDENTIAL_TYPES = {
Expand Down
16 changes: 10 additions & 6 deletions app/models/mixins/authentication_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@ module AuthenticationMixin
included do
has_many :authentications, :as => :resource, :dependent => :destroy, :autosave => true

virtual_column :authentication_status, :type => :string
has_one :authentication_status_severity_level,
-> { order(Authentication::STATUS_SEVERITY_AREL.desc) },
:as => :resource,
:inverse_of => :resource,
:class_name => "Authentication"

virtual_delegate :authentication_status,
:to => "authentication_status_severity_level.status",
:default => "None",
:allow_nil => true

def self.authentication_check_schedule
zone = MiqServer.my_server.zone
Expand Down Expand Up @@ -117,11 +126,6 @@ def missing_credentials?(type = nil)
!has_credentials?(type)
end

def authentication_status
ordered_auths = authentication_for_providers.sort_by(&:status_severity)
ordered_auths.last.try(:status) || "None"
end

def authentication_status_ok?(type = nil)
authentication_best_fit(type).try(:status) == "Valid"
end
Expand Down
79 changes: 79 additions & 0 deletions spec/models/mixins/authentication_mixin_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
describe AuthenticationMixin do
include Spec::Support::ArelHelper

let(:host) { FactoryGirl.create(:host) }
let(:invalid_auth) { FactoryGirl.create(:authentication, :resource => host, :status => "Invalid") }
let(:valid_auth) { FactoryGirl.create(:authentication, :resource => host, :status => "Valid") }
let(:authentications) { [invalid_auth, valid_auth] }

let(:test_class_instance) do
Class.new(ActiveRecord::Base) do
def self.name; "TestClass"; end
Expand Down Expand Up @@ -721,4 +728,76 @@ def self.name; "TestClass"; end
end
end
end

describe "#authentication_status_severity_level" do
it "uses the least valid status" do
EvmSpecHelper.local_miq_server
authentications
expect(host.authentication_status_severity_level.status).to eq("Invalid")
end
end

describe "#authentication_status" do
before do
EvmSpecHelper.local_miq_server
end

context "with no authentications" do
before { host }

it "is nil with sql" do
expect(virtual_column_sql_value(Host, "authentication_status")).to be_nil
end

it "is 'None' with pure ruby (via relations)" do
expect(host.authentication_status).to eq("None")
end

it "is 'None' when accessed via ruby, but fetched via sql" do
fetched_host = Host.select(:id, :authentication_status).first
expect(fetched_host.attributes[:authentication_status]).to be_nil
expect(fetched_host.authentication_status).to eq("None")
end
end

context "with a valid authentication" do
before { valid_auth }

it "is 'Valid' with sql" do
h = Host.select(:id, :authentication_status).first

expect do
expect(h.authentication_status).to eq("Valid")
end.to match_query_limit_of(0)
expect(h.association(:authentication_status_severity_level)).not_to be_loaded
end

it "is 'Valid' with ruby" do
h = Host.first # clean host record

expect(h.authentication_status).to eq("Valid")
expect(h.association(:authentication_status_severity_level)).to be_loaded
end
end

context "with a valid and invalid authentication" do
before { authentications }

it "is 'Invalid' with sql" do
h = Host.select(:id, :authentication_status).first

expect do
expect(h.authentication_status).to eq("Invalid")
end.to match_query_limit_of(0)
expect(h.association(:authentication_status_severity_level)).not_to be_loaded
end

it "is 'Valid' with ruby" do
h = Host.first # clean host record

expect(h.authentication_status).to eq("Invalid")
expect(h.association(:authentication_status_severity_level)).to be_loaded
end
end
end
end

0 comments on commit 49d854d

Please sign in to comment.