diff --git a/app/models/miq_report/generator.rb b/app/models/miq_report/generator.rb index 46a77e0549be..42c8a5742e59 100644 --- a/app/models/miq_report/generator.rb +++ b/app/models/miq_report/generator.rb @@ -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) diff --git a/app/models/miq_report/search.rb b/app/models/miq_report/search.rb index 748622d5ae5a..2e8729d3d4e5 100644 --- a/app/models/miq_report/search.rb +++ b/app/models/miq_report/search.rb @@ -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]) diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index fbd3243252fe..c75a65520f6b 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -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]) @@ -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. # @@ -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) diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index 3b4c4cc17dfb..8f65808bb5e5 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -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