diff options
Diffstat (limited to 'activestorage')
15 files changed, 81 insertions, 34 deletions
diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index 369c07929d..990ab27c9f 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -# Provides delayed purging of attachments or blobs using their +purge_later+ method. +# Provides delayed purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveJob::Base # FIXME: Limit this to a custom ActiveStorage error retry_on StandardError - def perform(attachment_or_blob) - attachment_or_blob.purge + def perform(blob) + blob.purge end end diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index ad43845e4e..2a1c20b7db 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -14,17 +14,15 @@ class ActiveStorage::Attachment < ActiveRecord::Base delegate_missing_to :blob - # Purging an attachment will purge the blob (delete the file on the service, then destroy the record) - # and then destroy the attachment itself. + # Synchronously purges the blob (deletes it from the configured service) and destroys the attachment. def purge blob.purge destroy end - # Purging an attachment means purging the blob, which means talking to the service, which means - # talking over the Internet. Whenever you're doing that, it's a good idea to put that work in a job, - # so it doesn't hold up other operations. That's what +purge_later+ provides. + # Destroys the attachment and asynchronously purges the blob (deletes it from the configured service). def purge_later - ActiveStorage::PurgeJob.perform_later(self) + blob.purge_later + destroy end end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 664a53a778..e6cf08ce83 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -3,8 +3,8 @@ # A blob is a record that contains the metadata about a file and a key for where that file resides on the service. # Blobs can be created in two ways: # -# 1) Subsequent to the file being uploaded server-side to the service via <tt>create_after_upload!</tt>. -# 2) Ahead of the file being directly uploaded client-side to the service via <tt>create_before_direct_upload!</tt>. +# 1. Subsequent to the file being uploaded server-side to the service via <tt>create_after_upload!</tt>. +# 2. Ahead of the file being directly uploaded client-side to the service via <tt>create_before_direct_upload!</tt>. # # The first option doesn't require any client-side JavaScript integration, and can be used by any other back-end # service that deals with files. The second option is faster, since you're not using your own server as a staging diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index c2ad5c844c..dead6b6d33 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Encapsulates a string representing a filename to provide convenience access to parts of it and a sanitized version. -# This is what's returned by `ActiveStorage::Blob#filename`. A Filename instance is comparable so it can be used for sorting. +# This is what's returned by ActiveStorage::Blob#filename. A Filename instance is comparable so it can be used for sorting. class ActiveStorage::Filename include Comparable @@ -9,25 +9,33 @@ class ActiveStorage::Filename @filename = filename end - # Filename.new("racecar.jpg").base # => "racecar" + # Returns the basename of the filename. + # + # ActiveStorage::Filename.new("racecar.jpg").base # => "racecar" def base File.basename @filename, extension_with_delimiter end - # Filename.new("racecar.jpg").extension_with_delimiter # => ".jpg" + # Returns the extension with delimiter of the filename. + # + # ActiveStorage::Filename.new("racecar.jpg").extension_with_delimiter # => ".jpg" def extension_with_delimiter File.extname @filename end - # Filename.new("racecar.jpg").extension_without_delimiter # => "jpg" + # Returns the extension without delimiter of the filename. + # + # ActiveStorage::Filename.new("racecar.jpg").extension_without_delimiter # => "jpg" def extension_without_delimiter extension_with_delimiter.from(1).to_s end alias_method :extension, :extension_without_delimiter - # Filename.new("foo:bar.jpg").sanitized # => "foo-bar.jpg" - # Filename.new("foo/bar.jpg").sanitized # => "foo-bar.jpg" + # Returns the sanitized filename. + # + # ActiveStorage::Filename.new("foo:bar.jpg").sanitized # => "foo-bar.jpg" + # ActiveStorage::Filename.new("foo/bar.jpg").sanitized # => "foo-bar.jpg" # # ...and any other character unsafe for URLs or storage is converted or stripped. def sanitized diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 40648a27f7..02bf32b352 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -4,7 +4,7 @@ # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the # original. # -# Variants rely on {MiniMagick}(https://github.com/minimagick/minimagick) for the actual transformations +# Variants rely on {MiniMagick}[https://github.com/minimagick/minimagick] for the actual transformations # of the file, so you must add <tt>gem "mini_magick"</tt> to your Gemfile if you wish to use variants. # # Note that to create a variant it's necessary to download the entire blob file from the service and load it diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index f657d90db4..bf269e2a8f 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -3,7 +3,7 @@ require "active_support/core_ext/object/inclusion" # A set of transformations that can be applied to a blob to create a variant. This class is exposed via -# the `ActiveStorage::Blob#variant` method and should rarely be used directly. +# the ActiveStorage::Blob#variant method and should rarely be used directly. # # In case you do need to use this directly, it's instantiated using a hash of transformations where # the key is the command and the value is the arguments. Example: diff --git a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb index 7a869874a4..9e31e3966a 100644 --- a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb +++ b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb @@ -5,7 +5,7 @@ class CreateActiveStorageTables < ActiveRecord::Migration[5.2] t.string :filename, null: false t.string :content_type t.text :metadata - t.integer :byte_size, null: false + t.bigint :byte_size, null: false t.string :checksum, null: false t.datetime :created_at, null: false diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb index 9c4b6d5d1d..c08fd56652 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -8,10 +8,10 @@ module ActiveStorage # Abstract base class for the concrete ActiveStorage::Attached::One and ActiveStorage::Attached::Many # classes that both provide proxy access to the blob association for a record. class Attached - attr_reader :name, :record + attr_reader :name, :record, :dependent - def initialize(name, record) - @name, @record = name, record + def initialize(name, record, dependent:) + @name, @record, @dependent = name, record, dependent end private diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index f3879ee2e3..35a081adc4 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -26,7 +26,7 @@ module ActiveStorage def has_one_attached(name, dependent: :purge_later) class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name} - @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self) + @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) end CODE @@ -65,7 +65,7 @@ module ActiveStorage def has_many_attached(name, dependent: :purge_later) class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name} - @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self) + @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"}) end CODE diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index 2e5831348e..ac90f32d95 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -21,11 +21,14 @@ module ActiveStorage # person.avatar.attach(io: File.open("~/face.jpg"), filename: "face.jpg", content_type: "image/jpg") # person.avatar.attach(avatar_blob) # ActiveStorage::Blob object def attach(attachable) - write_attachment \ - ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) + if attached? && dependent == :purge_later + replace attachable + else + write_attachment create_attachment_from(attachable) + end end - # Returns true if an attachment has been made. + # Returns +true+ if an attachment has been made. # # class User < ActiveRecord::Base # has_one_attached :avatar @@ -53,6 +56,19 @@ module ActiveStorage end private + def replace(attachable) + blob.tap do + transaction do + destroy + write_attachment create_attachment_from(attachable) + end + end.purge_later + end + + def create_attachment_from(attachable) + ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) + end + def write_attachment(attachment) record.public_send("#{name}_attachment=", attachment) end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index ce736b8728..b80fdea1ab 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -77,7 +77,7 @@ module ActiveStorage raise NotImplementedError end - # Return true if a file exists at the +key+. + # Return +true+ if a file exists at the +key+. def exist?(key) raise NotImplementedError end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 5aa8d74a5a..f600753a08 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -66,7 +66,7 @@ module ActiveStorage verified_key_with_expiration, filename: filename, disposition: disposition, content_type: content_type else - "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?disposition=#{disposition}&content_type=#{content_type}" + "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?content_type=#{content_type}&disposition=#{disposition}" end payload[:url] = generated_url diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 7cfd8683db..ac346c0087 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -31,6 +31,31 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 "access underlying associations of new blob" do @user.avatar.attach create_blob(filename: "funky.jpg") assert_equal @user, @user.avatar_attachment.record diff --git a/activestorage/test/service/disk_service_test.rb b/activestorage/test/service/disk_service_test.rb index f57e44536a..e07d1d88bc 100644 --- a/activestorage/test/service/disk_service_test.rb +++ b/activestorage/test/service/disk_service_test.rb @@ -8,7 +8,7 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests test "url generation" do - assert_match(/rails\/active_storage\/disk\/.*\/avatar\.png\?.+disposition=inline/, + assert_match(/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline/, @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: "inline; filename=\"avatar.png\"", filename: "avatar.png", content_type: "image/png")) end end diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb index 46dd97b3e9..dedc58452e 100644 --- a/activestorage/test/template/image_tag_test.rb +++ b/activestorage/test/template/image_tag_test.rb @@ -11,17 +11,17 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase end test "blob" do - assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url @blob}" />), image_tag(@blob) + assert_dom_equal %(<img src="#{polymorphic_url @blob}" />), image_tag(@blob) end test "variant" do variant = @blob.variant(resize: "100x100") - assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url variant}" />), image_tag(variant) + assert_dom_equal %(<img src="#{polymorphic_url variant}" />), image_tag(variant) end test "attachment" do attachment = ActiveStorage::Attachment.new(blob: @blob) - assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url attachment}" />), image_tag(attachment) + assert_dom_equal %(<img src="#{polymorphic_url attachment}" />), image_tag(attachment) end test "error when attachment's empty" do |