Support nil and Array in :conditions => { attr => value } hashes. Closes #6548.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5435 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 50 insertions, 33 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 52068e461e..606f87f678 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,8 @@
 *SVN*
 
+* Support nil and Array in :conditions => { attr => value } hashes.  #6548 [Assaf, Jeremy Kemper]
+    find(:all, :conditions => { :topic_id => [1, 2, 3], :last_read => nil }
+
 * Consistently use LOWER() for uniqueness validations (rather than mixing with UPPER()) so the database can always use a functional index on the lowercased column.  #6495 [Si]
 
 * SQLite: fix calculations workaround, remove count(distinct) query rewrite, cleanup test connection scripts.  [Jeremy Kemper]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 3fc9b4bddb..c09cef1118 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -986,7 +986,7 @@ module ActiveRecord #:nodoc:
           options.update(:limit => 1) unless options[:include]
           find_every(options).first
         end
-           
+
         def find_every(options)
           records = scoped?(:find, :include) || options[:include] ?
             find_with_associations(options) : 
@@ -996,11 +996,11 @@ module ActiveRecord #:nodoc:
 
           records
         end
- 
+
         def find_from_ids(ids, options)
-          expects_array = ids.first.kind_of?(Array)       
+          expects_array = ids.first.kind_of?(Array)
           return ids.first if expects_array && ids.first.empty?
-        
+
           ids = ids.flatten.compact.uniq
 
           case ids.size
@@ -1192,16 +1192,16 @@ module ActiveRecord #:nodoc:
             attribute_names = extract_attribute_names_from_match(match)
             super unless all_attributes_exists?(attribute_names)
 
-            conditions = construct_conditions_from_arguments(attribute_names, arguments)
+            attributes = construct_attributes_from_arguments(attribute_names, arguments)
 
             case extra_options = arguments[attribute_names.size]
               when nil
-                options = { :conditions => conditions }
+                options = { :conditions => attributes }
                 set_readonly_option!(options)
                 ActiveSupport::Deprecation.silence { send(finder, options) }
 
               when Hash
-                finder_options = extra_options.merge(:conditions => conditions)
+                finder_options = extra_options.merge(:conditions => attributes)
                 validate_find_options(finder_options)
                 set_readonly_option!(finder_options)
 
@@ -1215,7 +1215,7 @@ module ActiveRecord #:nodoc:
 
               else
                 ActiveSupport::Deprecation.silence do
-                  send(deprecated_finder, conditions, *arguments[attribute_names.length..-1])
+                  send(deprecated_finder, sanitize_sql(attributes), *arguments[attribute_names.length..-1])
                 end
             end
           elsif match = /find_or_(initialize|create)_by_([_a-zA-Z]\w*)/.match(method_id.to_s)
@@ -1223,9 +1223,11 @@ module ActiveRecord #:nodoc:
             attribute_names = extract_attribute_names_from_match(match)
             super unless all_attributes_exists?(attribute_names)
 
-            options = { :conditions => construct_conditions_from_arguments(attribute_names, arguments) }
+            attributes = construct_attributes_from_arguments(attribute_names, arguments)
+            options = { :conditions => attributes }
             set_readonly_option!(options)
-            find_initial(options) || send(instantiator, construct_attributes_from_arguments(attribute_names, arguments))
+
+            find_initial(options) || send(instantiator, attributes)
           else
             super
           end
@@ -1247,12 +1249,6 @@ module ActiveRecord #:nodoc:
           match.captures.last.split('_and_')
         end
 
-        def construct_conditions_from_arguments(attribute_names, arguments)
-          conditions = []
-          attribute_names.each_with_index { |name, idx| conditions << "#{table_name}.#{connection.quote_column_name(name)} #{attribute_condition(arguments[idx])} " }
-          [ conditions.join(" AND "), *arguments[0...attribute_names.length] ]
-        end
-        
         def construct_attributes_from_arguments(attribute_names, arguments)
           attributes = {}
           attribute_names.each_with_index { |name, idx| attributes[name] = arguments[idx] }
@@ -1275,7 +1271,7 @@ module ActiveRecord #:nodoc:
         def expand_id_conditions(id_or_conditions)
           case id_or_conditions
             when Array, Hash then id_or_conditions
-            else construct_conditions_from_arguments([primary_key], [id_or_conditions])
+            else sanitize_sql(primary_key => id_or_conditions)
           end
         end
 
@@ -1377,26 +1373,32 @@ module ActiveRecord #:nodoc:
           klass.base_class.name
         end
 
-        #Accepts an array, hash, or string of sql conditions and 
-        #deals with them accordingly
+        # Accepts an array, hash, or string of sql conditions and sanitizes
+        # them into a valid SQL fragment.
         #   ["name='%s' and group_id='%s'", "foo'bar", 4]  returns  "name='foo''bar' and group_id='4'"
         #   { :name => "foo'bar", :group_id => 4 }  returns "name='foo''bar' and group_id='4'"
         #   "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'"
         def sanitize_sql(condition)
-          return sanitize_sql_array(condition) if condition.is_a?(Array)
-          return sanitize_sql_hash(condition) if condition.is_a?(Hash)
-          condition
+          case condition
+            when Array; sanitize_sql_array(condition)
+            when Hash;  sanitize_sql_hash(condition)
+            else        condition
+          end
         end
-        
-        # Accepts a hash of conditions.  The hash has each key/value or attribute/value pair
-        # sanitized and interpolated into the sql statement.
-        #   { :name => "foo'bar", :group_id => 4 }  returns "name='foo''bar' and group_id= 4"
-        def sanitize_sql_hash(hash)
-          hash.collect { |attrib, value|
-            "#{table_name}.#{connection.quote_column_name(attrib)} = #{quote_value(value)}"
-          }.join(" AND ")
+
+        # Sanitizes a hash of attribute/value pairs into SQL conditions.
+        #   { :name => "foo'bar", :group_id => 4 }
+        #     # => "name='foo''bar' and group_id= 4"
+        #   { :status => nil, :group_id => [1,2,3] }
+        #     # => "status IS NULL and group_id IN (1,2,3)"
+        def sanitize_sql_hash(attrs)
+          conditions = attrs.map do |attr, value|
+            "#{table_name}.#{connection.quote_column_name(attr)} #{attribute_condition(value)}"
+          end.join(' AND ')
+
+          replace_bind_variables(conditions, attrs.values)
         end
-        
+
         # Accepts an array of conditions.  The array has each value
         # sanitized and interpolated into the sql statement.
         #   ["name='%s' and group_id='%s'", "foo'bar", 4]  returns  "name='foo''bar' and group_id='4'"
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index ebc21b5e0c..cf2dfcbb24 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -161,10 +161,22 @@ class FinderTest < Test::Unit::TestCase
       Company.find(:first, :conditions => { :id => 2, :dhh => true })
     }
   end
-  
+
   def test_hash_condition_find_with_escaped_characters
     Company.create("name" => "Ain't noth'n like' \#stuff")
-    assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff"})
+    assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff" })
+  end
+
+  def test_hash_condition_find_with_array
+    p1, p2 = Post.find(:all, :limit => 2, :order => 'id asc')
+    assert_equal [p1, p2], Post.find(:all, :conditions => { :id => [p1, p2] }, :order => 'id asc')
+    assert_equal [p1, p2], Post.find(:all, :conditions => { :id => [p1, p2.id] }, :order => 'id asc')
+  end
+
+  def test_hash_condition_find_with_nil
+    topic = Topic.find(:first, :conditions => { :last_read => nil } )
+    assert_not_nil topic
+    assert_nil topic.last_read
   end
 
   def test_bind_variables