From b42c3255bf22e54f459751d5370e8befc33e84ea Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 23 Oct 2015 16:13:36 -0500 Subject: Fix `first(limit)` to take advantage of `loaded?` records if available I realized that `first(2)`, etc. was unnecessarily querying for the records when they were already preloaded. This was because `find_nth_with_limit` can not know which `@records` to return because it conflates the `offset` and `index` into a single variable, while the `@records` only needs the `index` itself to select the proper record. Because `find_nth` and `find_nth_with_limit` are public methods, I instead introduced a private method `find_nth_with_limit_and_offset` which is called internally and handles the `loaded?` checking. Once the `offset` argument is removed from `find_nth`, `find_nth_with_limit_and_offset` can be collapsed into `find_nth_with_limit`, with `offset` always equal to `offset_index`. --- activerecord/test/cases/relations_test.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 7149c7d072..df0c2f1f92 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -111,10 +111,21 @@ class RelationTest < ActiveRecord::TestCase def test_loaded_first topics = Topic.all.order('id ASC') + topics.to_a # force load - assert_queries(1) do - topics.to_a # force load - 2.times { assert_equal "The First Topic", topics.first.title } + assert_no_queries do + assert_equal "The First Topic", topics.first.title + end + + assert topics.loaded? + end + + def test_loaded_first_with_limit + topics = Topic.all.order('id ASC') + topics.to_a # force load + + assert_no_queries do + assert_equal "The First Topic", topics.first(2).first.title end assert topics.loaded? -- cgit v1.2.3