[WIP] Polymorphic through

app/models/cloud_tenant.rb

@@ -185,7 +185,6 @@ def self.post_refresh_ems(ems_id, _)
   end
 
   def self.tenant_joins_clause(scope)
-    scope.includes(:source_tenant, :ext_management_system)
-         .references(:source_tenant, :ext_management_system)
+    scope.left_joins(:source_tenant, :ext_management_system)
   end
 end

app/models/flavor.rb

@@ -44,8 +44,7 @@ def self.class_by_ems(ext_management_system)
   end
 
   def self.tenant_joins_clause(scope)
-    scope.includes(:cloud_tenants => "source_tenant", :ext_management_system => {})
-         .references(:cloud_tenants, :tenants, :ext_management_system)
+    scope.left_joins(:cloud_tenants => :source_tenant, :ext_management_system => {})
   end
 
   # Create a flavor as a queued task and return the task id. The queue name and

app/models/miq_report.rb

@@ -47,7 +47,7 @@ class MiqReport < ApplicationRecord
   attr_accessor :ext_options
   attr_accessor_that_yamls :table, :sub_table, :filter_summary, :extras, :ids, :scoped_association, :html_title, :file_name,
                            :extras, :record_id, :tl_times, :user_categories, :trend_data, :performance, :include_for_find,
-                           :report_run_time, :chart, :skip_references
+                           :report_run_time, :chart
 
   attr_accessor_that_yamls :reserved # For legacy imports
 

app/models/miq_report/generator.rb

@@ -80,10 +80,14 @@ def table2class(table)
     @table2class[table]
   end
 
+  # models that are preloaded for later use
   def get_include_for_find
-    get_include.deep_merge(include_for_find || {}).presence
+    include_for_find.presence
   end
 
+  # models that are necessary / in the SELECT, WHERE, or ORDER clause
+  #
+  # get from include hash or derive from col_order)
   def get_include
     include_as_hash(include.presence || invent_report_includes)
   end

app/models/miq_report/search.rb

@@ -93,8 +93,7 @@ def paged_view_search(options = {})
     search_options = options.merge(:class            => db,
                                    :conditions       => conditions,
                                    :include_for_find => includes,
-                                   :references       => get_include,
-                                   :skip_references  => skip_references
+                                   :references       => get_include
                                   )
     search_options.merge!(:limit => limit, :offset => offset, :order => order) if order
     search_options[:extra_cols] = va_sql_cols if va_sql_cols.present?

app/models/mixins/cloud_tenancy_mixin.rb

@@ -13,8 +13,7 @@ def tenant_id_clause_format(tenant_ids)
     end
 
     def tenant_joins_clause(scope)
-      scope.includes(:cloud_tenant => "source_tenant", :ext_management_system => {})
-           .references(:cloud_tenants, :tenants, :ext_management_systems) # needed for the where to work
+      scope.left_joins(:cloud_tenant => :source_tenant, :ext_management_system => {})
     end
   end
 end

app/models/service_template.rb

@@ -100,7 +100,7 @@ def self.with_tenant(tenant_id)
   end
 
   def self.with_additional_tenants
-    references(table_name, :tenants).includes(:service_template_tenants => :tenant)
+    left_joins(:service_template_tenants => :tenant).includes(:service_template_tenants => :tenant)
   end
 
   def self.catalog_item_types

lib/rbac/filterer.rb

@@ -273,18 +273,15 @@ def search(options = {})
       scope = scope.except(:offset, :limit, :order)
       scope = scope_targets(klass, scope, user_filters, user, miq_group)
               .where(conditions).where(sub_filter).where(where_clause).where(exp_sql).where(ids_clause)
-              .includes(include_for_find).includes(exp_includes)
               .order(order)
 
+      scope = add_includes(scope, klass, include_for_find, exp_includes)
       scope = include_references(scope, klass, references, exp_includes)
       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) }
