Fix calling reference on polymorphic relationships

For the record:  I am not really proud or satisfied with the code added
here in it's current state, but it fixes the problem it seems.

Recently, the `lib/extensions/ar_merge_conditions.rb file was removed
from the repo as it was deemed no longer to be needed, and we had
converted everything over to using AREL.

The main method that was removed was the following:

    def self.apply_legacy_finder_options(options)
      unknown_keys = options.keys - LEGACY_FINDER_METHODS.map(&:first)
      raise "Unsupported options #{unknown_keys}" unless unknown_keys.empty?

      # Determine whether any of the included associations are polymorphic
      has_polymorphic = included_associations(options[:include]).any? { |name| self.reflection_is_polymorphic?(name) }

      LEGACY_FINDER_METHODS.inject(all) do |scope, (key, method)|
        # Don't call references method on scope if polymorphic associations are
        # included to avoid ActiveRecord::EagerLoadPolymorphicError
        next(scope) if method == :references && has_polymorphic
        #
        options[key] ? scope.send(method, options[key]) : scope
      end
    end

Which basically mapped the old Rails 2.3 methods (LEGACY_FINDER_METHODS,
not shown) to the new(er) Rails 3+ AREL syntax.  The important part of
note is the lines with `has_polymorphic`, which prevents applying the
values from the `:include` key (which is mapped to both the `.includes`
and `.reference` methods) to the `.reference` method.

When the refactoring was done in 062263c (part of the
ManageIQ#10175 pull request), the check
for the reference methods was not brought along, so when an invalid
`:include_for_find` key was passed in with a polymorphic relationship,
the code would break with a not so obvious error message:

    Error caught: [ActiveRecord::ConfigurationError] nil

Most of the code in ar_merge_conditions.rb is actually for properly
determining when a polymorphic relationship exists, so in reality, this
code maybe should have stayed in some way or another.  The changes here
are an attempt to reduce the amount of code added, but it might be to
limited in what keys are checked, and thus, not thorough enough.

Regardless, it does seem to prevent most cases.

lib/rbac/filterer.rb

@@ -199,10 +199,10 @@ def search(options = {})
       scope = scope.except(:offset, :limit)
       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).references(include_for_find)
-              .includes(exp_includes).references(exp_includes)
+              .includes(include_for_find).includes(exp_includes)
               .order(options[:order])
 
+      scope = include_references(scope, klass, include_for_find, exp_includes)
       scope = scope.limit(limit).offset(offset) if attrs[:apply_limit_in_sql]
       targets = scope
 
@@ -230,6 +230,21 @@ def search(options = {})
       return targets, attrs
     end
 
+    def include_references(scope, klass, include_for_find, exp_includes)
+      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)
+      end
+      scope
+    end
+
+    def polymorphic_include?(target_klass, includes)
+      includes.keys.any? do |incld|
+        reflection = target_klass.reflect_on_association(incld)
+        reflection && reflection.polymorphic?
+      end
+    end
+
     def filtered(objects, options = {})
       Rbac.search(options.reverse_merge(:targets => objects)).first
     end

spec/lib/rbac/filterer_spec.rb

@@ -234,6 +234,52 @@
         expect(results).to include(vm)
       end
     end
+
+    context "for Metrics::Rollup" do
+      before(:each) do
+        vm = FactoryGirl.create(:vm_vmware)
+        FactoryGirl.create(
+          :metric_rollup_vm_daily,
+          :resource_id => vm.id,
+          :timestamp   => "2010-04-14T00:00:00Z"
+        )
+
+        # Typical includes for rendering daily metrics charts
+        @include = {
+          :max_derived_cpu_available       => {},
+          :max_derived_cpu_reserved        => {},
+          :min_cpu_usagemhz_rate_average   => {},
+          :max_cpu_usagemhz_rate_average   => {},
+          :min_cpu_usage_rate_average      => {},
+          :max_cpu_usage_rate_average      => {},
+          :v_pct_cpu_ready_delta_summation => {},
+          :v_pct_cpu_wait_delta_summation  => {},
+          :v_pct_cpu_used_delta_summation  => {},
+          :max_derived_memory_available    => {},
+          :max_derived_memory_reserved     => {},
+          :min_derived_memory_used         => {},
+          :max_derived_memory_used         => {},
+          :min_disk_usage_rate_average     => {},
+          :max_disk_usage_rate_average     => {},
+          :min_net_usage_rate_average      => {},
+          :max_net_usage_rate_average      => {},
+          :v_derived_storage_used          => {},
+          :resource                        => {}
+        }
+      end
+
+      # NOTE:  Think long and hard before you consider removing this test.
+      # Many-a-hours wasted here determining this bug that resulted in
+      # re-adding this test again.
+      it "should not raise an error when a polymorphic reflection is included" do
+        result = nil
+        expect do
+          result = described_class.search :class            => "MetricRollup",
+                                          :include_for_find => @include
+        end.not_to raise_error
+        expect(result.first.length).to eq(1)
+      end
+    end
   end
 
   context "common setup" do