diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index a5601279d259e..fec3bdb88202a 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Setting custom metadata on blobs are now persisted to remote storage. + + *joshuamsager* + * Support direct uploads to multiple services. *Dmitry Tsepelev* diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 1731c2c9cae8c..086a4969488c9 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -52,7 +52,7 @@ class ActiveStorage::Blob < ActiveStorage::Record self.service_name ||= self.class.service&.name end - after_update_commit :update_service_metadata, if: :content_type_previously_changed? + after_update_commit :update_service_metadata, if: -> { content_type_previously_changed? || metadata_previously_changed? } before_destroy(prepend: true) do raise ActiveRecord::InvalidForeignKey if attachments.exists? @@ -168,6 +168,14 @@ def filename ActiveStorage::Filename.new(self[:filename]) end + def custom_metadata + self[:metadata][:custom] || {} + end + + def custom_metadata=(metadata) + self[:metadata] = self[:metadata].merge(custom: metadata) + end + # Returns true if the content_type of this blob is in the image range, like image/png. def image? content_type.start_with?("image") @@ -200,12 +208,12 @@ def url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline, # 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: ActiveStorage.service_urls_expire_in) - service.url_for_direct_upload key, expires_in: expires_in, content_type: content_type, content_length: byte_size, checksum: checksum + service.url_for_direct_upload key, expires_in: expires_in, content_type: content_type, content_length: byte_size, checksum: checksum, custom_metadata: custom_metadata end # Returns a Hash of headers for +service_url_for_direct_upload+ requests. def service_headers_for_direct_upload - service.headers_for_direct_upload key, filename: filename, content_type: content_type, content_length: byte_size, checksum: checksum + service.headers_for_direct_upload key, filename: filename, content_type: content_type, content_length: byte_size, checksum: checksum, custom_metadata: custom_metadata end def content_type_for_serving # :nodoc: @@ -362,11 +370,11 @@ def web_image? def service_metadata if forcibly_serve_as_binary? - { content_type: ActiveStorage.binary_content_type, disposition: :attachment, filename: filename } + { content_type: ActiveStorage.binary_content_type, disposition: :attachment, filename: filename, custom_metadata: custom_metadata } elsif !allowed_inline? - { content_type: content_type, disposition: :attachment, filename: filename } + { content_type: content_type, disposition: :attachment, filename: filename, custom_metadatata: custom_metadata } else - { content_type: content_type } + { content_type: content_type, custom_metadata: custom_metadata } end end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index 952e9cd019e2b..7ccd380107a3b 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -128,12 +128,12 @@ def url(key, **options) # The URL will be valid for the amount of seconds specified in +expires_in+. # You must also provide the +content_type+, +content_length+, and +checksum+ of the file # that will be uploaded. All these attributes will be validated by the service upon upload. - def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) + def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:, custom_metadata: {}) raise NotImplementedError end # Returns a Hash of headers for +url_for_direct_upload+ requests. - def headers_for_direct_upload(key, filename:, content_type:, content_length:, checksum:) + def headers_for_direct_upload(key, filename:, content_type:, content_length:, checksum:, custom_metadata: {}) {} end @@ -150,6 +150,9 @@ def public_url(key, **) raise NotImplementedError end + def custom_metadata_headers(metadata) + raise NotImplementedError + end def instrument(operation, payload = {}, &block) ActiveSupport::Notifications.instrument( diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index e078a98d0f64c..1e23e9113002a 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -19,12 +19,12 @@ def initialize(storage_account_name:, storage_access_key:, container:, public: f @public = public end - def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, custom_metadata: {}, **) instrument :upload, key: key, checksum: checksum do handle_errors do content_disposition = content_disposition_with(filename: filename, type: disposition) if disposition && filename - client.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition) + client.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition, metadata: custom_metadata) end end end @@ -86,7 +86,7 @@ def exist?(key) end end - def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) + def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:, custom_metadata: {}) instrument :url, key: key do |payload| generated_url = signer.signed_uri( uri_for(key), false, @@ -101,10 +101,10 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:, chec end end - def headers_for_direct_upload(key, content_type:, checksum:, filename: nil, disposition: nil, **) + def headers_for_direct_upload(key, content_type:, checksum:, filename: nil, disposition: nil, custom_metadata:, **) content_disposition = content_disposition_with(type: disposition, filename: filename) if filename - { "Content-Type" => content_type, "Content-MD5" => checksum, "x-ms-blob-content-disposition" => content_disposition, "x-ms-blob-type" => "BlockBlob" } + { "Content-Type" => content_type, "Content-MD5" => checksum, "x-ms-blob-content-disposition" => content_disposition, "x-ms-blob-type" => "BlockBlob", **custom_metadata_headers(custom_metadata) } end private @@ -166,5 +166,9 @@ def handle_errors raise end end + + def custom_metadata_headers(metadata) + metadata.transform_keys { |key| "x-ms-meta-#{key}" } + end end end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 59fb3f0226e81..9ed1f2238b7b4 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -72,7 +72,7 @@ def exist?(key) end end - def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) + def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:, custom_metadata: {}) instrument :url, key: key do |payload| verified_token_with_expiration = ActiveStorage.verifier.generate( { diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 9b39fd44a8ab7..f05bd3d7aee57 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -16,14 +16,14 @@ def initialize(public: false, **config) @public = public end - def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename: nil) + def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename: nil, custom_metadata: {}) instrument :upload, key: key, checksum: checksum do # GCS's signed URLs don't include params such as response-content-type response-content_disposition # in the signature, which means an attacker can modify them and bypass our effort to force these to # binary and attachment when the file's content type requires it. The only way to force them is to # store them as object's metadata. content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename - bucket.create_file(io, key, md5: checksum, cache_control: @config[:cache_control], content_type: content_type, content_disposition: content_disposition) + bucket.create_file(io, key, md5: checksum, cache_control: @config[:cache_control], content_type: content_type, content_disposition: content_disposition, metadata: custom_metadata) rescue Google::Cloud::InvalidArgumentError raise ActiveStorage::IntegrityError end @@ -43,11 +43,12 @@ def download(key, &block) end end - def update_metadata(key, content_type:, disposition: nil, filename: nil) + def update_metadata(key, content_type:, disposition: nil, filename: nil, custom_metadata: {}) instrument :update_metadata, key: key, content_type: content_type, disposition: disposition do file_for(key).update do |file| file.content_type = content_type file.content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename + file.metadata = custom_metadata end end end @@ -86,7 +87,7 @@ def exist?(key) end end - def url_for_direct_upload(key, expires_in:, checksum:, **) + def url_for_direct_upload(key, expires_in:, checksum:, custom_metadata: {}, **) instrument :url, key: key do |payload| headers = {} version = :v2 @@ -99,6 +100,8 @@ def url_for_direct_upload(key, expires_in:, checksum:, **) version = :v4 end + headers.merge!(custom_metadata_headers(custom_metadata)) + args = { content_md5: checksum, expires: expires_in, @@ -120,11 +123,10 @@ def url_for_direct_upload(key, expires_in:, checksum:, **) end end - def headers_for_direct_upload(key, checksum:, filename: nil, disposition: nil, **) + def headers_for_direct_upload(key, checksum:, filename: nil, disposition: nil, custom_metadata: {}, **) content_disposition = content_disposition_with(type: disposition, filename: filename) if filename - headers = { "Content-MD5" => checksum, "Content-Disposition" => content_disposition } - + headers = { "Content-MD5" => checksum, "Content-Disposition" => content_disposition, **custom_metadata_headers(custom_metadata) } if @config[:cache_control].present? headers["Cache-Control"] = @config[:cache_control] end @@ -223,5 +225,9 @@ def signer response.signed_blob end end + + def custom_metadata_headers(metadata) + metadata.transform_keys { |key| "x-goog-meta-#{key}" } + end end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 134831f6b03b1..e6192b5890067 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -23,14 +23,14 @@ def initialize(bucket:, upload: {}, public: false, **options) @upload_options[:acl] = "public-read" if public? end - def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, custom_metadata: {}, **) 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, content_disposition: content_disposition + upload_with_single_part key, io, checksum: checksum, content_type: content_type, content_disposition: content_disposition, custom_metadata: custom_metadata else - upload_with_multipart key, io, content_type: content_type, content_disposition: content_disposition + upload_with_multipart key, io, content_type: content_type, content_disposition: content_disposition, custom_metadata: custom_metadata end end end @@ -77,11 +77,11 @@ def exist?(key) end end - def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) + def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:, custom_metadata: {}) 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, - whitelist_headers: ["content-length"], **upload_options + metadata: custom_metadata, whitelist_headers: ["content-length"], **upload_options payload[:url] = generated_url @@ -89,10 +89,10 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:, chec end end - def headers_for_direct_upload(key, content_type:, checksum:, filename: nil, disposition: nil, **) + def headers_for_direct_upload(key, content_type:, checksum:, filename: nil, disposition: nil, custom_metadata: {}, **) content_disposition = content_disposition_with(type: disposition, filename: filename) if filename - { "Content-Type" => content_type, "Content-MD5" => checksum, "Content-Disposition" => content_disposition } + { "Content-Type" => content_type, "Content-MD5" => checksum, "Content-Disposition" => content_disposition, **custom_metadata_headers(custom_metadata) } end private @@ -110,16 +110,16 @@ def public_url(key, **client_opts) MAXIMUM_UPLOAD_PARTS_COUNT = 10000 MINIMUM_UPLOAD_PART_SIZE = 5.megabytes - 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) + def upload_with_single_part(key, io, checksum: nil, content_type: nil, content_disposition: nil, custom_metadata: {}) + object_for(key).put(body: io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition, metadata: custom_metadata, **upload_options) rescue Aws::S3::Errors::BadDigest raise ActiveStorage::IntegrityError end - def upload_with_multipart(key, io, content_type: nil, content_disposition: nil) + def upload_with_multipart(key, io, content_type: nil, content_disposition: nil, custom_metadata: {}) part_size = [ io.size.fdiv(MAXIMUM_UPLOAD_PARTS_COUNT).ceil, MINIMUM_UPLOAD_PART_SIZE ].max - object_for(key).upload_stream(content_type: content_type, content_disposition: content_disposition, 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, metadata: custom_metadata, **upload_options) do |out| IO.copy_stream(io, out) end end @@ -143,5 +143,9 @@ def stream(key) offset += chunk_size end end + + def custom_metadata_headers(metadata) + metadata.transform_keys { |key| "x-amz-meta-#{key}" } + end end end diff --git a/activestorage/test/controllers/direct_uploads_controller_test.rb b/activestorage/test/controllers/direct_uploads_controller_test.rb index 8bd5fd8b5c840..d768d5b019f0f 100644 --- a/activestorage/test/controllers/direct_uploads_controller_test.rb +++ b/activestorage/test/controllers/direct_uploads_controller_test.rb @@ -22,7 +22,10 @@ class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::Integration "my_key_1": "my_value_1", "my_key_2": "my_value_2", "platform": "my_platform", - "library_ID": "12345" + "library_ID": "12345", + custom: { + "my_key_3": "my_value_3" + } } ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "s3") do @@ -39,7 +42,7 @@ class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::Integration assert_equal "text/plain", details["content_type"] assert_match SERVICE_CONFIGURATIONS[:s3][:bucket], details["direct_upload"]["url"] assert_match(/s3(-[-a-z0-9]+)?\.(\S+)?amazonaws\.com/, details["direct_upload"]["url"]) - assert_equal({ "Content-Type" => "text/plain", "Content-MD5" => checksum, "Content-Disposition" => "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt" }, details["direct_upload"]["headers"]) + assert_equal({ "Content-Type" => "text/plain", "Content-MD5" => checksum, "Content-Disposition" => "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", "x-amz-meta-my_key_3" => "my_value_3" }, details["direct_upload"]["headers"]) end end end @@ -67,7 +70,10 @@ class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::Integratio "my_key_1": "my_value_1", "my_key_2": "my_value_2", "platform": "my_platform", - "library_ID": "12345" + "library_ID": "12345", + custom: { + "my_key_3": "my_value_3" + } } ActiveStorage::DirectUploadToken.stub(:verify_direct_upload_token, "gcs") do @@ -83,7 +89,7 @@ class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::Integratio assert_equal metadata, details["metadata"].transform_keys(&:to_sym) assert_equal "text/plain", details["content_type"] assert_match %r{storage\.googleapis\.com/#{@config[:bucket]}}, details["direct_upload"]["url"] - assert_equal({ "Content-MD5" => checksum, "Content-Disposition" => "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt" }, details["direct_upload"]["headers"]) + assert_equal({ "Content-MD5" => checksum, "Content-Disposition" => "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", "x-goog-meta-my_key_3" => "my_value_3" }, details["direct_upload"]["headers"]) end end end diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index b6a9355517f09..aa4c58dba3f0f 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -259,13 +259,31 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase test "updating the content_type updates service metadata" do blob = directly_upload_file_blob(filename: "racecar.jpg", content_type: "application/octet-stream") - expected_arguments = [blob.key, content_type: "image/jpeg"] + expected_arguments = [blob.key, content_type: "image/jpeg", custom_metadata: {}] assert_called_with(blob.service, :update_metadata, expected_arguments) do blob.update!(content_type: "image/jpeg") end end + test "updating the metadata updates service metadata" do + blob = directly_upload_file_blob(filename: "racecar.jpg", content_type: "application/octet-stream") + + expected_arguments = [ + blob.key, + { + content_type: "application/octet-stream", + disposition: :attachment, + filename: blob.filename, + custom_metadatata: { "test" => true } + } + ] + + assert_called_with(blob.service, :update_metadata, expected_arguments) do + blob.update!(metadata: { custom: { "test" => true } }) + end + end + test "scope_for_strict_loading adds includes only when track_variants and strict_loading_by_default" do assert_empty( ActiveStorage::Blob.scope_for_strict_loading.includes_values, diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 488c3fc5ddb39..bceb2e0f9483b 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -81,6 +81,19 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase @service.delete key end + test "upload with custom_metadata" do + key = SecureRandom.base58(24) + data = "Foobar" + + @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), custom_metadata: { "foo" => "baz" }) + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + + response = Net::HTTP.get_response(URI(url)) + assert_equal("baz", response["x-ms-meta-foo"]) + 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/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index 2b637cc0cd891..b49074101e2b5 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -132,17 +132,31 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase service.delete key end - test "update metadata" do + test "upload with custom_metadata" do key = SecureRandom.base58(24) data = "Something else entirely!" - @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html") + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain", custom_metadata: { "foo" => "baz" }) - @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + + response = Net::HTTP.get_response(URI(url)) + assert_equal("baz", response["x-goog-meta-foo"]) + ensure + @service.delete key + end + + test "update custom_metadata" do + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html", custom_metadata: { "foo" => "baz" }) + + @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain", custom_metadata: { "foo" => "bar" }) url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) response = Net::HTTP.get_response(URI(url)) assert_equal "text/plain", response.content_type assert_match(/inline;.*test.txt/, response["Content-Disposition"]) + assert_equal("bar", response["x-goog-meta-foo"]) ensure @service.delete key end diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index 8ee8fb34e9aef..11ca847b52dce 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -125,6 +125,26 @@ class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase @service.delete key end + test "upload with custom_metadata" do + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload( + key, + StringIO.new(data), + checksum: Digest::MD5.base64digest(data), + content_type: "text/plain", + custom_metadata: { "foo" => "baz" }, + filename: "custom_metadata.txt" + ) + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + + response = Net::HTTP.get_response(URI(url)) + assert_equal("baz", response["x-amz-meta-foo"]) + ensure + @service.delete key + end + test "upload with content disposition" do key = SecureRandom.base58(24) data = "Something else entirely!"