diff --git a/app/models/application_record.rb b/app/models/application_record.rb index c473c21cfa3..0e27a438fb9 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -13,6 +13,7 @@ class ApplicationRecord < ActiveRecord::Base include ToModelHash extend ArTableLock + extend ArReferences # FIXME: UI code - decorator support if defined?(ManageIQ::Decorators::Engine) diff --git a/app/models/miq_report/generator.rb b/app/models/miq_report/generator.rb index 584874924e5..372818d939a 100644 --- a/app/models/miq_report/generator.rb +++ b/app/models/miq_report/generator.rb @@ -258,7 +258,7 @@ def generate_daily_metric_rollup_results(options = {}) .where(options[:where_clause]) .where(:timestamp => time_range) .includes(db_includes) - .references(db_includes) + .references(db_klass.includes_to_references(db_includes)) .includes(exp_includes || []) .limit(options[:limit]) results = Rbac.filtered(results, :class => db, diff --git a/lib/acts_as_ar_model.rb b/lib/acts_as_ar_model.rb index 6b16e9ae70a..74475bf1aba 100644 --- a/lib/acts_as_ar_model.rb +++ b/lib/acts_as_ar_model.rb @@ -21,6 +21,10 @@ def self.base_class superclass == ActsAsArModel ? self : superclass.base_class end + def self.includes_to_references(_inc) + [] + end + class << self; alias_method :base_model, :base_class; end # diff --git a/lib/extensions/ar_references.rb b/lib/extensions/ar_references.rb new file mode 100644 index 00000000000..557e182f407 --- /dev/null +++ b/lib/extensions/ar_references.rb @@ -0,0 +1,21 @@ +module ArReferences + # 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 includes_to_references(inc) + return [] unless inc + + inc = Array(inc) unless inc.kind_of?(Hash) + inc.flat_map do |n, v| + if (ref = reflect_on_association(n.to_sym)) && !ref.polymorphic? + n_table = ref.table_name + v_tables = v ? ref.klass.try(:includes_to_references, v) : [] + [n_table] + v_tables + elsif reflection_with_virtual(n.to_sym) # ignore polymorphic and virtual attribute + [] + else # it is probably a table name - keep it + n + end + end + end +end diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index cafc9c0d2e0..47ce09aa878 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -173,7 +173,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) @@ -413,7 +413,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(klass.includes_to_references(include_for_find)).references(klass.includes_to_references(exp_includes)) end scope end diff --git a/spec/lib/extensions/ar_references_spec.rb b/spec/lib/extensions/ar_references_spec.rb new file mode 100644 index 00000000000..db87b666a07 --- /dev/null +++ b/spec/lib/extensions/ar_references_spec.rb @@ -0,0 +1,23 @@ +describe "ar_references" do + describe ".includes_to_references" do + it "supports none" do + expect(Vm.includes_to_references(nil)).to eq([]) + expect(Vm.includes_to_references([])).to eq([]) + end + + it "supports arrays" do + expect(Vm.includes_to_references(%w[host operating_system])).to eq(%w[hosts operating_systems]) + expect(Vm.includes_to_references(%i[host])).to eq(%w[hosts]) + end + + it "supports hashes" do + expect(Hardware.includes_to_references(:vm => {})).to eq(%w[vms]) + expect(Hardware.includes_to_references(:vm => :ext_management_system)).to eq(%w[vms ext_management_systems]) + expect(Hardware.includes_to_references(:vm => {:ext_management_system => {}})).to eq(%w[vms ext_management_systems]) + end + + it "supports table array" do + expect(Vm.includes_to_references(%w[hosts operating_systems])).to eq(%w[hosts operating_systems]) + end + end +end diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index 4faab84bf33..9de1db84cca 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -621,11 +621,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 @@ -638,7 +634,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 @@ -655,11 +651,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 @@ -671,7 +663,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 @@ -2461,21 +2453,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 @@ -2516,7 +2508,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