diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index c47136a9f7768..900c81676cb5f 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Allow to purge an attachment when record is not persisted for `has_many_attached` + + *Jacopo Beschi* + * Add `with_all_variant_records` method to eager load all variant records on an attachment at once. `with_attached_image` scope now eager loads variant records if using variant tracking. diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb index b72b584652348..b540f85fbefe6 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -16,10 +16,6 @@ def initialize(name, record) def change record.attachment_changes[name] end - - def reset_changes - record.attachment_changes.delete(name) - end end end diff --git a/activestorage/lib/active_storage/attached/changes.rb b/activestorage/lib/active_storage/attached/changes.rb index 1db3906a630d9..a3b2cf4ab5fd5 100644 --- a/activestorage/lib/active_storage/attached/changes.rb +++ b/activestorage/lib/active_storage/attached/changes.rb @@ -11,6 +11,9 @@ module Attached::Changes #:nodoc: autoload :DeleteOne autoload :DeleteMany + + autoload :PurgeOne + autoload :PurgeMany end end end diff --git a/activestorage/lib/active_storage/attached/changes/purge_many.rb b/activestorage/lib/active_storage/attached/changes/purge_many.rb new file mode 100644 index 0000000000000..c6b305c489e3f --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/purge_many.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::PurgeMany #:nodoc: + attr_reader :name, :record, :attachments + + def initialize(name, record, attachments) + @name, @record, @attachments = name, record, attachments + end + + def purge + attachments.each(&:purge) + reset + end + + def purge_later + attachments.each(&:purge_later) + reset + end + + private + def reset + record.attachment_changes.delete(name) + record.public_send("#{name}_attachments").reset + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/purge_one.rb b/activestorage/lib/active_storage/attached/changes/purge_one.rb new file mode 100644 index 0000000000000..b53c7db54b5e3 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/purge_one.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::PurgeOne #:nodoc: + attr_reader :name, :record, :attachment + + def initialize(name, record, attachment) + @name, @record, @attachment = name, record, attachment + end + + def purge + attachment&.purge + reset + end + + def purge_later + attachment&.purge_later + reset + end + + private + def reset + record.attachment_changes.delete(name) + record.public_send("#{name}_attachment=", nil) + end + end +end diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 5afa3a3951430..de78751b9d547 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -3,6 +3,19 @@ module ActiveStorage # Decorated proxy object representing of multiple attachments to a model. class Attached::Many < Attached + ## + # :method: purge + # + # Directly purges each associated attachment (i.e. destroys the blobs and + # attachments and deletes the files on the service). + delegate :purge, to: :purge_many + + ## + # :method: purge_later + # + # Purges each associated attachment through the queuing system. + delegate :purge_later, to: :purge_many + delegate_missing_to :attachments # Returns all the associated attachment records. @@ -52,15 +65,9 @@ def detach attachments.delete_all if attached? end - ## - # :method: purge - # - # Directly purges each associated attachment (i.e. destroys the blobs and - # attachments and deletes the files on the service). - - ## - # :method: purge_later - # - # Purges each associated attachment through the queuing system. + private + def purge_many + Attached::Changes::PurgeMany.new(name, record, attachments) + end end end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index 2ab78b206ca3f..f34d0512325e0 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -3,6 +3,19 @@ module ActiveStorage # Representation of a single attachment to a model. class Attached::One < Attached + ## + # :method: purge + # + # Directly purges the attachment (i.e. destroys the blob and + # attachment and deletes the file on the service). + delegate :purge, to: :purge_one + + ## + # :method: purge_later + # + # Purges the attachment through the queuing system. + delegate :purge_later, to: :purge_one + delegate_missing_to :attachment, allow_nil: true # Returns the associated attachment record. @@ -62,28 +75,13 @@ def detach end end - # Directly purges the attachment (i.e. destroys the blob and - # attachment and deletes the file on the service). - def purge - if attached? - attachment.purge - write_attachment nil - reset_changes - end - end - - # Purges the attachment through the queuing system. - def purge_later - if attached? - attachment.purge_later - write_attachment nil - reset_changes - end - end - private def write_attachment(attachment) record.public_send("#{name}_attachment=", attachment) end + + def purge_one + Attached::Changes::PurgeOne.new(name, record, attachment) + end end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index d823f2b9876b3..c6aefa2eed10c 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -483,6 +483,33 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "purging when record is not persisted" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + user = User.new + user.highlights.attach blobs + assert user.highlights.attached? + + attachments = user.highlights.attachments + user.highlights.purge + + assert_not user.highlights.attached? + assert attachments.all?(&:destroyed?) + blobs.each do |blob| + assert_not ActiveStorage::Blob.exists?(blob.id) + assert_not ActiveStorage::Blob.service.exist?(blob.key) + end + end + end + + test "purging delete changes when record is not persisted" do + user = User.new + user.highlights = [] + + user.highlights.purge + + assert_nil user.attachment_changes["highlights"] + end + test "purging later" do [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| @user.highlights.attach blobs @@ -531,6 +558,24 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "purging attachment later when record is not persisted" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + user = User.new + user.highlights.attach blobs + assert user.highlights.attached? + + perform_enqueued_jobs do + user.highlights.purge_later + end + + assert_not user.highlights.attached? + blobs.each do |blob| + assert_not ActiveStorage::Blob.exists?(blob.id) + assert_not ActiveStorage::Blob.service.exist?(blob.key) + end + end + end + test "purging dependent attachment later on destroy" do [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| @user.highlights.attach blobs diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index cfa4d2a5cbea7..f39b44a9bf22b 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -497,6 +497,15 @@ def upload.open end end + test "purging delete changes when record is not persisted" do + user = User.new + user.avatar = nil + + user.avatar.purge + + assert_nil user.attachment_changes["avatar"] + end + test "purging later" do create_blob(filename: "funky.jpg").tap do |blob| @user.avatar.attach blob