-        if inner_scope.order_values.present?
-          inner_scope = apply_select(klass, inner_scope, select_from_order_columns(inner_scope.order_values))
-        end
+        inner_scope = scope.except(:select, :references)
+        inner_scope = apply_select(klass, inner_scope, select_from_order_columns(inner_scope.order_values))
         scope = scope.from(Arel.sql("(#{inner_scope.to_sql})").as(scope.table_name))
                      .except(:offset, :limit, :where)
 
@@ -354,6 +351,8 @@ def inline_view?(options, scope)
     # We currently assume that all regular columns are already in the SELECT.
     # So this is just returning the functions (e.g. LOWER(name))
     def select_from_order_columns(columns)
+      return if columns.nil?
+
       columns.compact.map do |column|
         if column.kind_of?(Arel::Nodes::Ordering)
           column.expr
@@ -368,8 +367,17 @@ def select_from_order_columns(columns)
       end
     end
 
-    def include_references(scope, klass, include_for_find, exp_includes)
-      scope.references(klass.includes_to_references(include_for_find)).references(klass.includes_to_references(exp_includes))
+    def add_includes(scope, klass, *includeses)
+      includeses.compact.inject(scope) { |scp, includes| scp.includes(includes) }
+    end
+
+    def include_references(scope, klass, *includeses)
+      includeses.compact.inject(scope) do |scp, includes|
+        # add to join clause (add distinct if necessary)
+        scp = add_joins(klass, scp, includes)
+        # add select just in case a distinct column got in there
+        apply_select(klass, scp, select_from_order_columns(scp.order_values))
+      end
     end
 
     # @param includes [Array, Hash]

spec/lib/rbac/filterer_spec.rb

@@ -709,7 +709,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
       end
 
       it "does not add references without includes" do
-        expect(results.references_values).to eq []
+        expect(results.left_outer_joins_values).to eq []
       end
 
       context "with :include_for_find" do
@@ -722,7 +722,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 %w[users]
+          expect(results.left_outer_joins_values).to eq([:evm_owner => {}])
         end
       end
     end
@@ -739,7 +739,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
       end
 
       it "does not add references with no includes" do
-        expect(results.references_values).to eq []
+        expect(results.left_outer_joins_values).to eq []
       end
 
       context "with :include_for_find" do
@@ -751,7 +751,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 %w[users]
+          expect(results.left_outer_joins_values).to eq([:evm_owner => {}])
         end
       end
     end
@@ -1147,7 +1147,8 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
           :max_disk_usage_rate_average     => {},
           :min_net_usage_rate_average      => {},
           :max_net_usage_rate_average      => {},
-          :v_derived_storage_used          => {}
+          :v_derived_storage_used          => {},
+          :resource                        => {},
         }
       end
 
@@ -1157,8 +1158,10 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
       it "should not raise an error when a polymorphic reflection is included" do
         result = nil
         expect do
+          # having the virtual attribute in references is the current way.
+          # even though resource is polymorphic, it is ending up in the join tables
           result = described_class.search :class            => "MetricRollup",
-                                          :include_for_find => @include
+                                          :include_for_find => @include, :references => @include, :use_sql_view => true
         end.not_to raise_error
         expect(result.first.length).to eq(1)
       end
@@ -2512,35 +2515,27 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
     let(:exp_includes)     { { :host => {} } }
 
     it "adds include_for_find .references to the scope" do
-      method_args      = [scope, klass, include_for_find, nil]
-      resulting_scope  = subject.send(:include_references, *method_args)
-
-      expect(resulting_scope.references_values).to eq(%w[miq_servers])
+      resulting_scope = subject.send(:include_references, scope, klass, include_for_find, nil)
+      expect(resulting_scope.left_outer_joins_values).to eq([include_for_find])
     end
 
     it "adds exp_includes .references to the scope" do
-      method_args      = [scope, klass, nil, exp_includes]
-      resulting_scope  = subject.send(:include_references, *method_args)
-
-      expect(resulting_scope.references_values).to eq(%w[hosts])
+      resulting_scope = subject.send(:include_references, scope, klass, nil, exp_includes)
+      expect(resulting_scope.left_outer_joins_values).to eq([exp_includes])
     end
 
     it "adds include_for_find and exp_includes .references to the scope" do
