diff --git a/app/models/miq_report/generator.rb b/app/models/miq_report/generator.rb index 9ddd40488ff1..59b0ed48c83e 100644 --- a/app/models/miq_report/generator.rb +++ b/app/models/miq_report/generator.rb @@ -255,7 +255,7 @@ def generate_daily_metric_rollup_results(options = {}) .where(options[:where_clause]) .where(:timestamp => time_range) .includes(db_includes) - .references(db_includes) + .references(Rbac.includes_to_references(db_klass, db_includes)) .limit(options[:limit]) results = Rbac.filtered(results, :class => db, :filter => conditions, diff --git a/lib/rbac.rb b/lib/rbac.rb index 4dc28e125213..713b6554928d 100644 --- a/lib/rbac.rb +++ b/lib/rbac.rb @@ -15,6 +15,10 @@ def self.accessible_tenant_ids_strategy(*args) Filterer.accessible_tenant_ids_strategy(*args) end + class << self + delegate :includes_to_references, :to => Rbac::Filterer + end + def self.resources_shared_with(user) valid_resources = [] diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index 2e39e37cc701..d4eac603f913 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -171,7 +171,7 @@ def self.accessible_tenant_ids_strategy(klass) # @option options :conditions [Hash|String|Array] # @option options :where_clause [] # @option options :sub_filter - # @option options :include_for_find [Array] + # @option options :include_for_find [Array, Hash{Symbol => Symbol,Hash,Array}] models included but not in query # @option options :filter [MiqExpression] (optional) # @option options :user [User] (default: current_user) @@ -358,6 +358,26 @@ def select_from_order_columns(columns) end end + # Given a nested hash of associations (used by includes) + # convert into an array of table names (used by references) + # If given an array of table names, will output the same array + def self.includes_to_references(klass, inc) + return [] unless inc + + inc = Array(inc) unless inc.kind_of?(Hash) + inc.flat_map do |n, v| + if (ref = klass.reflect_on_association(n.to_sym)) && !ref.polymorphic? + n_table = ref.table_name + v_tables = v ? includes_to_references(ref.klass, v) : [] + [n_table] + v_tables + elsif klass.reflection_with_virtual(n.to_sym) # ignore polymorphic and virtual attribute + [] + else # it is probably a table name - keep it + n + end + end + end + # This is a very primitive way of determining whether we want to skip # adding references to the query. # @@ -411,7 +431,7 @@ def include_references(scope, klass, include_for_find, exp_includes, skip) ref_includes = Hash(include_for_find).merge(Hash(exp_includes)) unless polymorphic_include?(klass, ref_includes) - scope = scope.references(include_for_find).references(exp_includes) + scope = scope.references(self.class.includes_to_references(klass, include_for_find)).references(self.class.includes_to_references(klass, exp_includes)) end scope end diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index c2a11e8cc8fb..3f56619edf1f 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -584,11 +584,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt end it "does not add references without includes" do - # empty string here is basically passing `.references(nil)`, and the - # extra empty hash here is from the MiqExpression (which will result in - # the same), both of which will no-op to when determining if there are - # joins in ActiveRecord, and will not create a JoinDependency query - expect(results.references_values).to match_array ["", "{}"] + expect(results.references_values).to eq [] end context "with :include_for_find" do @@ -601,7 +597,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt end it "adds references" do - expect(results.references_values).to match_array ["{:evm_owner=>{}}", "{}"] + expect(results.references_values).to match_array %w[users] end end end @@ -618,11 +614,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt end it "does not add references with no includes" do - # The single empty string is the result of a nil from both the lack of - # a MiqExpression filter and the user filter, which is deduped in - # ActiveRecord's internals and results in a `.references(nil)` - # effectively - expect(results.references_values).to match_array [""] + expect(results.references_values).to eq [] end context "with :include_for_find" do @@ -634,7 +626,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt end it "adds references" do - expect(results.references_values).to match_array ["", "{:evm_owner=>{}}"] + expect(results.references_values).to match_array %w[users] end end end @@ -2424,21 +2416,21 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) method_args = [scope, klass, include_for_find, nil, skip] resulting_scope = subject.send(:include_references, *method_args) - expect(resulting_scope.references_values).to eq(["{:miq_server=>{}}", ""]) + expect(resulting_scope.references_values).to eq(%w[miq_servers]) end it "adds exp_includes .references to the scope" do method_args = [scope, klass, nil, exp_includes, skip] resulting_scope = subject.send(:include_references, *method_args) - expect(resulting_scope.references_values).to eq(["", "{:host=>{}}"]) + expect(resulting_scope.references_values).to eq(%w[hosts]) end it "adds include_for_find and exp_includes .references to the scope" do method_args = [scope, klass, include_for_find, exp_includes, skip] resulting_scope = subject.send(:include_references, *method_args) - expect(resulting_scope.references_values).to eq(["{:miq_server=>{}}", "{:host=>{}}"]) + expect(resulting_scope.references_values).to eq(%w[miq_servers hosts]) end context "if the include is polymorphic" do @@ -2479,7 +2471,7 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) it "adds .references to the scope" do allow(subject).to receive(:warn) - expect(resulting_scope.references_values).to eq(["{:miq_server=>{}}", "{:host=>{}}"]) + expect(resulting_scope.references_values).to eq(%w[miq_servers hosts]) end it "warns that there was an issue in test mode" do @@ -2692,6 +2684,28 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) end end + describe ".includes_to_references" do + it "supports none" do + expect(described_class.includes_to_references(Vm, nil)).to eq([]) + expect(described_class.includes_to_references(Vm, [])).to eq([]) + end + + it "supports arrays" do + expect(described_class.includes_to_references(Vm, %w[host operating_system])).to eq(%w[hosts operating_systems]) + expect(described_class.includes_to_references(Vm, %i[host])).to eq(%w[hosts]) + end + + it "supports hashes" do + expect(described_class.includes_to_references(Hardware, :vm => {})).to eq(%w[vms]) + expect(described_class.includes_to_references(Hardware, :vm => :ext_management_system)).to eq(%w[vms ext_management_systems]) + expect(described_class.includes_to_references(Hardware, :vm => {:ext_management_system => {}})).to eq(%w[vms ext_management_systems]) + end + + it "supports table array" do + expect(described_class.includes_to_references(Vm, %w[hosts operating_systems])).to eq(%w[hosts operating_systems]) + end + end + describe "cloud_tenant based search" do let(:ems_openstack) { FactoryBot.create(:ems_cloud) } let(:project1_tenant) { FactoryBot.create(:tenant, :source_type => 'CloudTenant') }