Skip to content

Commit 82d4ad5

Browse files
committed
Fix ActiveStorage::Blob inverse association:
- This is a fix needed to unblock rails#50284, because Active Storage relies on a Active Record bug. The problem is best understood with a small snippet: ``` blob = ActiveStorage::Blob.new ActiveStorage::Attachment.new(blob: blob) ActiveStorage::Attachment.new(blob: blob) # Currently: p blob.attachments #=> #<ActiveRecord::Associations::CollectionProxy []> # Once the Active Record bug is fixed: p blob.attachments #=> #<ActiveRecord::Associations::CollectionProxy [#<ActiveStorage::Attachment id: nil, name: nil, record_type: nil, record_id: nil, blob_id: nil, created_at: nil>, #<ActiveStorage::Attachment id: nil, name: nil, record_type: nil, record_id: nil, blob_id: nil, created_at: nil>]> # Trying to save the blob would result in trying to create 2 attachments which # fails because of unique constrainsts. ``` ### Code path The real code path that does what the snippet above does is located here: https://github.com/rails/rails/blob/9c3ffab47c3bf59320ba08e9dafdb0275cf91a5a/activestorage/lib/active_storage/attached/many.rb#L52 It's basically doing this: ``` user.images.attach "photo1.png" # Initialize a Blob record and an Attachment user.images.attach "photo2.png" # Get the Blob from above, create another Attachment # Initialize a new Blob record and an new Attachment # rinse and repeat every time `attach` is called ``` Basically each time we call `attach`, we grab the previous blobs that were attached (and that already have an Attachment record), and ### Solution - Explicitly set the `inverse_of`, so that it behaves as if rails#50284 is shipped - Don't build a new attachment for blob already having one. ### Tests I didn't add tests, the test suite is already well covered, adding the `inverse_of` without any changes breaks the test suite already. You can try by running for instance the `activestorage/test/models/attached/many_test.rb`.
1 parent a9562e2 commit 82d4ad5

File tree

2 files changed

+6
-2
lines changed

2 files changed

+6
-2
lines changed

activestorage/app/models/active_storage/attachment.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class ActiveStorage::Attachment < ActiveStorage::Record
2828
# :method:
2929
#
3030
# Returns the associated ActiveStorage::Blob.
31-
belongs_to :blob, class_name: "ActiveStorage::Blob", autosave: true
31+
belongs_to :blob, class_name: "ActiveStorage::Blob", autosave: true, inverse_of: :attachments
3232

3333
delegate_missing_to :blob
3434
delegate :signed_id, to: :blob

activestorage/lib/active_storage/attached/changes/create_one_of_many.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ module ActiveStorage
44
class Attached::Changes::CreateOneOfMany < Attached::Changes::CreateOne # :nodoc:
55
private
66
def find_attachment
7-
record.public_send("#{name}_attachments").detect { |attachment| attachment.blob_id == blob.id }
7+
if blob.persisted?
8+
record.public_send("#{name}_attachments").detect { |attachment| attachment.blob_id == blob.id }
9+
else
10+
blob.attachments.find { |attachment| attachment.record == record }
11+
end
812
end
913
end
1014
end

0 commit comments

Comments
 (0)