From 3bf95f95130ec00194c7a7d5dd9d6c69f702214c Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sat, 16 Sep 2017 14:51:26 +0900 Subject: Don't expose Active Storage routes These routes are only used internally in Active Storage, and it seems that there is no need for the user to directly use them. Therefore, I think that routes should not be exposed to users. --- activestorage/config/routes.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'activestorage') diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index 168788475c..c3194887be 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Rails.application.routes.draw do - get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob + get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob, internal: true direct :rails_blob do |blob| route_for(:rails_service_blob, blob.signed_id, blob.filename) @@ -11,7 +11,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Attachment") { |attachment| route_for(:rails_blob, attachment.blob) } - get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation + get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation, internal: true direct :rails_variant do |variant| signed_blob_id = variant.blob.signed_id @@ -24,7 +24,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Variant") { |variant| route_for(:rails_variant, variant) } - get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service - put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service - post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads + get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service, internal: true + put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service, internal: true + post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads, internal: true end -- cgit v1.2.3 From 194a93385b08be857172b8eb6287d335c5adf1ea Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Sun, 17 Sep 2017 17:40:20 +0900 Subject: Add local option to Message form [ci skip] * MessagesController redirects to `GET /message/:id`. * It looks it don't expect XHR request. * `form_with` behaves for XHR by default. * I've added `local: true` option to `form_with`. --- activestorage/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/README.md b/activestorage/README.md index 17d98978bb..5b73740f70 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -63,7 +63,7 @@ end ``` ```erb -<%= form_with model: @message do |form| %> +<%= form_with model: @message, local: true do |form| %> <%= form.text_field :title, placeholder: "Title" %>
<%= form.text_area :content %>

