diff options
Diffstat (limited to 'activestorage')
17 files changed, 138 insertions, 25 deletions
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index fdb0f143f4..1475a7a786 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `config.active_storage.draw_routes` to disable Active Storage routes. + + *Gannon McGibbon* + * Image analysis is skipped if ImageMagick returns an error. `ActiveStorage::Analyzer::ImageAnalyzer#metadata` would previously raise a diff --git a/activestorage/app/jobs/active_storage/mirror_job.rb b/activestorage/app/jobs/active_storage/mirror_job.rb index e34faedb56..e70629d6ec 100644 --- a/activestorage/app/jobs/active_storage/mirror_job.rb +++ b/activestorage/app/jobs/active_storage/mirror_job.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + # Provides asynchronous mirroring of directly-uploaded blobs. class ActiveStorage::MirrorJob < ActiveStorage::BaseJob queue_as { ActiveStorage.queues[:mirror] } diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index 3af7361cff..bde53e72f3 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -29,4 +29,4 @@ Rails.application.routes.draw do resolve("ActiveStorage::Blob") { |blob, options| route_for(:rails_blob, blob, options) } resolve("ActiveStorage::Attachment") { |attachment, options| route_for(:rails_blob, attachment.blob, options) } -end +end if ActiveStorage.draw_routes diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 5c5da551ae..c35a9920d6 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -43,17 +43,26 @@ module ActiveStorage mattr_accessor :logger mattr_accessor :verifier + mattr_accessor :variant_processor, default: :mini_magick + mattr_accessor :queues, default: {} + mattr_accessor :previewers, default: [] - mattr_accessor :analyzers, default: [] - mattr_accessor :variant_processor, default: :mini_magick + mattr_accessor :analyzers, default: [] + mattr_accessor :paths, default: {} - mattr_accessor :variable_content_types, default: [] + + mattr_accessor :variable_content_types, default: [] + mattr_accessor :binary_content_type, default: "application/octet-stream" mattr_accessor :content_types_to_serve_as_binary, default: [] - mattr_accessor :content_types_allowed_inline, default: [] - mattr_accessor :binary_content_type, default: "application/octet-stream" + mattr_accessor :content_types_allowed_inline, default: [] + mattr_accessor :service_urls_expire_in, default: 5.minutes + mattr_accessor :routes_prefix, default: "/rails/active_storage" + mattr_accessor :draw_routes, default: true + + mattr_accessor :replace_on_assign_to_many, default: false module Transformers extend ActiveSupport::Autoload diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb index ae7f0685f2..4bd6b56b6c 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActiveStorage # Provides the class-level DSL for declaring an Active Record model's attachments. module Attached::Model @@ -93,12 +95,19 @@ module ActiveStorage end def #{name}=(attachables) - attachment_changes["#{name}"] = - if attachables.nil? || Array(attachables).none? - ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self) - else - ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables) + if ActiveStorage.replace_on_assign_to_many + attachment_changes["#{name}"] = + if attachables.nil? || Array(attachables).none? + ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self) + else + ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables) + end + else + if !attachables.nil? || Array(attachables).any? + attachment_changes["#{name}"] = + ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, #{name}.blobs + attachables) end + end end CODE diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index f70d0a512a..9d9cd02d12 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -73,12 +73,15 @@ module ActiveStorage ActiveStorage.analyzers = app.config.active_storage.analyzers || [] ActiveStorage.paths = app.config.active_storage.paths || {} ActiveStorage.routes_prefix = app.config.active_storage.routes_prefix || "/rails/active_storage" + ActiveStorage.draw_routes = app.config.active_storage.draw_routes != false ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || [] ActiveStorage.content_types_to_serve_as_binary = app.config.active_storage.content_types_to_serve_as_binary || [] ActiveStorage.service_urls_expire_in = app.config.active_storage.service_urls_expire_in || 5.minutes ActiveStorage.content_types_allowed_inline = app.config.active_storage.content_types_allowed_inline || [] ActiveStorage.binary_content_type = app.config.active_storage.binary_content_type || "application/octet-stream" + + ActiveStorage.replace_on_assign_to_many = app.config.active_storage.replace_on_assign_to_many || false 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 993cc0e5f7..0648da70b5 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -17,10 +17,12 @@ module ActiveStorage @container = container end - def upload(key, io, checksum: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) instrument :upload, key: key, checksum: checksum do handle_errors do - blobs.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum) + content_disposition = content_disposition_with(filename: filename, type: disposition) if disposition && filename + + blobs.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition) end end end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 67892d43b2..764a447c69 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -84,8 +84,12 @@ module ActiveStorage purpose: :blob_key } ) + current_uri = URI.parse(current_host) + generated_url = url_helpers.rails_disk_service_url(verified_key_with_expiration, - host: current_host, + protocol: current_uri.scheme, + host: current_uri.host, + port: current_uri.port, disposition: content_disposition, content_type: content_type, filename: filename diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index e4bd57048a..a73f6ab526 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -20,12 +20,14 @@ module ActiveStorage @upload_options = upload end - def upload(key, io, checksum: nil, content_type: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) instrument :upload, key: key, checksum: checksum do + content_disposition = content_disposition_with(filename: filename, type: disposition) if disposition && filename + if io.size < multipart_upload_threshold - upload_with_single_part key, io, checksum: checksum, content_type: content_type + upload_with_single_part key, io, checksum: checksum, content_type: content_type, content_disposition: content_disposition else - upload_with_multipart key, io, content_type: content_type + upload_with_multipart key, io, content_type: content_type, content_disposition: content_disposition end end end @@ -103,16 +105,16 @@ module ActiveStorage MAXIMUM_UPLOAD_PARTS_COUNT = 10000 MINIMUM_UPLOAD_PART_SIZE = 5.megabytes - def upload_with_single_part(key, io, checksum: nil, content_type: nil) - object_for(key).put(body: io, content_md5: checksum, content_type: content_type, **upload_options) + def upload_with_single_part(key, io, checksum: nil, content_type: nil, content_disposition: nil) + object_for(key).put(body: io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition, **upload_options) rescue Aws::S3::Errors::BadDigest raise ActiveStorage::IntegrityError end - def upload_with_multipart(key, io, content_type: nil) + def upload_with_multipart(key, io, content_type: nil, content_disposition: nil) part_size = [ io.size.fdiv(MAXIMUM_UPLOAD_PARTS_COUNT).ceil, MINIMUM_UPLOAD_PART_SIZE ].max - object_for(key).upload_stream(content_type: content_type, part_size: part_size, **upload_options) do |out| + object_for(key).upload_stream(content_type: content_type, content_disposition: content_disposition, part_size: part_size, **upload_options) do |out| IO.copy_stream(io, out) end end diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index d30f49315a..172a2f0aae 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -24,7 +24,6 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase assert_equal 480, metadata[:width] assert_equal 640, metadata[:height] assert_equal [4, 3], metadata[:display_aspect_ratio] - assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] end diff --git a/activestorage/test/dummy/config/initializers/backtrace_silencers.rb b/activestorage/test/dummy/config/initializers/backtrace_silencers.rb index d0f0d3b5df..8452ef9236 100644 --- a/activestorage/test/dummy/config/initializers/backtrace_silencers.rb +++ b/activestorage/test/dummy/config/initializers/backtrace_silencers.rb @@ -2,7 +2,7 @@ # Be sure to restart your server when you modify this file. # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. -# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } +# Rails.backtrace_cleaner.add_silencer { |line| /my_noisy_library/.match?(line) } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. # Rails.backtrace_cleaner.remove_silencers! diff --git a/activestorage/test/fixtures/files/racecar.tif b/activestorage/test/fixtures/files/racecar.tif Binary files differindex 0a11b22896..97430741e2 100644 --- a/activestorage/test/fixtures/files/racecar.tif +++ b/activestorage/test/fixtures/files/racecar.tif diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 878e284049..39ddecb041 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -269,6 +269,24 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "updating an existing record with attachments when appending on assign" do + append_on_assign do + @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + + assert_difference -> { @user.reload.highlights.count }, +2 do + @user.update! highlights: [ create_blob(filename: "whenever.jpg"), create_blob(filename: "wherever.jpg") ] + end + + assert_no_difference -> { @user.reload.highlights.count } do + @user.update! highlights: [ ] + end + + assert_no_difference -> { @user.reload.highlights.count } do + @user.update! highlights: nil + end + end + end + test "attaching existing blobs to a new record" do User.new(name: "Jason").tap do |user| user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") @@ -538,4 +556,12 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase User.remove_method :highlights end end + + private + def append_on_assign + ActiveStorage.replace_on_assign_to_many, previous = false, ActiveStorage.replace_on_assign_to_many + yield + ensure + ActiveStorage.replace_on_assign_to_many = previous + end end diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 2b07902d07..9eea1b94c8 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -9,6 +9,35 @@ if SERVICE_CONFIGURATIONS[:azure] include ActiveStorage::Service::SharedServiceTests + test "upload with content_type" do + key = SecureRandom.base58(24) + data = "Foobar" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + + url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: nil, filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/attachment;.*test\.html/, response["Content-Disposition"]) + ensure + @service.delete key + end + + test "upload with content disposition" do + key = SecureRandom.base58(24) + data = "Foobar" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), disposition: :inline) + + assert_equal("inline; filename=\"test.txt\"; filename*=UTF-8''test.txt", @service.blobs.get_blob_properties(@service.container, key).properties[:content_disposition]) + + url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: nil, filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_match(/attachment;.*test\.html/, response["Content-Disposition"]) + ensure + @service.delete key + end + test "signed URL generation" do url = @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") diff --git a/activestorage/test/service/disk_service_test.rb b/activestorage/test/service/disk_service_test.rb index f3c4dd26bd..b766cc3f56 100644 --- a/activestorage/test/service/disk_service_test.rb +++ b/activestorage/test/service/disk_service_test.rb @@ -8,8 +8,14 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests test "URL generation" do - assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline/, - @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")) + original_url_options = Rails.application.routes.default_url_options.dup + Rails.application.routes.default_url_options.merge!(protocol: "http", host: "test.example.com", port: 3001) + begin + assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline/, + @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")) + ensure + Rails.application.routes.default_url_options = original_url_options + end end test "headers_for_direct_upload generation" do diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index b9120770e6..3dd1b59681 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -77,6 +77,23 @@ if SERVICE_CONFIGURATIONS[:s3] @service.delete key end + test "upload with content disposition" do + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload( + key, + StringIO.new(data), + checksum: Digest::MD5.base64digest(data), + filename: ActiveStorage::Filename.new("cool_data.txt"), + disposition: :attachment + ) + + assert_equal("attachment; filename=\"cool_data.txt\"; filename*=UTF-8''cool_data.txt", @service.bucket.object(key).content_disposition) + ensure + @service.delete key + end + test "uploading a large object in multiple parts" do service = build_service(upload: { multipart_threshold: 5.megabytes }) diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index ac38b9362c..164b0acd96 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -6,6 +6,7 @@ require_relative "dummy/config/environment.rb" require "bundler/setup" require "active_support" require "active_support/test_case" +require "active_support/core_ext/object/try" require "active_support/testing/autorun" require "active_storage/service/mirror_service" require "image_processing/mini_magick" |