WIP: fixing the limit bug and introducing new tests (failing for now) on .find(array) with offset

2 files changed, 30 insertions, 7 deletions

diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index a0e1420055..18324cf3c6 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -444,8 +444,6 @@ module ActiveRecord
     end
 
     def find_some(ids)
-      result = where(primary_key => ids).to_a
-
       expected_size =
         if limit_value && ids.size > limit_value
           limit_value
@@ -456,12 +454,16 @@ module ActiveRecord
       # 11 ids with limit 3, offset 9 should give 2 results.
       if offset_value && (ids.size - offset_value < expected_size)
         expected_size = ids.size - offset_value
+      else
+        ids = ids.first(expected_size) unless self.values[:order]
       end
 
+      result = where(primary_key => ids).to_a
+
       if result.size == expected_size
         return result if self.values[:order]
         records_by_id = result.index_by(&:id)
-        ids.first(expected_size).collect { |id| records_by_id[id.to_i] }
+        ids.collect { |id| records_by_id[id.to_i] }.compact
       else
         raise_record_not_found_exception!(ids, result.size, expected_size)
       end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 1ab91c5195..526623ec36 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -83,11 +83,28 @@ class FinderTest < ActiveRecord::TestCase
     assert_equal 'The Fifth Topic of the day', records[2].title
   end
 
-  def test_find_with_ids_and_limit
-    records = Topic.limit(2).find([4,2,5])
+  def test_find_with_ids_with_limit_and_order_clause
+    # The order clause takes precedence over the informed ids
+    records = Topic.limit(2).order(:id).find([5,3,1])
     assert_equal 2, records.size
-    assert_equal 'The Fourth Topic of the day', records[0].title
+    assert_equal 'The First Topic',            records[0].title
+    assert_equal 'The Third Topic of the day', records[1].title
+  end
+
+  def test_find_with_ids_and_limit
+    records = Topic.limit(3).find([3,2,5,1,4])
+    assert_equal 3, records.size
+    assert_equal 'The Third Topic of the day',  records[0].title
     assert_equal 'The Second Topic of the day', records[1].title
+    assert_equal 'The Fifth Topic of the day',  records[2].title
+  end
+
+  def test_find_with_ids_and_offset
+    records = Topic.offset(2).find([3,2,5,1,4])
+    assert_equal 3, records.size
+    assert_equal 'The Fifth Topic of the day',  records[0].title
+    assert_equal 'The First Topic',             records[1].title
+    assert_equal 'The Fourth Topic of the day', records[2].title
   end
 
   def test_find_passing_active_record_object_is_deprecated
@@ -236,7 +253,9 @@ class FinderTest < ActiveRecord::TestCase
 
   def test_find_by_ids_with_limit_and_offset
     assert_equal 2, Entrant.limit(2).find([1,3,2]).size
-    assert_equal 1, Entrant.limit(3).offset(2).find([1,3,2]).size
+    entrants = Entrant.limit(3).offset(2).find([1,3,2])
+    assert_equal 1, entrants.size
+    assert_equal 'Ruby Guru', entrants.first.name
 
     # Also test an edge case: If you have 11 results, and you set a
     #   limit of 3 and offset of 9, then you should find that there
@@ -244,6 +263,8 @@ class FinderTest < ActiveRecord::TestCase
     devs = Developer.all
     last_devs = Developer.limit(3).offset(9).find devs.map(&:id)
     assert_equal 2, last_devs.size
+    assert_equal 'fixture_10', last_devs[0].name
+    assert_equal 'Jamis', last_devs[1].name
   end
 
   def test_find_with_large_number