Prevent Active Storage Blob from autosaving Attachments:

- This commit is meant to fix a behaviour change introduced in rails#50800,
  which causes issues reported in rails#53451 and rails#53452.

  ### Context

  There are a lot of codepaths and callbacks happening and the issue
  is subtle which makes things a bit tricky to explain, but in short,
  adding the `inverse_of` on the `ActiveStorage::Attachment <-> Blob`
  relation added behavioural difference on how the Attachement
  get saved.

  ### Problem

  By adding the `ActiveStorage::Attachment <-> Blob` inverse_of in
  rails#50800, a Blob is now aware of its associated Attachment and
  this creates a problem: Order of association save callbacks

  ```ruby
  class User < ApplicationRecord
    has_one_attached :avatar
  end

  blob = create_active_storage_blob("avatar.png")
  User.create!(avatar: blob)
  ```

  Before rails#50800, this was the order of how the records were persisted:

  ```
  1. Blob is created
  2. User is created
  3. Blob is updated through the User<->Blob autosave association (Active storage [modifies some metadata](https://github.com/rails/rails/blob/ad9bc2a51d5980d1c0990db97e552015410778f0/activestorage/lib/active_storage/attached/changes/create_one.rb#L12) on the Blob when creating an attachment)
  4. Attachment is created through the User<->Attachment autosave association
  ```

  After rails#50800, this is the order:

  ```
  1-3. Similar as above
  4.   Attachment is created through the ~User~Blob<->Attachment autosave association
  ```

  The reasons this creates the issues in rails#53451 and rails#53452 are
  different so I'll explain them separately.

  #### Problem in rails#53451

  Because an Attachment now gets created through the Blob association in
  a [after_update](https://github.com/rails/rails/blob/ad9bc2a51d5980d1c0990db97e552015410778f0/activerecord/lib/active_record/autosave_association.rb#L198) callback
  that **runs prior** to the [`Blob#touch_attachments`](https://github.com/rails/rails/blob/ad9bc2a51d5980d1c0990db97e552015410778f0/activestorage/app/models/active_storage/blob.rb#L44) after_update callback, the Blob will now `touch` associated attachments which by repercusion ends up loading
  the record (e.g the User in the example above) and touching it.
  By calling `user.touch_later`, this adds the user (that was loaded
  from the db, not the one in memory), to the transaction which
  ultimately gets used and replace the one in memory
  to run the commit callbacks ([here](https://github.com/rails/rails/blob/ad9bc2a51d5980d1c0990db97e552015410778f0/activerecord/lib/active_record/touch_later.rb#L23) and [here](https://github.com/rails/rails/blob/ad9bc2a51d5980d1c0990db97e552015410778f0/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L301-L303)) A simple example is the following:

  ```ruby
  class Car < ApplicationRecord
    attr_accessor :engine_sound

    after_save :touch_car
    after_commit :rev_engine

    def touch_car
      Car.find(1).touch
    end

    def rev_engine
      puts engine_sound # => nil
    end
  end

  Car.new(id: 1, engine_sound: "vroom")
  ```

  #### Problem in rails#53452

  Previously, the User could see the `Attachment#previously_new_record?`
  being true. This is again because the Attachment used to be created
  thanks to the autosave association on their relation.

  Now that the Attachment is saved through the Blob autosave association,
  when the User<->Attachment autosave association is called,
  the attachment is already persisted so it's considered an updat and
  we end up in a different codepath which as a result ends up
  resetting the `@previously_new_record` flag. ([here](https://github.com/rails/rails/blob/67c6ef2e5957152f00050224281ed4eabaaebd92/activerecord/lib/active_record/associations/collection_association.rb#L377) and [here]https://github.com/rails/rails/blob/67c6ef2e5957152f00050224281ed4eabaaebd92/activerecord/lib/active_record/persistence.rb#L263())

  ### Solution

  I couldn't find a better way but to modify the Blob<->Attachment
  relation and sets the autosave to false.
  This way, the Blob is not responsible to save the attachment,
  but the record (User) is.
  I also don't think it made sense anyway to have the Blob be
  responsible to save the Attachment.

  I tried to think of reasons that this would create issues where
  we instantiate a Blob and an Attachment, saves the Blob and leave
  the Attachment unsaved but I couldn't think of any. The reason
  is because an Attachment needs a record (e.g. User) to be
  valid and that same record will be saving the attachment.

  > [!IMPORTANT]
  > If for reasons an application changes the metadata of an **existing** Blob
  > and saves it, its existing Attachment
  > and dirty attributes would not be saved with this commit.
  >
  > Is this a common scenario? I don't know.

  ### Other solution

  We could revert rails#50800 and set the inverse_of explicitly to `nil`
  on the Blob<->Attachment.

Author: Edouard-chin

activestorage/CHANGELOG.md

@@ -1,2 +1,13 @@
+*   A Blob will no longer autosave associated Attachment.
+
+    This fixes an issue where a record with an attachment would have
+    its dirty attributes reset, preventing your `after commit` callbacks
+    on that record to behave as expected.
+
+    Note that this change doesn't require any changes on your application
+    and is supposed to be internal. Active Storage Attachment will continue
+    to be autosaved (through a different relation).
+
+    *Edouard-chin*
 
 Please check [8-0-stable](https://github.com/rails/rails/blob/8-0-stable/activestorage/CHANGELOG.md) for previous changes.

activestorage/app/models/active_storage/blob.rb

@@ -29,7 +29,7 @@ class ActiveStorage::Blob < ActiveStorage::Record
   # :method:
   #
   # Returns the associated ActiveStorage::Attachment instances.
-  has_many :attachments
+  has_many :attachments, autosave: false
 
   ##
   # :singleton-method:

activestorage/test/models/attachment_test.rb

@@ -38,6 +38,27 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase
     assert_predicate blob.reload, :analyzed?
   end
 
+  test "attaching a blob doesn't touch the record" do
+    data = "Something else entirely!"
+    io = StringIO.new(data)
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob.upload(io)
+
+    user = User.create!(
+      name: "Roger",
+      avatar: blob.signed_id,
+      record_callbacks: true,
+    )
+
+    assert_equal(1, user.callback_counter)
+  end
+
+  test "attaching a record doesn't reset the previously_new_record flag" do
+    @user.highlights.attach(io: ::StringIO.new("dummy"), filename: "dummy.txt")
+
+    assert(@user.notification_sent)
+  end
+
   test "mirroring a directly-uploaded blob after attaching it" do
     with_service("mirror") do
       blob = directly_upload_file_blob

activestorage/test/test_helper.rb

@@ -121,6 +121,9 @@ def with_raise_on_open_redirects(service)
 ActiveRecord::Base.include GlobalID::Identification
 
 class User < ActiveRecord::Base
+  attr_accessor :record_callbacks, :callback_counter
+  attr_reader :notification_sent
+
   validates :name, presence: true
 
   has_one_attached :avatar
@@ -161,11 +164,25 @@ class User < ActiveRecord::Base
     attachable.variant :preview, resize_to_fill: [400, 400], preprocessed: true
   end
 
+  after_commit :increment_callback_counter
+  after_update_commit :notify
+
   accepts_nested_attributes_for :highlights_attachments, allow_destroy: true
 
   def should_preprocessed?
     name == "transform via method"
   end
+
+  def increment_callback_counter
+    if record_callbacks
+      @callback_counter ||= 0
+      @callback_counter += 1
+    end
+  end
+
+  def notify
+    @notification_sent = true if highlights_attachments.any?(&:previously_new_record?)
+  end
 end
 
 class Group < ActiveRecord::Base