Merge pull request #4222 from amatsuda/ar_predicate_builder_refactor

refactor AR::PredicateBuilder.build_from_hash

1 files changed, 37 insertions, 35 deletions

diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index 95b6efb402..88081edae2 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -15,47 +15,49 @@ module ActiveRecord
             table = Arel::Table.new(table_name, engine)
           end
 
-          attribute = table[column.to_sym]
-
-          case value
-          when ActiveRecord::Relation
-            value = value.select(value.klass.arel_table[value.klass.primary_key]) if value.select_values.empty?
-            attribute.in(value.arel.ast)
-          when Array, ActiveRecord::Associations::CollectionProxy
-            values = value.to_a.map {|x| x.is_a?(ActiveRecord::Model) ? x.id : x}
-            ranges, values = values.partition {|v| v.is_a?(Range) || v.is_a?(Arel::Relation)}
-
-            array_predicates = ranges.map {|range| attribute.in(range)}
+          build(table[column.to_sym], value)
+        end
+      end
+      predicates.flatten
+    end
 
-            if values.include?(nil)
-              values = values.compact
-              case values.length
-              when 0
-                array_predicates << attribute.eq(nil)
-              when 1
-                array_predicates << attribute.eq(values.first).or(attribute.eq(nil))
-              else
-                array_predicates << attribute.in(values).or(attribute.eq(nil))
-              end
+    private
+      def self.build(attribute, value)
+        case value
+        when ActiveRecord::Relation
+          value = value.select(value.klass.arel_table[value.klass.primary_key]) if value.select_values.empty?
+          attribute.in(value.arel.ast)
+        when Array, ActiveRecord::Associations::CollectionProxy
+          values = value.to_a.map {|x| x.is_a?(ActiveRecord::Model) ? x.id : x}
+          ranges, values = values.partition {|v| v.is_a?(Range) || v.is_a?(Arel::Relation)}
+
+          array_predicates = ranges.map {|range| attribute.in(range)}
+
+          if values.include?(nil)
+            values = values.compact
+            case values.length
+            when 0
+              array_predicates << attribute.eq(nil)
+            when 1
+              array_predicates << attribute.eq(values.first).or(attribute.eq(nil))
             else
-              array_predicates << attribute.in(values)
+              array_predicates << attribute.in(values).or(attribute.eq(nil))
             end
-
-            array_predicates.inject {|composite, predicate| composite.or(predicate)}
-          when Range, Arel::Relation
-            attribute.in(value)
-          when ActiveRecord::Model
-            attribute.eq(value.id)
-          when Class
-            # FIXME: I think we need to deprecate this behavior
-            attribute.eq(value.name)
           else
-            attribute.eq(value)
+            array_predicates << attribute.in(values)
           end
+
+          array_predicates.inject {|composite, predicate| composite.or(predicate)}
+        when Range, Arel::Relation
+          attribute.in(value)
+        when ActiveRecord::Model
+          attribute.eq(value.id)
+        when Class
+          # FIXME: I think we need to deprecate this behavior
+          attribute.eq(value.name)
+        else
+          attribute.eq(value)
         end
       end
-
-      predicates.flatten
-    end
   end
 end