Merge pull request rails#45943 from quadule/reset-relation-after-bulk-updates

Reset AR::Relation in bulk insert and upsert methods

Author: kamipo

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Reset relations after `insert_all`/`upsert_all`.
+
+    Bulk insert/upsert methods will now call `reset` if used on a relation, matching the behavior of `update_all`.
+
+    *Milo Winningham*
+
 *   Use `_N` as a parallel tests databases suffixes
 
     Peviously, `-N` was used as a suffix. This can cause problems for RDBMSes

activerecord/lib/active_record/association_relation.rb

@@ -17,7 +17,7 @@ def ==(other)
 
     %w(insert insert_all insert! insert_all! upsert upsert_all).each do |method|
       class_eval <<~RUBY, __FILE__, __LINE__ + 1
-        def #{method}(attributes, **kwargs)
+        def #{method}(...)
           if @association.reflection.through_reflection?
             raise ArgumentError, "Bulk insert or upsert is currently not supported for has_many through association"
           end

activerecord/lib/active_record/associations/collection_proxy.rb

@@ -1125,14 +1125,20 @@ def pretty_print(pp) # :nodoc:
         super
       end
 
+      %w(insert insert_all insert! insert_all! upsert upsert_all).each do |method|
+        class_eval <<~RUBY, __FILE__, __LINE__ + 1
+          def #{method}(...)
+            scope.#{method}(...).tap { reset }
+          end
+        RUBY
+      end
+
       delegate_methods = [
         QueryMethods,
         SpawnMethods,
       ].flat_map { |klass|
         klass.public_instance_methods(false)
-      } - self.public_instance_methods(false) - [:select] + [
-        :scoping, :values, :insert, :insert_all, :insert!, :insert_all!, :upsert, :upsert_all, :load_async
-      ]
+      } - self.public_instance_methods(false) - [ :select ] + [ :scoping, :values, :load_async ]
 
       delegate(*delegate_methods, to: :scope)
 

activerecord/lib/active_record/insert_all.rb

@@ -11,7 +11,7 @@ class << self
       def execute(relation, ...)
         relation.model.with_connection do |c|
           new(relation, c, ...).execute
-        end
+        end.tap { relation.reset }
       end
     end
 

activerecord/test/cases/collection_cache_key_test.rb

@@ -96,6 +96,24 @@ class CollectionCacheKeyTest < ActiveRecord::TestCase
       assert_match(/\Acomments\/query-(\h+)-(\d+)-(\d+)\z/, comments.cache_key)
     end
 
+    test "insert_all will update cache_key" do
+      developers = Developer.all
+      cache_key = developers.cache_key
+
+      developers.insert_all([{ name: "Alice" }, { name: "Bob" }])
+
+      assert_not_equal cache_key, developers.cache_key
+    end
+
+    test "upsert_all will update cache_key" do
+      developers = Developer.all
+      cache_key = developers.cache_key
+
+      developers.upsert_all([{ id: 1, name: "Alice" }, { id: 2, name: "Bob" }])
+
+      assert_not_equal cache_key, developers.cache_key
+    end
+
     test "update_all will update cache_key" do
       developers = Developer.where(name: "David")
       cache_key = developers.cache_key

activerecord/test/cases/insert_all_test.rb

@@ -729,6 +729,14 @@ def test_insert_all_on_relation_precedence
     end
   end
 
+  def test_insert_all_resets_relation
+    audit_logs = Developer.create!(name: "Alice").audit_logs.load
+
+    assert_changes "audit_logs.loaded?", from: true, to: false do
+      audit_logs.insert_all!([{ message: "event" }])
+    end
+  end
+
   def test_insert_all_create_with
     assert_difference "Book.where(format: 'X').count", +2 do
       Book.create_with(format: "X").insert_all!([ { name: "A" }, { name: "B" } ])
@@ -761,6 +769,14 @@ def test_upsert_all_on_relation_precedence
     end
   end
 
+  def test_upsert_all_resets_relation
+    audit_logs = Developer.create!(name: "Alice").audit_logs.load
+
+    assert_changes "audit_logs.loaded?", from: true, to: false do
+      audit_logs.upsert_all([{ id: 1, message: "event" }])
+    end
+  end
+
   def test_upsert_all_create_with
     skip unless supports_insert_on_duplicate_update?