-      method_args      = [scope, klass, include_for_find, exp_includes]
-      resulting_scope  = subject.send(:include_references, *method_args)
-
-      expect(resulting_scope.references_values).to eq(%w[miq_servers hosts])
+      resulting_scope = subject.send(:include_references, scope, klass, include_for_find, exp_includes)
+      expect(resulting_scope.left_outer_joins_values).to match_array([include_for_find, exp_includes])
     end
 
     context "if the include is polymorphic" do
       let(:klass)            { MetricRollup }
       let(:include_for_find) { { :resource => {} } }
 
       it "does not add .references to the scope" do
-        method_args      = [scope, klass, include_for_find, nil]
-        resulting_scope  = subject.send(:include_references, *method_args)
-
-        expect(resulting_scope.references_values).to eq([])
+        resulting_scope = subject.send(:include_references, scope, klass, include_for_find, nil)
+        expect(resulting_scope.left_outer_joins_values).to eq([])
       end
     end
   end

spec/models/metric_rollup_spec.rb

@@ -20,25 +20,6 @@
     end
   end
 
-  context "test" do
-    it "should not raise an error when a polymorphic reflection is included and references are specified in a query" do
-      skip "until ActiveRecord is fixed"
-      # TODO: A fix in ActiveRecord will make this test pass
-      expect do
-        MetricRollup.where(:id => 1)
-                    .includes(:resource => {}, :time_profile => {})
-                    .references(:time_profile => {}).last
-      end.not_to raise_error
-
-      # TODO: Also, there is a bug that exists in only the manageiq repo and not rails
-      # TODO: that causes the error "ActiveRecord::ConfigurationError: nil"
-      # TODO: instead of the expected "ActiveRecord::EagerLoadPolymorphicError" error.
-      expect do
-        Tagging.includes(:taggable => {}).where('bogus_table.column = 1').references(:bogus_table).to_a
-      end.to raise_error ActiveRecord::EagerLoadPolymorphicError
-    end
-  end
-
   context ".rollups_in_range" do
     before do
       @current = FactoryBot.create_list(:metric_rollup_vm_hr, 2)

spec/models/miq_report/generator_spec.rb

@@ -165,42 +165,70 @@
       expect(rpt.get_include_for_find).to be_nil
     end
 
-    it "includes virtual_includes from virtual_attributes that are not sql friendly" do
+    it "does not includes virtual_includes from virtual_attributes that are not sql friendly" do
       rpt = MiqReport.new(:db   => "VmOrTemplate",
-                          :cols => %w(name platform))
-      expect(rpt.get_include_for_find).to eq(:platform => {})
+                          :cols => %w[name platform])
+      expect(rpt.get_include_for_find).to be_nil
     end
 
     it "does not include sql friendly virtual_attributes" do
       rpt = MiqReport.new(:db   => "VmOrTemplate",
-                          :cols => %w(name v_total_snapshots))
+                          :cols => %w[name v_total_snapshots])
       expect(rpt.get_include_for_find).to be_nil
     end
 