-- cgit v1.2.3 From 7d14bda3a7c74ec0262fa021e641a4235f5405d5 Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Sun, 17 Sep 2017 10:06:47 +0900 Subject: Fix file missing in activestorage's example code [ci skip] * File.open("~/face.jpg") raise error: `Errno::ENOENT: No such file or directory @ rb_sysopen - ~/face.jpg` --- activestorage/README.md | 2 +- activestorage/lib/active_storage/attached/many.rb | 2 +- activestorage/lib/active_storage/attached/one.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activestorage') diff --git a/activestorage/README.md b/activestorage/README.md index 17d98978bb..f4aae4fc00 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -24,7 +24,7 @@ class User < ApplicationRecord end # Attach an avatar to the user. -user.avatar.attach(io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg") +user.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") # Does the user have an avatar? user.avatar.attached? # => true diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 59b7d7d559..1e0657c33c 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -17,7 +17,7 @@ module ActiveStorage # # 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("~/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpg") + # 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| diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index ac90f32d95..c66be08f58 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -18,7 +18,7 @@ module ActiveStorage # # 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("~/face.jpg"), filename: "face.jpg", content_type: "image/jpg") + # 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) if attached? && dependent == :purge_later -- cgit v1.2.3 From 0993cbe3e0564c8dfa8b258e3a88059d311a352b Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Mon, 18 Sep 2017 09:13:00 +0900 Subject: Remove unused require in ActiveStorage::Variation --- activestorage/app/models/active_storage/variation.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index bf269e2a8f..cf04a879eb 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -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. # -- cgit v1.2.3 From 704bf9b9c64a2f92c6e72d74350bd8f00aaca5c5 Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Sun, 17 Sep 2017 18:24:16 +0900 Subject: Add `with_attached_*` scope to `has_one_attached` macro * For avoiding N+1 problem, added `with_attached_*` scope to `has_one_attached` macro. --- activestorage/lib/active_storage/attached/macros.rb | 6 ++++++ activestorage/test/models/attachments_test.rb | 13 +++++++++++++ 2 files changed, 19 insertions(+) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index 35a081adc4..f0256718ac 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -12,6 +12,10 @@ module ActiveStorage # 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+ @@ -33,6 +37,8 @@ module ActiveStorage has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record 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 before_destroy { public_send(name).purge_later } end diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index ac346c0087..379ae0a416 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -84,6 +84,19 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase end 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") -- cgit v1.2.3 From 91edf754c4ccdb55bfebb2fcb1458ca0a4d769d9 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Wed, 20 Sep 2017 15:34:04 -0400 Subject: Flesh out ActiveStorage::Filename docs --- activestorage/app/models/active_storage/filename.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index dead6b6d33..8e3cd488a4 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. +# Encapsulates a string representing a filename to provide convenient access to parts of it sanitization. +# A Filename instance is returned by ActiveStorage::Blob#filename, and is comparable so it can be used for sorting. class ActiveStorage::Filename include Comparable @@ -9,23 +9,31 @@ class ActiveStorage::Filename @filename = filename end - # Returns the basename of the filename. + # Returns the part of the filename preceding any extension. # # ActiveStorage::Filename.new("racecar.jpg").base # => "racecar" + # ActiveStorage::Filename.new("racecar").base # => "racecar" + # ActiveStorage::Filename.new(".gitignore").base # => ".gitignore" def base File.basename @filename, extension_with_delimiter end - # Returns the extension with delimiter of the filename. + # Returns the extension of the filename (i.e. the substring following the last dot, excluding a dot at the + # beginning) with the dot that precedes it. If the filename has no extension, an empty string is returned. # # ActiveStorage::Filename.new("racecar.jpg").extension_with_delimiter # => ".jpg" + # ActiveStorage::Filename.new("racecar").extension_with_delimiter # => "" + # ActiveStorage::Filename.new(".gitignore").extension_with_delimiter # => "" def extension_with_delimiter File.extname @filename end - # Returns the extension without delimiter of the filename. + # Returns the extension of the filename (i.e. the substring following the last dot, excluding a dot at + # the beginning). If the filename has no extension, an empty string is returned. # # ActiveStorage::Filename.new("racecar.jpg").extension_without_delimiter # => "jpg" + # ActiveStorage::Filename.new("racecar").extension_without_delimiter # => "" + # ActiveStorage::Filename.new(".gitignore").extension_without_delimiter # => "" def extension_without_delimiter extension_with_delimiter.from(1).to_s end @@ -37,7 +45,7 @@ class ActiveStorage::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. + # Characters considered unsafe for storage (e.g. \, $, and the RTL override character) are replaced with a dash. def sanitized @filename.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") end -- cgit v1.2.3 From 4e68525e338685c1c77b76a488eb06af021f0e05 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Wed, 20 Sep 2017 15:35:01 -0400 Subject: Add missing word [ci skip] --- activestorage/app/models/active_storage/filename.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index 8e3cd488a4..79d55dc889 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Encapsulates a string representing a filename to provide convenient access to parts of it sanitization. +# 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 include Comparable -- cgit v1.2.3 From cadf6f85f3a44a3c7782e03b266af94c9fed42c0 Mon Sep 17 00:00:00 2001 From: yalab Date: Tue, 26 Sep 2017 17:56:38 +0900 Subject: Fixed broken `bundle exec rake install` --- activestorage/Rakefile | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activestorage') diff --git a/activestorage/Rakefile b/activestorage/Rakefile index aa71a65f6e..2aa4d2a76f 100644 --- a/activestorage/Rakefile +++ b/activestorage/Rakefile @@ -11,4 +11,6 @@ Rake::TestTask.new do |test| test.warning = false end +task :package + task default: :test -- cgit v1.2.3 From 7f3ba970b4892262169971ccafd09fd10e21f3df Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Wed, 27 Sep 2017 23:59:21 +0300 Subject: Set version in activestorage/package.json in proper format. `5.2.0.alpha` => `5.2.0-alpha` System versioning isn't compliant with npm. --- activestorage/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/package.json b/activestorage/package.json index b481295a8d..8e6dd1c57f 100644 --- a/activestorage/package.json +++ b/activestorage/package.json @@ -1,6 +1,6 @@ { "name": "activestorage", - "version": "5.2.0.alpha", + "version": "5.2.0-alpha", "description": "Attach cloud and local files in Rails applications", "main": "app/assets/javascripts/activestorage.js", "files": [ -- cgit v1.2.3 From d30586211b41e018869a1a3f4e3af778a31591db Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 28 Sep 2017 16:43:37 -0400 Subject: Preview PDFs and videos --- .../controllers/active_storage/blobs_controller.rb | 15 +--- .../controllers/active_storage/disk_controller.rb | 6 +- .../active_storage/previews_controller.rb | 12 +++ .../active_storage/variants_controller.rb | 19 +---- activestorage/app/models/active_storage/blob.rb | 47 +++++++++-- activestorage/app/models/active_storage/preview.rb | 90 +++++++++++++++++++++ activestorage/app/models/active_storage/variant.rb | 6 +- .../app/models/active_storage/variation.rb | 9 +++ activestorage/config/routes.rb | 13 +++ activestorage/lib/active_storage.rb | 2 + activestorage/lib/active_storage/engine.rb | 10 +++ activestorage/lib/active_storage/previewer.rb | 69 ++++++++++++++++ .../lib/active_storage/previewer/pdf_previewer.rb | 17 ++++ .../active_storage/previewer/video_previewer.rb | 17 ++++ activestorage/lib/active_storage/service.rb | 7 ++ .../service/azure_storage_service.rb | 2 +- .../lib/active_storage/service/disk_service.rb | 5 +- .../lib/active_storage/service/gcs_service.rb | 11 ++- .../lib/active_storage/service/s3_service.rb | 2 +- .../test/controllers/blobs_controller_test.rb | 2 +- .../test/controllers/previews_controller_test.rb | 24 ++++++ .../test/controllers/variants_controller_test.rb | 4 +- activestorage/test/fixtures/files/report.pdf | Bin 0 -> 13469 bytes activestorage/test/fixtures/files/video.mp4 | Bin 0 -> 275433 bytes activestorage/test/models/preview_test.rb | 38 +++++++++ activestorage/test/models/variant_test.rb | 6 +- activestorage/test/previewer/pdf_previewer_test.rb | 20 +++++ .../test/previewer/video_previewer_test.rb | 20 +++++ .../test/service/azure_storage_service_test.rb | 2 +- activestorage/test/service/disk_service_test.rb | 2 +- activestorage/test/service/gcs_service_test.rb | 4 +- activestorage/test/service/mirror_service_test.rb | 6 +- activestorage/test/service/s3_service_test.rb | 2 +- activestorage/test/template/image_tag_test.rb | 8 +- activestorage/test/test_helper.rb | 7 +- 35 files changed, 436 insertions(+), 68 deletions(-) create mode 100644 activestorage/app/controllers/active_storage/previews_controller.rb create mode 100644 activestorage/app/models/active_storage/preview.rb create mode 100644 activestorage/lib/active_storage/previewer.rb create mode 100644 activestorage/lib/active_storage/previewer/pdf_previewer.rb create mode 100644 activestorage/lib/active_storage/previewer/video_previewer.rb create mode 100644 activestorage/test/controllers/previews_controller_test.rb create mode 100644 activestorage/test/fixtures/files/report.pdf create mode 100644 activestorage/test/fixtures/files/video.mp4 create mode 100644 activestorage/test/models/preview_test.rb create mode 100644 activestorage/test/previewer/pdf_previewer_test.rb create mode 100644 activestorage/test/previewer/video_previewer_test.rb (limited to 'activestorage') diff --git a/activestorage/app/controllers/active_storage/blobs_controller.rb b/activestorage/app/controllers/active_storage/blobs_controller.rb index 00aa8567c8..a17e3852f9 100644 --- a/activestorage/app/controllers/active_storage/blobs_controller.rb +++ b/activestorage/app/controllers/active_storage/blobs_controller.rb @@ -6,20 +6,11 @@ # authenticated redirection controller. class ActiveStorage::BlobsController < ActionController::Base def show - if blob = find_signed_blob - expires_in 5.minutes # service_url defaults to 5 minutes - redirect_to blob.service_url(disposition: disposition_param) + if blob = ActiveStorage::Blob.find_signed(params[:signed_id]) + expires_in ActiveStorage::Blob.service.url_expires_in + redirect_to blob.service_url(disposition: params[:disposition]) else head :not_found end end - - private - def find_signed_blob - ActiveStorage::Blob.find_signed(params[:signed_id]) - end - - def disposition_param - params[:disposition].presence_in(%w( inline attachment )) || "inline" - end end diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index 41e6d61bff..a4fd427cb2 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -8,7 +8,7 @@ class ActiveStorage::DiskController < ActionController::Base def show if key = decode_verified_key send_data disk_service.download(key), - disposition: disposition_param, content_type: params[:content_type] + disposition: params[:disposition], content_type: params[:content_type] else head :not_found end @@ -38,10 +38,6 @@ class ActiveStorage::DiskController < ActionController::Base ActiveStorage.verifier.verified(params[:encoded_key], purpose: :blob_key) end - def disposition_param - params[:disposition].presence || "inline" - end - def decode_verified_token ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token) diff --git a/activestorage/app/controllers/active_storage/previews_controller.rb b/activestorage/app/controllers/active_storage/previews_controller.rb new file mode 100644 index 0000000000..9e8cf27b6e --- /dev/null +++ b/activestorage/app/controllers/active_storage/previews_controller.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class ActiveStorage::PreviewsController < ActionController::Base + def show + if blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id]) + expires_in ActiveStorage::Blob.service.url_expires_in + redirect_to ActiveStorage::Preview.new(blob, params[:variation_key]).processed.service_url(disposition: params[:disposition]) + else + head :not_found + end + end +end diff --git a/activestorage/app/controllers/active_storage/variants_controller.rb b/activestorage/app/controllers/active_storage/variants_controller.rb index 02e3010626..dc5e78ecc0 100644 --- a/activestorage/app/controllers/active_storage/variants_controller.rb +++ b/activestorage/app/controllers/active_storage/variants_controller.rb @@ -6,24 +6,11 @@ # authenticated redirection controller. class ActiveStorage::VariantsController < ActionController::Base def show - if blob = find_signed_blob - expires_in 5.minutes # service_url defaults to 5 minutes - redirect_to ActiveStorage::Variant.new(blob, decoded_variation).processed.service_url(disposition: disposition_param) + if blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id]) + expires_in ActiveStorage::Blob.service.url_expires_in + redirect_to ActiveStorage::Variant.new(blob, params[:variation_key]).processed.service_url(disposition: params[:disposition]) else head :not_found end end - - private - def find_signed_blob - ActiveStorage::Blob.find_signed(params[:signed_blob_id]) - end - - def decoded_variation - ActiveStorage::Variation.decode(params[:variation_key]) - end - - def disposition_param - params[:disposition].presence_in(%w( inline attachment )) || "inline" - end end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index e6cf08ce83..7477b09d09 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -14,6 +14,8 @@ # update a blob's metadata on a subsequent pass, but you should not update the key or change the uploaded file. # If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one. class ActiveStorage::Blob < ActiveRecord::Base + class UnpreviewableError < StandardError; end + self.table_name = "active_storage_blobs" has_secure_token :key @@ -21,6 +23,8 @@ class ActiveStorage::Blob < ActiveRecord::Base class_attribute :service + has_one_attached :preview_image + class << self # You can used 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 @@ -101,19 +105,18 @@ class ActiveStorage::Blob < ActiveRecord::Base content_type.start_with?("text") end - # Returns an ActiveStorage::Variant instance with the set of +transformations+ - # passed in. This is only relevant for image files, and it allows any image to - # be transformed for size, colors, and the like. Example: + # 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: "100x100").processed.service_url # - # This will create and process a variant of the avatar blob that's constrained to a height and width of 100. + # 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. # # 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 url_for(Current.user.avatar.variant(resize: "100x100")) %> + # <%= image_tag Current.user.avatar.variant(resize: "100x100") %> # # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::VariantsController # can then produce on-demand. @@ -122,17 +125,45 @@ class ActiveStorage::Blob < ActiveRecord::Base end + # Returns an ActiveStorage::Preview instance with the set of +transformations+ provided. A preview is an image generated + # 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: "100x100").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: "100x100") %> + # + # This method raises ActiveStorage::Blob::UnpreviewableError if no previewer accepts the receiving blob. To determine + # whether a blob is accepted by any previewer, call ActiveStorage::Blob#previewable?. + def preview(transformations) + if previewable? + ActiveStorage::Preview.new(self, ActiveStorage::Variation.new(transformations)) + else + raise UnpreviewableError + end + end + + # Returns true if any registered previewer accepts the blob. By default, this will return true for videos and PDF documents. + def previewable? + ActiveStorage.previewers.any? { |klass| klass.accept?(self) } + end + + # Returns the URL of the blob on the service. This URL is intended to be short-lived for security and not used directly # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL. # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And # it allows permanent URLs that redirect to the +service_url+ to be cached in the view. - def service_url(expires_in: 5.minutes, disposition: :inline) - service.url key, expires_in: expires_in, disposition: "#{disposition}; #{filename.parameters}", filename: filename, content_type: content_type + def service_url(expires_in: service.url_expires_in, disposition: "inline") + service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type 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 # short-lived for security and only generated on-demand by the client-side JavaScript responsible for doing the uploading. - def service_url_for_direct_upload(expires_in: 5.minutes) + def service_url_for_direct_upload(expires_in: service.url_expires_in) service.url_for_direct_upload key, expires_in: expires_in, content_type: content_type, content_length: byte_size, checksum: checksum end diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb new file mode 100644 index 0000000000..42c4bbc5a4 --- /dev/null +++ b/activestorage/app/models/active_storage/preview.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +# Some non-image blobs can be previewed: that is, they can be presented as images. A video blob can be previewed by +# extracting its first frame, and a PDF blob can be previewed by extracting its first page. +# +# A previewer extracts a preview image from a blob. Active Storage provides previewers for videos and PDFs: +# ActiveStorage::Previewer::VideoPreviewer and ActiveStorage::Previewer::PDFPreviewer. Build custom previewers by +# subclassing ActiveStorage::Previewer and implementing the requisite methods. Consult the ActiveStorage::Previewer +# documentation for more details on what's required of previewers. +# +# To choose the previewer for a blob, Active Storage calls +accept?+ on each registered previewer in order. It uses the +# first previewer for which +accept?+ returns true when given the blob. In a Rails application, add or remove previewers +# by manipulating +Rails.application.config.active_storage.previewers+ in an initializer: +# +# Rails.application.config.active_storage.previewers +# # => [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] +# +# # Add a custom previewer for Microsoft Office documents: +# Rails.application.config.active_storage.previewers << DOCXPreviewer +# # => [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer, DOCXPreviewer ] +# +# Outside of a Rails application, modify +ActiveStorage.previewers+ instead. +# +# The built-in previewers rely on third-party system libraries: +# +# * {ffmpeg}[https://www.ffmpeg.org] +# * {mupdf}[https://mupdf.com] +# +# These libraries are not provided by Rails. You must install them yourself to use the built-in previewers. Before you +# install and use third-party software, make sure you understand the licensing implications of doing so. +class ActiveStorage::Preview + class UnprocessedError < StandardError; end + + attr_reader :blob, :variation + + def initialize(blob, variation_or_variation_key) + @blob, @variation = blob, ActiveStorage::Variation.wrap(variation_or_variation_key) + end + + # Processes the preview if it has not been processed yet. Returns the receiving Preview instance for convenience: + # + # blob.preview(resize: "100x100").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. + def processed + process unless processed? + self + end + + # Returns the blob's attached preview image. + def image + blob.preview_image + end + + # Returns the URL of the preview's variant on the service. Raises ActiveStorage::Preview::UnprocessedError if the + # preview has not been processed yet. + # + # This method synchronously processes a variant of the preview image, so do not call it in views. Instead, generate + # a stable URL that redirects to the short-lived URL returned by this method. + def service_url(**options) + if processed? + variant.service_url(options) + else + raise UnprocessedError + end + end + + private + def processed? + image.attached? + end + + def process + previewer.preview { |attachable| image.attach(attachable) } + end + + def variant + ActiveStorage::Variant.new(image, variation).processed + end + + + def previewer + previewer_class.new(blob) + end + + def previewer_class + ActiveStorage.previewers.detect { |klass| klass.accept?(blob) } + end +end diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 02bf32b352..54685b4c0e 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -38,8 +38,8 @@ class ActiveStorage::Variant attr_reader :blob, :variation delegate :service, to: :blob - def initialize(blob, variation) - @blob, @variation = blob, variation + def initialize(blob, variation_or_variation_key) + @blob, @variation = blob, ActiveStorage::Variation.wrap(variation_or_variation_key) end # Returns the variant instance itself after it's been processed or an existing processing has been found on the service. @@ -61,7 +61,7 @@ class ActiveStorage::Variant # Use url_for(variant) (or the implied form, like +link_to variant+ or +redirect_to variant+) to get the stable URL # for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +service_call+ method # for its redirection. - def service_url(expires_in: 5.minutes, disposition: :inline) + def service_url(expires_in: service.url_expires_in, disposition: :inline) service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index cf04a879eb..df2643442a 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -13,6 +13,15 @@ class ActiveStorage::Variation attr_reader :transformations class << self + def wrap(variation_or_key) + case variation_or_key + when self + variation_or_key + else + decode variation_or_key + end + end + # Returns a variation instance with the transformations that were encoded by +encode+. def decode(key) new ActiveStorage.verifier.verify(key, purpose: :variation) diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index c3194887be..c659e079fd 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -24,6 +24,19 @@ Rails.application.routes.draw do resolve("ActiveStorage::Variant") { |variant| route_for(:rails_variant, variant) } + get "/rails/active_storage/previews/:signed_blob_id/:variation_key/*filename" => "active_storage/previews#show", as: :rails_blob_preview, internal: true + + direct :rails_preview do |preview| + signed_blob_id = preview.blob.signed_id + variation_key = preview.variation.key + filename = preview.blob.filename + + route_for(:rails_blob_preview, signed_blob_id, variation_key, filename) + end + + resolve("ActiveStorage::Preview") { |preview| route_for(:rails_preview, preview) } + + get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service, internal: true put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service, internal: true post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads, internal: true diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index ccc1d4a163..44d9c25504 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -33,6 +33,8 @@ module ActiveStorage autoload :Attached autoload :Service + autoload :Previewer mattr_accessor :verifier + mattr_accessor :previewers, default: [] end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 590a36a30a..335eae8dd8 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -3,11 +3,15 @@ require "rails" require "active_storage" +require "active_storage/previewer/pdf_previewer" +require "active_storage/previewer/video_previewer" + module ActiveStorage class Engine < Rails::Engine # :nodoc: isolate_namespace ActiveStorage config.active_storage = ActiveSupport::OrderedOptions.new + config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] config.eager_load_namespaces << ActiveStorage @@ -59,5 +63,11 @@ module ActiveStorage end end end + + initializer "active_storage.previewers" do + config.after_initialize do |app| + ActiveStorage.previewers = app.config.active_storage.previewers || [] + end + end end end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb new file mode 100644 index 0000000000..c91f64ac65 --- /dev/null +++ b/activestorage/lib/active_storage/previewer.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module ActiveStorage + # This is an abstract base class for previewers, which generate images from blobs. See + # ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of + # concrete subclasses. + class Previewer + attr_reader :blob + + # Implement this method in a concrete subclass. Have it return true when given a blob from which + # the previewer can generate an image. + def self.accept?(blob) + false + end + + def initialize(blob) + @blob = blob + end + + # Override this method in a concrete subclass. Have it yield an attachable preview image (i.e. + # anything accepted by ActiveStorage::Attached::One#attach). + def preview + raise NotImplementedError + end + + private + # Downloads the blob to a new tempfile. Yields the tempfile. + # + # Use this method to get a tempfile that you can provide to a drawing command. + def open # :doc: + Tempfile.open("input") do |file| + download_blob_to file + yield file + end + end + + def download_blob_to(file) + file.binmode + blob.download { |chunk| file.write(chunk) } + file.rewind + end + + + # Executes a system command, capturing its binary output in a tempfile. Yields the tempfile. + # + # Use this method to shell out to system libraries (e.g. mupdf or ffmpeg) for preview image + # generation. The resulting tempfile can be used as the +:io+ value in an attachable Hash: + # + # def preview + # open do |input| + # draw "my-drawing-command", input.path, "--format", "png", "-" do |output| + # yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" + # end + # end + # end + def draw(*argv) # :doc: + Tempfile.open("output") do |file| + capture *argv, to: file + yield file + end + end + + def capture(*argv, to:) + to.binmode + IO.popen(argv) { |out| IO.copy_stream(out, to) } + to.rewind + end + end +end diff --git a/activestorage/lib/active_storage/previewer/pdf_previewer.rb b/activestorage/lib/active_storage/previewer/pdf_previewer.rb new file mode 100644 index 0000000000..31a2a8f120 --- /dev/null +++ b/activestorage/lib/active_storage/previewer/pdf_previewer.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ActiveStorage + class Previewer::PDFPreviewer < Previewer + def self.accept?(blob) + blob.content_type == "application/pdf" + end + + def preview + open do |input| + draw "mutool", "draw", "-F", "png", "-o", "-", input.path, "1" do |output| + yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" + end + end + end + end +end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb new file mode 100644 index 0000000000..840d87f100 --- /dev/null +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ActiveStorage + class Previewer::VideoPreviewer < Previewer + def self.accept?(blob) + blob.video? + end + + def preview + open do |input| + draw "ffmpeg", "-i", input.path, "-y", "-vcodec", "png", "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-" do |output| + yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" + end + end + end + end +end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index b80fdea1ab..1f012da1e7 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -4,6 +4,7 @@ require "active_storage/log_subscriber" module ActiveStorage class IntegrityError < StandardError; end + # Abstract class serving as an interface for concrete services. # # The available services are: @@ -42,6 +43,8 @@ module ActiveStorage class_attribute :logger + class_attribute :url_expires_in, default: 5.minutes + class << self # Configure an Active Storage service by name from a set of configurations, # typically loaded from a YAML file. The Active Storage engine uses this @@ -113,5 +116,9 @@ module ActiveStorage # ActiveStorage::Service::DiskService => Disk self.class.name.split("::").third.remove("Service") end + + def content_disposition_with(type: "inline", filename:) + (type.to_s.presence_in(%w( attachment inline )) || "inline") + "; #{filename.parameters}" + end end end diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 895cc9c2f1..27dd192ce6 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -66,7 +66,7 @@ module ActiveStorage URI(base_url), false, permissions: "r", expiry: format_expiry(expires_in), - content_disposition: disposition, + content_disposition: content_disposition_with(type: disposition, filename: filename), content_type: content_type ).to_s diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index f600753a08..52eaba4e7b 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -64,9 +64,10 @@ module ActiveStorage if defined?(Rails.application) Rails.application.routes.url_helpers.rails_disk_service_path \ verified_key_with_expiration, - filename: filename, disposition: disposition, content_type: content_type + filename: filename, disposition: content_disposition_with(type: disposition, filename: filename), content_type: content_type else - "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?content_type=#{content_type}&disposition=#{disposition}" + "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?content_type=#{content_type}" \ + "&disposition=#{content_disposition_with(type: disposition, filename: filename)}" end payload[:url] = generated_url diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 685dd61a0a..b4ffeeeb8a 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -24,12 +24,17 @@ module ActiveStorage end end - # FIXME: Add streaming when given a block + # FIXME: Download in chunks when given a block. def download(key) instrument :download, key do io = file_for(key).download io.rewind - io.read + + if block_given? + yield io.read + else + io.read + end end end @@ -54,7 +59,7 @@ module ActiveStorage def url(key, expires_in:, filename:, content_type:, disposition:) instrument :url, key do |payload| generated_url = file_for(key).signed_url expires: expires_in, query: { - "response-content-disposition" => disposition, + "response-content-disposition" => content_disposition_with(type: disposition, filename: filename), "response-content-type" => content_type } diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index e074269353..3e93cdd072 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -55,7 +55,7 @@ module ActiveStorage def url(key, expires_in:, filename:, disposition:, content_type:) instrument :url, key do |payload| generated_url = object_for(key).presigned_url :get, expires_in: expires_in.to_i, - response_content_disposition: disposition, + response_content_disposition: content_disposition_with(type: disposition, filename: filename), response_content_type: content_type payload[:url] = generated_url diff --git a/activestorage/test/controllers/blobs_controller_test.rb b/activestorage/test/controllers/blobs_controller_test.rb index c37b9c8a10..97177e64c2 100644 --- a/activestorage/test/controllers/blobs_controller_test.rb +++ b/activestorage/test/controllers/blobs_controller_test.rb @@ -5,7 +5,7 @@ require "database/setup" class ActiveStorage::BlobsControllerTest < ActionDispatch::IntegrationTest setup do - @blob = create_image_blob filename: "racecar.jpg" + @blob = create_file_blob filename: "racecar.jpg" end test "showing blob utilizes browser caching" do diff --git a/activestorage/test/controllers/previews_controller_test.rb b/activestorage/test/controllers/previews_controller_test.rb new file mode 100644 index 0000000000..c3151a710e --- /dev/null +++ b/activestorage/test/controllers/previews_controller_test.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::PreviewsControllerTest < ActionDispatch::IntegrationTest + setup do + @blob = create_file_blob filename: "report.pdf", content_type: "application/pdf" + end + + test "showing preview inline" do + get rails_blob_preview_url( + filename: @blob.filename, + signed_blob_id: @blob.signed_id, + variation_key: ActiveStorage::Variation.encode(resize: "100x100")) + + assert @blob.preview_image.attached? + assert_redirected_to(/report\.png\?.*disposition=inline/) + + image = read_image(@blob.preview_image.variant(resize: "100x100")) + assert_equal 77, image.width + assert_equal 100, image.height + end +end diff --git a/activestorage/test/controllers/variants_controller_test.rb b/activestorage/test/controllers/variants_controller_test.rb index 0a049f3bc4..6c70d73786 100644 --- a/activestorage/test/controllers/variants_controller_test.rb +++ b/activestorage/test/controllers/variants_controller_test.rb @@ -5,7 +5,7 @@ require "database/setup" class ActiveStorage::VariantsControllerTest < ActionDispatch::IntegrationTest setup do - @blob = create_image_blob filename: "racecar.jpg" + @blob = create_file_blob filename: "racecar.jpg" end test "showing variant inline" do @@ -16,7 +16,7 @@ class ActiveStorage::VariantsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to(/racecar\.jpg\?.*disposition=inline/) - image = read_image_variant(@blob.variant(resize: "100x100")) + image = read_image(@blob.variant(resize: "100x100")) assert_equal 100, image.width assert_equal 67, image.height end diff --git a/activestorage/test/fixtures/files/report.pdf b/activestorage/test/fixtures/files/report.pdf new file mode 100644 index 0000000000..cccb9b5d64 Binary files /dev/null and b/activestorage/test/fixtures/files/report.pdf differ diff --git a/activestorage/test/fixtures/files/video.mp4 b/activestorage/test/fixtures/files/video.mp4 new file mode 100644 index 0000000000..8fb1c5b24d Binary files /dev/null and b/activestorage/test/fixtures/files/video.mp4 differ diff --git a/activestorage/test/models/preview_test.rb b/activestorage/test/models/preview_test.rb new file mode 100644 index 0000000000..317a2d5c58 --- /dev/null +++ b/activestorage/test/models/preview_test.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::PreviewTest < ActiveSupport::TestCase + test "previewing a PDF" do + blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") + preview = blob.preview(resize: "640x280").processed + assert preview.image.attached? + assert_equal "report.png", preview.image.filename.to_s + assert_equal "image/png", preview.image.content_type + + image = read_image(preview.image) + assert_equal 612, image.width + assert_equal 792, image.height + end + + test "previewing an MP4 video" do + blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") + preview = blob.preview(resize: "640x280").processed + assert preview.image.attached? + assert_equal "video.png", preview.image.filename.to_s + assert_equal "image/png", preview.image.content_type + + image = read_image(preview.image) + assert_equal 640, image.width + assert_equal 480, image.height + end + + test "previewing an unpreviewable blob" do + blob = create_file_blob + + assert_raises ActiveStorage::Blob::UnpreviewableError do + blob.preview resize: "640x280" + end + end +end diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index ca112ab907..d7cbef4e55 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -5,14 +5,14 @@ require "database/setup" class ActiveStorage::VariantTest < ActiveSupport::TestCase setup do - @blob = create_image_blob filename: "racecar.jpg" + @blob = create_file_blob filename: "racecar.jpg" end test "resized variation" do variant = @blob.variant(resize: "100x100").processed assert_match(/racecar\.jpg/, variant.service_url) - image = read_image_variant(variant) + image = read_image(variant) assert_equal 100, image.width assert_equal 67, image.height end @@ -21,7 +21,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase variant = @blob.variant(resize: "100x100", monochrome: true).processed assert_match(/racecar\.jpg/, variant.service_url) - image = read_image_variant(variant) + image = read_image(variant) assert_equal 100, image.width assert_equal 67, image.height assert_match(/Gray/, image.colorspace) diff --git a/activestorage/test/previewer/pdf_previewer_test.rb b/activestorage/test/previewer/pdf_previewer_test.rb new file mode 100644 index 0000000000..60f075d1b2 --- /dev/null +++ b/activestorage/test/previewer/pdf_previewer_test.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require "active_storage/previewer/pdf_previewer" + +class ActiveStorage::Previewer::PDFPreviewerTest < ActiveSupport::TestCase + setup do + @blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") + end + + test "previewing a PDF document" do + ActiveStorage::Previewer::PDFPreviewer.new(@blob).preview do |attachable| + assert_equal "image/png", attachable[:content_type] + assert_equal "report.png", attachable[:filename] + + image = MiniMagick::Image.read(attachable[:io]) + assert_equal 612, image.width + assert_equal 792, image.height + end + end +end diff --git a/activestorage/test/previewer/video_previewer_test.rb b/activestorage/test/previewer/video_previewer_test.rb new file mode 100644 index 0000000000..967d5d5ba9 --- /dev/null +++ b/activestorage/test/previewer/video_previewer_test.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require "active_storage/previewer/video_previewer" + +class ActiveStorage::Previewer::VideoPreviewerTest < ActiveSupport::TestCase + setup do + @blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") + end + + test "previewing an MP4 video" do + ActiveStorage::Previewer::VideoPreviewer.new(@blob).preview do |attachable| + assert_equal "image/png", attachable[:content_type] + assert_equal "video.png", attachable[:filename] + + image = MiniMagick::Image.read(attachable[:io]) + assert_equal 640, image.width + assert_equal 480, image.height + end + end +end diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 4729bdfbc5..4b7e70b8b1 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -11,7 +11,7 @@ if SERVICE_CONFIGURATIONS[:azure] test "signed URL generation" do url = @service.url(FIXTURE_KEY, expires_in: 5.minutes, - disposition: "inline; filename=\"avatar.png\"", filename: "avatar.png", content_type: "image/png") + disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") assert_match(/(\S+)&rscd=inline%3B\+filename%3D%22avatar.png%22&rsct=image%2Fpng/, url) assert_match SERVICE_CONFIGURATIONS[:azure][:container], url diff --git a/activestorage/test/service/disk_service_test.rb b/activestorage/test/service/disk_service_test.rb index e07d1d88bc..4a6361b920 100644 --- a/activestorage/test/service/disk_service_test.rb +++ b/activestorage/test/service/disk_service_test.rb @@ -9,6 +9,6 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase test "url generation" do 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")) + @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")) end end diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index f664cee90b..5566c664a9 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -34,10 +34,10 @@ if SERVICE_CONFIGURATIONS[:gcs] test "signed URL generation" do freeze_time do url = SERVICE.bucket.signed_url(FIXTURE_KEY, expires: 120) + - "&response-content-disposition=inline%3B+filename%3D%22test.txt%22" + + "&response-content-disposition=inline%3B+filename%3D%22test.txt%22%3B+filename%2A%3DUTF-8%27%27test.txt" + "&response-content-type=text%2Fplain" - assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: "inline; filename=\"test.txt\"", filename: "test.txt", content_type: "text/plain") + assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") end end end diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index 93e86eff70..92101b1282 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -47,9 +47,11 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase end test "URL generation in primary service" do + filename = ActiveStorage::Filename.new("test.txt") + freeze_time do - assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt", content_type: "text/plain"), - @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt", content_type: "text/plain") + assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain"), + @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain") end end diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index c07d6396b1..c3818422aa 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -33,7 +33,7 @@ if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].pr test "signed URL generation" do url = @service.url(FIXTURE_KEY, expires_in: 5.minutes, - disposition: "inline; filename=\"avatar.png\"", filename: "avatar.png", content_type: "image/png") + disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") assert_match(/s3\.(\S+)?amazonaws.com.*response-content-disposition=inline.*avatar\.png.*response-content-type=image%2Fpng/, url) assert_match SERVICE_CONFIGURATIONS[:s3][:bucket], url diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb index dedc58452e..80f183e0e3 100644 --- a/activestorage/test/template/image_tag_test.rb +++ b/activestorage/test/template/image_tag_test.rb @@ -7,7 +7,7 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase tests ActionView::Helpers::AssetTagHelper setup do - @blob = create_image_blob filename: "racecar.jpg" + @blob = create_file_blob filename: "racecar.jpg" end test "blob" do @@ -19,6 +19,12 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase assert_dom_equal %(), image_tag(variant) end + test "preview" do + blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") + preview = blob.preview(resize: "100x100") + assert_dom_equal %(), image_tag(preview) + end + test "attachment" do attachment = ActiveStorage::Attachment.new(blob: @blob) assert_dom_equal %(), image_tag(attachment) diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 2a969fa326..dd37239060 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -6,6 +6,7 @@ require "bundler/setup" require "active_support" require "active_support/test_case" require "active_support/testing/autorun" +require "mini_magick" begin require "byebug" @@ -44,7 +45,7 @@ class ActiveSupport::TestCase ActiveStorage::Blob.create_after_upload! io: StringIO.new(data), filename: filename, content_type: content_type end - def create_image_blob(filename: "racecar.jpg", content_type: "image/jpeg") + def create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") ActiveStorage::Blob.create_after_upload! \ io: file_fixture(filename).open, filename: filename, content_type: content_type @@ -54,8 +55,8 @@ class ActiveSupport::TestCase ActiveStorage::Blob.create_before_direct_upload! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type end - def read_image_variant(variant) - MiniMagick::Image.open variant.service.send(:path_for, variant.key) + def read_image(blob_or_variant) + MiniMagick::Image.open blob_or_variant.service.send(:path_for, blob_or_variant.key) end end -- cgit v1.2.3 From 83b7cb3a4711794ca1716d1b593643d2309f0019 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 30 Sep 2017 05:39:27 +0900 Subject: Fix "warning: `*' interpreted as argument prefix" --- activestorage/lib/active_storage/previewer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index c91f64ac65..930b376067 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -55,7 +55,7 @@ module ActiveStorage # end def draw(*argv) # :doc: Tempfile.open("output") do |file| - capture *argv, to: file + capture(*argv, to: file) yield file end end -- cgit v1.2.3 From 325c06fbc499aa4da1ce50d9b85dbf5c6ed3321e Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 30 Sep 2017 05:48:23 +0900 Subject: Fix `test "signed URL generation"` failure https://travis-ci.org/rails/rails/jobs/281044755#L5582-L5586 --- activestorage/test/service/azure_storage_service_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 4b7e70b8b1..be31bbe858 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -13,7 +13,7 @@ if SERVICE_CONFIGURATIONS[:azure] url = @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") - assert_match(/(\S+)&rscd=inline%3B\+filename%3D%22avatar.png%22&rsct=image%2Fpng/, url) + assert_match(/(\S+)&rscd=inline%3B\+filename%3D%22avatar\.png%22%3B\+filename\*%3DUTF-8%27%27avatar\.png&rsct=image%2Fpng/, url) assert_match SERVICE_CONFIGURATIONS[:azure][:container], url end end -- cgit v1.2.3 From 45ed61ac4731eb91f39d3762889dce0da899af45 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Tue, 3 Oct 2017 08:27:21 -0500 Subject: Associate blobs with their attachments --- activestorage/app/models/active_storage/blob.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 7477b09d09..ff785d4f61 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -23,6 +23,8 @@ class ActiveStorage::Blob < ActiveRecord::Base class_attribute :service + has_many :attachments + has_one_attached :preview_image class << self -- cgit v1.2.3 From 53c16188924c7f4179555cdc6ae6911e44743d60 Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Wed, 4 Oct 2017 03:36:21 +0900 Subject: Fix third-party system libraries list in ActiveStorage::Preview [ci skip] --- activestorage/app/models/active_storage/preview.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb index 42c4bbc5a4..be5053edae 100644 --- a/activestorage/app/models/active_storage/preview.rb +++ b/activestorage/app/models/active_storage/preview.rb @@ -23,8 +23,8 @@ # # The built-in previewers rely on third-party system libraries: # -# * {ffmpeg}[https://www.ffmpeg.org] -# * {mupdf}[https://mupdf.com] +# * {ffmpeg}[https://www.ffmpeg.org] +# * {mupdf}[https://mupdf.com] # # These libraries are not provided by Rails. You must install them yourself to use the built-in previewers. Before you # install and use third-party software, make sure you understand the licensing implications of doing so. -- cgit v1.2.3 From ead60686e810df4b49bf19f4f113b48f16ae560f Mon Sep 17 00:00:00 2001 From: khall Date: Wed, 4 Oct 2017 12:26:04 -0700 Subject: Replace variation key use with SHA256 of key to prevent long filenames If a variant has a large set of options associated with it, the generated filename will be too long, causing Errno::ENAMETOOLONG to be raised. This change replaces those potentially long filenames with a much more compact SHA256 hash. Fixes #30662. --- activestorage/app/models/active_storage/variant.rb | 2 +- activestorage/test/models/variant_test.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 54685b4c0e..90a3605331 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -50,7 +50,7 @@ class ActiveStorage::Variant # Returns a combination key of the blob and the variation that together identifies a specific variant. def key - "variants/#{blob.key}/#{variation.key}" + "variants/#{blob.key}/#{Digest::SHA256.hexdigest(variation.key)}" end # Returns the URL of the variant on the service. This URL is intended to be short-lived for security and not used directly diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index d7cbef4e55..b7d20ab55a 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -26,4 +26,9 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_equal 67, image.height assert_match(/Gray/, image.colorspace) end + + test "service_url doesn't grow in length despite long variant options" do + variant = @blob.variant(font: "a" * 10_000).processed + assert_operator variant.service_url.length, :<, 500 + end end -- cgit v1.2.3 From 2ab561932e7ba76f4b39e9e32e1163567a0dc3fe Mon Sep 17 00:00:00 2001 From: Oleg Date: Fri, 6 Oct 2017 12:50:05 -0700 Subject: how do we install active_storage? [skip ci] --- activestorage/README.md | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activestorage') diff --git a/activestorage/README.md b/activestorage/README.md index 8814887950..78e4463c5a 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -12,6 +12,10 @@ A key difference to how Active Storage works compared to other attachment soluti `Blob` models store attachment metadata (filename, content-type, etc.), and their identifier key in the storage service. Blob models do not store the actual binary data. They are intended to be immutable in spirit. One file, one blob. You can associate the same blob with multiple application models as well. And if you want to do transformations of a given `Blob`, the idea is that you'll simply create a new one, rather than attempt to mutate the existing one (though of course you can delete the previous version later if you don't need it). +## Installation + +Run `rails active_storage:install` to copy over active_storage migrations. + ## Examples One attachment: -- cgit v1.2.3 From 445c682a8465b1a42f1335ae2cf7d20b9a112fcd Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 12 Oct 2017 11:47:21 -0400 Subject: Introduce ActiveStorage::Blob#representation --- activestorage/app/models/active_storage/blob.rb | 26 ++++++++++++++ activestorage/app/models/active_storage/variant.rb | 4 +++ activestorage/test/models/preview_test.rb | 2 ++ activestorage/test/models/representation_test.rb | 41 ++++++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 activestorage/test/models/representation_test.rb (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index ff785d4f61..d43049e32c 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -15,6 +15,7 @@ # If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one. class ActiveStorage::Blob < ActiveRecord::Base class UnpreviewableError < StandardError; end + class UnrepresentableError < StandardError; end self.table_name = "active_storage_blobs" @@ -155,6 +156,31 @@ class ActiveStorage::Blob < ActiveRecord::Base end + # Returns an ActiveStorage::Preview instance for a previewable blob or an ActiveStorage::Variant instance for an image blob. + # + # blob.representation(resize: "100x100").processed.service_url + # + # Raises ActiveStorage::Blob::UnrepresentableError if the receiving blob is neither an image nor previewable. Call + # ActiveStorage::Blob#representable? to determine whether a blob is representable. + # + # See ActiveStorage::Blob#preview and ActiveStorage::Blob#variant for more information. + def representation(transformations) + case + when previewable? + preview transformations + when image? + variant transformations + else + raise UnrepresentableError + end + end + + # Returns true if the blob is an image or is previewable. + def representable? + image? || previewable? + end + + # Returns the URL of the blob on the service. This URL is intended to be short-lived for security and not used directly # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL. # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 90a3605331..915b78162c 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -65,6 +65,10 @@ class ActiveStorage::Variant service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type end + # Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be duck-typed. + def image + self + end private def processed? diff --git a/activestorage/test/models/preview_test.rb b/activestorage/test/models/preview_test.rb index 317a2d5c58..bcd8442f4b 100644 --- a/activestorage/test/models/preview_test.rb +++ b/activestorage/test/models/preview_test.rb @@ -7,6 +7,7 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase test "previewing a PDF" do blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") preview = blob.preview(resize: "640x280").processed + assert preview.image.attached? assert_equal "report.png", preview.image.filename.to_s assert_equal "image/png", preview.image.content_type @@ -19,6 +20,7 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase test "previewing an MP4 video" do blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") preview = blob.preview(resize: "640x280").processed + assert preview.image.attached? assert_equal "video.png", preview.image.filename.to_s assert_equal "image/png", preview.image.content_type diff --git a/activestorage/test/models/representation_test.rb b/activestorage/test/models/representation_test.rb new file mode 100644 index 0000000000..29fe61aee4 --- /dev/null +++ b/activestorage/test/models/representation_test.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::RepresentationTest < ActiveSupport::TestCase + test "representing an image" do + blob = create_file_blob + representation = blob.representation(resize: "100x100").processed + + image = read_image(representation.image) + assert_equal 100, image.width + assert_equal 67, image.height + end + + test "representing a PDF" do + blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") + representation = blob.representation(resize: "640x280").processed + + image = read_image(representation.image) + assert_equal 612, image.width + assert_equal 792, image.height + end + + test "representing an MP4 video" do + blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") + representation = blob.representation(resize: "640x280").processed + + image = read_image(representation.image) + assert_equal 640, image.width + assert_equal 480, image.height + end + + test "representing an unrepresentable blob" do + blob = create_blob + + assert_raises ActiveStorage::Blob::UnrepresentableError do + blob.representation resize: "100x100" + end + end +end -- cgit v1.2.3 From 62ff514d33d3a3b0930956a4b4866e6b228c278c Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 12 Oct 2017 13:40:25 -0400 Subject: Accept variation keys in #preview and #variant --- activestorage/app/models/active_storage/blob.rb | 4 ++-- activestorage/app/models/active_storage/variation.rb | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index d43049e32c..84b8f3827b 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -124,7 +124,7 @@ class ActiveStorage::Blob < ActiveRecord::Base # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::VariantsController # can then produce on-demand. def variant(transformations) - ActiveStorage::Variant.new(self, ActiveStorage::Variation.new(transformations)) + ActiveStorage::Variant.new(self, ActiveStorage::Variation.wrap(transformations)) end @@ -144,7 +144,7 @@ class ActiveStorage::Blob < ActiveRecord::Base # whether a blob is accepted by any previewer, call ActiveStorage::Blob#previewable?. def preview(transformations) if previewable? - ActiveStorage::Preview.new(self, ActiveStorage::Variation.new(transformations)) + ActiveStorage::Preview.new(self, ActiveStorage::Variation.wrap(transformations)) else raise UnpreviewableError end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index df2643442a..13bad87cac 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -13,16 +13,21 @@ class ActiveStorage::Variation attr_reader :transformations class << self - def wrap(variation_or_key) - case variation_or_key + # Returns a Variation instance based on the given variator. If the variator is a Variation, it is + # returned unmodified. If it is a String, it is passed to ActiveStorage::Variation.decode. Otherwise, + # it is assumed to be a transformations Hash and is passed directly to the constructor. + def wrap(variator) + case variator when self - variation_or_key + variator + when String + decode variator else - decode variation_or_key + new variator end end - # Returns a variation instance with the transformations that were encoded by +encode+. + # Returns a Variation instance with the transformations that were encoded by +encode+. def decode(key) new ActiveStorage.verifier.verify(key, purpose: :variation) end -- cgit v1.2.3 From 29da7d1ff510a9f376fc6c780273dfa89298ff51 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 13 Oct 2017 07:52:39 -0400 Subject: Clarify comment [ci skip] --- activestorage/app/models/active_storage/variant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 915b78162c..fa5aa69bd3 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -65,7 +65,7 @@ class ActiveStorage::Variant service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type end - # Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be duck-typed. + # Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be used interchangeably. def image self end -- cgit v1.2.3 From f1e47b0348bd6c18dd94659af549a25684ca78ff Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Fri, 13 Oct 2017 20:47:56 +0300 Subject: Define path with __dir__ in activestorage/ Related to #29176 --- activestorage/test/database/setup.rb | 2 +- activestorage/test/dummy/bin/bundle | 2 +- activestorage/test/test_helper.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'activestorage') diff --git a/activestorage/test/database/setup.rb b/activestorage/test/database/setup.rb index 87564499e6..705650a25d 100644 --- a/activestorage/test/database/setup.rb +++ b/activestorage/test/database/setup.rb @@ -3,5 +3,5 @@ require_relative "create_users_migration" ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") -ActiveRecord::Migrator.migrate File.expand_path("../../../db/migrate", __FILE__) +ActiveRecord::Migrator.migrate File.expand_path("../../db/migrate", __dir__) ActiveStorageCreateUsers.migrate(:up) diff --git a/activestorage/test/dummy/bin/bundle b/activestorage/test/dummy/bin/bundle index 277e128251..5015ba6f8b 100755 --- a/activestorage/test/dummy/bin/bundle +++ b/activestorage/test/dummy/bin/bundle @@ -1,5 +1,5 @@ #!/usr/bin/env ruby # frozen_string_literal: true -ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", __FILE__) +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) load Gem.bin_path("bundler", "bundle") diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index dd37239060..e206cf1b8b 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -23,7 +23,7 @@ Minitest.backtrace_filter = Minitest::BacktraceFilter.new require "yaml" SERVICE_CONFIGURATIONS = begin - erb = ERB.new(Pathname.new(File.expand_path("../service/configurations.yml", __FILE__)).read) + erb = ERB.new(Pathname.new(File.expand_path("service/configurations.yml", __dir__)).read) configuration = YAML.load(erb.result) || {} configuration.deep_symbolize_keys rescue Errno::ENOENT @@ -38,7 +38,7 @@ ActiveStorage::Service.logger = ActiveSupport::Logger.new(nil) ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing") class ActiveSupport::TestCase - self.file_fixture_path = File.expand_path("../fixtures/files", __FILE__) + self.file_fixture_path = File.expand_path("fixtures/files", __dir__) private def create_blob(data: "Hello world!", filename: "hello.txt", content_type: "text/plain") -- cgit v1.2.3 From 19323d3bcc828e36b2d7ce23bc0e0031a05a012e Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Fri, 13 Oct 2017 20:50:03 +0300 Subject: Use `require_relative` instead of `require` with full path in activestorage/ Related to #29417 --- activestorage/test/test_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index e206cf1b8b..60656feb80 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require File.expand_path("../../test/dummy/config/environment.rb", __FILE__) +require_relative "dummy/config/environment.rb" require "bundler/setup" require "active_support" -- cgit v1.2.3 From 20c91119903f70eb19aed33fe78417789dbf070f Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sat, 21 Oct 2017 22:27:49 +0900 Subject: [Active Storage] require_relative => require --- activestorage/lib/active_storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 44d9c25504..9ac1e7ab54 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -26,7 +26,7 @@ require "active_record" require "active_support" require "active_support/rails" -require_relative "active_storage/version" +require "active_storage/version" module ActiveStorage extend ActiveSupport::Autoload -- cgit v1.2.3 From 605484079d297d1ba6835628465be81f03c052ee Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sun, 22 Oct 2017 13:16:59 -0400 Subject: Extract metadata from images and videos --- .../app/jobs/active_storage/analyze_job.rb | 8 +++ activestorage/app/jobs/active_storage/purge_job.rb | 2 +- .../app/models/active_storage/attachment.rb | 7 ++ activestorage/app/models/active_storage/blob.rb | 57 ++++++++++++++- activestorage/lib/active_storage.rb | 3 + activestorage/lib/active_storage/analyzer.rb | 33 +++++++++ .../lib/active_storage/analyzer/image_analyzer.rb | 36 ++++++++++ .../lib/active_storage/analyzer/null_analyzer.rb | 13 ++++ .../lib/active_storage/analyzer/video_analyzer.rb | 79 +++++++++++++++++++++ activestorage/lib/active_storage/attached/one.rb | 6 +- activestorage/lib/active_storage/downloading.rb | 21 ++++++ activestorage/lib/active_storage/engine.rb | 14 +++- activestorage/lib/active_storage/log_subscriber.rb | 2 +- activestorage/lib/active_storage/previewer.rb | 27 ++----- .../lib/active_storage/previewer/pdf_previewer.rb | 2 +- .../active_storage/previewer/video_previewer.rb | 10 ++- activestorage/lib/active_storage/service.rb | 2 - activestorage/test/analyzer/image_analyzer_test.rb | 16 +++++ activestorage/test/analyzer/video_analyzer_test.rb | 35 +++++++++ .../test/fixtures/files/rotated_video.mp4 | Bin 0 -> 275090 bytes .../fixtures/files/video_without_video_stream.mp4 | Bin 0 -> 16252 bytes activestorage/test/models/attachments_test.rb | 59 ++++++++++++++- activestorage/test/previewer/pdf_previewer_test.rb | 3 + .../test/previewer/video_previewer_test.rb | 3 + activestorage/test/test_helper.rb | 6 +- 25 files changed, 407 insertions(+), 37 deletions(-) create mode 100644 activestorage/app/jobs/active_storage/analyze_job.rb create mode 100644 activestorage/lib/active_storage/analyzer.rb create mode 100644 activestorage/lib/active_storage/analyzer/image_analyzer.rb create mode 100644 activestorage/lib/active_storage/analyzer/null_analyzer.rb create mode 100644 activestorage/lib/active_storage/analyzer/video_analyzer.rb create mode 100644 activestorage/lib/active_storage/downloading.rb create mode 100644 activestorage/test/analyzer/image_analyzer_test.rb create mode 100644 activestorage/test/analyzer/video_analyzer_test.rb create mode 100644 activestorage/test/fixtures/files/rotated_video.mp4 create mode 100644 activestorage/test/fixtures/files/video_without_video_stream.mp4 (limited to 'activestorage') diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb new file mode 100644 index 0000000000..a11a73d030 --- /dev/null +++ b/activestorage/app/jobs/active_storage/analyze_job.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later. +class ActiveStorage::AnalyzeJob < ActiveJob::Base + def perform(blob) + blob.analyze + end +end diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index 990ab27c9f..188840f702 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Provides delayed purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. +# Provides asynchronous 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 diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 29226e8ee9..9f61a5dbf3 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -14,6 +14,8 @@ class ActiveStorage::Attachment < ActiveRecord::Base delegate_missing_to :blob + after_create_commit :analyze_blob_later + # Synchronously purges the blob (deletes it from the configured service) and destroys the attachment. def purge blob.purge @@ -25,4 +27,9 @@ class ActiveStorage::Attachment < ActiveRecord::Base blob.purge_later destroy end + + private + def analyze_blob_later + blob.analyze_later unless blob.analyzed? + end end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 84b8f3827b..99823e14c6 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_storage/analyzer/null_analyzer" + # 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: # @@ -20,7 +22,7 @@ class ActiveStorage::Blob < ActiveRecord::Base self.table_name = "active_storage_blobs" has_secure_token :key - store :metadata, coder: JSON + store :metadata, accessors: [ :analyzed ], coder: JSON class_attribute :service @@ -224,6 +226,46 @@ class ActiveStorage::Blob < ActiveRecord::Base end + # Extracts and stores metadata from the file associated with this blob using a relevant analyzer. Active Storage comes + # with built-in analyzers for images and videos. See ActiveStorage::Analyzer::ImageAnalyzer and + # ActiveStorage::Analyzer::VideoAnalyzer for information about the specific attributes they extract and the third-party + # libraries they require. + # + # To choose the analyzer for a blob, Active Storage calls +accept?+ on each registered analyzer in order. It uses the + # first analyzer for which +accept?+ returns true when given the blob. If no registered analyzer accepts the blob, no + # metadata is extracted from it. + # + # In a Rails application, add or remove analyzers by manipulating +Rails.application.config.active_storage.analyzers+ + # in an initializer: + # + # # Add a custom analyzer for Microsoft Office documents: + # Rails.application.config.active_storage.analyzers.append DOCXAnalyzer + # + # # Remove the built-in video analyzer: + # Rails.application.config.active_storage.analyzers.delete ActiveStorage::Analyzer::VideoAnalyzer + # + # Outside of a Rails application, manipulate +ActiveStorage.analyzers+ instead. + # + # You won't ordinarily need to call this method from a Rails application. New blobs are automatically and asynchronously + # analyzed via #analyze_later when they're attached for the first time. + def analyze + update! metadata: extract_metadata_via_analyzer + end + + # Enqueues an ActiveStorage::AnalyzeJob which calls #analyze. + # + # This method is automatically called for a blob when it's attached for the first time. You can call it to analyze a blob + # again (e.g. if you add a new analyzer or modify an existing one). + def analyze_later + ActiveStorage::AnalyzeJob.perform_later(self) + end + + # Returns true if the blob has been analyzed. + def analyzed? + analyzed + end + + # Deletes the file on the service that's associated with this blob. This should only be done if the blob is going to be # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+ # methods in most circumstances. @@ -255,4 +297,17 @@ class ActiveStorage::Blob < ActiveRecord::Base io.rewind end.base64digest end + + + def extract_metadata_via_analyzer + analyzer.metadata.merge(analyzed: true) + end + + def analyzer + analyzer_class.new(self) + end + + def analyzer_class + ActiveStorage.analyzers.detect { |klass| klass.accept?(self) } || ActiveStorage::Analyzer::NullAnalyzer + end end diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 9ac1e7ab54..cfdb2a8acd 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -34,7 +34,10 @@ module ActiveStorage autoload :Attached autoload :Service autoload :Previewer + autoload :Analyzer + mattr_accessor :logger mattr_accessor :verifier mattr_accessor :previewers, default: [] + mattr_accessor :analyzers, default: [] end diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb new file mode 100644 index 0000000000..837785a12b --- /dev/null +++ b/activestorage/lib/active_storage/analyzer.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "active_storage/downloading" + +module ActiveStorage + # This is an abstract base class for analyzers, which extract metadata from blobs. See + # ActiveStorage::Analyzer::ImageAnalyzer for an example of a concrete subclass. + class Analyzer + include Downloading + + attr_reader :blob + + # Implement this method in a concrete subclass. Have it return true when given a blob from which + # the analyzer can extract metadata. + def self.accept?(blob) + false + end + + def initialize(blob) + @blob = blob + end + + # Override this method in a concrete subclass. Have it return a Hash of metadata. + def metadata + raise NotImplementedError + end + + private + def logger + ActiveStorage.logger + end + end +end diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb new file mode 100644 index 0000000000..b25d2092b9 --- /dev/null +++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module ActiveStorage + # Extracts width and height in pixels from an image blob. + # + # Example: + # + # ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata + # # => { width: 4104, height: 2736 } + # + # This analyzer relies on the third-party [MiniMagick]{https://github.com/minimagick/minimagick} gem. MiniMagick requires + # the [ImageMagick]{http://www.imagemagick.org} system library. These libraries are not provided by Rails; you must + # install them yourself to use this analyzer. + class Analyzer::ImageAnalyzer < Analyzer + def self.accept?(blob) + blob.image? + end + + def metadata + read_image do |image| + { width: image.width, height: image.height } + end + rescue LoadError + logger.info "Skipping image analysis because the mini_magick gem isn't installed" + {} + end + + private + def read_image + download_blob_to_tempfile do |file| + require "mini_magick" + yield MiniMagick::Image.new(file.path) + end + end + end +end diff --git a/activestorage/lib/active_storage/analyzer/null_analyzer.rb b/activestorage/lib/active_storage/analyzer/null_analyzer.rb new file mode 100644 index 0000000000..8ff7ce48e5 --- /dev/null +++ b/activestorage/lib/active_storage/analyzer/null_analyzer.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module ActiveStorage + class Analyzer::NullAnalyzer < Analyzer # :nodoc: + def self.accept?(blob) + true + end + + def metadata + {} + end + end +end diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb new file mode 100644 index 0000000000..408b5e58e9 --- /dev/null +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require "active_support/core_ext/hash/compact" + +module ActiveStorage + # Extracts the following from a video blob: + # + # * Width (pixels) + # * Height (pixels) + # * Duration (seconds) + # * Angle (degrees) + # * Aspect ratio + # + # Example: + # + # ActiveStorage::VideoAnalyzer.new(blob).metadata + # # => { width: 640, height: 480, duration: 5.0, angle: 0, aspect_ratio: [4, 3] } + # + # This analyzer requires the {ffmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. You must + # install ffmpeg yourself to use this analyzer. + class Analyzer::VideoAnalyzer < Analyzer + def self.accept?(blob) + blob.video? + end + + def metadata + { width: width, height: height, duration: duration, angle: angle, aspect_ratio: aspect_ratio }.compact + end + + private + def width + Integer(video_stream["width"]) if video_stream["width"] + end + + def height + Integer(video_stream["height"]) if video_stream["height"] + end + + def duration + Float(video_stream["duration"]) if video_stream["duration"] + end + + def angle + Integer(tags["rotate"]) if tags["rotate"] + end + + def aspect_ratio + if descriptor = video_stream["display_aspect_ratio"] + descriptor.split(":", 2).collect(&:to_i) + end + end + + + def tags + @tags ||= video_stream["tags"] || {} + end + + def video_stream + @video_stream ||= streams.detect { |stream| stream["codec_type"] == "video" } || {} + end + + def streams + probe["streams"] || [] + end + + def probe + download_blob_to_tempfile { |file| probe_from(file) } + end + + def probe_from(file) + IO.popen([ "ffprobe", "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output| + JSON.parse(output.read) + end + rescue Errno::ENOENT + logger.info "Skipping video analysis because ffmpeg isn't installed" + {} + end + end +end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index c66be08f58..dc19512484 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -59,12 +59,16 @@ module ActiveStorage def replace(attachable) blob.tap do transaction do - destroy + destroy_attachment write_attachment create_attachment_from(attachable) end end.purge_later end + def destroy_attachment + attachment.destroy + end + def create_attachment_from(attachable) ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) end diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb new file mode 100644 index 0000000000..bcf42f610e --- /dev/null +++ b/activestorage/lib/active_storage/downloading.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module ActiveStorage + module Downloading + private + # Opens a new tempfile and copies blob data into it. Yields the tempfile. + def download_blob_to_tempfile # :doc: + Tempfile.open("ActiveStorage") do |file| + download_blob_to file + yield file + end + end + + # Efficiently download blob data into the given file. + def download_blob_to(file) # :doc: + file.binmode + blob.download { |chunk| file.write(chunk) } + file.rewind + end + end +end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 335eae8dd8..a01a14cd83 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -6,20 +6,22 @@ require "active_storage" require "active_storage/previewer/pdf_previewer" require "active_storage/previewer/video_previewer" +require "active_storage/analyzer/image_analyzer" +require "active_storage/analyzer/video_analyzer" + module ActiveStorage class Engine < Rails::Engine # :nodoc: isolate_namespace ActiveStorage config.active_storage = ActiveSupport::OrderedOptions.new config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] + config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] config.eager_load_namespaces << ActiveStorage initializer "active_storage.logger" do - require "active_storage/service" - config.after_initialize do |app| - ActiveStorage::Service.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.logger = app.config.active_storage.logger || Rails.logger end end @@ -69,5 +71,11 @@ module ActiveStorage ActiveStorage.previewers = app.config.active_storage.previewers || [] end end + + initializer "active_storage.analyzers" do + config.after_initialize do |app| + ActiveStorage.analyzers = app.config.active_storage.analyzers || [] + end + end end end diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index 0d00a75c0e..5cbf4bd1a5 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -27,7 +27,7 @@ module ActiveStorage end def logger - ActiveStorage::Service.logger + ActiveStorage.logger end private diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index 930b376067..460f6d5678 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true +require "active_storage/downloading" + module ActiveStorage # This is an abstract base class for previewers, which generate images from blobs. See # ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of # concrete subclasses. class Previewer + include Downloading + attr_reader :blob # Implement this method in a concrete subclass. Have it return true when given a blob from which @@ -24,37 +28,20 @@ module ActiveStorage end private - # Downloads the blob to a new tempfile. Yields the tempfile. - # - # Use this method to get a tempfile that you can provide to a drawing command. - def open # :doc: - Tempfile.open("input") do |file| - download_blob_to file - yield file - end - end - - def download_blob_to(file) - file.binmode - blob.download { |chunk| file.write(chunk) } - file.rewind - end - - # Executes a system command, capturing its binary output in a tempfile. Yields the tempfile. # - # Use this method to shell out to system libraries (e.g. mupdf or ffmpeg) for preview image + # Use this method to shell out to a system library (e.g. mupdf or ffmpeg) for preview image # generation. The resulting tempfile can be used as the +:io+ value in an attachable Hash: # # def preview - # open do |input| + # download_blob_to_tempfile do |input| # draw "my-drawing-command", input.path, "--format", "png", "-" do |output| # yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" # end # end # end def draw(*argv) # :doc: - Tempfile.open("output") do |file| + Tempfile.open("ActiveStorage") do |file| capture(*argv, to: file) yield file end diff --git a/activestorage/lib/active_storage/previewer/pdf_previewer.rb b/activestorage/lib/active_storage/previewer/pdf_previewer.rb index 31a2a8f120..a2f05c74a6 100644 --- a/activestorage/lib/active_storage/previewer/pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/pdf_previewer.rb @@ -7,7 +7,7 @@ module ActiveStorage end def preview - open do |input| + download_blob_to_tempfile do |input| draw "mutool", "draw", "-F", "png", "-o", "-", input.path, "1" do |output| yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb index 840d87f100..49f128d142 100644 --- a/activestorage/lib/active_storage/previewer/video_previewer.rb +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -7,11 +7,17 @@ module ActiveStorage end def preview - open do |input| - draw "ffmpeg", "-i", input.path, "-y", "-vcodec", "png", "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-" do |output| + download_blob_to_tempfile do |input| + draw_relevant_frame_from input do |output| yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" end end end + + private + def draw_relevant_frame_from(file, &block) + draw "ffmpeg", "-i", file.path, "-y", "-vcodec", "png", + "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-", &block + end end end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index 1f012da1e7..aa150e4d8a 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -41,8 +41,6 @@ module ActiveStorage extend ActiveSupport::Autoload autoload :Configurator - class_attribute :logger - class_attribute :url_expires_in, default: 5.minutes class << self diff --git a/activestorage/test/analyzer/image_analyzer_test.rb b/activestorage/test/analyzer/image_analyzer_test.rb new file mode 100644 index 0000000000..9087072215 --- /dev/null +++ b/activestorage/test/analyzer/image_analyzer_test.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +require "active_storage/analyzer/image_analyzer" + +class ActiveStorage::Analyzer::ImageAnalyzerTest < ActiveSupport::TestCase + test "analyzing an image" do + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") + metadata = blob.tap(&:analyze).metadata + + assert_equal 4104, metadata[:width] + assert_equal 2736, metadata[:height] + end +end diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb new file mode 100644 index 0000000000..4a3c4a8bfc --- /dev/null +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +require "active_storage/analyzer/video_analyzer" + +class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase + test "analyzing a video" do + blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") + metadata = blob.tap(&:analyze).metadata + + assert_equal 640, metadata[:width] + assert_equal 480, metadata[:height] + assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal 5.166648, metadata[:duration] + assert_not_includes metadata, :angle + end + + test "analyzing a rotated video" do + blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") + metadata = blob.tap(&:analyze).metadata + + assert_equal 640, metadata[:width] + assert_equal 480, metadata[:height] + assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal 5.227975, metadata[:duration] + assert_equal 90, metadata[:angle] + end + + test "analyzing a video without a video stream" do + blob = create_file_blob(filename: "video_without_video_stream.mp4", content_type: "video/mp4") + assert_equal({ "analyzed" => true }, blob.tap(&:analyze).metadata) + end +end diff --git a/activestorage/test/fixtures/files/rotated_video.mp4 b/activestorage/test/fixtures/files/rotated_video.mp4 new file mode 100644 index 0000000000..4c7a4e9e57 Binary files /dev/null and b/activestorage/test/fixtures/files/rotated_video.mp4 differ diff --git a/activestorage/test/fixtures/files/video_without_video_stream.mp4 b/activestorage/test/fixtures/files/video_without_video_stream.mp4 new file mode 100644 index 0000000000..e6a55f868b Binary files /dev/null and b/activestorage/test/fixtures/files/video_without_video_stream.mp4 differ diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 379ae0a416..47f2bd7911 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -15,7 +15,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase assert_equal "funky.jpg", @user.avatar.filename.to_s end - test "attach existing sgid blob" do + 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 @@ -63,6 +63,31 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase assert_equal "funky.jpg", @user.avatar_attachment.blob.filename.to_s 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 blob.reload.analyzed? + + @user.avatar.attachment.destroy + + assert_no_enqueued_jobs do + @user.reload.avatar.attach blob + end + end + test "purge attached blob" do @user.avatar.attach create_blob(filename: "funky.jpg") avatar_key = @user.avatar.key @@ -135,6 +160,38 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 "purge attached blobs" do @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") diff --git a/activestorage/test/previewer/pdf_previewer_test.rb b/activestorage/test/previewer/pdf_previewer_test.rb index 60f075d1b2..fe32f39be4 100644 --- a/activestorage/test/previewer/pdf_previewer_test.rb +++ b/activestorage/test/previewer/pdf_previewer_test.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "test_helper" +require "database/setup" + require "active_storage/previewer/pdf_previewer" class ActiveStorage::Previewer::PDFPreviewerTest < ActiveSupport::TestCase diff --git a/activestorage/test/previewer/video_previewer_test.rb b/activestorage/test/previewer/video_previewer_test.rb index 967d5d5ba9..dba9b0d7e2 100644 --- a/activestorage/test/previewer/video_previewer_test.rb +++ b/activestorage/test/previewer/video_previewer_test.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "test_helper" +require "database/setup" + require "active_storage/previewer/video_previewer" class ActiveStorage::Previewer::VideoPreviewerTest < ActiveSupport::TestCase diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 60656feb80..38408cdad3 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -33,8 +33,8 @@ end require "tmpdir" ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: Dir.mktmpdir("active_storage_tests")) -ActiveStorage::Service.logger = ActiveSupport::Logger.new(nil) +ActiveStorage.logger = ActiveSupport::Logger.new(nil) ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing") class ActiveSupport::TestCase @@ -46,9 +46,7 @@ class ActiveSupport::TestCase end def create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") - ActiveStorage::Blob.create_after_upload! \ - io: file_fixture(filename).open, - filename: filename, content_type: content_type + ActiveStorage::Blob.create_after_upload! io: file_fixture(filename).open, filename: filename, content_type: content_type end def create_blob_before_direct_upload(filename: "hello.txt", byte_size:, checksum:, content_type: "text/plain") -- cgit v1.2.3 From 398e4fecde6f8b3263cc7c04face664c248d3966 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sun, 22 Oct 2017 13:36:37 -0400 Subject: Fix links [ci skip] --- activestorage/lib/active_storage/analyzer/image_analyzer.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb index b25d2092b9..25e0251e6e 100644 --- a/activestorage/lib/active_storage/analyzer/image_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb @@ -8,8 +8,8 @@ module ActiveStorage # ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata # # => { width: 4104, height: 2736 } # - # This analyzer relies on the third-party [MiniMagick]{https://github.com/minimagick/minimagick} gem. MiniMagick requires - # the [ImageMagick]{http://www.imagemagick.org} system library. These libraries are not provided by Rails; you must + # This analyzer relies on the third-party {MiniMagick}[https://github.com/minimagick/minimagick] gem. MiniMagick requires + # the {ImageMagick}[http://www.imagemagick.org] system library. These libraries are not provided by Rails; you must # install them yourself to use this analyzer. class Analyzer::ImageAnalyzer < Analyzer def self.accept?(blob) -- cgit v1.2.3 From f4d1aa5310284ebceb51970140fa08f6c8ea19b6 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sun, 22 Oct 2017 23:14:44 -0400 Subject: Use the indicative mood consistently [ci skip] --- activestorage/lib/active_storage/downloading.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index bcf42f610e..ceb7cce0c7 100644 --- a/activestorage/lib/active_storage/downloading.rb +++ b/activestorage/lib/active_storage/downloading.rb @@ -11,7 +11,7 @@ module ActiveStorage end end - # Efficiently download blob data into the given file. + # Efficiently downloads blob data into the given file. def download_blob_to(file) # :doc: file.binmode blob.download { |chunk| file.write(chunk) } -- cgit v1.2.3 From aa6bcbbac8517d5b077f21073e9902637d7c7157 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 2 Nov 2017 15:07:04 -0400 Subject: Allow third-party previewers/analyzers to customize their tempdirs --- activestorage/lib/active_storage/downloading.rb | 9 +++++++-- activestorage/lib/active_storage/previewer.rb | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index ceb7cce0c7..3dac6b116a 100644 --- a/activestorage/lib/active_storage/downloading.rb +++ b/activestorage/lib/active_storage/downloading.rb @@ -3,9 +3,9 @@ module ActiveStorage module Downloading private - # Opens a new tempfile and copies blob data into it. Yields the tempfile. + # Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile. def download_blob_to_tempfile # :doc: - Tempfile.open("ActiveStorage") do |file| + Tempfile.open("ActiveStorage", tempdir) do |file| download_blob_to file yield file end @@ -17,5 +17,10 @@ module ActiveStorage blob.download { |chunk| file.write(chunk) } file.rewind end + + # Returns the directory in which tempfiles should be opened. Defaults to +Dir.tmpdir+. + def tempdir # :doc: + Dir.tmpdir + end end end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index 460f6d5678..ed75bae3b5 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -40,8 +40,10 @@ module ActiveStorage # end # end # end + # + # The output tempfile is opened in the directory returned by ActiveStorage::Downloading#tempdir. def draw(*argv) # :doc: - Tempfile.open("ActiveStorage") do |file| + Tempfile.open("ActiveStorage", tempdir) do |file| capture(*argv, to: file) yield file end -- cgit v1.2.3 From 9ec67362054e874ed905310a79b670941fa397af Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 3 Nov 2017 11:29:21 -0400 Subject: Permit configuring Active Storage's job queue --- activestorage/app/jobs/active_storage/analyze_job.rb | 2 +- activestorage/app/jobs/active_storage/base_job.rb | 5 +++++ activestorage/app/jobs/active_storage/purge_job.rb | 2 +- activestorage/lib/active_storage.rb | 1 + activestorage/lib/active_storage/engine.rb | 19 +++++-------------- 5 files changed, 13 insertions(+), 16 deletions(-) create mode 100644 activestorage/app/jobs/active_storage/base_job.rb (limited to 'activestorage') diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb index a11a73d030..2a952f9f74 100644 --- a/activestorage/app/jobs/active_storage/analyze_job.rb +++ b/activestorage/app/jobs/active_storage/analyze_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later. -class ActiveStorage::AnalyzeJob < ActiveJob::Base +class ActiveStorage::AnalyzeJob < ActiveStorage::BaseJob 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 new file mode 100644 index 0000000000..6caab42a2d --- /dev/null +++ b/activestorage/app/jobs/active_storage/base_job.rb @@ -0,0 +1,5 @@ +# 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 188840f702..98874d2250 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. -class ActiveStorage::PurgeJob < ActiveJob::Base +class ActiveStorage::PurgeJob < ActiveStorage::BaseJob # FIXME: Limit this to a custom ActiveStorage error retry_on StandardError diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index cfdb2a8acd..d1ff6b7032 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -38,6 +38,7 @@ module ActiveStorage mattr_accessor :logger mattr_accessor :verifier + mattr_accessor :queue mattr_accessor :previewers, default: [] mattr_accessor :analyzers, default: [] end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index a01a14cd83..6cf6635c4f 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -19,9 +19,12 @@ module ActiveStorage config.eager_load_namespaces << ActiveStorage - initializer "active_storage.logger" do + initializer "active_storage.configs" do config.after_initialize do |app| - ActiveStorage.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.queue = app.config.active_storage.queue + ActiveStorage.previewers = app.config.active_storage.previewers || [] + ActiveStorage.analyzers = app.config.active_storage.analyzers || [] end end @@ -65,17 +68,5 @@ module ActiveStorage end end end - - initializer "active_storage.previewers" do - config.after_initialize do |app| - ActiveStorage.previewers = app.config.active_storage.previewers || [] - end - end - - initializer "active_storage.analyzers" do - config.after_initialize do |app| - ActiveStorage.analyzers = app.config.active_storage.analyzers || [] - end - end end end -- cgit v1.2.3 From 86938c495e282e6a61c16a9e1d77582e22c0a4fc Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 6 Nov 2017 21:29:37 -0500 Subject: Fix streaming downloads from S3/Azure Storage Closes #31073. --- .../lib/active_storage/service/azure_storage_service.rb | 8 ++++---- activestorage/lib/active_storage/service/s3_service.rb | 6 +++--- activestorage/test/service/shared_service_tests.rb | 10 ++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 27dd192ce6..f3877ad9c9 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -28,7 +28,7 @@ module ActiveStorage end end - def download(key) + def download(key, &block) if block_given? instrument :streaming_download, key do stream(key, &block) @@ -108,15 +108,15 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, options = {}, &block) + def stream(key) blob = blob_for(key) chunk_size = 5.megabytes offset = 0 while offset < blob.properties[:content_length] - _, io = blobs.get_blob(container, key, start_range: offset, end_range: offset + chunk_size - 1) - yield io + _, chunk = blobs.get_blob(container, key, start_range: offset, end_range: offset + chunk_size - 1) + yield chunk.force_encoding(Encoding::BINARY) offset += chunk_size end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 3e93cdd072..958b172efb 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -26,7 +26,7 @@ module ActiveStorage end end - def download(key) + def download(key, &block) if block_given? instrument :streaming_download, key do stream(key, &block) @@ -85,14 +85,14 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, options = {}, &block) + def stream(key, &block) object = object_for(key) chunk_size = 5.megabytes offset = 0 while offset < object.content_length - yield object.read(options.merge(range: "bytes=#{offset}-#{offset + chunk_size - 1}")) + yield object.get(range: "bytes=#{offset}-#{offset + chunk_size - 1}").body.read.force_encoding(Encoding::BINARY) offset += chunk_size end end diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index a9e1cb6ce9..ade91ab89a 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -50,6 +50,16 @@ module ActiveStorage::Service::SharedServiceTests assert_equal FIXTURE_DATA, @service.download(FIXTURE_KEY) end + test "downloading in chunks" do + chunks = [] + + @service.download(FIXTURE_KEY) do |chunk| + chunks << chunk + end + + assert_equal [ FIXTURE_DATA ], chunks + end + test "existing" do assert @service.exist?(FIXTURE_KEY) assert_not @service.exist?(FIXTURE_KEY + "nonsense") -- cgit v1.2.3 From daf77db65d9d5e295ee3ba86605988875cb834e4 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Tue, 7 Nov 2017 09:06:23 -0500 Subject: Remove needless block parameter --- activestorage/lib/active_storage/service/s3_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 958b172efb..6957119780 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -85,7 +85,7 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, &block) + def stream(key) object = object_for(key) chunk_size = 5.megabytes -- cgit v1.2.3 From 704a7e425ca99af1b778c764a86e5388647631dd Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 13 Nov 2017 16:36:39 -0500 Subject: Preserve existing metadata when analyzing a blob Closes #31138. --- activestorage/app/models/active_storage/blob.rb | 2 +- activestorage/test/models/attachments_test.rb | 25 +++++++++++++++++++++++++ activestorage/test/test_helper.rb | 4 ++-- 3 files changed, 28 insertions(+), 3 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 99823e14c6..2aa05d665e 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -249,7 +249,7 @@ class ActiveStorage::Blob < ActiveRecord::Base # You won't ordinarily need to call this method from a Rails application. New blobs are automatically and asynchronously # analyzed via #analyze_later when they're attached for the first time. def analyze - update! metadata: extract_metadata_via_analyzer + update! metadata: metadata.merge(extract_metadata_via_analyzer) end # Enqueues an ActiveStorage::AnalyzeJob which calls #analyze. diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 47f2bd7911..96bc963cff 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -88,6 +88,16 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 "purge attached blob" do @user.avatar.attach create_blob(filename: "funky.jpg") avatar_key = @user.avatar.key @@ -193,6 +203,21 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 "purge attached blobs" do @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") highlight_keys = @user.highlights.collect(&:key) diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 38408cdad3..55da781f2a 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -45,8 +45,8 @@ class ActiveSupport::TestCase ActiveStorage::Blob.create_after_upload! io: StringIO.new(data), filename: filename, content_type: content_type end - def create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") - ActiveStorage::Blob.create_after_upload! io: file_fixture(filename).open, filename: filename, content_type: content_type + def create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg", metadata: nil) + ActiveStorage::Blob.create_after_upload! io: file_fixture(filename).open, filename: filename, content_type: content_type, metadata: metadata end def create_blob_before_direct_upload(filename: "hello.txt", byte_size:, checksum:, content_type: "text/plain") -- cgit v1.2.3 From 499a4164ce9816c4913bf3db14787ea99ef2c266 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Tue, 14 Nov 2017 10:42:10 -0500 Subject: Introduce ActiveStorage::Attached::{One,Many}#detach --- activestorage/lib/active_storage/attached/many.rb | 6 +++++- activestorage/lib/active_storage/attached/one.rb | 15 +++++++------ activestorage/test/models/attachments_test.rb | 26 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 1e0657c33c..0b3400bccf 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -13,7 +13,6 @@ module ActiveStorage end # Associates one or several attachments with the current record, saving them to the database. - # Examples: # # 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 @@ -36,6 +35,11 @@ module ActiveStorage attachments.any? end + # Deletes associated attachments without purging them, leaving their respective blobs in place. + def detach + attachments.destroy_all if attached? + end + # Directly purges each associated attachment (i.e. destroys the blobs and # attachments and deletes the files on the service). def purge diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index dc19512484..7092f6b109 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -14,7 +14,6 @@ module ActiveStorage end # Associates a given attachment with the current record, saving it to the database. - # Examples: # # person.avatar.attach(params[:avatar]) # ActionDispatch::Http::UploadedFile object # person.avatar.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload @@ -39,6 +38,14 @@ module ActiveStorage attachment.present? end + # Deletes the attachment without purging it, leaving its blob in place. + def detach + if attached? + attachment.destroy + write_attachment nil + end + end + # Directly purges the attachment (i.e. destroys the blob and # attachment and deletes the file on the service). def purge @@ -59,16 +66,12 @@ module ActiveStorage def replace(attachable) blob.tap do transaction do - destroy_attachment + detach write_attachment create_attachment_from(attachable) end end.purge_later end - def destroy_attachment - attachment.destroy - end - def create_attachment_from(attachable) ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable)) end diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 96bc963cff..e645d868ce 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -98,6 +98,17 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 @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 @@ -218,6 +229,21 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 @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) -- cgit v1.2.3 From 1a0f85e13963f0765dad8b378b651ccece051e2c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 20 Nov 2017 05:11:12 +0900 Subject: Fix ASt CI failure with rack-test 0.7.1 Due to https://github.com/rack-test/rack-test/commit/5fd3631078e7c73aaed7d4371f70fb2a79384be9. --- activestorage/test/models/attachments_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index e645d868ce..23533ece1e 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -27,7 +27,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase test "attach new blob from an UploadedFile" do file = file_fixture "racecar.jpg" - @user.avatar.attach Rack::Test::UploadedFile.new file + @user.avatar.attach Rack::Test::UploadedFile.new file.to_s assert_equal "racecar.jpg", @user.avatar.filename.to_s end -- cgit v1.2.3 From e05e2ae44f1ecf8e9bb5949f531305c15bc3c665 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sun, 19 Nov 2017 17:34:07 -0500 Subject: Permit attaching files to new records Closes #31164. --- activestorage/lib/active_storage/attached/many.rb | 6 +++- activestorage/lib/active_storage/attached/one.rb | 8 ++--- activestorage/test/models/attachments_test.rb | 44 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 0b3400bccf..6eace65b79 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -20,7 +20,11 @@ module ActiveStorage # document.images.attach([ first_blob, second_blob ]) def attach(*attachables) attachables.flatten.collect do |attachable| - attachments.create!(name: name, blob: create_blob_from(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 end end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index 7092f6b109..0244232b2c 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -23,7 +23,7 @@ module ActiveStorage if attached? && dependent == :purge_later replace attachable else - write_attachment create_attachment_from(attachable) + write_attachment build_attachment_from(attachable) end end @@ -67,13 +67,13 @@ module ActiveStorage blob.tap do transaction do detach - write_attachment create_attachment_from(attachable) + write_attachment build_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)) + def build_attachment_from(attachable) + ActiveStorage::Attachment.new(record: record, name: name, blob: create_blob_from(attachable)) end def write_attachment(attachment) diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 23533ece1e..edd68d3051 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -56,6 +56,26 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase assert ActiveStorage::Blob.service.exist?(@user.avatar.key) 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 user.avatar.attached? + assert_equal "funky.jpg", user.avatar.filename.to_s + + assert_difference -> { ActiveStorage::Attachment.count }, +1 do + user.save! + end + + assert user.reload.avatar.attached? + assert_equal "funky.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 @@ -160,6 +180,30 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 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 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" }, -- cgit v1.2.3 From 2d20a7696a761b1840bc2fbe09a2fd4bff2a779f Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 20 Nov 2017 10:52:54 -0500 Subject: Fix direct uploads to local service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disable CSRF protection for ActiveStorage::DiskController#update. The local disk service is intended to imitate a third-party service like S3 or GCS, so we don't care where direct uploads originate: they’re authorized by signed tokens. Closes #30290. [Shinichi Maeshima & George Claghorn] --- activestorage/app/controllers/active_storage/disk_controller.rb | 2 ++ activestorage/test/dummy/config/environments/test.rb | 3 +++ activestorage/test/test_helper.rb | 1 + 3 files changed, 6 insertions(+) (limited to 'activestorage') diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index a4fd427cb2..8caecfff49 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -5,6 +5,8 @@ # Always go through the BlobsController, or your own authenticated controller, rather than directly # to the service url. class ActiveStorage::DiskController < ActionController::Base + skip_forgery_protection + def show if key = decode_verified_key send_data disk_service.download(key), diff --git a/activestorage/test/dummy/config/environments/test.rb b/activestorage/test/dummy/config/environments/test.rb index ce0889e8ae..74a802d98c 100644 --- a/activestorage/test/dummy/config/environments/test.rb +++ b/activestorage/test/dummy/config/environments/test.rb @@ -30,6 +30,9 @@ Rails.application.configure do # Print deprecation notices to the stderr. config.active_support.deprecation = :stderr + # Disable request forgery protection in test environment. + config.action_controller.allow_forgery_protection = false + # Raises error for missing translations # config.action_view.raise_on_missing_translations = true end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 55da781f2a..aaf1d452ea 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +ENV["RAILS_ENV"] ||= "test" require_relative "dummy/config/environment.rb" require "bundler/setup" -- cgit v1.2.3 From ae7593e7e842ec5efb8d58a7ce005ba55fd4c886 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 20 Nov 2017 10:57:29 -0500 Subject: Load 5.2 defaults in ASt dummy app --- activestorage/test/dummy/config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/test/dummy/config/application.rb b/activestorage/test/dummy/config/application.rb index 7ee6625bb5..06cbc453a2 100644 --- a/activestorage/test/dummy/config/application.rb +++ b/activestorage/test/dummy/config/application.rb @@ -19,7 +19,7 @@ Bundler.require(*Rails.groups) module Dummy class Application < Rails::Application - config.load_defaults 5.1 + config.load_defaults 5.2 config.active_storage.service = :local end -- cgit v1.2.3 From 1d24e47140356f136471d15e3ce3fa427f4430c2 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 20 Nov 2017 18:06:06 -0500 Subject: Provide attachment writers Permit creating a record and attaching files in a single step. # Before: User.create!(user_params.except(:avatar)).tap do |user| user.avatar.attach(user_params[:avatar]) end # After: User.create!(user_params) [Yoshiyuki Hirano & George Claghorn] --- .../lib/active_storage/attached/macros.rb | 8 ++++++ activestorage/test/models/attachments_test.rb | 32 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index f0256718ac..2b38a9b887 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -32,6 +32,10 @@ module ActiveStorage 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 @@ -73,6 +77,10 @@ module ActiveStorage 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" diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index edd68d3051..20eec3c220 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -76,6 +76,20 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 @user.new_record? + assert @user.avatar.attached? + assert_equal "town.jpg", @user.avatar.filename.to_s + + @user.save! + assert @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 @@ -204,6 +218,24 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase 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 @user.new_record? + assert @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 @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" }, -- cgit v1.2.3 From 3fa812615a28f9c6392c433f3b08c41c5efb999f Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Tue, 21 Nov 2017 14:17:12 -0500 Subject: Fix that some ASt route helpers silently discarded options --- activestorage/config/routes.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'activestorage') diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index c659e079fd..1eae21445a 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -3,38 +3,38 @@ Rails.application.routes.draw do get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob, internal: true - direct :rails_blob do |blob| - route_for(:rails_service_blob, blob.signed_id, blob.filename) + direct :rails_blob do |blob, options| + route_for(:rails_service_blob, blob.signed_id, blob.filename, options) end - resolve("ActiveStorage::Blob") { |blob| route_for(:rails_blob, blob) } - resolve("ActiveStorage::Attachment") { |attachment| route_for(:rails_blob, attachment.blob) } + resolve("ActiveStorage::Blob") { |blob, options| route_for(:rails_blob, blob) } + resolve("ActiveStorage::Attachment") { |attachment, options| route_for(:rails_blob, attachment.blob, options) } get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation, internal: true - direct :rails_variant do |variant| + direct :rails_variant do |variant, options| signed_blob_id = variant.blob.signed_id variation_key = variant.variation.key filename = variant.blob.filename - route_for(:rails_blob_variation, signed_blob_id, variation_key, filename) + route_for(:rails_blob_variation, signed_blob_id, variation_key, filename, options) end - resolve("ActiveStorage::Variant") { |variant| route_for(:rails_variant, variant) } + resolve("ActiveStorage::Variant") { |variant, options| route_for(:rails_variant, variant, options) } get "/rails/active_storage/previews/:signed_blob_id/:variation_key/*filename" => "active_storage/previews#show", as: :rails_blob_preview, internal: true - direct :rails_preview do |preview| + direct :rails_preview do |preview, options| signed_blob_id = preview.blob.signed_id variation_key = preview.variation.key filename = preview.blob.filename - route_for(:rails_blob_preview, signed_blob_id, variation_key, filename) + route_for(:rails_blob_preview, signed_blob_id, variation_key, filename, options) end - resolve("ActiveStorage::Preview") { |preview| route_for(:rails_preview, preview) } + resolve("ActiveStorage::Preview") { |preview, options| route_for(:rails_preview, preview, options) } get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service, internal: true -- cgit v1.2.3 From 4d5f0bb30b5ac76407c9864b83b69b8a83ac3dd6 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Tue, 21 Nov 2017 14:59:30 -0500 Subject: Fix loading ActiveStorage::DiskController when CSRF protection is disabled by default --- activestorage/app/controllers/active_storage/disk_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index 8caecfff49..a7e10c0696 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -5,7 +5,7 @@ # Always go through the BlobsController, or your own authenticated controller, rather than directly # to the service url. class ActiveStorage::DiskController < ActionController::Base - skip_forgery_protection + skip_forgery_protection if default_protect_from_forgery def show if key = decode_verified_key -- cgit v1.2.3 From fbb12910bdf256bbe2a95909fff114243d75424d Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 23 Nov 2017 19:48:25 -0500 Subject: Avoid connecting to GCS during app boot --- activestorage/lib/active_storage/service/gcs_service.rb | 17 ++++++++++++----- activestorage/test/service/gcs_service_test.rb | 9 ++------- 2 files changed, 14 insertions(+), 12 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index b4ffeeeb8a..be6ddf32a0 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -7,11 +7,8 @@ module ActiveStorage # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API # documentation that applies to all services. class Service::GCSService < Service - attr_reader :client, :bucket - - def initialize(project:, keyfile:, bucket:, **options) - @client = Google::Cloud::Storage.new(project: project, keyfile: keyfile, **options) - @bucket = @client.bucket(bucket) + def initialize(**config) + @config = config end def upload(key, io, checksum: nil) @@ -85,8 +82,18 @@ module ActiveStorage end private + attr_reader :config + def file_for(key) bucket.file(key, skip_lookup: true) end + + def bucket + @bucket ||= client.bucket(config.fetch(:bucket)) + end + + def client + @client ||= Google::Cloud::Storage.new(config.except(:bucket)) + end end end diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index 5566c664a9..1860149da9 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -32,13 +32,8 @@ if SERVICE_CONFIGURATIONS[:gcs] end test "signed URL generation" do - freeze_time do - url = SERVICE.bucket.signed_url(FIXTURE_KEY, expires: 120) + - "&response-content-disposition=inline%3B+filename%3D%22test.txt%22%3B+filename%2A%3DUTF-8%27%27test.txt" + - "&response-content-type=text%2Fplain" - - assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") - end + assert_match(/storage\.googleapis\.com\/.*response-content-disposition=inline.*test\.txt.*response-content-type=text%2Fplain/, + @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")) end end else -- cgit v1.2.3 From fb09e05bdf6b2cd73a19e7a71087130f6b95c961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 27 Nov 2017 12:55:20 -0500 Subject: Update yarn lock --- activestorage/yarn.lock | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'activestorage') diff --git a/activestorage/yarn.lock b/activestorage/yarn.lock index dd09577445..41742be201 100644 --- a/activestorage/yarn.lock +++ b/activestorage/yarn.lock @@ -1219,12 +1219,6 @@ escope@^3.6.0: esrecurse "^4.1.0" estraverse "^4.1.1" -eslint-config-airbnb-base@^11.3.1: - version "11.3.1" - resolved "https://registry.yarnpkg.com/eslint-config-airbnb-base/-/eslint-config-airbnb-base-11.3.1.tgz#c0ab108c9beed503cb999e4c60f4ef98eda0ed30" - dependencies: - eslint-restricted-globals "^0.1.1" - eslint-import-resolver-node@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/eslint-import-resolver-node/-/eslint-import-resolver-node-0.3.1.tgz#4422574cde66a9a7b099938ee4d508a199e0e3cc" @@ -1254,10 +1248,6 @@ eslint-plugin-import@^2.7.0: minimatch "^3.0.3" read-pkg-up "^2.0.0" -eslint-restricted-globals@^0.1.1: - version "0.1.1" - resolved "https://registry.yarnpkg.com/eslint-restricted-globals/-/eslint-restricted-globals-0.1.1.tgz#35f0d5cbc64c2e3ed62e93b4b1a7af05ba7ed4d7" - eslint-scope@^3.7.1: version "3.7.1" resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-3.7.1.tgz#3d63c3edfda02e06e01a452ad88caacc7cdcb6e8" -- cgit v1.2.3 From cceeeb6e57f1cf8b24d507e2da9ed85d374c8bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 27 Nov 2017 13:01:15 -0500 Subject: Preparing for 5.2.0.beta1 release --- activestorage/CHANGELOG.md | 2 ++ activestorage/lib/active_storage/gem_version.rb | 2 +- activestorage/package.json | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) (limited to 'activestorage') diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 358552313f..1006e4e06f 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,5 @@ +## Rails 5.2.0.beta1 (November 27, 2017) ## + * Added to Rails. *DHH* diff --git a/activestorage/lib/active_storage/gem_version.rb b/activestorage/lib/active_storage/gem_version.rb index e1d7b3493a..b5e783793b 100644 --- a/activestorage/lib/active_storage/gem_version.rb +++ b/activestorage/lib/active_storage/gem_version.rb @@ -10,7 +10,7 @@ module ActiveStorage MAJOR = 5 MINOR = 2 TINY = 0 - PRE = "alpha" + PRE = "beta1" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activestorage/package.json b/activestorage/package.json index 8e6dd1c57f..77ef35060b 100644 --- a/activestorage/package.json +++ b/activestorage/package.json @@ -1,6 +1,6 @@ { "name": "activestorage", - "version": "5.2.0-alpha", + "version": "5.2.0-beta1", "description": "Attach cloud and local files in Rails applications", "main": "app/assets/javascripts/activestorage.js", "files": [ -- cgit v1.2.3 From acaa0795b26a811456daab5b175831553090e8fc Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Tue, 28 Nov 2017 07:48:56 +0900 Subject: Include migration files in gem Fixes #31245 --- activestorage/activestorage.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 911e1a0469..7f7f1a26ac 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |s| s.email = "david@loudthinking.com" s.homepage = "http://rubyonrails.org" - s.files = Dir["CHANGELOG.md", "MIT-LICENSE", "README.md", "lib/**/*", "app/**/*", "config/**/*"] + s.files = Dir["CHANGELOG.md", "MIT-LICENSE", "README.md", "lib/**/*", "app/**/*", "config/**/*", "db/**/*"] s.require_path = "lib" s.metadata = { -- cgit v1.2.3 From 37cf9b34667965402e804d75a78ccdeeb7d55166 Mon Sep 17 00:00:00 2001 From: Fatos Morina Date: Tue, 28 Nov 2017 19:27:43 +0100 Subject: Fix typos and add a few suggestions --- activestorage/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'activestorage') diff --git a/activestorage/README.md b/activestorage/README.md index 78e4463c5a..8af0409ec5 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -143,3 +143,17 @@ Active Storage, with its included JavaScript library, supports uploading directl ## License Active Storage is released under the [MIT License](https://opensource.org/licenses/MIT). + + ## Support + +API documentation is at: + +* http://api.rubyonrails.org + +Bug reports for the Ruby on Rails project can be filed here: + +* https://github.com/rails/rails/issues + +Feature requests should be discussed on the rails-core mailing list here: + +* https://groups.google.com/forum/?fromgroups#!forum/rubyonrails-core -- cgit v1.2.3 From 2837d0f3347e747a8c12bd3c097bc7282072d42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 28 Nov 2017 00:01:45 -0500 Subject: Preparing for 5.2.0.beta2 release --- activestorage/CHANGELOG.md | 7 +++++++ activestorage/lib/active_storage/gem_version.rb | 2 +- activestorage/package.json | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) (limited to 'activestorage') diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 1006e4e06f..c5171e7490 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,10 @@ +## Rails 5.2.0.beta2 (November 28, 2017) ## + +* Fix the gem adding the migrations files to the package. + + *Yuji Yaginuma* + + ## Rails 5.2.0.beta1 (November 27, 2017) ## * Added to Rails. diff --git a/activestorage/lib/active_storage/gem_version.rb b/activestorage/lib/active_storage/gem_version.rb index b5e783793b..f048bb0b77 100644 --- a/activestorage/lib/active_storage/gem_version.rb +++ b/activestorage/lib/active_storage/gem_version.rb @@ -10,7 +10,7 @@ module ActiveStorage MAJOR = 5 MINOR = 2 TINY = 0 - PRE = "beta1" + PRE = "beta2" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activestorage/package.json b/activestorage/package.json index 77ef35060b..621706000b 100644 --- a/activestorage/package.json +++ b/activestorage/package.json @@ -1,6 +1,6 @@ { "name": "activestorage", - "version": "5.2.0-beta1", + "version": "5.2.0-beta2", "description": "Attach cloud and local files in Rails applications", "main": "app/assets/javascripts/activestorage.js", "files": [ -- cgit v1.2.3 From 9d65ac30fde3977d4773ff6052f31b99a5084f0f Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Wed, 29 Nov 2017 13:08:33 +0900 Subject: Use `credentials` instead of `keyfile` in GCS sevice The `keyfile` was renamed to `credentials` in `google-cloud-storage` 1.8.0. https://github.com/GoogleCloudPlatform/google-cloud-ruby/blob/master/google-cloud-storage/CHANGELOG.md#180--2017-11-14 Although `keyfile` can still be used, but it looks like deprecate. https://github.com/GoogleCloudPlatform/google-cloud-ruby/blob/ddf7b2a856d676316525eb581c1a4cc83ca6097b/google-cloud-storage/lib/google/cloud/storage.rb#L589...L590 Therefore, I think that should use `credentials` in newly generated applications. Ref: https://github.com/GoogleCloudPlatform/google-cloud-ruby/issues/1802 --- activestorage/lib/active_storage/service/gcs_service.rb | 2 ++ activestorage/test/service/configurations.example.yml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index be6ddf32a0..fd9916634a 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "google-cloud-storage", "~> 1.8" + require "google/cloud/storage" require "active_support/core_ext/object/to_query" diff --git a/activestorage/test/service/configurations.example.yml b/activestorage/test/service/configurations.example.yml index 56ed37be5d..43cc013bc8 100644 --- a/activestorage/test/service/configurations.example.yml +++ b/activestorage/test/service/configurations.example.yml @@ -7,7 +7,7 @@ # # gcs: # service: GCS -# keyfile: { +# credentials: { # type: "service_account", # project_id: "", # private_key_id: "", -- cgit v1.2.3 From d041a1dcbaced9f44ed976a48312c9f492fffe5e Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 30 Nov 2017 23:54:03 -0500 Subject: Add ActiveStorage::Previewer#logger to match ActiveStorage::Analyzer#logger --- activestorage/lib/active_storage/previewer.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index ed75bae3b5..3d485988e9 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -54,5 +54,9 @@ module ActiveStorage IO.popen(argv) { |out| IO.copy_stream(out, to) } to.rewind end + + def logger + ActiveStorage.logger + end end end -- cgit v1.2.3 From b852ef2660dac36e348865b455fab7fbcc0d2a7f Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 1 Dec 2017 11:07:30 -0500 Subject: Make ASt previewer/analyzer binary paths configurable --- .../lib/active_storage/analyzer/video_analyzer.rb | 4 +++- activestorage/lib/active_storage/engine.rb | 19 ++++++++++++++++++- .../lib/active_storage/previewer/pdf_previewer.rb | 9 ++++++++- .../lib/active_storage/previewer/video_previewer.rb | 4 +++- 4 files changed, 32 insertions(+), 4 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index 408b5e58e9..b6fc54d917 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -19,6 +19,8 @@ module ActiveStorage # This analyzer requires the {ffmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. You must # install ffmpeg yourself to use this analyzer. class Analyzer::VideoAnalyzer < Analyzer + class_attribute :ffprobe_path, default: "ffprobe" + def self.accept?(blob) blob.video? end @@ -68,7 +70,7 @@ module ActiveStorage end def probe_from(file) - IO.popen([ "ffprobe", "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output| + IO.popen([ ffprobe_path, "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output| JSON.parse(output.read) end rescue Errno::ENOENT diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 6cf6635c4f..b870e6d4d6 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -15,7 +15,8 @@ module ActiveStorage config.active_storage = ActiveSupport::OrderedOptions.new config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] - config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] + config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] + config.active_storage.paths = ActiveSupport::OrderedOptions.new config.eager_load_namespaces << ActiveStorage @@ -68,5 +69,21 @@ module ActiveStorage end end end + + initializer "active_storage.paths" do + config.after_initialize do |app| + if ffprobe_path = app.config.active_storage.paths.ffprobe + ActiveStorage::Analyzer::VideoAnalyzer.ffprobe_path = ffprobe_path + end + + if ffmpeg_path = app.config.active_storage.paths.ffmpeg + ActiveStorage::Previewer::VideoPreviewer.ffmpeg_path = ffmpeg_path + end + + if mutool_path = app.config.active_storage.paths.mutool + ActiveStorage::Previewer::PDFPreviewer.mutool_path = mutool_path + end + end + end end end diff --git a/activestorage/lib/active_storage/previewer/pdf_previewer.rb b/activestorage/lib/active_storage/previewer/pdf_previewer.rb index a2f05c74a6..b84aefcc9c 100644 --- a/activestorage/lib/active_storage/previewer/pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/pdf_previewer.rb @@ -2,16 +2,23 @@ module ActiveStorage class Previewer::PDFPreviewer < Previewer + class_attribute :mutool_path, default: "mutool" + def self.accept?(blob) blob.content_type == "application/pdf" end def preview download_blob_to_tempfile do |input| - draw "mutool", "draw", "-F", "png", "-o", "-", input.path, "1" do |output| + draw_first_page_from input do |output| yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" end end end + + private + def draw_first_page_from(file, &block) + draw mutool_path, "draw", "-F", "png", "-o", "-", file.path, "1", &block + end end end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb index 49f128d142..5d06e33f44 100644 --- a/activestorage/lib/active_storage/previewer/video_previewer.rb +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -2,6 +2,8 @@ module ActiveStorage class Previewer::VideoPreviewer < Previewer + class_attribute :ffmpeg_path, default: "ffmpeg" + def self.accept?(blob) blob.video? end @@ -16,7 +18,7 @@ module ActiveStorage private def draw_relevant_frame_from(file, &block) - draw "ffmpeg", "-i", file.path, "-y", "-vcodec", "png", + draw ffmpeg_path, "-i", file.path, "-y", "-vcodec", "png", "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-", &block end end -- cgit v1.2.3 From 8c5a7fbefd3cad403e7594d0b6a5488d80d4c98e Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Sat, 2 Dec 2017 22:43:28 -0500 Subject: Purge variants with their blobs --- activestorage/app/models/active_storage/blob.rb | 3 ++- activestorage/lib/active_storage/log_subscriber.rb | 4 +++ activestorage/lib/active_storage/service.rb | 9 +++++-- .../service/azure_storage_service.rb | 30 +++++++++++++++++----- .../lib/active_storage/service/disk_service.rb | 22 +++++++++++----- .../lib/active_storage/service/gcs_service.rb | 18 ++++++++----- .../lib/active_storage/service/mirror_service.rb | 5 ++++ .../lib/active_storage/service/s3_service.rb | 20 ++++++++++----- activestorage/test/models/blob_test.rb | 10 +++++++- activestorage/test/service/shared_service_tests.rb | 17 ++++++++++++ 10 files changed, 107 insertions(+), 31 deletions(-) (limited to 'activestorage') diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 2aa05d665e..acaf22fac1 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -270,7 +270,8 @@ class ActiveStorage::Blob < ActiveRecord::Base # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+ # methods in most circumstances. def delete - service.delete key + service.delete(key) + service.delete_prefixed("variants/#{key}/") if image? end # Deletes the file on the service and then destroys the blob record. This is the recommended way to dispose of unwanted diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index 5cbf4bd1a5..a4e148c1a5 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -18,6 +18,10 @@ module ActiveStorage info event, color("Deleted file from key: #{key_in(event)}", RED) end + def service_delete_prefixed(event) + info event, color("Deleted files by key prefix: #{event.payload[:prefix]}", RED) + end + def service_exist(event) debug event, color("Checked if file exists at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE) end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index aa150e4d8a..c8f675db86 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -78,6 +78,11 @@ module ActiveStorage raise NotImplementedError end + # Delete files at keys starting with the +prefix+. + def delete_prefixed(prefix) + raise NotImplementedError + end + # Return +true+ if a file exists at the +key+. def exist?(key) raise NotImplementedError @@ -104,10 +109,10 @@ module ActiveStorage end private - def instrument(operation, key, payload = {}, &block) + def instrument(operation, payload = {}, &block) ActiveSupport::Notifications.instrument( "service_#{operation}.active_storage", - payload.merge(key: key, service: service_name), &block) + payload.merge(service: service_name), &block) end def service_name diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index f3877ad9c9..98de0836de 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -19,7 +19,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin blobs.create_block_blob(container, key, io, content_md5: checksum) rescue Azure::Core::Http::HTTPError @@ -30,11 +30,11 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do _, io = blobs.get_blob(container, key) io.force_encoding(Encoding::BINARY) end @@ -42,7 +42,7 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin blobs.delete_blob(container, key) rescue Azure::Core::Http::HTTPError @@ -51,8 +51,24 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_all, prefix: prefix do + marker = nil + + loop do + results = blobs.list_blobs(container, prefix: prefix, marker: marker) + + results.each do |blob| + blobs.delete_blob(container, blob.name) + end + + break unless marker = results.continuation_token.presence + end + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = blob_for(key).present? payload[:exist] = answer answer @@ -60,7 +76,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri( URI(base_url), false, @@ -77,7 +93,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri(URI(base_url), false, permissions: "rw", expiry: format_expiry(expires_in)).to_s diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 52eaba4e7b..a8728c5bc3 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -16,7 +16,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do IO.copy_stream(io, make_path_for(key)) ensure_integrity_of(key, checksum) if checksum end @@ -24,7 +24,7 @@ module ActiveStorage def download(key) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do File.open(path_for(key), "rb") do |file| while data = file.read(64.kilobytes) yield data @@ -32,14 +32,14 @@ module ActiveStorage end end else - instrument :download, key do + instrument :download, key: key do File.binread path_for(key) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin File.delete path_for(key) rescue Errno::ENOENT @@ -48,8 +48,16 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + Dir.glob(path_for("#{prefix}*")).each do |path| + FileUtils.rm_rf(path) + end + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = File.exist? path_for(key) payload[:exist] = answer answer @@ -57,7 +65,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key) generated_url = @@ -77,7 +85,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_token_with_expiration = ActiveStorage.verifier.generate( { key: key, diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index fd9916634a..c13ce4786d 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -14,7 +14,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin bucket.create_file(io, key, md5: checksum) rescue Google::Cloud::InvalidArgumentError @@ -25,7 +25,7 @@ module ActiveStorage # FIXME: Download in chunks when given a block. def download(key) - instrument :download, key do + instrument :download, key: key do io = file_for(key).download io.rewind @@ -38,7 +38,7 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin file_for(key).delete rescue Google::Cloud::NotFoundError @@ -47,8 +47,14 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.files(prefix: prefix).all(&:delete) + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = file_for(key).exists? payload[:exist] = answer answer @@ -56,7 +62,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, content_type:, disposition:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = file_for(key).signed_url expires: expires_in, query: { "response-content-disposition" => content_disposition_with(type: disposition, filename: filename), "response-content-type" => content_type @@ -69,7 +75,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_type: content_type, content_md5: checksum diff --git a/activestorage/lib/active_storage/service/mirror_service.rb b/activestorage/lib/active_storage/service/mirror_service.rb index 39e922f7ab..7eca8ce7f4 100644 --- a/activestorage/lib/active_storage/service/mirror_service.rb +++ b/activestorage/lib/active_storage/service/mirror_service.rb @@ -35,6 +35,11 @@ module ActiveStorage perform_across_services :delete, key end + # Delete files at keys starting with the +prefix+ on all services. + def delete_prefixed(prefix) + perform_across_services :delete_prefixed, prefix + end + private def each_service(&block) [ primary, *mirrors ].each(&block) diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 6957119780..c95672f338 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -17,7 +17,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) rescue Aws::S3::Errors::BadDigest @@ -28,24 +28,30 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do object_for(key).get.body.read.force_encoding(Encoding::BINARY) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do object_for(key).delete end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.objects(prefix: prefix).batch_delete! + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = object_for(key).exists? payload[:exist] = answer answer @@ -53,7 +59,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :get, expires_in: expires_in.to_i, response_content_disposition: content_disposition_with(type: disposition, filename: filename), response_content_type: content_type @@ -65,7 +71,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i, content_type: content_type, content_length: content_length, content_md5: checksum diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 6e815997ba..f94e65ed77 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -41,13 +41,21 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "purge removes from external service" do + test "purge deletes file from external service" do blob = create_blob blob.purge assert_not ActiveStorage::Blob.service.exist?(blob.key) end + test "purge deletes variants from external service" do + blob = create_file_blob + variant = blob.variant(resize: "100>").processed + + blob.purge + assert_not ActiveStorage::Blob.service.exist?(variant.key) + end + private def expected_url_for(blob, disposition: :inline) query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index ade91ab89a..ce28c4393a 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -75,5 +75,22 @@ module ActiveStorage::Service::SharedServiceTests @service.delete SecureRandom.base58(24) end end + + test "deleting by prefix" do + begin + @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) + @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) + @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) + + @service.delete_prefixed("a/a/") + assert_not @service.exist?("a/a/a") + assert_not @service.exist?("a/a/b") + assert @service.exist?("a/b/a") + ensure + @service.delete("a/a/a") + @service.delete("a/a/b") + @service.delete("a/b/a") + end + end end end -- cgit v1.2.3 From 7609ca08ce5000689838eeb04ed37084bf364f78 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sun, 3 Dec 2017 18:06:29 +0100 Subject: Fix instrumention name: delete_prefixed like the others. --- activestorage/lib/active_storage/service/azure_storage_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 98de0836de..19b09991b3 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -52,7 +52,7 @@ module ActiveStorage end def delete_prefixed(prefix) - instrument :delete_all, prefix: prefix do + instrument :delete_prefixed, prefix: prefix do marker = nil loop do -- cgit v1.2.3 From e8286ee272a3e51daebc198519accd1f6895a8d2 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 7 Dec 2017 15:14:22 -0500 Subject: Fix customizing Content-Type via GCS service URLs --- activestorage/lib/active_storage/service/gcs_service.rb | 8 +++++++- activestorage/test/service/gcs_service_test.rb | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index c13ce4786d..6f6f4105fe 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -16,7 +16,13 @@ module ActiveStorage def upload(key, io, checksum: nil) instrument :upload, key: key, checksum: checksum do begin - bucket.create_file(io, key, md5: checksum) + # The official GCS client library doesn't allow us to create a file with no Content-Type metadata. + # We need the file we create to have no Content-Type so we can control it via the response-content-type + # param in signed URLs. Workaround: let the GCS client create the file with an inferred + # Content-Type (usually "application/octet-stream") then clear it. + bucket.create_file(io, key, md5: checksum).update do |file| + file.content_type = nil + end rescue Google::Cloud::InvalidArgumentError raise ActiveStorage::IntegrityError end diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index 1860149da9..7efcd60fb7 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -35,6 +35,20 @@ if SERVICE_CONFIGURATIONS[:gcs] assert_match(/storage\.googleapis\.com\/.*response-content-disposition=inline.*test\.txt.*response-content-type=text%2Fplain/, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")) end + + test "signed URL response headers" do + begin + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.header["Content-Type"] + ensure + @service.delete key + end + end end else puts "Skipping GCS Service tests because no GCS configuration was supplied" -- cgit v1.2.3 From da8e0ba03cbae33857954c0c1a228bd6dae562da Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 8 Dec 2017 13:15:04 -0500 Subject: Swap raw video width and height if angle is 90 or 270 degrees --- .../lib/active_storage/analyzer/video_analyzer.rb | 14 +++++++++++++- activestorage/test/analyzer/video_analyzer_test.rb | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'activestorage') diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index b6fc54d917..1c144baa37 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -31,10 +31,18 @@ module ActiveStorage private def width - Integer(video_stream["width"]) if video_stream["width"] + rotated? ? raw_height : raw_width end def height + rotated? ? raw_width : raw_height + end + + def raw_width + Integer(video_stream["width"]) if video_stream["width"] + end + + def raw_height Integer(video_stream["height"]) if video_stream["height"] end @@ -52,6 +60,10 @@ module ActiveStorage end end + def rotated? + angle == 90 || angle == 270 + end + def tags @tags ||= video_stream["tags"] || {} diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 4a3c4a8bfc..b3b9c97fe4 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -21,8 +21,8 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") metadata = blob.tap(&:analyze).metadata - assert_equal 640, metadata[:width] - assert_equal 480, metadata[:height] + assert_equal 480, metadata[:width] + assert_equal 640, metadata[:height] assert_equal [4, 3], metadata[:aspect_ratio] assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] -- cgit v1.2.3