Merge pull request #17886 from mrgilman/refactor-predicate-builder

Refactor `PredicateBuilder` from singleton to instance

activerecord/lib/active_record/relation.rb

@@ -677,5 +677,9 @@ def tables_in_string(string)
       # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries
       string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map(&:downcase).uniq - ['raw_sql_']
     end
+
+    def predicate_builder
+      @predicate_builder ||= PredicateBuilder.new(klass, table)
+    end
   end
 end

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -5,7 +5,12 @@ class PredicateBuilder # :nodoc:
     autoload :RelationHandler, 'active_record/relation/predicate_builder/relation_handler'
     autoload :ArrayHandler, 'active_record/relation/predicate_builder/array_handler'
 
-    def self.resolve_column_aliases(klass, hash)
+    def initialize(klass, table)
+      @klass = klass
+      @table = table
+    end
+
+    def resolve_column_aliases(hash)
       hash = hash.dup
       hash.keys.grep(Symbol) do |key|
         if klass.attribute_alias? key
@@ -15,21 +20,21 @@ def self.resolve_column_aliases(klass, hash)
       hash
     end
 
-    def self.build_from_hash(klass, attributes, default_table)
+    def build_from_hash(attributes)
       queries = []
+      builder = self
 
       attributes.each do |column, value|
-        table = default_table
-
         if value.is_a?(Hash)
           if value.empty?
             queries << '1=0'
           else
-            table       = Arel::Table.new(column)
+            arel_table = Arel::Table.new(column)
             association = klass._reflect_on_association(column)
+            builder = self.class.new(association && association.klass, arel_table)
 
             value.each do |k, v|
-              queries.concat expand(association && association.klass, table, k, v)
+              queries.concat builder.expand(k, v)
             end
           end
         else
@@ -37,17 +42,17 @@ def self.build_from_hash(klass, attributes, default_table)
 
           if column.include?('.')
             table_name, column = column.split('.', 2)
-            table = Arel::Table.new(table_name)
+            arel_table = Arel::Table.new(table_name)
+            builder = self.class.new(klass, arel_table)
           end
-
-          queries.concat expand(klass, table, column, value)
+          queries.concat builder.expand(column, value)
         end
       end
 
       queries
     end
 
-    def self.expand(klass, table, column, value)
+    def expand(column, value)
       queries = []
 
       # Find the foreign key when using queries such as:
@@ -57,17 +62,17 @@ def self.expand(klass, table, column, value)
       # PriceEstimate.where(estimate_of: treasure)
       if klass && reflection = klass._reflect_on_association(column)
         if reflection.polymorphic? && base_class = polymorphic_base_class_from_value(value)
-          queries << build(table[reflection.foreign_type], base_class)
+          queries << self.class.build(table[reflection.foreign_type], base_class)
         end
 
         column = reflection.foreign_key
       end
 
-      queries << build(table[column], value)
+      queries << self.class.build(table[column], value)
       queries
     end
 
-    def self.polymorphic_base_class_from_value(value)
+    def polymorphic_base_class_from_value(value)
       case value
       when Relation
         value.klass.base_class
@@ -116,11 +121,14 @@ def self.register_handler(klass, handler)
     def self.build(attribute, value)
       handler_for(value).call(attribute, value)
     end
-    private_class_method :build
 
     def self.handler_for(object)
       @handlers.detect { |klass, _| klass === object }.last
     end
     private_class_method :handler_for
+
+    protected
+
+    attr_reader :klass, :table
   end
 end

activerecord/lib/active_record/relation/query_methods.rb

@@ -947,7 +947,7 @@ def build_where(opts, other = [])
       when String, Array
         [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))]
       when Hash
-        opts = PredicateBuilder.resolve_column_aliases(klass, opts)
+        opts = predicate_builder.resolve_column_aliases(opts)
 
         tmp_opts, bind_values = create_binds(opts)
         self.bind_values += bind_values
@@ -955,7 +955,7 @@ def build_where(opts, other = [])
         attributes = @klass.send(:expand_hash_conditions_for_aggregates, tmp_opts)
         add_relations_to_bind_values(attributes)
 
-        PredicateBuilder.build_from_hash(klass, attributes, table)
+        predicate_builder.build_from_hash(attributes)
       else
         [opts]
       end

activerecord/lib/active_record/sanitization.rb

@@ -87,14 +87,15 @@ def expand_hash_conditions_for_aggregates(attrs)
       #   { address: Address.new("123 abc st.", "chicago") }
       #     # => "address_street='123 abc st.' and address_city='chicago'"
       def sanitize_sql_hash_for_conditions(attrs, default_table_name = self.table_name)
+        table = Arel::Table.new(table_name).alias(default_table_name)
+        predicate_builder = PredicateBuilder.new(self, table)
         ActiveSupport::Deprecation.warn(<<-EOWARN)
 sanitize_sql_hash_for_conditions is deprecated, and will be removed in Rails 5.0
         EOWARN
-        attrs = PredicateBuilder.resolve_column_aliases self, attrs
+        attrs = predicate_builder.resolve_column_aliases(attrs)
         attrs = expand_hash_conditions_for_aggregates(attrs)
 
-        table = Arel::Table.new(table_name).alias(default_table_name)
-        PredicateBuilder.build_from_hash(self, attrs, table).map { |b|
+        predicate_builder.build_from_hash(attrs).map { |b|
           connection.visitor.compile b
         }.join(' AND ')
       end