Skip to content

Commit

Permalink
Merge pull request #18543 from kbrock/rbac_virtual_attributes
Browse files Browse the repository at this point in the history
Rbac virtual attributes
  • Loading branch information
Fryguy committed Mar 22, 2019
2 parents 99e4014 + 652260b commit 3dfb0b0
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 1 deletion.
1 change: 1 addition & 0 deletions app/models/miq_report/generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ def generate_basic_results(options = {})

## add in virtual attributes that can be calculated from sql
rbac_opts[:extra_cols] = va_sql_cols unless va_sql_cols.blank?
rbac_opts[:use_sql_view] = true if db_options && db_options[:use_sql_view]

results, attrs = Rbac.search(rbac_opts)
results = Metric::Helper.remove_duplicate_timestamps(results)
Expand Down
1 change: 1 addition & 0 deletions app/models/miq_report/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def paged_view_search(options = {})
)
search_options.merge!(:limit => limit, :offset => offset, :order => order) if order
search_options[:extra_cols] = va_sql_cols if va_sql_cols.present?
search_options[:use_sql_view] = true if db_options && db_options[:use_sql_view]

if options[:parent]
targets = get_parent_targets(options[:parent], options[:association] || options[:parent_method])
Expand Down
46 changes: 45 additions & 1 deletion lib/rbac/filterer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,22 @@ def search(options = {})

scope = include_references(scope, klass, include_for_find, exp_includes, skip_references)
scope = scope.limit(limit).offset(offset) if attrs[:apply_limit_in_sql]

if inline_view?(options, scope)
inner_scope = scope.except(:select, :includes, :references)
scope.includes_values.each { |hash| inner_scope = add_joins(klass, inner_scope, hash) }
scope = scope.from(Arel.sql("(#{inner_scope.to_sql})").as(scope.table_name))
.except(:offset, :limit, :order, :where)

# the auth_count needs to come from the inner query (the query with the limit)
if !options[:skip_counts] && (attrs[:apply_limit_in_sql] && limit)
auth_count = inner_scope.except(:offset, :limit, :order).count(:all)
end
end
targets = scope

unless options[:skip_counts]
auth_count = attrs[:apply_limit_in_sql] && limit ? targets.except(:offset, :limit, :order).count(:all) : targets.length
auth_count ||= attrs[:apply_limit_in_sql] && limit ? targets.except(:offset, :limit, :order).count(:all) : targets.length
end

if search_filter && targets && (!exp_attrs || !exp_attrs[:supported_by_sql])
Expand Down Expand Up @@ -286,6 +298,20 @@ def is_sti?(klass)
klass.respond_to?(:finder_needs_type_condition?) ? klass.finder_needs_type_condition? : false
end

# We would like to use an inline view if:
# - we enabled viewing an inline view
# - we have virtual attributes
# - we are not bringing back the whole table (i.e. we do have a where() or a limit())
# - we have a non qualified table name (otherwise our inline view will blow up)
# - we are using a scope (instead of a relation - which doesn't do includes so well)
def inline_view?(options, scope)
options[:use_sql_view] &&
options[:extra_cols] &&
(scope.limit_value || scope.where_values_hash.present?) &&
!scope.table_name&.include?(".") &&
scope.respond_to?(:includes_values)
end

# This is a very primitive way of determining whether we want to skip
# adding references to the query.
#
Expand Down Expand Up @@ -344,6 +370,24 @@ def include_references(scope, klass, include_for_find, exp_includes, skip)
scope
end

# @param includes [Array, Hash]
def add_joins(klass, scope, includes)
return scope unless includes
includes = Array(includes) unless includes.kind_of?(Enumerable)
includes.each do |association, value|
if table_include?(klass, association)
scope = value ? scope.left_outer_joins(association => value) : scope.left_outer_joins(association)
end
end
scope
end

# this is a reference to a non polymorphic table
def table_include?(target_klass, association)
reflection = target_klass.reflect_on_association(association)
reflection && !reflection.polymorphic?
end

def polymorphic_include?(target_klass, includes)
includes.keys.any? do |incld|
reflection = target_klass.reflect_on_association(incld)
Expand Down
71 changes: 71 additions & 0 deletions spec/lib/rbac/filterer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,77 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
expect(results.length).to eq(VmdbDatabaseSetting.all.length)
end
end

describe "with inline view" do
let!(:tagged_group) { FactoryBot.create(:miq_group, :tenant => default_tenant) }
let!(:user) { FactoryBot.create(:user, :miq_groups => [tagged_group]) }

before do
# the user can see production environments
tagged_group.entitlement = Entitlement.new
tagged_group.entitlement.set_belongsto_filters([])
tagged_group.entitlement.set_managed_filters([["/managed/environment/prod"]])
tagged_group.save!
end

it "handles added include from miq_expression" do
st = FactoryBot.create(:service_template)
service1, service2, _service3 = FactoryBot.create_list(:service, 3, :service_template => st)
service1.tag_with("/managed/environment/prod", :ns => "*")
service2.tag_with("/managed/environment/prod", :ns => "*")
service2.update_attributes(:service_template => nil)

# exclude service2 (no service template)
exp = YAML.safe_load("--- !ruby/object:MiqExpression
exp:
and:
- IS NOT EMPTY:
field: Service.service_template-name
- IS NOT EMPTY:
field: Service-name
")
results = described_class.search(:class => "Service",
:filter => exp,
:limit => 20,
:extra_cols => "v_total_vms",
:use_sql_view => true,
:user => user,
:include_for_find => {:service_templates => :pictures},
:order => "services.name desc")
expect(results.first).to eq([service1])
expect(results.last[:auth_count]).to eq(1)
end

it "handles ruby and sql miq_expression" do
root_service = FactoryBot.create(:service)
services1 = FactoryBot.create_list(:service, 4, :parent => root_service) # matches both (NOTE: more than limit)
services2 = FactoryBot.create_list(:service, 2) # matches rbac, and sql filter, not ruby filter
services1.each { |s| s.tag_with("/managed/environment/prod", :ns => "*") }
services2.each { |s| s.tag_with("/managed/environment/prod", :ns => "*") }
FactoryBot.create_list(:service, 1, :parent => root_service) # matches ruby and sql filter, not rbac

# expression with sql AND ruby
exp = YAML.safe_load("--- !ruby/object:MiqExpression
exp:
and:
- IS NOT EMPTY:
field: Service-name
- EQUAL:
field: Service-service_id
value: #{root_service.id}
")

results = described_class.search(:class => "Service",
:filter => exp,
:limit => 3,
:extra_cols => "v_total_vms",
:use_sql_view => true,
:user => user,
:order => "services.id")
expect(results.first).to eq(services1[0..2])
expect(results.last[:auth_count]).to eq(4)
end
end
end

describe ".filtered" do
Expand Down

0 comments on commit 3dfb0b0

Please sign in to comment.