Merge pull request #28715 from kamipo/convert_association_queries_to_poro_queries

Convert association queries to PORO queries

4 files changed, 32 insertions, 60 deletions

diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index fca914aedd..133b3d837c 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -20,8 +20,6 @@ module ActiveRecord
       register_handler(RangeHandler::RangeWithBinds, RangeHandler.new)
       register_handler(Relation, RelationHandler.new)
       register_handler(Array, ArrayHandler.new(self))
-      register_handler(AssociationQueryValue, AssociationQueryHandler.new(self))
-      register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self))
     end
 
     def build_from_hash(attributes)
@@ -87,7 +85,6 @@ module ActiveRecord
         binds = []
 
         attributes.each do |column_name, value|
-          binds.concat(value.bound_attributes) if value.is_a?(Relation)
           case
           when value.is_a?(Hash) && !table.has_column?(column_name)
             attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value)
@@ -99,7 +96,21 @@ module ActiveRecord
             #
             # For polymorphic relationships, find the foreign key and type:
             # PriceEstimate.where(estimate_of: treasure)
-            result[column_name] = AssociationQueryHandler.value_for(table, column_name, value)
+            associated_table = table.associated_table(column_name)
+            if associated_table.polymorphic_association?
+              case value.is_a?(Array) ? value.first : value
+              when Base, Relation
+                value = [value] unless value.is_a?(Array)
+                klass = PolymorphicArrayValue
+              end
+            end
+
+            klass ||= AssociationQueryValue
+            result[column_name] = klass.new(associated_table, value).queries.map do |query|
+              attrs, bvs = create_binds_for_hash(query)
+              binds.concat(bvs)
+              attrs
+            end
           when value.is_a?(Range) && !table.type(column_name).respond_to?(:subtype)
             first = value.begin
             last = value.end
@@ -117,6 +128,8 @@ module ActiveRecord
             if can_be_bound?(column_name, value)
               result[column_name] = Arel::Nodes::BindParam.new
               binds << build_bind_param(column_name, value)
+            elsif value.is_a?(Relation)
+              binds.concat(value.bound_attributes)
             end
           end
         end
diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
index 88b6c37d43..54e9910598 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
@@ -10,6 +10,7 @@ module ActiveRecord
         nils, values = values.partition(&:nil?)
 
         return attribute.in([]) if values.empty? && nils.empty?
+        return queries_predicates(values) if nils.empty? && values.all? { |v| v.is_a?(Hash) }
 
         ranges, values = values.partition { |v| v.is_a?(Range) }
 
@@ -26,7 +27,7 @@ module ActiveRecord
 
         array_predicates = ranges.map { |range| predicate_builder.build(attribute, range) }
         array_predicates.unshift(values_predicate)
-        array_predicates.inject { |composite, predicate| composite.or(predicate) }
+        array_predicates.inject(&:or)
       end
 
       # TODO Change this to private once we've dropped Ruby 2.2 support.
@@ -40,6 +41,17 @@ module ActiveRecord
             other
           end
         end
+
+      private
+        def queries_predicates(queries)
+          if queries.size > 1
+            queries.map do |query|
+              Arel::Nodes::And.new(predicate_builder.build_from_hash(query))
+            end.inject(&:or)
+          else
+            predicate_builder.build_from_hash(queries.first)
+          end
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
index 9141d9c537..2fe0f81cab 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
@@ -1,35 +1,5 @@
 module ActiveRecord
   class PredicateBuilder
-    class AssociationQueryHandler # :nodoc:
-      def self.value_for(table, column, value)
-        associated_table = table.associated_table(column)
-        if associated_table.polymorphic_association?
-          case value.is_a?(Array) ? value.first : value
-          when Base, Relation
-            value = [value] unless value.is_a?(Array)
-            klass = PolymorphicArrayValue
-          end
-        end
-
-        klass ||= AssociationQueryValue
-        klass.new(associated_table, value)
-      end
-
-      def initialize(predicate_builder)
-        @predicate_builder = predicate_builder
-      end
-
-      def call(attribute, value)
-        predicate_builder.build_from_hash(value.queries)
-      end
-
-      # TODO Change this to private once we've dropped Ruby 2.2 support.
-      # Workaround for Ruby 2.2 "private attribute?" warning.
-      protected
-
-        attr_reader :predicate_builder
-    end
-
     class AssociationQueryValue # :nodoc:
       attr_reader :associated_table, :value
 
@@ -39,7 +9,7 @@ module ActiveRecord
       end
 
       def queries
-        { associated_table.association_foreign_key.to_s => ids }
+        [associated_table.association_foreign_key.to_s => ids]
       end
 
       private
diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb
index c2f136256b..9bb2f8c8dc 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb
@@ -1,28 +1,5 @@
 module ActiveRecord
   class PredicateBuilder
-    class PolymorphicArrayHandler # :nodoc:
-      def initialize(predicate_builder)
-        @predicate_builder = predicate_builder
-      end
-
-      def call(attribute, value)
-        predicates = value.queries.map { |query| predicate_builder.build_from_hash(query) }
-
-        if predicates.size > 1
-          type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) }
-          type_and_ids_predicates.inject(&:or)
-        else
-          predicates.first
-        end
-      end
-
-      # TODO Change this to private once we've dropped Ruby 2.2 support.
-      # Workaround for Ruby 2.2 "private attribute?" warning.
-      protected
-
-        attr_reader :predicate_builder
-    end
-
     class PolymorphicArrayValue # :nodoc:
       attr_reader :associated_table, :values
 
@@ -35,7 +12,7 @@ module ActiveRecord
         type_to_ids_mapping.map do |type, ids|
           {
             associated_table.association_foreign_type.to_s => type,
-            associated_table.association_foreign_key.to_s => ids
+            associated_table.association_foreign_key.to_s => ids.size > 1 ? ids : ids.first
           }
         end
       end