Records and arrays of records are bound as quoted ids.

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

activerecord/CHANGELOG

@@ -1,5 +1,9 @@
 *SVN*
 
+* Records and arrays of records are bound as quoted ids. [Jeremy Kemper]
+    Foo.find(:all, :conditions => ['bar_id IN (?)', bars])
+    Foo.find(:first, :conditions => ['bar_id = ?', bar])
+
 * Fixed that Base.find :all, :conditions => [ "id IN (?)", collection ] would fail if collection was empty [DHH]
 
 * Add a list of regexes assert_queries skips in the ActiveRecord test suite.  [Rick]

activerecord/lib/active_record/base.rb

@@ -1308,9 +1308,9 @@ module ActiveRecord #:nodoc:
         end
 
         def quote_bound_value(value) #:nodoc:
-          if (value.respond_to?(:map) && !value.is_a?(String))
-            if value.empty?
-              "null"
+          if value.respond_to?(:map) && !value.is_a?(String)
+            if value.respond_to?(:empty?) && value.empty?
+              connection.quote(nil)
             else
               value.map { |v| connection.quote(v) }.join(',')
             end

activerecord/lib/active_record/connection_adapters/abstract/quoting.rb

@@ -4,6 +4,9 @@ module ActiveRecord
       # Quotes the column value to help prevent
       # {SQL injection attacks}[http://en.wikipedia.org/wiki/SQL_injection].
       def quote(value, column = nil)
+        # records are quoted as their primary key
+        return value.quoted_id if value.respond_to?(:quoted_id)
+
         case value
           when String
             if column && column.type == :binary && column.class.respond_to?(:string_to_binary)

activerecord/test/finder_test.rb

@@ -186,10 +186,25 @@ class FinderTest < Test::Unit::TestCase
     assert_equal %('a','b','c'), bind(':a', :a => Set.new(%w(a b c))) # '
   end
 
+  def test_bind_empty_enumerable
+    quoted_nil = ActiveRecord::Base.connection.quote(nil)
+    assert_equal quoted_nil, bind('?', [])
+    assert_equal " in (#{quoted_nil})", bind(' in (?)', [])
+    assert_equal "foo in (#{quoted_nil})", bind('foo in (?)', [])
+  end
+
   def test_bind_string
     assert_equal "''", bind('?', '')
   end
 
+  def test_bind_record
+    o = Struct.new(:quoted_id).new(1)
+    assert_equal '1', bind('?', o)
+
+    os = [o] * 3
+    assert_equal '1,1,1', bind('?', os)
+  end
+
   def test_string_sanitation
     assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1")
     assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table")
@@ -363,6 +378,24 @@ class FinderTest < Test::Unit::TestCase
     end
   end
 
+  def test_find_by_empty_ids
+    assert_equal [], Post.find([])
+  end
+
+  def test_find_by_empty_in_condition
+    assert_equal [], Post.find(:all, :conditions => ['id in (?)', []])
+  end
+
+  def test_find_by_records
+    p1, p2 = Post.find(1, 2)
+    assert_equal [p1, p2], Post.find(:all, :conditions => ['id in (?)', [p1, p2]]).sort_by { |p| p.id }
+  end
+
+  def test_find_by_records_and_ids
+    p1, p2 = Post.find(1, 2)
+    assert_equal [p1, p2], Post.find(:all, :conditions => ['id in (?)', [p1, p2.id]]).sort_by { |p| p.id }
+  end
+
   def test_select_value
     assert_equal "37signals", Company.connection.select_value("SELECT name FROM companies WHERE id = 1")
     assert_nil Company.connection.select_value("SELECT name FROM companies WHERE id = -1")