diff options
Diffstat (limited to 'activestorage/app')
19 files changed, 189 insertions, 175 deletions
diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index a22f644238..e2bcb520b9 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -484,7 +484,7 @@ }, { key: "readNextChunk", value: function readNextChunk() { - if (this.chunkIndex < this.chunkCount) { + if (this.chunkIndex < this.chunkCount || this.chunkIndex == 0 && this.chunkCount == 0) { var start = this.chunkIndex * this.chunkSize; var end = Math.min(start + this.chunkSize, this.file.size); var bytes = fileSlice.call(this.file, start, end); @@ -560,7 +560,10 @@ this.xhr.setRequestHeader("Content-Type", "application/json"); this.xhr.setRequestHeader("Accept", "application/json"); this.xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest"); - this.xhr.setRequestHeader("X-CSRF-Token", getMetaValue("csrf-token")); + var csrfToken = getMetaValue("csrf-token"); + if (csrfToken != undefined) { + this.xhr.setRequestHeader("X-CSRF-Token", csrfToken); + } this.xhr.addEventListener("load", function(event) { return _this.requestDidLoad(event); }); @@ -855,14 +858,22 @@ return DirectUploadsController; }(); var processingAttribute = "data-direct-uploads-processing"; + var submitButtonsByForm = new WeakMap(); var started = false; function start() { if (!started) { started = true; + document.addEventListener("click", didClick, true); document.addEventListener("submit", didSubmitForm); document.addEventListener("ajax:before", didSubmitRemoteElement); } } + function didClick(event) { + var target = event.target; + if ((target.tagName == "INPUT" || target.tagName == "BUTTON") && target.type == "submit" && target.form) { + submitButtonsByForm.set(target.form, target); + } + } function didSubmitForm(event) { handleFormSubmissionEvent(event); } @@ -894,7 +905,7 @@ } } function submitForm(form) { - var button = findElement(form, "input[type=submit]"); + var button = submitButtonsByForm.get(form) || findElement(form, "input[type=submit], button[type=submit]"); if (button) { var _button = button, disabled = _button.disabled; button.disabled = false; @@ -909,6 +920,7 @@ button.click(); form.removeChild(button); } + submitButtonsByForm.delete(form); } function disable(input) { input.disabled = true; diff --git a/activestorage/app/controllers/active_storage/base_controller.rb b/activestorage/app/controllers/active_storage/base_controller.rb index 59312ac8df..b27d2bd8aa 100644 --- a/activestorage/app/controllers/active_storage/base_controller.rb +++ b/activestorage/app/controllers/active_storage/base_controller.rb @@ -1,10 +1,8 @@ # frozen_string_literal: true -# The base controller for all ActiveStorage controllers. +# The base class for all Active Storage controllers. class ActiveStorage::BaseController < ActionController::Base - protect_from_forgery with: :exception + include ActiveStorage::SetCurrent - before_action do - ActiveStorage::Current.host = request.base_url - end + protect_from_forgery with: :exception end diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index 75cc11d6ff..df3116afd7 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -3,16 +3,18 @@ # Serves files stored with the disk service in the same way that the cloud services do. # This means using expiring, signed URLs that are meant for immediate access, not permanent linking. # Always go through the BlobsController, or your own authenticated controller, rather than directly -# to the service url. +# to the service URL. class ActiveStorage::DiskController < ActiveStorage::BaseController skip_forgery_protection def show if key = decode_verified_key - serve_file disk_service.path_for(key), content_type: params[:content_type], disposition: params[:disposition] + serve_file disk_service.path_for(key[:key]), content_type: key[:content_type], disposition: key[:disposition] else head :not_found end + rescue Errno::ENOENT + head :not_found end def update @@ -59,6 +61,6 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController end def acceptable_content?(token) - token[:content_type] == request.content_type && token[:content_length] == request.content_length + token[:content_type] == request.content_mime_type && token[:content_length] == request.content_length end end diff --git a/activestorage/app/controllers/concerns/active_storage/set_current.rb b/activestorage/app/controllers/concerns/active_storage/set_current.rb new file mode 100644 index 0000000000..597afe7064 --- /dev/null +++ b/activestorage/app/controllers/concerns/active_storage/set_current.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Sets the <tt>ActiveStorage::Current.host</tt> attribute, which the disk service uses to generate URLs. +# Include this concern in custom controllers that call ActiveStorage::Blob#service_url, +# ActiveStorage::Variant#service_url, or ActiveStorage::Preview#service_url so the disk service can +# generate URLs using the same host, protocol, and base path as the current request. +module ActiveStorage::SetCurrent + extend ActiveSupport::Concern + + included do + before_action do + ActiveStorage::Current.host = request.base_url + end + end +end diff --git a/activestorage/app/javascript/activestorage/blob_record.js b/activestorage/app/javascript/activestorage/blob_record.js index ff847892b2..7fbe315f76 100644 --- a/activestorage/app/javascript/activestorage/blob_record.js +++ b/activestorage/app/javascript/activestorage/blob_record.js @@ -17,7 +17,12 @@ export class BlobRecord { this.xhr.setRequestHeader("Content-Type", "application/json") this.xhr.setRequestHeader("Accept", "application/json") this.xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest") - this.xhr.setRequestHeader("X-CSRF-Token", getMetaValue("csrf-token")) + + const csrfToken = getMetaValue("csrf-token") + if (csrfToken != undefined) { + this.xhr.setRequestHeader("X-CSRF-Token", csrfToken) + } + this.xhr.addEventListener("load", event => this.requestDidLoad(event)) this.xhr.addEventListener("error", event => this.requestDidError(event)) } diff --git a/activestorage/app/javascript/activestorage/file_checksum.js b/activestorage/app/javascript/activestorage/file_checksum.js index ffaec1a128..a9dbef69ea 100644 --- a/activestorage/app/javascript/activestorage/file_checksum.js +++ b/activestorage/app/javascript/activestorage/file_checksum.js @@ -39,7 +39,7 @@ export class FileChecksum { } readNextChunk() { - if (this.chunkIndex < this.chunkCount) { + if (this.chunkIndex < this.chunkCount || (this.chunkIndex == 0 && this.chunkCount == 0)) { const start = this.chunkIndex * this.chunkSize const end = Math.min(start + this.chunkSize, this.file.size) const bytes = fileSlice.call(this.file, start, end) diff --git a/activestorage/app/javascript/activestorage/ujs.js b/activestorage/app/javascript/activestorage/ujs.js index 08c535470d..98fcba60fa 100644 --- a/activestorage/app/javascript/activestorage/ujs.js +++ b/activestorage/app/javascript/activestorage/ujs.js @@ -2,16 +2,25 @@ import { DirectUploadsController } from "./direct_uploads_controller" import { findElement } from "./helpers" const processingAttribute = "data-direct-uploads-processing" +const submitButtonsByForm = new WeakMap let started = false export function start() { if (!started) { started = true + document.addEventListener("click", didClick, true) document.addEventListener("submit", didSubmitForm) document.addEventListener("ajax:before", didSubmitRemoteElement) } } +function didClick(event) { + const { target } = event + if ((target.tagName == "INPUT" || target.tagName == "BUTTON") && target.type == "submit" && target.form) { + submitButtonsByForm.set(target.form, target) + } +} + function didSubmitForm(event) { handleFormSubmissionEvent(event) } @@ -49,7 +58,8 @@ function handleFormSubmissionEvent(event) { } function submitForm(form) { - let button = findElement(form, "input[type=submit]") + let button = submitButtonsByForm.get(form) || findElement(form, "input[type=submit], button[type=submit]") + if (button) { const { disabled } = button button.disabled = false @@ -64,6 +74,7 @@ function submitForm(form) { button.click() form.removeChild(button) } + submitButtonsByForm.delete(form) } function disable(input) { diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb index 2a952f9f74..35d043d508 100644 --- a/activestorage/app/jobs/active_storage/analyze_job.rb +++ b/activestorage/app/jobs/active_storage/analyze_job.rb @@ -2,6 +2,10 @@ # Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later. class ActiveStorage::AnalyzeJob < ActiveStorage::BaseJob + queue_as { ActiveStorage.queues[:analysis] } + + retry_on ActiveStorage::IntegrityError, attempts: 10, wait: :exponentially_longer + def perform(blob) blob.analyze end diff --git a/activestorage/app/jobs/active_storage/base_job.rb b/activestorage/app/jobs/active_storage/base_job.rb index 6caab42a2d..7bc2064dc5 100644 --- a/activestorage/app/jobs/active_storage/base_job.rb +++ b/activestorage/app/jobs/active_storage/base_job.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true class ActiveStorage::BaseJob < ActiveJob::Base - queue_as { ActiveStorage.queue } end diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index b021b5f2d0..5ceb222005 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -2,7 +2,10 @@ # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveStorage::BaseJob - discard_on ActiveRecord::RecordNotFound, ActiveRecord::InvalidForeignKey + queue_as { ActiveStorage.queues[:purge] } + + discard_on ActiveRecord::RecordNotFound + retry_on ActiveRecord::Deadlocked, attempts: 10, wait: :exponentially_longer def perform(blob) blob.purge diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 1c4dc25094..874ba80ca8 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -3,9 +3,8 @@ require "active_support/core_ext/module/delegation" # Attachments associate records with blobs. Usually that's a one record-many blobs relationship, -# but it is possible to associate many different records with the same blob. If you're doing that, -# you'll want to declare with <tt>has_one/many_attached :thingy, dependent: false</tt>, so that destroying -# any one record won't destroy the blob as well. (Then you'll need to do your own garbage collecting, though). +# but it is possible to associate many different records with the same blob. A foreign-key constraint +# on the attachments table prevents blobs from being purged if they’re still attached to any records. class ActiveStorage::Attachment < ActiveRecord::Base self.table_name = "active_storage_attachments" @@ -20,13 +19,13 @@ class ActiveStorage::Attachment < ActiveRecord::Base # Synchronously deletes the attachment and {purges the blob}[rdoc-ref:ActiveStorage::Blob#purge]. def purge delete - blob.purge + blob&.purge end # Deletes the attachment and {enqueues a background job}[rdoc-ref:ActiveStorage::Blob#purge_later] to purge the blob. def purge_later delete - blob.purge_later + blob&.purge_later end private @@ -39,7 +38,7 @@ class ActiveStorage::Attachment < ActiveRecord::Base end def purge_dependent_blob_later - blob.purge_later if dependent == :purge_later + blob&.purge_later if dependent == :purge_later end @@ -47,3 +46,5 @@ class ActiveStorage::Attachment < ActiveRecord::Base record.attachment_reflections[name]&.options[:dependent] end end + +ActiveSupport.run_load_hooks :active_storage_attachment, ActiveStorage::Attachment diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index bf87598a66..c9fbafad1f 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -35,8 +35,12 @@ class ActiveStorage::Blob < ActiveRecord::Base scope :unattached, -> { left_joins(:attachments).where(ActiveStorage::Attachment.table_name => { blob_id: nil }) } + before_destroy(prepend: true) do + raise ActiveRecord::InvalidForeignKey if attachments.exists? + end + class << self - # You can used the signed ID of a blob to refer to it on the client side without fear of tampering. + # You can use the signed ID of a blob to refer to it on the client side without fear of tampering. # This is particularly helpful for direct uploads where the client-side needs to refer to the blob # that was created ahead of the upload itself on form submission. # @@ -75,6 +79,15 @@ class ActiveStorage::Blob < ActiveRecord::Base def create_before_direct_upload!(filename:, byte_size:, checksum:, content_type: nil, metadata: nil) create! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type, metadata: metadata end + + # To prevent problems with case-insensitive filesystems, especially in combination + # with databases which treat indices as case-sensitive, all blob keys generated are going + # to only contain the base-36 character alphabet and will therefore be lowercase. To maintain + # the same or higher amount of entropy as in the base-58 encoding used by `has_secure_token` + # the number of bytes used is increased to 28 from the standard 24 + def generate_unique_secure_token + SecureRandom.base36(28) + end end # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering. @@ -83,9 +96,10 @@ class ActiveStorage::Blob < ActiveRecord::Base ActiveStorage.verifier.generate(id, purpose: :blob_id) end - # Returns the key pointing to the file on the service that's associated with this blob. The key is in the - # standard secure-token format from Rails. So it'll look like: XTAPjJCJiuDrLk3TmwyJGpUo. This key is not intended - # to be revealed directly to the user. Always refer to blobs using the signed_id or a verified form of the key. + # Returns the key pointing to the file on the service that's associated with this blob. The key is the + # secure-token format from Rails in lower case. So it'll look like: xtapjjcjiudrlk3tmwyjgpuobabd. + # This key is not intended to be revealed directly to the user. + # Always refer to blobs using the signed_id or a verified form of the key. def key # We can't wait until the record is first saved to have a key for it self[:key] ||= self.class.generate_unique_secure_token @@ -126,8 +140,8 @@ class ActiveStorage::Blob < ActiveRecord::Base def service_url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline, filename: nil, **options) filename = ActiveStorage::Filename.wrap(filename || self.filename) - service.url key, expires_in: expires_in, filename: filename, content_type: content_type, - disposition: forcibly_serve_as_binary? ? :attachment : disposition, **options + service.url key, expires_in: expires_in, filename: filename, content_type: content_type_for_service_url, + disposition: forced_disposition_for_service_url || disposition, **options end # Returns a URL that can be used to directly upload a file for this blob on the service. This URL is intended to be @@ -166,7 +180,7 @@ class ActiveStorage::Blob < ActiveRecord::Base end def upload_without_unfurling(io) #:nodoc: - service.upload key, io, checksum: checksum + service.upload key, io, checksum: checksum, **service_metadata end # Downloads the file associated with this blob. If no block is given, the entire file is read into memory and returned. @@ -179,17 +193,18 @@ class ActiveStorage::Blob < ActiveRecord::Base # # The tempfile's name is prefixed with +ActiveStorage-+ and the blob's ID. Its extension matches that of the blob. # - # By default, the tempfile is created in <tt>Dir.tmpdir</tt>. Pass +tempdir:+ to create it in a different directory: + # By default, the tempfile is created in <tt>Dir.tmpdir</tt>. Pass +tmpdir:+ to create it in a different directory: # - # blob.open(tempdir: "/path/to/tmp") do |file| + # blob.open(tmpdir: "/path/to/tmp") do |file| # # ... # end # # The tempfile is automatically closed and unlinked after the given block is executed. # # Raises ActiveStorage::IntegrityError if the downloaded data does not match the blob's checksum. - def open(tempdir: nil, &block) - ActiveStorage::Downloader.new(self, tempdir: tempdir).download_blob_to_tempfile(&block) + def open(tmpdir: nil, &block) + service.open key, checksum: checksum, + name: [ "ActiveStorage-#{id}-", filename.extension_with_delimiter ], tmpdir: tmpdir, &block end @@ -207,6 +222,7 @@ class ActiveStorage::Blob < ActiveRecord::Base def purge destroy delete + rescue ActiveRecord::InvalidForeignKey end # Enqueues an ActiveStorage::PurgeJob to call #purge. This is the recommended way to purge blobs from a transaction, @@ -234,5 +250,29 @@ class ActiveStorage::Blob < ActiveRecord::Base ActiveStorage.content_types_to_serve_as_binary.include?(content_type) end - ActiveSupport.run_load_hooks(:active_storage_blob, self) + def allowed_inline? + ActiveStorage.content_types_allowed_inline.include?(content_type) + end + + def content_type_for_service_url + forcibly_serve_as_binary? ? ActiveStorage.binary_content_type : content_type + end + + def forced_disposition_for_service_url + if forcibly_serve_as_binary? || !allowed_inline? + :attachment + end + end + + def service_metadata + if forcibly_serve_as_binary? + { content_type: ActiveStorage.binary_content_type, disposition: :attachment, filename: filename } + elsif !allowed_inline? + { content_type: content_type, disposition: :attachment, filename: filename } + else + { content_type: content_type } + end + end end + +ActiveSupport.run_load_hooks :active_storage_blob, ActiveStorage::Blob diff --git a/activestorage/app/models/active_storage/blob/identifiable.rb b/activestorage/app/models/active_storage/blob/identifiable.rb index 049e45dc3e..924bd06131 100644 --- a/activestorage/app/models/active_storage/blob/identifiable.rb +++ b/activestorage/app/models/active_storage/blob/identifiable.rb @@ -2,7 +2,10 @@ module ActiveStorage::Blob::Identifiable def identify - update! content_type: identify_content_type, identified: true unless identified? + unless identified? + update! content_type: identify_content_type, identified: true + update_service_metadata + end end def identified? @@ -15,6 +18,14 @@ module ActiveStorage::Blob::Identifiable end def download_identifiable_chunk - service.download_chunk key, 0...4.kilobytes + if byte_size.positive? + service.download_chunk key, 0...4.kilobytes + else + "" + end + end + + def update_service_metadata + service.update_metadata key, service_metadata if service_metadata.any? end end diff --git a/activestorage/app/models/active_storage/blob/representable.rb b/activestorage/app/models/active_storage/blob/representable.rb index 03d5511481..32e8fcefdf 100644 --- a/activestorage/app/models/active_storage/blob/representable.rb +++ b/activestorage/app/models/active_storage/blob/representable.rb @@ -10,7 +10,7 @@ module ActiveStorage::Blob::Representable # Returns an ActiveStorage::Variant instance with the set of +transformations+ provided. This is only relevant for image # files, and it allows any image to be transformed for size, colors, and the like. Example: # - # avatar.variant(resize_to_fit: [100, 100]).processed.service_url + # avatar.variant(resize_to_limit: [100, 100]).processed.service_url # # This will create and process a variant of the avatar blob that's constrained to a height and width of 100px. # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations. @@ -18,7 +18,7 @@ module ActiveStorage::Blob::Representable # Frequently, though, you don't actually want to transform the variant right away. But rather simply refer to a # specific variant that can be created by a controller on-demand. Like so: # - # <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %> + # <%= image_tag Current.user.avatar.variant(resize_to_limit: [100, 100]) %> # # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController # can then produce on-demand. @@ -43,13 +43,13 @@ module ActiveStorage::Blob::Representable # from a non-image blob. Active Storage comes with built-in previewers for videos and PDF documents. The video previewer # extracts the first frame from a video and the PDF previewer extracts the first page from a PDF document. # - # blob.preview(resize_to_fit: [100, 100]).processed.service_url + # blob.preview(resize_to_limit: [100, 100]).processed.service_url # # Avoid processing previews synchronously in views. Instead, link to a controller action that processes them on demand. # Active Storage provides one, but you may want to create your own (for example, if you need authentication). Here’s # how to use the built-in version: # - # <%= image_tag video.preview(resize_to_fit: [100, 100]) %> + # <%= image_tag video.preview(resize_to_limit: [100, 100]) %> # # This method raises ActiveStorage::UnpreviewableError if no previewer accepts the receiving blob. To determine # whether a blob is accepted by any previewer, call ActiveStorage::Blob#previewable?. @@ -69,7 +69,7 @@ module ActiveStorage::Blob::Representable # Returns an ActiveStorage::Preview for a previewable blob or an ActiveStorage::Variant for a variable image blob. # - # blob.representation(resize_to_fit: [100, 100]).processed.service_url + # blob.representation(resize_to_limit: [100, 100]).processed.service_url # # Raises ActiveStorage::UnrepresentableError if the receiving blob is neither variable nor previewable. Call # ActiveStorage::Blob#representable? to determine whether a blob is representable. diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index bebb5e61b3..2a03e0173d 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -3,8 +3,6 @@ # Encapsulates a string representing a filename to provide convenient access to parts of it and sanitization. # A Filename instance is returned by ActiveStorage::Blob#filename, and is comparable so it can be used for sorting. class ActiveStorage::Filename - require_dependency "active_storage/filename/parameters" - include Comparable class << self @@ -60,10 +58,6 @@ class ActiveStorage::Filename @filename.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") end - def parameters #:nodoc: - Parameters.new self - end - # Returns the sanitized version of the filename. def to_s sanitized.to_s diff --git a/activestorage/app/models/active_storage/filename/parameters.rb b/activestorage/app/models/active_storage/filename/parameters.rb deleted file mode 100644 index fb9ea10e49..0000000000 --- a/activestorage/app/models/active_storage/filename/parameters.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -class ActiveStorage::Filename::Parameters #:nodoc: - attr_reader :filename - - def initialize(filename) - @filename = filename - end - - def combined - "#{ascii}; #{utf8}" - end - - TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/ - - def ascii - 'filename="' + percent_escape(I18n.transliterate(filename.sanitized), TRADITIONAL_ESCAPED_CHAR) + '"' - end - - RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/ - - def utf8 - "filename*=UTF-8''" + percent_escape(filename.sanitized, RFC_5987_ESCAPED_CHAR) - end - - def to_s - combined - end - - private - def percent_escape(string, pattern) - string.gsub(pattern) do |char| - char.bytes.map { |byte| "%%%02X" % byte }.join - end - end -end diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb index dd50494799..bb9d960443 100644 --- a/activestorage/app/models/active_storage/preview.rb +++ b/activestorage/app/models/active_storage/preview.rb @@ -38,7 +38,7 @@ class ActiveStorage::Preview # Processes the preview if it has not been processed yet. Returns the receiving Preview instance for convenience: # - # blob.preview(resize_to_fit: [100, 100]).processed.service_url + # blob.preview(resize_to_limit: [100, 100]).processed.service_url # # Processing a preview generates an image from its blob and attaches the preview image to the blob. Because the preview # image is stored with the blob, it is only generated once. diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 056fd2be99..bc0058967a 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "ostruct" + # Image blobs can have variants that are the result of a set of transformations applied to the original. # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the # original. @@ -25,7 +27,7 @@ # To refer to such a delayed on-demand variant, simply link to the variant through the resolved route provided # by Active Storage like so: # -# <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %> +# <%= image_tag Current.user.avatar.variant(resize_to_limit: [100, 100]) %> # # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController # can then produce on-demand. @@ -34,15 +36,15 @@ # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform # the transformations, upload the variant to the service, and return itself again. Example: # -# avatar.variant(resize_to_fit: [100, 100]).processed.service_url +# avatar.variant(resize_to_limit: [100, 100]).processed.service_url # # This will create and process a variant of the avatar blob that's constrained to a height and width of 100. # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations. # # You can combine any number of ImageMagick/libvips operations into a variant, as well as any macros provided by the -# ImageProcessing gem (such as +resize_to_fit+): +# ImageProcessing gem (such as +resize_to_limit+): # -# avatar.variant(resize_to_fit: [800, 800], monochrome: true, rotate: "-90") +# avatar.variant(resize_to_limit: [800, 800], monochrome: true, rotate: "-90") # # Visit the following links for a list of available ImageProcessing commands and ImageMagick/libvips operations: # @@ -51,7 +53,7 @@ # * {ImageProcessing::Vips}[https://github.com/janko-m/image_processing/blob/master/doc/vips.md#methods] # * {ruby-vips reference}[http://www.rubydoc.info/gems/ruby-vips/Vips/Image] class ActiveStorage::Variant - WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif ) + WEB_IMAGE_CONTENT_TYPES = %w[ image/png image/jpeg image/jpg image/gif ] attr_reader :blob, :variation delegate :service, to: :blob @@ -95,37 +97,35 @@ class ActiveStorage::Variant def process blob.open do |image| - transform image do |output| - upload output - end + transform(image) { |output| upload(output) } end end - - def filename - if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) - blob.filename - else - ActiveStorage::Filename.new("#{blob.filename.base}.png") - end + def transform(image, &block) + variation.transform(image, format: format, &block) end - def content_type - blob.content_type.presence_in(WEB_IMAGE_CONTENT_TYPES) || "image/png" + def upload(file) + service.upload(key, file) end - def transform(image) - format = "png" unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) - result = variation.transform(image, format: format) - begin - yield result - ensure - result.close! - end + def specification + @specification ||= + if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) + Specification.new \ + filename: blob.filename, + content_type: blob.content_type, + format: nil + else + Specification.new \ + filename: ActiveStorage::Filename.new("#{blob.filename.base}.png"), + content_type: "image/png", + format: "png" + end end - def upload(file) - service.upload(key, file) - end + delegate :filename, :content_type, :format, to: :specification + + class Specification < OpenStruct; end end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index ae1376a6cf..41b5a45f53 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -6,7 +6,7 @@ # 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: # -# ActiveStorage::Variation.new(resize_to_fit: [100, 100], monochrome: true, trim: true, rotate: "-90") +# ActiveStorage::Variation.new(resize_to_limit: [100, 100], monochrome: true, trim: true, rotate: "-90") # # The options map directly to {ImageProcessing}[https://github.com/janko-m/image_processing] commands. class ActiveStorage::Variation @@ -47,13 +47,9 @@ class ActiveStorage::Variation # saves the transformed image into a temporary file. If +format+ is specified # it will be the format of the result image, otherwise the result image # retains the source format. - def transform(file, format: nil) + def transform(file, format: nil, &block) ActiveSupport::Notifications.instrument("transform.active_storage") do - if processor - image_processing_transform(file, format) - else - mini_magick_transform(file, format) - end + transformer.transform(file, format: format, &block) end end @@ -63,63 +59,22 @@ class ActiveStorage::Variation end private - # Applies image transformations using the ImageProcessing gem. - def image_processing_transform(file, format) - operations = transformations.each_with_object([]) do |(name, argument), list| - if name.to_s == "combine_options" - ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.") - list.concat argument.keep_if { |key, value| value.present? }.to_a - elsif argument.present? - list << [name, argument] + def transformer + if ActiveStorage.variant_processor + begin + require "image_processing" + rescue LoadError + ActiveSupport::Deprecation.warn <<~WARNING.squish + Generating image variants will require the image_processing gem in Rails 6.1. + Please add `gem 'image_processing', '~> 1.2'` to your Gemfile. + WARNING + + ActiveStorage::Transformers::MiniMagickTransformer.new(transformations) + else + ActiveStorage::Transformers::ImageProcessingTransformer.new(transformations) end - end - - processor - .source(file) - .loader(page: 0) - .convert(format) - .apply(operations) - .call - end - - # Applies image transformations using the MiniMagick gem. - def mini_magick_transform(file, format) - image = MiniMagick::Image.new(file.path, file) - - transformations.each do |name, argument_or_subtransformations| - image.mogrify do |command| - if name.to_s == "combine_options" - argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument| - pass_transform_argument(command, subtransformation_name, subtransformation_argument) - end - else - pass_transform_argument(command, name, argument_or_subtransformations) - end - end - end - - image.format(format) if format - - image.tempfile.tap(&:open) - end - - # Returns the ImageProcessing processor class specified by `ActiveStorage.variant_processor`. - def processor - begin - require "image_processing" - rescue LoadError - ActiveSupport::Deprecation.warn("Using mini_magick gem directly is deprecated and will be removed in Rails 6.1. Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.") - return nil - end - - ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize) if ActiveStorage.variant_processor - end - - def pass_transform_argument(command, method, argument) - if argument == true - command.public_send(method) - elsif argument.present? - command.public_send(method, argument) + else + ActiveStorage::Transformers::MiniMagickTransformer.new(transformations) end end end |