Move clearing of @offsets cache to reset_scope

ragesoss · Jun 21, 2017 · adcd307 · adcd307
1 parent c7f669a
commit adcd307
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 14 deletions.
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -1088,7 +1088,6 @@ def clear
       #   # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>]
       def reload
         proxy_association.reload
-        @offsets = {}
         reset_scope
       end
 
@@ -1111,11 +1110,11 @@ def reload
       def reset
         proxy_association.reset
         proxy_association.reset_scope
-        @offsets = {}
         reset_scope
       end
 
       def reset_scope # :nodoc:
+        @offsets = {}
         @scope = nil
         self
       end

diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -741,30 +741,39 @@ def test_find_first_sanitized
     assert_equal client2, firm.clients.merge!(where: ["#{QUOTED_TYPE} = :type", { type: "Client" }], order: "id").first
   end
 
-  def test_find_first_after_reset
+  def test_find_first_after_reset_scope
     firm = Firm.all.merge!(order: "id").first
+    collection = firm.clients
+
+    original_object_id = collection.first.object_id
+    assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object"
+
+    # It should return a different object, since the association has been reloaded
+    assert_not_equal original_object_id, firm.clients.first.object_id, "Expected #first to return a new object"
+  end
 
-    # Calling first twice should return the same object
-    original_object_id = firm.clients.first.object_id
-    assert_equal firm.clients.first.object_id, original_object_id, "Expected multiple invocations of #first to return the same object"
+  def test_find_first_after_reset
+    firm = Firm.all.merge!(order: "id").first
+    collection = firm.clients
 
-    firm.clients.reset
+    original_object_id = collection.first.object_id
+    assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object"
+    collection.reset
 
     # It should return a different object, since the association has been reloaded
-    assert_not_equal original_object_id, firm.clients.first.object_id, "Expected #first after #reload to return a new object"
+    assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object"
   end
 
   def test_find_first_after_reload
     firm = Firm.all.merge!(order: "id").first
+    collection = firm.clients
 
-    # Calling first twice should return the same object
-    original_object_id = firm.clients.first.object_id
-    assert_equal firm.clients.first.object_id, original_object_id, "Expected multiple invocations of #first to return the same object"
-
-    firm.clients.reload
+    original_object_id = collection.first.object_id
+    assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object"
+    collection.reset
 
     # It should return a different object, since the association has been reloaded
-    assert_not_equal original_object_id, firm.clients.first.object_id, "Expected #first after #reload to return a new object"
+    assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object"
   end
 
   def test_find_all_with_include_and_conditions