pass references in as an array

we are currently passing a nested hash into references
This is (and has always been) invalid.

Currently rails just treats this as "just use references for all"
It is undefined behavior and not guaranteed to keep working
in the future

includes_to_references converts an includes to a references array.

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,

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 = []
 

lib/rbac/filterer.rb

@@ -171,7 +171,7 @@ def self.accessible_tenant_ids_strategy(klass)
     # @option options :conditions    [Hash|String|Array<String>]
     # @option options :where_clause  []
     # @option options :sub_filter
-    # @option options :include_for_find [Array<Symbol>]
+    # @option options :include_for_find [Array<Symbol>, 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

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') }