+    it "uses only include_for_find" do
+      rpt = MiqReport.new(:db               => "VmOrTemplate",
+                          :cols             => %w[name platform],
+                          :include          => {:host => {:columns => %w[name]}, :storage => {:columns => %w[name]}},
+                          :include_for_find => {:snapshots => {}})
+      expect(rpt.get_include_for_find).to eq(:snapshots => {})
+    end
+  end
+
+  describe "#get_include (private)" do
+    it "returns nil with empty include" do
+      rpt = MiqReport.new(:db      => "VmOrTemplate",
+                          :include => {})
+      expect(rpt.get_include).to be_blank
+    end
+
+    it "includes virtual_includes from virtual_attributes that are not sql friendly" do
+      rpt = MiqReport.new(:db   => "VmOrTemplate",
+                          :cols => %w[name platform])
+      expect(rpt.get_include).to eq(:platform => {})
+    end
+
+    it "does not include sql friendly virtual_attributes" do
+      rpt = MiqReport.new(:db   => "VmOrTemplate",
+                          :cols => %w[name v_total_snapshots])
+      expect(rpt.get_include).to be_blank
+    end
+
     it "uses include and include_as_hash" do
       rpt = MiqReport.new(:db               => "VmOrTemplate",
-                          :cols             => %w(name platform),
-                          :include          => {:host => {:columns => %w(name)}, :storage => {:columns => %w(name)}},
+                          :cols             => %w[name platform],
+                          :include          => {:host => {:columns => %w[name]}, :storage => {:columns => %w[name]}},
                           :include_for_find => {:snapshots => {}})
-      expect(rpt.get_include_for_find).to eq(:platform => {}, :host => {}, :storage => {}, :snapshots => {})
+      expect(rpt.get_include).to eq(:platform => {}, :host => {}, :storage => {})
     end
 
     it "uses col, col_order, and virtual attributes and ignores empty include" do
       # it also allows cols to override col_order for requesting extra columns
       rpt = MiqReport.new(:db               => "VmOrTemplate",
                           :include          => {},
                           :cols             => %w[name v_datastore_path],
-                          :col_order        => %w(name host.name storage.name),
+                          :col_order        => %w[name host.name storage.name],
                           :include_for_find => {:snapshots => {}})
-      expect(rpt.get_include_for_find).to eq(:v_datastore_path => {}, :host => {}, :storage => {}, :snapshots => {})
+      expect(rpt.get_include).to eq(:v_datastore_path => {}, :host => {}, :storage => {})
     end
 
     it "uses col_order and virtual attributes" do
       rpt = MiqReport.new(:db               => "VmOrTemplate",
                           :include          => {},
                           :col_order        => %w[name v_datastore_path host.name storage.name],
                           :include_for_find => {:snapshots => {}})
-      expect(rpt.get_include_for_find).to eq(:v_datastore_path => {}, :host => {}, :storage => {}, :snapshots => {})
+      expect(rpt.get_include).to eq(:v_datastore_path => {}, :host => {}, :storage => {})
     end
   end
 end

spec/models/miq_report_spec.rb

@@ -755,8 +755,13 @@
       end
     end
     context "performance reports" do
-      let(:report) do
-        MiqReport.new(
+      let(:miq_server) { EvmSpecHelper.local_miq_server }
+      let(:ems) { FactoryBot.create(:ems_vmware, :zone => miq_server.zone) }
+      let(:vm) { FactoryBot.create(:vm_vmware, :ext_management_system => ems) }
+      let(:time_profile) { FactoryBot.create(:time_profile_utc) }
+
+      it "runs daily report" do
+        report = MiqReport.new(
           :title   => "vim_perf_daily.yaml",
           :db      => "VimPerformanceDaily",
           :cols    => %w(timestamp cpu_usagemhz_rate_average max_derived_cpu_available),
@@ -765,10 +770,17 @@
                             cpu_usagemhz_rate_average_low_over_time_period
                             derived_memory_used_high_over_time_period
                             derived_memory_used_low_over_time_period)}})
+        report.generate_table(:userid => "admin")
       end
-      let(:ems) { FactoryBot.create(:ems_vmware, :zone => @server.zone) }
 
-      it "runs report" do
+      it "runs report with polymorphic references" do
+        FactoryBot.create(:metric_rollup_vm_daily, :resource => vm, :time_profile => time_profile)
+
+        report = MiqReport.new(
+            :title     => "vim_perf_daily.yaml",
+            :db        => "VimPerformanceDaily",
+            :col_order => %w(timestamp cpu_usagemhz_rate_average max_derived_cpu_available
+                             resource.derived_memory_used_low_over_time_period))
         report.generate_table(:userid => "admin")
       end
     end