diff options
Diffstat (limited to 'activestorage')
30 files changed, 1442 insertions, 751 deletions
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/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index 63918eb6f4..75cc11d6ff 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -5,30 +5,20 @@ # Always go through the BlobsController, or your own authenticated controller, rather than directly # to the service url. class ActiveStorage::DiskController < ActiveStorage::BaseController - include ActionController::Live - skip_forgery_protection def show if key = decode_verified_key - response.headers["Content-Type"] = params[:content_type] || DEFAULT_SEND_FILE_TYPE - response.headers["Content-Disposition"] = params[:disposition] || DEFAULT_SEND_FILE_DISPOSITION - - disk_service.download key do |chunk| - response.stream.write chunk - end + serve_file disk_service.path_for(key), content_type: params[:content_type], disposition: params[:disposition] else head :not_found end - ensure - response.stream.close end def update if token = decode_verified_token if acceptable_content?(token) disk_service.upload token[:key], request.body, checksum: token[:checksum] - head :no_content else head :unprocessable_entity end @@ -37,8 +27,6 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController end rescue ActiveStorage::IntegrityError head :unprocessable_entity - ensure - response.stream.close end private @@ -51,6 +39,20 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController ActiveStorage.verifier.verified(params[:encoded_key], purpose: :blob_key) end + def serve_file(path, content_type:, disposition:) + Rack::File.new(nil).serving(request, path).tap do |(status, headers, body)| + self.status = status + self.response_body = body + + headers.each do |name, value| + response.headers[name] = value + end + + response.headers["Content-Type"] = content_type || DEFAULT_SEND_FILE_TYPE + response.headers["Content-Disposition"] = disposition || DEFAULT_SEND_FILE_DISPOSITION + end + end + def decode_verified_token ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token) diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index 98874d2250..fa15e0451d 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -2,8 +2,7 @@ # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveStorage::BaseJob - # FIXME: Limit this to a custom ActiveStorage error - retry_on StandardError + discard_on ActiveRecord::RecordNotFound def perform(blob) blob.purge 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 <tt>identify: false</tt> 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..0ccf76f00b 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "action_dispatch" -require "action_dispatch/http/upload" require "active_support/core_ext/module/delegation" module ActiveStorage @@ -15,26 +13,13 @@ module ActiveStorage end private - def create_blob_from(attachable) - case attachable - when ActiveStorage::Blob - attachable - when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile - ActiveStorage::Blob.create_after_upload! \ - io: attachable.open, - filename: attachable.original_filename, - content_type: attachable.content_type - when Hash - ActiveStorage::Blob.create_after_upload!(attachable) - when String - ActiveStorage::Blob.find_signed(attachable) - else - nil - end + def change + record.attachment_changes[name] end 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..af19328a61 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/create_many.rb @@ -0,0 +1,36 @@ +# 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 blobs + @blobs ||= subchanges.collect(&:blob) + 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..5812fd2b08 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/create_one.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "action_dispatch" +require "action_dispatch/http/upload" + +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 ArgumentError, "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..25f88284df 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -9,22 +9,29 @@ 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. + # Returns all attached blobs. + def blobs + change.present? ? change.blobs : record.public_send("#{name}_blobs") + end + + # Attaches one or more +attachables+ to the record. + # + # If the record is persisted and unchanged, the attachments are saved to + # the database immediately. Otherwise, they'll be saved to the DB when the + # record is next saved. # # document.images.attach(params[:images]) # Array of ActionDispatch::Http::UploadedFile objects # document.images.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload # document.images.attach(io: File.open("/path/to/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpg") # document.images.attach([ first_blob, second_blob ]) def attach(*attachables) - attachables.flatten.collect do |attachable| - if record.new_record? - attachments.build(record: record, blob: create_blob_from(attachable)) - else - attachments.create!(record: record, blob: create_blob_from(attachable)) - end + if record.persisted? && !record.changed? + record.update(name => blobs + attachables.flatten) + else + record.public_send("#{name}=", blobs + attachables.flatten) end end @@ -41,7 +48,7 @@ module ActiveStorage # Deletes associated attachments without purging them, leaving their respective blobs in place. def detach - attachments.destroy_all if attached? + attachments.delete_all if attached? end ## @@ -50,7 +57,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..aa5c769cb8 --- /dev/null +++ b/activestorage/lib/active_storage/attached/model.rb @@ -0,0 +1,140 @@ +# 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 } + + after_commit(on: %i[ create update ]) { attachment_changes.delete(name.to_s).try(:upload) } + + 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 } + + after_commit(on: %i[ create update ]) { attachment_changes.delete(name.to_s).try(:upload) } + + ActiveRecord::Reflection.add_attachment_reflection( + self, + name, + ActiveRecord::Reflection.create(:has_many_attached, name, nil, { dependent: dependent }, self) + ) + end + end + + def attachment_changes #:nodoc: + @attachment_changes ||= {} + end + + def reload(*) #:nodoc: + super.tap { @attachment_changes = nil } + end + end +end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index f992cb5f84..c039226fcd 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -10,30 +10,28 @@ 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. + # Attaches an +attachable+ to the record. + # + # If the record is persisted and unchanged, the attachment is saved to + # the database immediately. Otherwise, it'll be saved to the DB when the + # record is next saved. # # person.avatar.attach(params[:avatar]) # ActionDispatch::Http::UploadedFile object # person.avatar.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload # 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) - - 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 record.persisted? && !record.changed? + record.update(name => attachable) + else + record.public_send("#{name}=", attachable) end end @@ -51,7 +49,7 @@ module ActiveStorage # Deletes the attachment without purging it, leaving its blob in place. def detach if attached? - attachment.destroy + attachment.delete write_attachment nil end end @@ -69,16 +67,11 @@ module ActiveStorage def purge_later if attached? attachment.purge_later + write_attachment nil end end private - delegate :transaction, to: :record - - def build_attachment(blob:) - ActiveStorage::Attachment.new(record: record, name: name, blob: blob) - end - def write_attachment(attachment) record.public_send("#{name}_attachment=", attachment) end 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/lib/active_storage/reflection.rb b/activestorage/lib/active_storage/reflection.rb index 04a1b20882..ce248c88b5 100644 --- a/activestorage/lib/active_storage/reflection.rb +++ b/activestorage/lib/active_storage/reflection.rb @@ -19,8 +19,8 @@ module ActiveStorage end module ReflectionExtension # :nodoc: - def add_attachment_reflection(ar, name, reflection) - ar.attachment_reflections.merge!(name.to_s => reflection) + def add_attachment_reflection(model, name, reflection) + model.attachment_reflections = model.attachment_reflections.merge(name.to_s => reflection) end private diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index b1b6f1ddcf..9f304b7e01 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -117,11 +117,11 @@ module ActiveStorage { "Content-Type" => content_type } end - private - def path_for(key) - File.join root, folder_for(key), key - end + def path_for(key) #:nodoc: + File.join root, folder_for(key), key + end + private def folder_for(key) [ key[0..1], key[2..3] ].join("/") end diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 38acef81f4..fdde01d4a3 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -6,7 +6,6 @@ require "google/cloud/storage" require "net/http" require "active_support/core_ext/object/to_query" -require "active_storage/filename" module ActiveStorage # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb index 9af7c83bdf..c053052f6f 100644 --- a/activestorage/test/controllers/disk_controller_test.rb +++ b/activestorage/test/controllers/disk_controller_test.rb @@ -6,8 +6,8 @@ require "database/setup" class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest test "showing blob inline" do blob = create_blob - get blob.service_url + assert_response :ok assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"] assert_equal "text/plain", response.headers["Content-Type"] assert_equal "Hello world!", response.body @@ -15,13 +15,22 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest test "showing blob as attachment" do blob = create_blob - get blob.service_url(disposition: :attachment) + assert_response :ok assert_equal "attachment; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"] assert_equal "text/plain", response.headers["Content-Type"] assert_equal "Hello world!", response.body end + test "showing blob range inline" do + blob = create_blob + get blob.service_url, headers: { "Range" => "bytes=5-9" } + assert_response :partial_content + assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"] + assert_equal "text/plain", response.headers["Content-Type"] + assert_equal " worl", response.body + end + test "directly uploading blob with integrity" do data = "Something else entirely!" @@ -58,4 +67,10 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity assert_not blob.service.exist?(blob.key) end + + test "directly uploading blob with invalid token" do + put update_rails_disk_service_url(encoded_token: "invalid"), + params: "Something else entirely!", headers: { "Content-Type" => "text/plain" } + assert_response :not_found + end end diff --git a/activestorage/test/jobs/purge_job_test.rb b/activestorage/test/jobs/purge_job_test.rb new file mode 100644 index 0000000000..251022a96f --- /dev/null +++ b/activestorage/test/jobs/purge_job_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::PurgeJobTest < ActiveJob::TestCase + setup { @blob = create_blob } + + test "purges" do + assert_difference -> { ActiveStorage::Blob.count }, -1 do + ActiveStorage::PurgeJob.perform_now @blob + end + + assert_not ActiveStorage::Blob.exists?(@blob.id) + assert_not ActiveStorage::Blob.service.exist?(@blob.key) + end + + test "ignores missing blob" do + @blob.purge + + perform_enqueued_jobs do + assert_nothing_raised do + ActiveStorage::PurgeJob.perform_later @blob + end + end + 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..fc89dae658 --- /dev/null +++ b/activestorage/test/models/attached/many_test.rb @@ -0,0 +1,497 @@ +# 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 "attaching existing blobs to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @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 + assert_not @user.highlights.first.persisted? + assert_not @user.highlights.second.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "funky.jpg", @user.highlights.reload.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, changed record" do + @user.name = "Tina" + assert @user.changed? + + @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 + assert_not @user.highlights.first.persisted? + assert_not @user.highlights.second.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "funky.jpg", @user.highlights.reload.first.filename.to_s + assert_equal "town.jpg", @user.highlights.second.filename.to_s + end + + test "attaching new blobs from Hashes to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @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 + assert_not @user.highlights.first.persisted? + assert_not @user.highlights.second.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "funky.jpg", @user.highlights.reload.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, changed record" do + @user.name = "Tina" + assert @user.changed? + + @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 + assert_not @user.highlights.first.persisted? + assert_not @user.highlights.second.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "racecar.jpg", @user.highlights.reload.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 "analyzing a new blob from an uploaded file after attaching it to an existing record" do + perform_enqueued_jobs do + @user.highlights.attach fixture_file_upload("racecar.jpg") + end + + assert @user.highlights.reload.first.analyzed? + assert_equal 4104, @user.highlights.first.metadata[:width] + assert_equal 2736, @user.highlights.first.metadata[:height] + end + + test "analyzing a new blob from an uploaded file after attaching it to an existing record via update" do + perform_enqueued_jobs do + @user.update! highlights: [ fixture_file_upload("racecar.jpg") ] + end + + assert @user.highlights.reload.first.analyzed? + assert_equal 4104, @user.highlights.first.metadata[:width] + assert_equal 2736, @user.highlights.first.metadata[:height] + end + + test "analyzing a directly-uploaded blob after attaching it to an existing record" do + perform_enqueued_jobs do + @user.highlights.attach directly_upload_file_blob(filename: "racecar.jpg") + end + + assert @user.highlights.reload.first.analyzed? + assert_equal 4104, @user.highlights.first.metadata[:width] + assert_equal 2736, @user.highlights.first.metadata[:height] + end + + test "analyzing a directly-uploaded blob after attaching it to an existing record via update" do + perform_enqueued_jobs do + @user.update! highlights: [ directly_upload_file_blob(filename: "racecar.jpg") ] + end + + assert @user.highlights.reload.first.analyzed? + assert_equal 4104, @user.highlights.first.metadata[:width] + assert_equal 2736, @user.highlights.first.metadata[:height] + 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 blobs 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 user.highlights.first.blob.new_record? + assert 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_not ActiveStorage::Blob.service.exist?(user.highlights.first.key) + assert_not ActiveStorage::Blob.service.exist?(user.highlights.second.key) + + user.save! + assert user.highlights.first.persisted? + assert user.highlights.second.persisted? + assert user.highlights.first.blob.persisted? + assert user.highlights.second.blob.persisted? + assert_equal "funky.jpg", user.reload.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) + 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 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 user.highlights.first.persisted? + assert user.highlights.second.persisted? + assert user.highlights.first.blob.persisted? + assert user.highlights.second.blob.persisted? + assert_equal "racecar.jpg", user.reload.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) + 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(ArgumentError) { User.create!(name: "Jason", highlights: :foo) } + assert_equal "Could not find or build blob: expected attachable, got :foo", error.message + end + + test "analyzing a new blob from an uploaded file after attaching it to a new record" do + perform_enqueued_jobs do + user = User.create!(name: "Jason", highlights: [ fixture_file_upload("racecar.jpg") ]) + assert user.highlights.reload.first.analyzed? + assert_equal 4104, user.highlights.first.metadata[:width] + assert_equal 2736, user.highlights.first.metadata[:height] + end + end + + test "analyzing a directly-uploaded blob after attaching it to a new record" do + perform_enqueued_jobs do + user = User.create!(name: "Jason", highlights: [ directly_upload_file_blob(filename: "racecar.jpg") ]) + assert user.highlights.reload.first.analyzed? + assert_equal 4104, user.highlights.first.metadata[:width] + assert_equal 2736, user.highlights.first.metadata[:height] + end + end + + test "detaching" 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.detach + end + + assert_not @user.highlights.attached? + assert ActiveStorage::Blob.exists?(blobs.first.id) + assert ActiveStorage::Blob.exists?(blobs.second.id) + assert ActiveStorage::Blob.service.exist?(blobs.first.key) + assert ActiveStorage::Blob.service.exist?(blobs.second.key) + end + 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 "clearing change on reload" do + @user.highlights = [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ] + assert @user.highlights.attached? + + @user.reload + assert_not @user.highlights.attached? + 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..33f9122644 --- /dev/null +++ b/activestorage/test/models/attached/one_test.rb @@ -0,0 +1,455 @@ +# 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 "attaching an existing blob to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @user.avatar.attach create_blob(filename: "funky.jpg") + assert_equal "funky.jpg", @user.avatar.filename.to_s + assert_not @user.avatar.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "funky.jpg", @user.reload.avatar.filename.to_s + end + + test "attaching an existing blob from a signed ID to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @user.avatar.attach create_blob(filename: "funky.jpg").signed_id + assert_equal "funky.jpg", @user.avatar.filename.to_s + assert_not @user.avatar.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "funky.jpg", @user.reload.avatar.filename.to_s + end + + test "attaching a new blob from a Hash to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + assert_equal "town.jpg", @user.avatar.filename.to_s + assert_not @user.avatar.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "town.jpg", @user.reload.avatar.filename.to_s + end + + test "attaching a new blob from an uploaded file to an existing, changed record" do + @user.name = "Tina" + assert @user.changed? + + @user.avatar.attach fixture_file_upload("racecar.jpg") + assert_equal "racecar.jpg", @user.avatar.filename.to_s + assert_not @user.avatar.persisted? + assert @user.will_save_change_to_name? + + @user.save! + assert_equal "racecar.jpg", @user.reload.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 "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 "replacing an attached blob on an existing record with itself" 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.reload.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 "analyzing a new blob from an uploaded file after attaching it to an existing record" do + perform_enqueued_jobs do + @user.avatar.attach fixture_file_upload("racecar.jpg") + end + + assert @user.avatar.reload.analyzed? + assert_equal 4104, @user.avatar.metadata[:width] + assert_equal 2736, @user.avatar.metadata[:height] + end + + test "analyzing a new blob from an uploaded file after attaching it to an existing record via update" do + perform_enqueued_jobs do + @user.update! avatar: fixture_file_upload("racecar.jpg") + end + + assert @user.avatar.reload.analyzed? + assert_equal 4104, @user.avatar.metadata[:width] + assert_equal 2736, @user.avatar.metadata[:height] + end + + test "analyzing a directly-uploaded blob after attaching it to an existing record" do + perform_enqueued_jobs do + @user.avatar.attach directly_upload_file_blob(filename: "racecar.jpg") + end + + assert @user.avatar.reload.analyzed? + assert_equal 4104, @user.avatar.metadata[:width] + assert_equal 2736, @user.avatar.metadata[:height] + end + + test "analyzing a directly-uploaded blob after attaching it to an existing record via updates" do + perform_enqueued_jobs do + @user.update! avatar: directly_upload_file_blob(filename: "racecar.jpg") + end + + assert @user.avatar.reload.analyzed? + assert_equal 4104, @user.avatar.metadata[:width] + assert_equal 2736, @user.avatar.metadata[:height] + 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 user.avatar.blob.new_record? + assert_equal "town.jpg", user.avatar.filename.to_s + assert_not ActiveStorage::Blob.service.exist?(user.avatar.key) + + user.save! + assert user.avatar.attachment.persisted? + assert user.avatar.blob.persisted? + assert_equal "town.jpg", user.reload.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.avatar.key) + 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 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 user.avatar.attachment.persisted? + assert user.avatar.blob.persisted? + assert_equal "racecar.jpg", user.reload.avatar.filename.to_s + assert ActiveStorage::Blob.service.exist?(user.avatar.key) + 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(ArgumentError) { User.create!(name: "Jason", avatar: :foo) } + assert_equal "Could not find or build blob: expected attachable, got :foo", error.message + end + + test "analyzing a new blob from an uploaded file after attaching it to a new record" do + perform_enqueued_jobs do + user = User.create!(name: "Jason", avatar: fixture_file_upload("racecar.jpg")) + assert user.avatar.reload.analyzed? + assert_equal 4104, user.avatar.metadata[:width] + assert_equal 2736, user.avatar.metadata[:height] + end + end + + test "analyzing a directly-uploaded blob after attaching it to a new record" do + perform_enqueued_jobs do + user = User.create!(name: "Jason", avatar: directly_upload_file_blob(filename: "racecar.jpg")) + assert user.avatar.reload.analyzed? + assert_equal 4104, user.avatar.metadata[:width] + assert_equal 2736, user.avatar.metadata[:height] + end + end + + test "detaching" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + assert @user.avatar.attached? + + perform_enqueued_jobs do + @user.avatar.detach + end + + assert_not @user.avatar.attached? + assert ActiveStorage::Blob.exists?(blob.id) + assert ActiveStorage::Blob.service.exist?(blob.key) + end + 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 "clearing change on reload" do + @user.avatar = create_blob(filename: "funky.jpg") + assert @user.avatar.attached? + + @user.reload + assert_not @user.avatar.attached? + 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/models/reflection_test.rb b/activestorage/test/models/reflection_test.rb index da866ca996..98606b0617 100644 --- a/activestorage/test/models/reflection_test.rb +++ b/activestorage/test/models/reflection_test.rb @@ -3,27 +3,32 @@ require "test_helper" class ActiveStorage::ReflectionTest < ActiveSupport::TestCase - test "allows reflecting for all attachment" do - expected_classes = - User.reflect_on_all_attachments.all? do |reflection| - reflection.is_a?(ActiveStorage::Reflection::HasOneAttachedReflection) || - reflection.is_a?(ActiveStorage::Reflection::HasManyAttachedReflection) - end - - assert expected_classes - end - - test "allows reflecting on a singular has_one_attached attachment" do + test "reflecting on a singular attachment" do reflection = User.reflect_on_attachment(:avatar) - + assert_equal User, reflection.active_record assert_equal :avatar, reflection.name assert_equal :has_one_attached, reflection.macro + assert_equal :purge_later, reflection.options[:dependent] end - test "allows reflecting on a singular has_many_attached attachment" do - reflection = User.reflect_on_attachment(:highlights) + test "reflection on a singular attachment with the same name as an attachment on another model" do + reflection = Group.reflect_on_attachment(:avatar) + assert_equal Group, reflection.active_record + end + test "reflecting on a collection attachment" do + reflection = User.reflect_on_attachment(:highlights) + assert_equal User, reflection.active_record assert_equal :highlights, reflection.name assert_equal :has_many_attached, reflection.macro + assert_equal :purge_later, reflection.options[:dependent] + end + + test "reflecting on all attachments" do + reflections = User.reflect_on_all_attachments.sort_by(&:name) + assert_equal [ User ], reflections.collect(&:active_record).uniq + assert_equal %i[ avatar cover_photo highlights vlogs ], reflections.collect(&:name) + assert_equal %i[ has_one_attached has_one_attached has_many_attached has_many_attached ], reflections.collect(&:macro) + assert_equal [ :purge_later, false, :purge_later, false ], reflections.collect { |reflection| reflection.options[:dependent] } end end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 573a8e0b0b..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,9 +90,15 @@ 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 has_many_attached :highlights has_many_attached :vlogs, dependent: false end + +class Group < ActiveRecord::Base + has_one_attached :avatar +end |