From e8682c5bf051517b0b265e446aa1a7eccfd47bf7 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sat, 7 Jul 2018 23:25:33 -0400 Subject: Store newly-uploaded files on save rather than assignment --- activestorage/CHANGELOG.md | 14 + .../app/models/active_storage/attachment.rb | 18 +- activestorage/app/models/active_storage/blob.rb | 21 +- activestorage/lib/active_storage/attached.rb | 7 +- .../lib/active_storage/attached/changes.rb | 16 + .../active_storage/attached/changes/create_many.rb | 32 ++ .../active_storage/attached/changes/create_one.rb | 65 +++ .../attached/changes/create_one_of_many.rb | 10 + .../active_storage/attached/changes/delete_many.rb | 19 + .../active_storage/attached/changes/delete_one.rb | 19 + .../lib/active_storage/attached/macros.rb | 122 ------ activestorage/lib/active_storage/attached/many.rb | 3 +- activestorage/lib/active_storage/attached/model.rb | 150 +++++++ activestorage/lib/active_storage/attached/one.rb | 19 +- activestorage/lib/active_storage/engine.rb | 2 +- activestorage/test/models/attached/many_test.rb | 335 +++++++++++++++ activestorage/test/models/attached/one_test.rb | 329 +++++++++++++++ activestorage/test/models/attached_test.rb | 56 --- activestorage/test/models/attachments_test.rb | 459 --------------------- activestorage/test/models/blob_test.rb | 22 +- .../test/models/presence_validation_test.rb | 4 +- activestorage/test/test_helper.rb | 6 + 22 files changed, 1049 insertions(+), 679 deletions(-) create mode 100644 activestorage/lib/active_storage/attached/changes.rb create mode 100644 activestorage/lib/active_storage/attached/changes/create_many.rb create mode 100644 activestorage/lib/active_storage/attached/changes/create_one.rb create mode 100644 activestorage/lib/active_storage/attached/changes/create_one_of_many.rb create mode 100644 activestorage/lib/active_storage/attached/changes/delete_many.rb create mode 100644 activestorage/lib/active_storage/attached/changes/delete_one.rb delete mode 100644 activestorage/lib/active_storage/attached/macros.rb create mode 100644 activestorage/lib/active_storage/attached/model.rb create mode 100644 activestorage/test/models/attached/many_test.rb create mode 100644 activestorage/test/models/attached/one_test.rb delete mode 100644 activestorage/test/models/attached_test.rb delete mode 100644 activestorage/test/models/attachments_test.rb diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 4aa551781b..0e9c2b5619 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,17 @@ +* Uploaded files assigned to a record are persisted to storage when the record + is saved instead of immediately. + + In Rails 5.2, the following causes an uploaded file in `params[:avatar]` to + be stored: + + ```ruby + @user.avatar = params[:avatar] + ``` + + In Rails 6, the uploaded file is stored when `@user` is successfully saved. + + *George Claghorn* + * Add the ability to reflect on defined attachments using the existing ActiveRecord reflection mechanism. diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index c59877a9a5..bb80799044 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -15,17 +15,18 @@ class ActiveStorage::Attachment < ActiveRecord::Base delegate_missing_to :blob after_create_commit :analyze_blob_later, :identify_blob + after_destroy_commit :purge_dependent_blob_later - # Synchronously purges the blob (deletes it from the configured service) and destroys the attachment. + # Synchronously purges the blob (deletes it from the configured service) and deletes the attachment. def purge blob.purge - destroy + delete end - # Destroys the attachment and asynchronously purges the blob (deletes it from the configured service). + # Deletes the attachment and queues a background job to purge the blob (delete it from the configured service). def purge_later blob.purge_later - destroy + delete end private @@ -36,4 +37,13 @@ class ActiveStorage::Attachment < ActiveRecord::Base def analyze_blob_later blob.analyze_later unless blob.analyzed? end + + def purge_dependent_blob_later + blob.purge_later if dependent == :purge_later + end + + + def dependent + record.attachment_reflections[name]&.options[:dependent] + end end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 73029f21a1..86f3dba524 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -48,15 +48,17 @@ class ActiveStorage::Blob < ActiveRecord::Base # Returns a new, unsaved blob instance after the +io+ has been uploaded to the service. # When providing a content type, pass identify: false to bypass automatic content type inference. def build_after_upload(io:, filename:, content_type: nil, metadata: nil, identify: true) - new.tap do |blob| - blob.filename = filename - blob.content_type = content_type - blob.metadata = metadata - + new(filename: filename, content_type: content_type, metadata: metadata).tap do |blob| blob.upload(io, identify: identify) end end + def build_after_unfurling(io:, filename:, content_type: nil, metadata: nil, identify: true) #:nodoc: + new(filename: filename, content_type: content_type, metadata: metadata).tap do |blob| + blob.unfurl(io, identify: identify) + end + end + # Returns a saved blob instance after the +io+ has been uploaded to the service. Note, the blob is first built, # then the +io+ is uploaded, then the blob is saved. This is done this way to avoid uploading (which may take # time), while having an open database transaction. @@ -152,12 +154,19 @@ class ActiveStorage::Blob < ActiveRecord::Base # Normally, you do not have to call this method directly at all. Use the factory class methods of +build_after_upload+ # and +create_after_upload!+. def upload(io, identify: true) + unfurl io, identify: identify + upload_without_unfurling io + end + + def unfurl(io, identify: true) #:nodoc: self.checksum = compute_checksum_in_chunks(io) self.content_type = extract_content_type(io) if content_type.nil? || identify self.byte_size = io.size self.identified = true + end - service.upload(key, io, checksum: checksum) + def upload_without_unfurling(io) #:nodoc: + service.upload key, io, checksum: checksum end # Downloads the file associated with this blob. If no block is given, the entire file is read into memory and returned. diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb index c08fd56652..1b53818581 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -15,6 +15,10 @@ module ActiveStorage end private + def change + record.attachment_changes[name] + end + def create_blob_from(attachable) case attachable when ActiveStorage::Blob @@ -35,6 +39,7 @@ module ActiveStorage end end +require "active_storage/attached/model" require "active_storage/attached/one" require "active_storage/attached/many" -require "active_storage/attached/macros" +require "active_storage/attached/changes" diff --git a/activestorage/lib/active_storage/attached/changes.rb b/activestorage/lib/active_storage/attached/changes.rb new file mode 100644 index 0000000000..1db3906a63 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module ActiveStorage + module Attached::Changes #:nodoc: + extend ActiveSupport::Autoload + + eager_autoload do + autoload :CreateOne + autoload :CreateMany + autoload :CreateOneOfMany + + autoload :DeleteOne + autoload :DeleteMany + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/create_many.rb b/activestorage/lib/active_storage/attached/changes/create_many.rb new file mode 100644 index 0000000000..3f7ca6a25f --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/create_many.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::CreateMany #:nodoc: + attr_reader :name, :record, :attachables + + def initialize(name, record, attachables) + @name, @record, @attachables = name, record, Array(attachables) + end + + def attachments + @attachments ||= subchanges.collect(&:attachment) + end + + def upload + subchanges.each(&:upload) + end + + def save + record.public_send("#{name}_attachments=", attachments) + end + + private + def subchanges + @subchanges ||= attachables.collect { |attachable| build_subchange_from(attachable) } + end + + def build_subchange_from(attachable) + ActiveStorage::Attached::Changes::CreateOneOfMany.new(name, record, attachable) + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/create_one.rb b/activestorage/lib/active_storage/attached/changes/create_one.rb new file mode 100644 index 0000000000..bb59a651ba --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/create_one.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::CreateOne #:nodoc: + attr_reader :name, :record, :attachable + + def initialize(name, record, attachable) + @name, @record, @attachable = name, record, attachable + end + + def attachment + @attachment ||= find_or_build_attachment + end + + def blob + @blob ||= find_or_build_blob + end + + def upload + case attachable + when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile + blob.upload_without_unfurling(attachable.open) + when Hash + blob.upload_without_unfurling(attachable.fetch(:io)) + end + end + + def save + record.public_send("#{name}_attachment=", attachment) + end + + private + def find_or_build_attachment + find_attachment || build_attachment + end + + def find_attachment + if record.public_send("#{name}_blob") == blob + record.public_send("#{name}_attachment") + end + end + + def build_attachment + ActiveStorage::Attachment.new(record: record, name: name, blob: blob) + end + + def find_or_build_blob + case attachable + when ActiveStorage::Blob + attachable + when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile + ActiveStorage::Blob.build_after_unfurling \ + io: attachable.open, + filename: attachable.original_filename, + content_type: attachable.content_type + when Hash + ActiveStorage::Blob.build_after_unfurling(attachable) + when String + ActiveStorage::Blob.find_signed(attachable) + else + raise "Could not find or build blob: expected attachable, got #{attachable.inspect}" + end + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/create_one_of_many.rb b/activestorage/lib/active_storage/attached/changes/create_one_of_many.rb new file mode 100644 index 0000000000..7268e87316 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/create_one_of_many.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::CreateOneOfMany < Attached::Changes::CreateOne #:nodoc: + private + def find_attachment + record.public_send("#{name}_attachments").detect { |attachment| attachment.blob_id == blob.id } + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/delete_many.rb b/activestorage/lib/active_storage/attached/changes/delete_many.rb new file mode 100644 index 0000000000..5c7fe385de --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/delete_many.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::DeleteMany #:nodoc: + attr_reader :name, :record + + def initialize(name, record) + @name, @record = name, record + end + + def attachments + ActiveStorage::Attachment.none + end + + def save + record.public_send("#{name}_attachments=", []) + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/delete_one.rb b/activestorage/lib/active_storage/attached/changes/delete_one.rb new file mode 100644 index 0000000000..2f7d356613 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/delete_one.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::DeleteOne #:nodoc: + attr_reader :name, :record + + def initialize(name, record) + @name, @record = name, record + end + + def attachment + nil + end + + def save + record.public_send("#{name}_attachment=", nil) + end + end +end diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb deleted file mode 100644 index 6ad9fc43d7..0000000000 --- a/activestorage/lib/active_storage/attached/macros.rb +++ /dev/null @@ -1,122 +0,0 @@ -# frozen_string_literal: true - -module ActiveStorage - # Provides the class-level DSL for declaring that an Active Record model has attached blobs. - module Attached::Macros - # Specifies the relation between a single attachment and the model. - # - # class User < ActiveRecord::Base - # has_one_attached :avatar - # end - # - # There is no column defined on the model side, Active Storage takes - # care of the mapping between your records and the attachment. - # - # To avoid N+1 queries, you can include the attached blobs in your query like so: - # - # User.with_attached_avatar - # - # Under the covers, this relationship is implemented as a +has_one+ association to a - # ActiveStorage::Attachment record and a +has_one-through+ association to a - # ActiveStorage::Blob record. These associations are available as +avatar_attachment+ - # and +avatar_blob+. But you shouldn't need to work with these associations directly in - # most circumstances. - # - # The system has been designed to having you go through the ActiveStorage::Attached::One - # proxy that provides the dynamic proxy to the associations and factory methods, like +attach+. - # - # If the +:dependent+ option isn't set, the attachment will be purged - # (i.e. destroyed) whenever the record is destroyed. - def has_one_attached(name, dependent: :purge_later) - generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name} - @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) - end - - def #{name}=(attachable) - #{name}.attach(attachable) - end - CODE - - has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record, inverse_of: :record, dependent: false - has_one :"#{name}_blob", through: :"#{name}_attachment", class_name: "ActiveStorage::Blob", source: :blob - - scope :"with_attached_#{name}", -> { includes("#{name}_attachment": :blob) } - - if dependent == :purge_later - after_destroy_commit { public_send(name).purge_later } - else - before_destroy { public_send(name).detach } - end - - ActiveRecord::Reflection.add_attachment_reflection( - self, - name, - ActiveRecord::Reflection.create(:has_one_attached, name, nil, { dependent: dependent }, self) - ) - end - - # Specifies the relation between multiple attachments and the model. - # - # class Gallery < ActiveRecord::Base - # has_many_attached :photos - # end - # - # There are no columns defined on the model side, Active Storage takes - # care of the mapping between your records and the attachments. - # - # To avoid N+1 queries, you can include the attached blobs in your query like so: - # - # Gallery.where(user: Current.user).with_attached_photos - # - # Under the covers, this relationship is implemented as a +has_many+ association to a - # ActiveStorage::Attachment record and a +has_many-through+ association to a - # ActiveStorage::Blob record. These associations are available as +photos_attachments+ - # and +photos_blobs+. But you shouldn't need to work with these associations directly in - # most circumstances. - # - # The system has been designed to having you go through the ActiveStorage::Attached::Many - # proxy that provides the dynamic proxy to the associations and factory methods, like +#attach+. - # - # If the +:dependent+ option isn't set, all the attachments will be purged - # (i.e. destroyed) whenever the record is destroyed. - def has_many_attached(name, dependent: :purge_later) - generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name} - @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) - end - - def #{name}=(attachables) - #{name}.attach(attachables) - end - CODE - - has_many :"#{name}_attachments", -> { where(name: name) }, as: :record, class_name: "ActiveStorage::Attachment", inverse_of: :record, dependent: false do - def purge - each(&:purge) - reset - end - - def purge_later - each(&:purge_later) - reset - end - end - has_many :"#{name}_blobs", through: :"#{name}_attachments", class_name: "ActiveStorage::Blob", source: :blob - - scope :"with_attached_#{name}", -> { includes("#{name}_attachments": :blob) } - - if dependent == :purge_later - after_destroy_commit { public_send(name).purge_later } - else - before_destroy { public_send(name).detach } - end - - ActiveRecord::Reflection.add_attachment_reflection( - self, - name, - ActiveRecord::Reflection.create(:has_many_attached, name, nil, { dependent: dependent }, self) - ) - end - end -end diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index d61acb6fad..204d6604c8 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -9,7 +9,7 @@ module ActiveStorage # # All methods called on this proxy object that aren't listed here will automatically be delegated to +attachments+. def attachments - record.public_send("#{name}_attachments") + change.present? ? change.attachments : record.public_send("#{name}_attachments") end # Associates one or several attachments with the current record, saving them to the database. @@ -50,7 +50,6 @@ module ActiveStorage # Directly purges each associated attachment (i.e. destroys the blobs and # attachments and deletes the files on the service). - ## # :method: purge_later # diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb new file mode 100644 index 0000000000..287c7a9253 --- /dev/null +++ b/activestorage/lib/active_storage/attached/model.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +module ActiveStorage + # Provides the class-level DSL for declaring an Active Record model's attachments. + module Attached::Model + extend ActiveSupport::Concern + + class_methods do + # Specifies the relation between a single attachment and the model. + # + # class User < ActiveRecord::Base + # has_one_attached :avatar + # end + # + # There is no column defined on the model side, Active Storage takes + # care of the mapping between your records and the attachment. + # + # To avoid N+1 queries, you can include the attached blobs in your query like so: + # + # User.with_attached_avatar + # + # Under the covers, this relationship is implemented as a +has_one+ association to a + # ActiveStorage::Attachment record and a +has_one-through+ association to a + # ActiveStorage::Blob record. These associations are available as +avatar_attachment+ + # and +avatar_blob+. But you shouldn't need to work with these associations directly in + # most circumstances. + # + # The system has been designed to having you go through the ActiveStorage::Attached::One + # proxy that provides the dynamic proxy to the associations and factory methods, like +attach+. + # + # If the +:dependent+ option isn't set, the attachment will be purged + # (i.e. destroyed) whenever the record is destroyed. + def has_one_attached(name, dependent: :purge_later) + generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name} + @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) + end + + def #{name}=(attachable) + attachment_changes["#{name}"] = + if attachable.nil? + ActiveStorage::Attached::Changes::DeleteOne.new("#{name}", self) + else + ActiveStorage::Attached::Changes::CreateOne.new("#{name}", self, attachable) + end + end + CODE + + has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record, inverse_of: :record, dependent: :destroy + has_one :"#{name}_blob", through: :"#{name}_attachment", class_name: "ActiveStorage::Blob", source: :blob + + scope :"with_attached_#{name}", -> { includes("#{name}_attachment": :blob) } + + after_save { attachment_changes[name.to_s]&.save } + + ActiveRecord::Reflection.add_attachment_reflection( + self, + name, + ActiveRecord::Reflection.create(:has_one_attached, name, nil, { dependent: dependent }, self) + ) + end + + # Specifies the relation between multiple attachments and the model. + # + # class Gallery < ActiveRecord::Base + # has_many_attached :photos + # end + # + # There are no columns defined on the model side, Active Storage takes + # care of the mapping between your records and the attachments. + # + # To avoid N+1 queries, you can include the attached blobs in your query like so: + # + # Gallery.where(user: Current.user).with_attached_photos + # + # Under the covers, this relationship is implemented as a +has_many+ association to a + # ActiveStorage::Attachment record and a +has_many-through+ association to a + # ActiveStorage::Blob record. These associations are available as +photos_attachments+ + # and +photos_blobs+. But you shouldn't need to work with these associations directly in + # most circumstances. + # + # The system has been designed to having you go through the ActiveStorage::Attached::Many + # proxy that provides the dynamic proxy to the associations and factory methods, like +#attach+. + # + # If the +:dependent+ option isn't set, all the attachments will be purged + # (i.e. destroyed) whenever the record is destroyed. + def has_many_attached(name, dependent: :purge_later) + generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name} + @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) + end + + def #{name}=(attachables) + attachment_changes["#{name}"] = + if attachables.nil? || Array(attachables).none? + ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self) + else + ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables) + end + end + CODE + + has_many :"#{name}_attachments", -> { where(name: name) }, as: :record, class_name: "ActiveStorage::Attachment", inverse_of: :record, dependent: :destroy do + def purge + each(&:purge) + reset + end + + def purge_later + each(&:purge_later) + reset + end + end + has_many :"#{name}_blobs", through: :"#{name}_attachments", class_name: "ActiveStorage::Blob", source: :blob + + scope :"with_attached_#{name}", -> { includes("#{name}_attachments": :blob) } + + after_save { attachment_changes[name.to_s]&.save } + + ActiveRecord::Reflection.add_attachment_reflection( + self, + name, + ActiveRecord::Reflection.create(:has_many_attached, name, nil, { dependent: dependent }, self) + ) + end + end + + def committed!(*) #:nodoc: + unless destroyed? + upload_attachment_changes + clear_attachment_changes + end + + super + end + + def attachment_changes #:nodoc: + @attachment_changes ||= {} + end + + private + def upload_attachment_changes + attachment_changes.each_value { |change| change.try(:upload) } + end + + def clear_attachment_changes + @attachment_changes = {} + end + end +end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index f992cb5f84..960ff99e63 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -10,11 +10,11 @@ module ActiveStorage # You don't have to call this method to access the attachment's methods as # they are all available at the model level. def attachment - record.public_send("#{name}_attachment") + change.present? ? change.attachment : record.public_send("#{name}_attachment") end def blank? - attachment.blank? + !attached? end # Associates a given attachment with the current record, saving it to the database. @@ -24,16 +24,10 @@ module ActiveStorage # person.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") # person.avatar.attach(avatar_blob) # ActiveStorage::Blob object def attach(attachable) - blob_was = blob if attached? - blob = create_blob_from(attachable) + new_blob = create_blob_from(attachable) - unless blob == blob_was - transaction do - detach - write_attachment build_attachment(blob: blob) - end - - blob_was.purge_later if blob_was && dependent == :purge_later + if !attached? || new_blob != blob + write_attachment build_attachment(blob: new_blob) end end @@ -69,6 +63,7 @@ module ActiveStorage def purge_later if attached? attachment.purge_later + write_attachment nil end end @@ -76,7 +71,7 @@ module ActiveStorage delegate :transaction, to: :record def build_attachment(blob:) - ActiveStorage::Attachment.new(record: record, name: name, blob: blob) + Attachment.new(record: record, name: name, blob: blob) end def write_attachment(attachment) diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 93c5c55b95..9d6a27eabe 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -62,7 +62,7 @@ module ActiveStorage require "active_storage/attached" ActiveSupport.on_load(:active_record) do - extend ActiveStorage::Attached::Macros + include ActiveStorage::Attached::Model end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb new file mode 100644 index 0000000000..ff2d29de11 --- /dev/null +++ b/activestorage/test/models/attached/many_test.rb @@ -0,0 +1,335 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + setup do + @user = User.create!(name: "Josh") + end + + teardown { ActiveStorage::Blob.all.each(&:purge) } + + test "attaching existing blobs to an existing record" do + @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + end + + test "attaching existing blobs from signed IDs to an existing record" do + @user.highlights.attach create_blob(filename: "funky.jpg").signed_id, create_blob(filename: "town.jpg").signed_id + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + end + + test "attaching new blobs from Hashes to an existing record" do + @user.highlights.attach( + { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" }, + { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg" }) + + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + end + + test "attaching new blobs from uploaded files to an existing record" do + @user.highlights.attach fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") + assert_equal "racecar.jpg", @user.highlights.first.filename.to_s + assert_equal "video.mp4", @user.highlights.second.filename.to_s + end + + test "updating an existing record to attach existing blobs" do + @user.update! highlights: [ create_file_blob(filename: "racecar.jpg"), create_file_blob(filename: "video.mp4") ] + assert_equal "racecar.jpg", @user.highlights.first.filename.to_s + assert_equal "video.mp4", @user.highlights.second.filename.to_s + end + + test "updating an existing record to attach existing blobs from signed IDs" do + @user.update! highlights: [ create_blob(filename: "funky.jpg").signed_id, create_blob(filename: "town.jpg").signed_id ] + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + end + + test "successfully updating an existing record to attach new blobs from uploaded files" do + @user.highlights = [ fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") ] + assert_equal "racecar.jpg", @user.highlights.first.filename.to_s + assert_equal "video.mp4", @user.highlights.second.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.first.key) + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.second.key) + + @user.save! + assert ActiveStorage::Blob.service.exist?(@user.highlights.first.key) + assert ActiveStorage::Blob.service.exist?(@user.highlights.second.key) + end + + test "unsuccessfully updating an existing record to attach new blobs from uploaded files" do + assert_not @user.update(name: "", highlights: [ fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") ]) + assert_equal "racecar.jpg", @user.highlights.first.filename.to_s + assert_equal "video.mp4", @user.highlights.second.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.first.key) + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.second.key) + end + + test "successfully updating an existing record to replace existing, dependent attachments" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |old_blobs| + @user.highlights.attach old_blobs + + perform_enqueued_jobs do + @user.update! highlights: [ create_blob(filename: "whenever.jpg"), create_blob(filename: "wherever.jpg") ] + end + + assert_equal "whenever.jpg", @user.highlights.first.filename.to_s + assert_equal "wherever.jpg", @user.highlights.second.filename.to_s + assert_not ActiveStorage::Blob.exists?(old_blobs.first.id) + assert_not ActiveStorage::Blob.exists?(old_blobs.second.id) + assert_not ActiveStorage::Blob.service.exist?(old_blobs.first.key) + assert_not ActiveStorage::Blob.service.exist?(old_blobs.second.key) + end + end + + test "successfully updating an existing record to replace existing, independent attachments" do + @user.vlogs.attach create_blob(filename: "funky.mp4"), create_blob(filename: "town.mp4") + + assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do + @user.update! vlogs: [ create_blob(filename: "whenever.mp4"), create_blob(filename: "wherever.mp4") ] + end + + assert_equal "whenever.mp4", @user.vlogs.first.filename.to_s + assert_equal "wherever.mp4", @user.vlogs.second.filename.to_s + end + + test "unsuccessfully updating an existing record to replace existing attachments" do + @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + + assert_no_enqueued_jobs do + assert_not @user.update(name: "", highlights: [ fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") ]) + end + + assert_equal "racecar.jpg", @user.highlights.first.filename.to_s + assert_equal "video.mp4", @user.highlights.second.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.first.key) + assert_not ActiveStorage::Blob.service.exist?(@user.highlights.second.key) + end + + test "updating an existing record to attach one new blob and one previously-attached blob" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + @user.highlights.attach blobs.first + + perform_enqueued_jobs do + assert_no_changes -> { @user.highlights_attachments.first.id } do + @user.update! highlights: blobs + end + end + + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + assert ActiveStorage::Blob.service.exist?(@user.highlights.first.key) + end + end + + test "successfully updating an existing record to remove dependent attachments" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + @user.highlights.attach blobs + + perform_enqueued_jobs do + @user.update! highlights: [] + end + + assert_not @user.highlights.attached? + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert_not ActiveStorage::Blob.service.exist?(blobs.second.key) + end + end + + test "successfully updating an existing record to remove independent attachments" do + [ create_blob(filename: "funky.mp4"), create_blob(filename: "town.mp4") ].tap do |blobs| + @user.vlogs.attach blobs + + assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do + @user.update! vlogs: [] + end + + assert_not @user.vlogs.attached? + assert ActiveStorage::Blob.service.exist?(blobs.first.key) + assert ActiveStorage::Blob.service.exist?(blobs.second.key) + end + end + + test "attaching existing blobs to a new record" do + User.new(name: "Jason").tap do |user| + user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + assert user.new_record? + assert_equal "funky.jpg", user.highlights.first.filename.to_s + assert_equal "town.jpg", user.highlights.second.filename.to_s + + user.save! + assert_equal "funky.jpg", user.highlights.first.filename.to_s + assert_equal "town.jpg", user.highlights.second.filename.to_s + end + end + + test "attaching an existing blob from a signed ID to a new record" do + User.new(name: "Jason").tap do |user| + user.avatar.attach create_blob(filename: "funky.jpg").signed_id + assert user.new_record? + assert_equal "funky.jpg", user.avatar.filename.to_s + + user.save! + assert_equal "funky.jpg", user.reload.avatar.filename.to_s + end + end + + test "attaching new blob from Hashes to a new record" do + User.new(name: "Jason").tap do |user| + user.highlights.attach( + { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" }, + { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpg" }) + + assert user.new_record? + assert user.highlights.first.new_record? + assert user.highlights.second.new_record? + assert_not user.highlights.first.blob.new_record? + assert_not user.highlights.second.blob.new_record? + assert_equal "funky.jpg", user.highlights.first.filename.to_s + assert_equal "town.jpg", user.highlights.second.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.highlights.first.key) + assert ActiveStorage::Blob.service.exist?(user.highlights.second.key) + + user.save! + assert_equal "funky.jpg", user.reload.highlights.first.filename.to_s + assert_equal "town.jpg", user.highlights.second.filename.to_s + end + end + + test "attaching new blobs from uploaded files to a new record" do + User.new(name: "Jason").tap do |user| + user.highlights.attach fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") + assert user.new_record? + assert user.highlights.first.new_record? + assert user.highlights.second.new_record? + assert_not user.highlights.first.blob.new_record? + assert_not user.highlights.second.blob.new_record? + assert_equal "racecar.jpg", user.highlights.first.filename.to_s + assert_equal "video.mp4", user.highlights.second.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.highlights.first.key) + assert ActiveStorage::Blob.service.exist?(user.highlights.second.key) + + user.save! + assert_equal "racecar.jpg", user.reload.highlights.first.filename.to_s + assert_equal "video.mp4", user.highlights.second.filename.to_s + end + end + + test "creating a record with existing blobs attached" do + user = User.create!(name: "Jason", highlights: [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ]) + assert_equal "funky.jpg", user.reload.highlights.first.filename.to_s + assert_equal "town.jpg", user.reload.highlights.second.filename.to_s + end + + test "creating a record with an existing blob from signed IDs attached" do + user = User.create!(name: "Jason", highlights: [ + create_blob(filename: "funky.jpg").signed_id, create_blob(filename: "town.jpg").signed_id ]) + assert_equal "funky.jpg", user.reload.highlights.first.filename.to_s + assert_equal "town.jpg", user.reload.highlights.second.filename.to_s + end + + test "creating a record with new blobs from uploaded files attached" do + User.new(name: "Jason", highlights: [ fixture_file_upload("racecar.jpg"), fixture_file_upload("video.mp4") ]).tap do |user| + assert user.new_record? + assert user.highlights.first.new_record? + assert user.highlights.second.new_record? + assert user.highlights.first.blob.new_record? + assert user.highlights.second.blob.new_record? + assert_equal "racecar.jpg", user.highlights.first.filename.to_s + assert_equal "video.mp4", user.highlights.second.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(user.highlights.first.key) + assert_not ActiveStorage::Blob.service.exist?(user.highlights.second.key) + + user.save! + assert_equal "racecar.jpg", user.highlights.first.filename.to_s + assert_equal "video.mp4", user.highlights.second.filename.to_s + end + end + + test "creating a record with an unexpected object attached" do + error = assert_raises { User.create!(name: "Jason", highlights: :foo) } + assert_equal "Could not find or build blob: expected attachable, got :foo", error.message + end + + test "purging" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + @user.highlights.attach blobs + assert @user.highlights.attached? + + @user.highlights.purge + assert_not @user.highlights.attached? + assert_not ActiveStorage::Blob.exists?(blobs.first.id) + assert_not ActiveStorage::Blob.exists?(blobs.second.id) + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert_not ActiveStorage::Blob.service.exist?(blobs.second.key) + end + end + + test "purging later" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + @user.highlights.attach blobs + assert @user.highlights.attached? + + perform_enqueued_jobs do + @user.highlights.purge_later + end + + assert_not @user.highlights.attached? + assert_not ActiveStorage::Blob.exists?(blobs.first.id) + assert_not ActiveStorage::Blob.exists?(blobs.second.id) + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert_not ActiveStorage::Blob.service.exist?(blobs.second.key) + 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 + + perform_enqueued_jobs do + @user.destroy! + end + + assert_not ActiveStorage::Blob.exists?(blobs.first.id) + assert_not ActiveStorage::Blob.exists?(blobs.second.id) + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert_not ActiveStorage::Blob.service.exist?(blobs.second.key) + end + end + + test "not purging independent attachment on destroy" do + [ create_blob(filename: "funky.mp4"), create_blob(filename: "town.mp4") ].tap do |blobs| + @user.vlogs.attach blobs + + assert_no_enqueued_jobs do + @user.destroy! + end + end + end + + test "overriding attached reader" do + @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + + assert_equal "funky.jpg", @user.highlights.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + + begin + User.class_eval do + def highlights + super.reverse + end + end + + assert_equal "town.jpg", @user.highlights.first.filename.to_s + assert_equal "funky.jpg", @user.highlights.second.filename.to_s + ensure + User.send(:remove_method, :highlights) + end + end +end diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb new file mode 100644 index 0000000000..af45b696ae --- /dev/null +++ b/activestorage/test/models/attached/one_test.rb @@ -0,0 +1,329 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + setup do + @user = User.create!(name: "Josh") + end + + teardown { ActiveStorage::Blob.all.each(&:purge) } + + test "attaching an existing blob to an existing record" do + @user.avatar.attach create_blob(filename: "funky.jpg") + assert_equal "funky.jpg", @user.avatar.filename.to_s + end + + test "attaching an existing blob from a signed ID to an existing record" do + @user.avatar.attach create_blob(filename: "funky.jpg").signed_id + assert_equal "funky.jpg", @user.avatar.filename.to_s + end + + test "attaching a new blob from a Hash to an existing record" do + @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + assert_equal "town.jpg", @user.avatar.filename.to_s + end + + test "attaching a new blob from an uploaded file to an existing record" do + @user.avatar.attach fixture_file_upload("racecar.jpg") + assert_equal "racecar.jpg", @user.avatar.filename.to_s + end + + test "updating an existing record to attach an existing blob" do + @user.update! avatar: create_blob(filename: "funky.jpg") + assert_equal "funky.jpg", @user.avatar.filename.to_s + end + + test "updating an existing record to attach an existing blob from a signed ID" do + @user.update! avatar: create_blob(filename: "funky.jpg").signed_id + assert_equal "funky.jpg", @user.avatar.filename.to_s + end + + test "successfully updating an existing record to attach a new blob from an uploaded file" do + @user.avatar = fixture_file_upload("racecar.jpg") + assert_equal "racecar.jpg", @user.avatar.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.avatar.key) + + @user.save! + assert ActiveStorage::Blob.service.exist?(@user.avatar.key) + end + + test "unsuccessfully updating an existing record to attach a new blob from an uploaded file" do + assert_not @user.update(name: "", avatar: fixture_file_upload("racecar.jpg")) + assert_equal "racecar.jpg", @user.avatar.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.avatar.key) + end + + test "successfully replacing an existing, dependent attachment on an existing record" do + create_blob(filename: "funky.jpg").tap do |old_blob| + @user.avatar.attach old_blob + + perform_enqueued_jobs do + @user.avatar.attach create_blob(filename: "town.jpg") + end + + assert_equal "town.jpg", @user.avatar.filename.to_s + assert_not ActiveStorage::Blob.exists?(old_blob.id) + assert_not ActiveStorage::Blob.service.exist?(old_blob.key) + end + end + + test "successfully replacing an existing, independent attachment on an existing record" do + @user.cover_photo.attach create_blob(filename: "funky.jpg") + + assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do + @user.cover_photo.attach create_blob(filename: "town.jpg") + end + + assert_equal "town.jpg", @user.cover_photo.filename.to_s + end + + test "unsuccessfully replacing an existing attachment on an existing record" do + @user.avatar.attach create_blob(filename: "funky.jpg") + + assert_no_enqueued_jobs do + assert_raises do + @user.avatar.attach nil + end + end + + assert_equal "funky.jpg", @user.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(@user.avatar.key) + end + + test "replacing an existing attachment on an existing record with the same blob" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + + assert_no_changes -> { @user.reload.avatar_attachment.id } do + assert_no_enqueued_jobs do + @user.avatar.attach blob + end + end + + assert_equal "funky.jpg", @user.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(@user.avatar.key) + end + end + + test "successfully updating an existing record to replace an existing, dependent attachment" do + create_blob(filename: "funky.jpg").tap do |old_blob| + @user.avatar.attach old_blob + + perform_enqueued_jobs do + @user.update! avatar: create_blob(filename: "town.jpg") + end + + assert_equal "town.jpg", @user.avatar.filename.to_s + assert_not ActiveStorage::Blob.exists?(old_blob.id) + assert_not ActiveStorage::Blob.service.exist?(old_blob.key) + end + end + + test "successfully updating an existing record to replace an existing, independent attachment" do + @user.cover_photo.attach create_blob(filename: "funky.jpg") + + assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do + @user.update! cover_photo: create_blob(filename: "town.jpg") + end + + assert_equal "town.jpg", @user.cover_photo.filename.to_s + end + + test "unsuccessfully updating an existing record to replace an existing attachment" do + @user.avatar.attach create_blob(filename: "funky.jpg") + + assert_no_enqueued_jobs do + assert_not @user.update(name: "", avatar: fixture_file_upload("racecar.jpg")) + end + + assert_equal "racecar.jpg", @user.avatar.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(@user.avatar.key) + end + + test "updating an existing record to replace an attached blob with itself" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + + assert_no_enqueued_jobs do + assert_no_changes -> { @user.avatar_attachment.id } do + @user.update! avatar: blob + end + end + end + end + + test "successfully updating an existing record to remove a dependent attachment" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + + perform_enqueued_jobs do + @user.update! avatar: nil + end + + assert_not @user.avatar.attached? + end + end + + test "successfully updating an existing record to remove an independent attachment" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.cover_photo.attach blob + + assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do + @user.update! cover_photo: nil + end + + assert_not @user.cover_photo.attached? + assert ActiveStorage::Blob.service.exist?(blob.key) + end + end + + test "attaching an existing blob to a new record" do + User.new(name: "Jason").tap do |user| + user.avatar.attach create_blob(filename: "funky.jpg") + assert user.new_record? + assert_equal "funky.jpg", user.avatar.filename.to_s + + user.save! + assert_equal "funky.jpg", user.reload.avatar.filename.to_s + end + end + + test "attaching an existing blob from a signed ID to a new record" do + User.new(name: "Jason").tap do |user| + user.avatar.attach create_blob(filename: "funky.jpg").signed_id + assert user.new_record? + assert_equal "funky.jpg", user.avatar.filename.to_s + + user.save! + assert_equal "funky.jpg", user.reload.avatar.filename.to_s + end + end + + test "attaching a new blob from a Hash to a new record" do + User.new(name: "Jason").tap do |user| + user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + assert user.new_record? + assert user.avatar.attachment.new_record? + assert_not user.avatar.blob.new_record? + assert_equal "town.jpg", user.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.avatar.key) + + user.save! + assert_equal "town.jpg", user.reload.avatar.filename.to_s + end + end + + test "attaching a new blob from an uploaded file to a new record" do + User.new(name: "Jason").tap do |user| + user.avatar.attach fixture_file_upload("racecar.jpg") + assert user.new_record? + assert user.avatar.attachment.new_record? + assert_not user.avatar.blob.new_record? + assert_equal "racecar.jpg", user.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.avatar.key) + + user.save! + assert_equal "racecar.jpg", user.reload.avatar.filename.to_s + end + end + + test "creating a record with an existing blob attached" do + user = User.create!(name: "Jason", avatar: create_blob(filename: "funky.jpg")) + assert_equal "funky.jpg", user.reload.avatar.filename.to_s + end + + test "creating a record with an existing blob from a signed ID attached" do + user = User.create!(name: "Jason", avatar: create_blob(filename: "funky.jpg").signed_id) + assert_equal "funky.jpg", user.reload.avatar.filename.to_s + end + + test "creating a record with a new blob from an uploaded file attached" do + User.new(name: "Jason", avatar: fixture_file_upload("racecar.jpg")).tap do |user| + assert user.new_record? + assert user.avatar.attachment.new_record? + assert user.avatar.blob.new_record? + assert_equal "racecar.jpg", user.avatar.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(user.avatar.key) + + user.save! + assert_equal "racecar.jpg", user.reload.avatar.filename.to_s + end + end + + test "creating a record with an unexpected object attached" do + error = assert_raises { User.create!(name: "Jason", avatar: :foo) } + assert_equal "Could not find or build blob: expected attachable, got :foo", error.message + end + + test "purging" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + assert @user.avatar.attached? + + @user.avatar.purge + assert_not @user.avatar.attached? + assert_not ActiveStorage::Blob.exists?(blob.id) + assert_not ActiveStorage::Blob.service.exist?(blob.key) + end + end + + test "purging later" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + assert @user.avatar.attached? + + perform_enqueued_jobs do + @user.avatar.purge_later + end + + assert_not @user.avatar.attached? + assert_not ActiveStorage::Blob.exists?(blob.id) + assert_not ActiveStorage::Blob.service.exist?(blob.key) + end + end + + test "purging dependent attachment later on destroy" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + + perform_enqueued_jobs do + @user.destroy! + end + + assert_not ActiveStorage::Blob.exists?(blob.id) + assert_not ActiveStorage::Blob.service.exist?(blob.key) + end + end + + test "not purging independent attachment on destroy" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.cover_photo.attach blob + + assert_no_enqueued_jobs do + @user.destroy! + end + end + end + + test "overriding attached reader" do + @user.avatar.attach create_blob(filename: "funky.jpg") + + assert_equal "funky.jpg", @user.avatar.filename.to_s + + begin + User.class_eval do + def avatar + super.filename.to_s.reverse + end + end + + assert_equal "gpj.yknuf", @user.avatar + ensure + User.send(:remove_method, :avatar) + end + end +end diff --git a/activestorage/test/models/attached_test.rb b/activestorage/test/models/attached_test.rb deleted file mode 100644 index b10d2bebe3..0000000000 --- a/activestorage/test/models/attached_test.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" -require "database/setup" - -class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase - setup do - @user = User.create!(name: "Josh") - end - - teardown { ActiveStorage::Blob.all.each(&:purge) } - - test "overriding has_one_attached methods works" do - # attach blob before messing with getter, which breaks `#attach` - @user.avatar.attach create_blob(filename: "funky.jpg") - - # inherited only - assert_equal "funky.jpg", @user.avatar.filename.to_s - - begin - User.class_eval do - def avatar - super.filename.to_s.reverse - end - end - - # override with super - assert_equal "funky.jpg".reverse, @user.avatar - ensure - User.send(:remove_method, :avatar) - end - end - - test "overriding has_many_attached methods works" do - # attach blobs before messing with getter, which breaks `#attach` - @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") - - # inherited only - assert_equal "funky.jpg", @user.highlights.first.filename.to_s - assert_equal "wonky.jpg", @user.highlights.second.filename.to_s - - begin - User.class_eval do - def highlights - super.reverse - end - end - - # override with super - assert_equal "wonky.jpg", @user.highlights.first.filename.to_s - assert_equal "funky.jpg", @user.highlights.second.filename.to_s - ensure - User.send(:remove_method, :highlights) - end - end -end diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb deleted file mode 100644 index ce83ec27d2..0000000000 --- a/activestorage/test/models/attachments_test.rb +++ /dev/null @@ -1,459 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" -require "database/setup" - -class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase - include ActiveJob::TestHelper - - setup { @user = User.create!(name: "DHH") } - - teardown { ActiveStorage::Blob.all.each(&:purge) } - - test "attach existing blob" do - @user.avatar.attach create_blob(filename: "funky.jpg") - assert_equal "funky.jpg", @user.avatar.filename.to_s - end - - test "attach existing blob from a signed ID" do - @user.avatar.attach create_blob(filename: "funky.jpg").signed_id - assert_equal "funky.jpg", @user.avatar.filename.to_s - end - - test "attach new blob from a Hash" do - @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" - assert_equal "town.jpg", @user.avatar.filename.to_s - end - - test "attach new blob from an UploadedFile" do - file = file_fixture "racecar.jpg" - @user.avatar.attach Rack::Test::UploadedFile.new file.to_s - assert_equal "racecar.jpg", @user.avatar.filename.to_s - end - - test "replace attached blob" do - @user.avatar.attach create_blob(filename: "funky.jpg") - - perform_enqueued_jobs do - assert_no_difference -> { ActiveStorage::Blob.count } do - @user.avatar.attach create_blob(filename: "town.jpg") - end - end - - assert_equal "town.jpg", @user.avatar.filename.to_s - end - - test "replace attached blob unsuccessfully" do - @user.avatar.attach create_blob(filename: "funky.jpg") - - perform_enqueued_jobs do - assert_raises do - @user.avatar.attach nil - end - end - - assert_equal "funky.jpg", @user.reload.avatar.filename.to_s - assert ActiveStorage::Blob.service.exist?(@user.avatar.key) - end - - test "replace attached blob with itself" do - @user.avatar.attach create_blob(filename: "funky.jpg") - - assert_no_changes -> { @user.reload.avatar.blob } do - assert_no_changes -> { @user.reload.avatar.attachment } do - assert_no_enqueued_jobs do - @user.avatar.attach @user.avatar.blob - end - end - end - end - - test "replaced attached blob with itself by signed ID" do - @user.avatar.attach create_blob(filename: "funky.jpg") - - assert_no_changes -> { @user.reload.avatar.blob } do - assert_no_changes -> { @user.reload.avatar.attachment } do - assert_no_enqueued_jobs do - @user.avatar.attach @user.avatar.blob.signed_id - end - end - end - end - - test "replace independent attached blob" do - @user.cover_photo.attach create_blob(filename: "funky.jpg") - - perform_enqueued_jobs do - assert_difference -> { ActiveStorage::Blob.count }, +1 do - assert_no_difference -> { ActiveStorage::Attachment.count } do - @user.cover_photo.attach create_blob(filename: "town.jpg") - end - end - end - - assert_equal "town.jpg", @user.cover_photo.filename.to_s - end - - test "attach blob to new record" do - user = User.new(name: "Jason") - - assert_no_changes -> { user.new_record? } do - assert_no_difference -> { ActiveStorage::Attachment.count } do - user.avatar.attach create_blob(filename: "funky.jpg") - end - end - - assert_predicate user.avatar, :attached? - assert_equal "funky.jpg", user.avatar.filename.to_s - - assert_difference -> { ActiveStorage::Attachment.count }, +1 do - user.save! - end - - assert_predicate user.reload.avatar, :attached? - assert_equal "funky.jpg", user.avatar.filename.to_s - end - - test "build new record with attached blob" do - assert_no_difference -> { ActiveStorage::Attachment.count } do - @user = User.new(name: "Jason", avatar: { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }) - end - - assert_predicate @user, :new_record? - assert_predicate @user.avatar, :attached? - assert_equal "town.jpg", @user.avatar.filename.to_s - - @user.save! - assert_predicate @user.reload.avatar, :attached? - assert_equal "town.jpg", @user.avatar.filename.to_s - end - - test "access underlying associations of new blob" do - @user.avatar.attach create_blob(filename: "funky.jpg") - assert_equal @user, @user.avatar_attachment.record - assert_equal @user.avatar_attachment.blob, @user.avatar_blob - assert_equal "funky.jpg", @user.avatar_attachment.blob.filename.to_s - end - - test "identify newly-attached, directly-uploaded blob" do - blob = directly_upload_file_blob(content_type: "application/octet-stream") - - @user.avatar.attach(blob) - - assert_equal "image/jpeg", @user.avatar.reload.content_type - assert_predicate @user.avatar, :identified? - end - - test "identify and analyze newly-attached, directly-uploaded blob" do - blob = directly_upload_file_blob(content_type: "application/octet-stream") - - perform_enqueued_jobs do - @user.avatar.attach blob - end - - assert_equal true, @user.avatar.reload.metadata[:identified] - assert_equal 4104, @user.avatar.metadata[:width] - assert_equal 2736, @user.avatar.metadata[:height] - end - - test "identify newly-attached blob only once" do - blob = create_file_blob - assert_predicate blob, :identified? - - # The blob's backing file is a PNG image. Fudge its content type so we can tell if it's identified when we attach it. - blob.update! content_type: "application/octet-stream" - - @user.avatar.attach blob - assert_equal "application/octet-stream", blob.content_type - end - - test "analyze newly-attached blob" do - perform_enqueued_jobs do - @user.avatar.attach create_file_blob - end - - assert_equal 4104, @user.avatar.reload.metadata[:width] - assert_equal 2736, @user.avatar.metadata[:height] - end - - test "analyze attached blob only once" do - blob = create_file_blob - - perform_enqueued_jobs do - @user.avatar.attach blob - end - - assert_predicate blob.reload, :analyzed? - - @user.avatar.detach - - assert_no_enqueued_jobs do - @user.reload.avatar.attach blob - end - end - - test "preserve existing metadata when analyzing a newly-attached blob" do - blob = create_file_blob(metadata: { foo: "bar" }) - - perform_enqueued_jobs do - @user.avatar.attach blob - end - - assert_equal "bar", blob.reload.metadata[:foo] - end - - test "detach blob" do - @user.avatar.attach create_blob(filename: "funky.jpg") - avatar_blob_id = @user.avatar.blob.id - avatar_key = @user.avatar.key - - @user.avatar.detach - assert_not_predicate @user.avatar, :attached? - assert ActiveStorage::Blob.exists?(avatar_blob_id) - assert ActiveStorage::Blob.service.exist?(avatar_key) - end - - test "purge attached blob" do - @user.avatar.attach create_blob(filename: "funky.jpg") - avatar_key = @user.avatar.key - - @user.avatar.purge - assert_not_predicate @user.avatar, :attached? - assert_not ActiveStorage::Blob.service.exist?(avatar_key) - end - - test "purge attached blob later when the record is destroyed" do - @user.avatar.attach create_blob(filename: "funky.jpg") - avatar_key = @user.avatar.key - - perform_enqueued_jobs do - @user.reload.destroy - - assert_nil ActiveStorage::Blob.find_by(key: avatar_key) - assert_not ActiveStorage::Blob.service.exist?(avatar_key) - end - end - - test "delete attachment for independent blob when record is destroyed" do - @user.cover_photo.attach create_blob(filename: "funky.jpg") - - @user.destroy - assert_not ActiveStorage::Attachment.exists?(record: @user, name: "cover_photo") - end - - test "find with attached blob" do - records = %w[alice bob].map do |name| - User.create!(name: name).tap do |user| - user.avatar.attach create_blob(filename: "#{name}.jpg") - end - end - - users = User.where(id: records.map(&:id)).with_attached_avatar.all - - assert_equal "alice.jpg", users.first.avatar.filename.to_s - assert_equal "bob.jpg", users.second.avatar.filename.to_s - end - - - test "attach existing blobs" do - @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") - - assert_equal "funky.jpg", @user.highlights.first.filename.to_s - assert_equal "wonky.jpg", @user.highlights.second.filename.to_s - end - - test "attach new blobs" do - @user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, - { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }) - - assert_equal "town.jpg", @user.highlights.first.filename.to_s - assert_equal "country.jpg", @user.highlights.second.filename.to_s - end - - test "attach blobs to new record" do - user = User.new(name: "Jason") - - assert_no_changes -> { user.new_record? } do - assert_no_difference -> { ActiveStorage::Attachment.count } do - user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, - { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }) - end - end - - assert_predicate user.highlights, :attached? - assert_equal "town.jpg", user.highlights.first.filename.to_s - assert_equal "country.jpg", user.highlights.second.filename.to_s - - assert_difference -> { ActiveStorage::Attachment.count }, +2 do - user.save! - end - - assert_predicate user.reload.highlights, :attached? - assert_equal "town.jpg", user.highlights.first.filename.to_s - assert_equal "country.jpg", user.highlights.second.filename.to_s - end - - test "build new record with attached blobs" do - assert_no_difference -> { ActiveStorage::Attachment.count } do - @user = User.new(name: "Jason", highlights: [ - { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, - { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }]) - end - - assert_predicate @user, :new_record? - assert_predicate @user.highlights, :attached? - assert_equal "town.jpg", @user.highlights.first.filename.to_s - assert_equal "country.jpg", @user.highlights.second.filename.to_s - - @user.save! - assert_predicate @user.reload.highlights, :attached? - assert_equal "town.jpg", @user.highlights.first.filename.to_s - assert_equal "country.jpg", @user.highlights.second.filename.to_s - end - - test "find attached blobs" do - @user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, - { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }) - - highlights = User.where(id: @user.id).with_attached_highlights.first.highlights - - assert_equal "town.jpg", highlights.first.filename.to_s - assert_equal "country.jpg", highlights.second.filename.to_s - end - - test "access underlying associations of new blobs" do - @user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, - { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }) - - assert_equal @user, @user.highlights_attachments.first.record - assert_equal @user.highlights_attachments.collect(&:blob).sort, @user.highlights_blobs.sort - assert_equal "town.jpg", @user.highlights_attachments.first.blob.filename.to_s - end - - test "analyze newly-attached blobs" do - perform_enqueued_jobs do - @user.highlights.attach( - create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg"), - create_file_blob(filename: "video.mp4", content_type: "video/mp4")) - end - - assert_equal 4104, @user.highlights.first.metadata[:width] - assert_equal 2736, @user.highlights.first.metadata[:height] - - assert_equal 640, @user.highlights.second.metadata[:width] - assert_equal 480, @user.highlights.second.metadata[:height] - end - - test "analyze attached blobs only once" do - blobs = [ - create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg"), - create_file_blob(filename: "video.mp4", content_type: "video/mp4") - ] - - perform_enqueued_jobs do - @user.highlights.attach(blobs) - end - - assert blobs.each(&:reload).all?(&:analyzed?) - - @user.highlights.attachments.destroy_all - - assert_no_enqueued_jobs do - @user.highlights.attach(blobs) - end - end - - test "preserve existing metadata when analyzing newly-attached blobs" do - blobs = [ - create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg", metadata: { foo: "bar" }), - create_file_blob(filename: "video.mp4", content_type: "video/mp4", metadata: { foo: "bar" }) - ] - - perform_enqueued_jobs do - @user.highlights.attach(blobs) - end - - blobs.each do |blob| - assert_equal "bar", blob.reload.metadata[:foo] - end - end - - test "detach blobs" do - @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") - highlight_blob_ids = @user.highlights.collect { |highlight| highlight.blob.id } - highlight_keys = @user.highlights.collect(&:key) - - @user.highlights.detach - assert_not_predicate @user.highlights, :attached? - - assert ActiveStorage::Blob.exists?(highlight_blob_ids.first) - assert ActiveStorage::Blob.exists?(highlight_blob_ids.second) - - assert ActiveStorage::Blob.service.exist?(highlight_keys.first) - assert ActiveStorage::Blob.service.exist?(highlight_keys.second) - end - - test "purge attached blobs" do - @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") - highlight_keys = @user.highlights.collect(&:key) - - @user.highlights.purge - assert_not_predicate @user.highlights, :attached? - assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) - assert_not ActiveStorage::Blob.service.exist?(highlight_keys.second) - end - - test "purge attached blobs later when the record is destroyed" do - @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") - highlight_keys = @user.highlights.collect(&:key) - - perform_enqueued_jobs do - @user.reload.destroy - - assert_nil ActiveStorage::Blob.find_by(key: highlight_keys.first) - assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) - - assert_nil ActiveStorage::Blob.find_by(key: highlight_keys.second) - assert_not ActiveStorage::Blob.service.exist?(highlight_keys.second) - end - end - - test "delete attachments for independent blobs when the record is destroyed" do - @user.vlogs.attach create_blob(filename: "funky.mp4"), create_blob(filename: "wonky.mp4") - - @user.destroy - assert_not ActiveStorage::Attachment.exists?(record: @user, name: "vlogs") - end - - test "selectively purge one attached blob of many" do - first_blob = create_blob(filename: "funky.jpg") - second_blob = create_blob(filename: "wonky.jpg") - attachments = @user.highlights.attach(first_blob, second_blob) - - assert_difference -> { ActiveStorage::Blob.count }, -1 do - @user.highlights.where(id: attachments.first.id).purge - end - - assert_not ActiveStorage::Blob.exists?(key: first_blob.key) - assert ActiveStorage::Blob.exists?(key: second_blob.key) - end - - test "selectively purge one attached blob of many later" do - first_blob = create_blob(filename: "funky.jpg") - second_blob = create_blob(filename: "wonky.jpg") - attachments = @user.highlights.attach(first_blob, second_blob) - - perform_enqueued_jobs do - assert_difference -> { ActiveStorage::Blob.count }, -1 do - @user.highlights.where(id: attachments.first.id).purge_later - end - end - - assert_not ActiveStorage::Blob.exists?(key: first_blob.key) - assert ActiveStorage::Blob.exists?(key: second_blob.key) - end -end diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 40b30acd3e..a0e207642a 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -7,21 +7,15 @@ require "active_support/testing/method_call_assertions" class ActiveStorage::BlobTest < ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions - test ".unattached scope returns not attached blobs" do - class UserWithHasOneAttachedDependentFalse < User - has_one_attached :avatar, dependent: false + test "unattached scope" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + User.create! name: "DHH", avatar: blobs.first + assert_includes ActiveStorage::Blob.unattached, blobs.second + assert_not_includes ActiveStorage::Blob.unattached, blobs.first + + User.create! name: "Jason", avatar: blobs.second + assert_not_includes ActiveStorage::Blob.unattached, blobs.second end - - ActiveStorage::Blob.delete_all - blob_1 = create_blob filename: "funky.jpg" - blob_2 = create_blob filename: "town.jpg" - - user = UserWithHasOneAttachedDependentFalse.create! - user.avatar.attach blob_1 - - assert_equal [blob_2], ActiveStorage::Blob.unattached - user.destroy - assert_equal [blob_1, blob_2].map(&:id).sort, ActiveStorage::Blob.unattached.pluck(:id).sort end test "create after upload sets byte size and checksum" do diff --git a/activestorage/test/models/presence_validation_test.rb b/activestorage/test/models/presence_validation_test.rb index aa804506dd..13ba3c900d 100644 --- a/activestorage/test/models/presence_validation_test.rb +++ b/activestorage/test/models/presence_validation_test.rb @@ -12,7 +12,7 @@ class ActiveStorage::PresenceValidationTest < ActiveSupport::TestCase test "validates_presence_of has_one_attached" do Admin.validates_presence_of :avatar - a = Admin.new + a = Admin.new(name: "DHH") assert_predicate a, :invalid? a.avatar.attach create_blob(filename: "funky.jpg") @@ -21,7 +21,7 @@ class ActiveStorage::PresenceValidationTest < ActiveSupport::TestCase test "validates_presence_of has_many_attached" do Admin.validates_presence_of :highlights - a = Admin.new + a = Admin.new(name: "DHH") assert_predicate a, :invalid? a.highlights.attach create_blob(filename: "funky.jpg") diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 9985a76832..7b7926ac79 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -79,6 +79,10 @@ class ActiveSupport::TestCase def extract_metadata_from(blob) blob.tap(&:analyze).metadata end + + def fixture_file_upload(filename) + Rack::Test::UploadedFile.new file_fixture(filename).to_s + end end require "global_id" @@ -86,6 +90,8 @@ GlobalID.app = "ActiveStorageExampleApp" ActiveRecord::Base.send :include, GlobalID::Identification class User < ActiveRecord::Base + validates :name, presence: true + has_one_attached :avatar has_one_attached :cover_photo, dependent: false -- cgit v1.2.3