Skip references if sql EXPLAIN passes

Currently, there are a lot of factors that determine whether or not a
query can be executed without the `.references` call.  ActiveRecord
itself is even smart enough to figured it out and add it for you if you
use the hash `.where` syntax:

    ContainerImage.includes(:containers => {})
                  .where(:containers => {:name => "foo"})
                  .to_sql

So as a fail safe, if `skip` is passed to method
`Rbac::Filterer#include_references`, then we can do an much cheap
explain to figure out if it is a valid query before deciding if skipping
is a bad idea.

This means 1 extra query to every RBac call search that is currently
"skippable" by our criteria, but they should hopefully be quick to
execute and be a fail safe for what we miss.

Note, doing a `scope.explain` will both execute the regular query and
the explain, and since we have a lot of heavy Rbac calls, this would not
be ideal.  We work around this by calling a private API with `.to_sql`
to only execute the EXPLAIN query, which still returns an error of
`ActiveRecord::StatementInvalid` when it is malformed.

lib/rbac/filterer.rb

@@ -289,7 +289,22 @@ def skip_references?(options, attrs)
     end
 
     def include_references(scope, klass, include_for_find, exp_includes, skip)
-      return scope if skip
+      if skip
+        # We want to no-op here, since the rescue means we should just execute
+        # the rest of the method as usual and skip the `return`.
+        begin
+          ActiveRecord::Base.connection.explain(scope.to_sql)
+          return scope
+        rescue ActiveRecord::StatementInvalid => e
+          unless Rails.env.production?
+            warn "There was an issue with the Rbac filter without references!"
+            warn "Consider trying to fix this edge case in Rbac::Filterer!  Error Below:"
+            warn e.message
+            warn e.backtrace
+          end
+        end
+      end
+
       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)

spec/lib/rbac/filterer_spec.rb

@@ -1804,6 +1804,69 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
 
         expect(resulting_scope.references_values).to eq([])
       end
+
+      context "when the scope is invalid without .references" do
+        let(:scope)           { klass.where("host.name = 'foo'") }
+        let(:method_args)     { [scope, klass, include_for_find, exp_includes, skip] }
+        let(:resulting_scope) { subject.send(:include_references, *method_args) }
+
+        let(:explain_error_match) {
+          Regexp.new(Regexp.escape(<<~ERR.chomp))
+            PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "host"
+            LINE 1: ...nfraManager::Vm') AND "vms"."template" = 'f' AND (host.name ...
+                                                                         ^
+            : EXPLAIN SELECT "vms".* FROM "vms" WHERE "vms"."type"
+          ERR
+        }
+
+        before { allow(subject).to receive(:warn) }
+
+        it "adds .references to the scope" do
+          expect(resulting_scope.references_values).to eq(["{:miq_server=>{}}", "{:host=>{}}"])
+        end
+
+        it "warns that there was an issue in test mode" do
+          # Make sure part of the warn output includes stacktrace lines from
+          # rbac and this file.  Does a bit of line addition to avoid this
+          # being too brittle and breaking easily, but expect it to break if
+          # you update Rbac::Filterer#include_references
+          method_file, method_line = subject.method(:include_references).source_location
+          explain_stacktrace_includes = [
+            "#{method_file}:#{method_line + 5}:in `include_references'",
+            Thread.current.backtrace[1].gsub(/:\d*:/) {|sub| ":#{sub.tr(":", "").to_i + 7}:"}
+          ]
+
+          expect(subject).to receive(:warn).with("There was an issue with the Rbac filter without references!")
+          expect(subject).to receive(:warn).with("Consider trying to fix this edge case in Rbac::Filterer!  Error Below:")
+          expect(subject).to receive(:warn).with(explain_error_match)
+          expect(subject).to receive(:warn).with(array_including(explain_stacktrace_includes))
+          resulting_scope
+        end
+
+        it "warns that there was an issue in development mode" do
+          expect(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("developement"))
+
+          # See above
+          method_file, method_line = subject.method(:include_references).source_location
+          explain_stacktrace_includes = [
+            "#{method_file}:#{method_line + 5}:in `include_references'",
+            Thread.current.backtrace[1].gsub(/:\d*:/) {|sub| ":#{sub.tr(":", "").to_i + 7}:"}
+          ]
+
+          expect(subject).to receive(:warn).with("There was an issue with the Rbac filter without references!")
+          expect(subject).to receive(:warn).with("Consider trying to fix this edge case in Rbac::Filterer!  Error Below:")
+          expect(subject).to receive(:warn).with(explain_error_match)
+          expect(subject).to receive(:warn).with(array_including(explain_stacktrace_includes))
+          resulting_scope
+        end
+
+        it "warns that there was an issue in production mode" do
+          expect(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))
+
+          expect(subject).to receive(:warn).never
+          resulting_scope
+        end
+      end
     end
 
     context "if the include is polymorphic" do