Follow-up to PR #1188

lib/paper_trail/record_trail.rb

@@ -70,25 +70,13 @@ module PaperTrail
 
       build_version_on_create(in_after_callback: true).tap do |version|
         version.save!
-        # One more Version object has been created. Need to invalidate
-        # the `versions` association cache, otherwise the new version
-        # can stay invisible within that association
+        # Because the version object was created using version_class.new instead
+        # of versions_assoc.build?, the association cache is unaware. So, we
+        # invalidate the `versions` association cache with `reset`.
         versions.reset
       end
     end
 
-    # @api private
-    def build_version_on_create(in_after_callback:)
-      event = Events::Create.new(@record, in_after_callback)
-
-      # Merge data from `Event` with data from PT-AT. We no longer use
-      # `data_for_create` but PT-AT still does.
-      data = event.data.merge!(data_for_create)
-
-      # Pure `version_class.new` reduces memory usage compared to `versions_assoc.build`
-      @record.class.paper_trail.version_class.new(data)
-    end
-
     # PT-AT extends this method to add its transaction id.
     #
     # @api private
@@ -140,9 +128,9 @@ module PaperTrail
       return unless version
 
       if version.save
-        # One more Version object has been created. Need to invalidate
-        # the `versions` association cache, otherwise the new version
-        # can stay invisible within that association
+        # Because the version object was created using version_class.new instead
+        # of versions_assoc.build?, the association cache is unaware. So, we
+        # invalidate the `versions` association cache with `reset`.
         versions.reset
         version
       else
@@ -150,19 +138,6 @@ module PaperTrail
       end
     end
 
-    # @api private
-    def build_version_on_update(force:, in_after_callback:, is_touch:)
-      event = Events::Update.new(@record, in_after_callback, is_touch, nil)
-      return unless force || event.changed_notably?
-
-      # Merge data from `Event` with data from PT-AT. We no longer use
-      # `data_for_update` but PT-AT still does.
-      data = event.data.merge!(data_for_update)
-
-      # Pure `version_class.new` reduces memory usage compared to `versions_assoc.build`
-      @record.class.paper_trail.version_class.new(data)
-    end
-
     # PT-AT extends this method to add its transaction id.
     #
     # @api private
@@ -278,6 +253,35 @@ module PaperTrail
       @record.send(@record.class.versions_association_name).reset
     end
 
+    # @api private
+    def build_version_on_create(in_after_callback:)
+      event = Events::Create.new(@record, in_after_callback)
+
+      # Merge data from `Event` with data from PT-AT. We no longer use
+      # `data_for_create` but PT-AT still does.
+      data = event.data.merge!(data_for_create)
+
+      # Pure `version_class.new` reduces memory usage compared to `versions_assoc.build`
+      @record.class.paper_trail.version_class.new(data)
+    end
+
+    # @api private
+    def build_version_on_update(force:, in_after_callback:, is_touch:)
+      event = Events::Update.new(@record, in_after_callback, is_touch, nil)
+      return unless force || event.changed_notably?
+
+      # Merge data from `Event` with data from PT-AT. We no longer use
+      # `data_for_update` but PT-AT still does. To save memory, we use `merge!`
+      # instead of `merge`.
+      data = event.data.merge!(data_for_update)
+
+      # Using `version_class.new` reduces memory usage compared to
+      # `versions_assoc.build`. It's a trade-off though. We have to clear
+      # the association cache (see `versions.reset`) and that could cause an
+      # additional query in certain applications.
+      @record.class.paper_trail.version_class.new(data)
+    end
+
     def log_version_errors(version, action)
       version.logger&.warn(
         "Unable to create version for #{action} of #{@record.class.name}" \

spec/paper_trail/model_spec.rb

@@ -896,7 +896,12 @@ RSpec.describe(::PaperTrail, versioning: true) do
 
     describe "#build_version_on_create" do
       let(:version_building) do
-        -> { widget.paper_trail.build_version_on_create(in_after_callback: false) }
+        lambda do
+          widget.paper_trail.send(
+            :build_version_on_create,
+            in_after_callback: false
+          )
+        end
       end
 
       it "is frugal enough" do
@@ -923,7 +928,8 @@ RSpec.describe(::PaperTrail, versioning: true) do
       end
       let(:version_building) do
         lambda do
-          widget.paper_trail.build_version_on_update(
+          widget.paper_trail.send(
+            :build_version_on_update,
             force: false,
             in_after_callback: false,
             is_touch: false