Merge branch 'master' into variants

10 files changed, 42 insertions, 29 deletions

diff --git a/lib/active_storage/blob.rb b/lib/active_storage/blob.rb
index 3336c4ebc3..1a15361747 100644
--- a/lib/active_storage/blob.rb
+++ b/lib/active_storage/blob.rb
@@ -56,8 +56,8 @@ class ActiveStorage::Blob < ActiveRecord::Base
     service.upload(key, io, checksum: checksum)
   end
 
-  def download
-    service.download key
+  def download(&block)
+    service.download key, &block
   end
 
 
diff --git a/lib/active_storage/disk_controller.rb b/lib/active_storage/disk_controller.rb
index 7149cc17a6..16a295d00d 100644
--- a/lib/active_storage/disk_controller.rb
+++ b/lib/active_storage/disk_controller.rb
@@ -33,6 +33,6 @@ class ActiveStorage::DiskController < ActionController::Base
     end
 
     def disposition_param
-      params[:disposition].presence_in(%w( inline attachment )) || 'inline'
+      params[:disposition].presence_in(%w( inline attachment )) || "inline"
     end
 end
diff --git a/lib/active_storage/download.rb b/lib/active_storage/download.rb
index 4d656942d8..6040a32de9 100644
--- a/lib/active_storage/download.rb
+++ b/lib/active_storage/download.rb
@@ -14,7 +14,7 @@ class ActiveStorage::Download
     application/xhtml+xml
   )
 
-  BINARY_CONTENT_TYPE = 'application/octet-stream'
+  BINARY_CONTENT_TYPE = "application/octet-stream"
 
   def initialize(stored_file)
     @stored_file = stored_file
@@ -22,11 +22,11 @@ class ActiveStorage::Download
 
   def headers(force_attachment: false)
     {
-      x_accel_redirect:    '/reproxy',
+      x_accel_redirect:    "/reproxy",
       x_reproxy_url:       reproxy_url,
       content_type:        content_type,
       content_disposition: content_disposition(force_attachment),
-      x_frame_options:     'SAMEORIGIN'
+      x_frame_options:     "SAMEORIGIN"
     }
   end
 
diff --git a/lib/active_storage/log_subscriber.rb b/lib/active_storage/log_subscriber.rb
index 5c486b9161..4ac34a3b25 100644
--- a/lib/active_storage/log_subscriber.rb
+++ b/lib/active_storage/log_subscriber.rb
@@ -2,9 +2,9 @@ require "active_support/log_subscriber"
 
 class ActiveStorage::LogSubscriber < ActiveSupport::LogSubscriber
   def service_upload(event)
-    message = color("Uploaded file to key: #{key_in(event)}", GREEN)
-    message << color(" (checksum: #{event.payload[:checksum]})", GREEN) if event.payload[:checksum]
-    info event, message
+    message = "Uploaded file to key: #{key_in(event)}"
+    message << " (checksum: #{event.payload[:checksum]})" if event.payload[:checksum]
+    info event, color(message, GREEN)
   end
 
   def service_download(event)
diff --git a/lib/active_storage/service.rb b/lib/active_storage/service.rb
index d0d4362006..cba9cd9c83 100644
--- a/lib/active_storage/service.rb
+++ b/lib/active_storage/service.rb
@@ -85,7 +85,7 @@ class ActiveStorage::Service
   private
     def instrument(operation, key, payload = {}, &block)
       ActiveSupport::Notifications.instrument(
-        "service_#{operation}.active_storage", 
+        "service_#{operation}.active_storage",
         payload.merge(key: key, service: service_name), &block)
     end
 
diff --git a/lib/active_storage/service/disk_service.rb b/lib/active_storage/service/disk_service.rb
index 87fc06c799..a2a27528c1 100644
--- a/lib/active_storage/service/disk_service.rb
+++ b/lib/active_storage/service/disk_service.rb
@@ -20,8 +20,8 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service
   def download(key)
     if block_given?
       instrument :streaming_download, key do
-        File.open(path_for(key)) do |file|
-          while data = file.binread(64.kilobytes)
+        File.open(path_for(key), "rb") do |file|
+          while data = file.read(64.kilobytes)
             yield data
           end
         end
@@ -55,7 +55,7 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service
     instrument :url, key do |payload|
       verified_key_with_expiration = ActiveStorage::VerifiedKeyWithExpiration.encode(key, expires_in: expires_in)
 
-      generated_url = 
+      generated_url =
         if defined?(Rails) && defined?(Rails.application)
           Rails.application.routes.url_helpers.rails_disk_blob_path(verified_key_with_expiration, disposition: disposition, filename: filename)
         else
@@ -63,7 +63,7 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service
         end
 
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
diff --git a/lib/active_storage/service/gcs_service.rb b/lib/active_storage/service/gcs_service.rb
index bca4ab5331..7053a130c0 100644
--- a/lib/active_storage/service/gcs_service.rb
+++ b/lib/active_storage/service/gcs_service.rb
@@ -44,11 +44,22 @@ class ActiveStorage::Service::GCSService < ActiveStorage::Service
 
   def url(key, expires_in:, disposition:, filename:)
     instrument :url, key do |payload|
-      generated_url = file_for(key).signed_url(expires: expires_in) + "&" +
-        { "response-content-disposition" => "#{disposition}; filename=\"#{filename}\"" }.to_query
-      
+      query = { "response-content-disposition" => "#{disposition}; filename=\"#{filename}\"" }
+      generated_url = file_for(key).signed_url(expires: expires_in, query: query)
+
+      payload[:url] = generated_url
+
+      generated_url
+    end
+  end
+
+  def url_for_direct_upload(key, expires_in:, content_type:, content_length:)
+    instrument :url, key do |payload|
+      generated_url = bucket.signed_url key, method: "PUT", expires: expires_in,
+        content_type: content_type
+
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
diff --git a/lib/active_storage/service/s3_service.rb b/lib/active_storage/service/s3_service.rb
index c3b6688bb9..efffdec157 100644
--- a/lib/active_storage/service/s3_service.rb
+++ b/lib/active_storage/service/s3_service.rb
@@ -2,17 +2,19 @@ require "aws-sdk"
 require "active_support/core_ext/numeric/bytes"
 
 class ActiveStorage::Service::S3Service < ActiveStorage::Service
-  attr_reader :client, :bucket
+  attr_reader :client, :bucket, :upload_options
 
-  def initialize(access_key_id:, secret_access_key:, region:, bucket:)
-    @client = Aws::S3::Resource.new(access_key_id: access_key_id, secret_access_key: secret_access_key, region: region)
+  def initialize(access_key_id:, secret_access_key:, region:, bucket:, upload: {}, **options)
+    @client = Aws::S3::Resource.new(access_key_id: access_key_id, secret_access_key: secret_access_key, region: region, **options)
     @bucket = @client.bucket(bucket)
+
+    @upload_options = upload
   end
 
   def upload(key, io, checksum: nil)
     instrument :upload, key, checksum: checksum do
       begin
-        object_for(key).put(body: io, content_md5: checksum)
+        object_for(key).put(upload_options.merge(body: io, content_md5: checksum))
       rescue Aws::S3::Errors::BadDigest
         raise ActiveStorage::IntegrityError
       end
@@ -49,9 +51,9 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service
     instrument :url, key do |payload|
       generated_url = object_for(key).presigned_url :get, expires_in: expires_in,
         response_content_disposition: "#{disposition}; filename=\"#{filename}\""
-      
+
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
@@ -60,9 +62,9 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service
     instrument :url, key do |payload|
       generated_url = object_for(key).presigned_url :put, expires_in: expires_in,
         content_type: content_type, content_length: content_length
-      
+
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
diff --git a/lib/active_storage/storage_services.yml b/lib/active_storage/storage_services.yml
index a93304d88f..c80a3e8453 100644
--- a/lib/active_storage/storage_services.yml
+++ b/lib/active_storage/storage_services.yml
@@ -9,8 +9,8 @@ local:
 # Use rails secrets:edit to set the AWS secrets (as shared:aws:access_key_id|secret_access_key)
 amazon:
   service: S3
-  access_key_id: <%= Rails.application.secrets.aws[:access_key_id] %>
-  secret_access_key: <%= Rails.application.secrets.aws[:secret_access_key] %>
+  access_key_id: <%= Rails.application.secrets.dig(:aws, :access_key_id) %>
+  secret_access_key: <%= Rails.application.secrets.dig(:aws, :secret_access_key) %>
   region: us-east-1
   bucket: your_own_bucket
 
diff --git a/lib/tasks/activestorage.rake b/lib/tasks/activestorage.rake
index 17dab0854a..ea83707224 100644
--- a/lib/tasks/activestorage.rake
+++ b/lib/tasks/activestorage.rake
@@ -7,7 +7,7 @@ namespace :activestorage do
     FileUtils.mkdir_p Rails.root.join("tmp/storage")
     puts "Made storage and tmp/storage directories for development and testing"
 
-    FileUtils.cp  File.expand_path("../../active_storage/storage_services.yml", __FILE__), Rails.root.join("config")
+    FileUtils.cp File.expand_path("../../active_storage/storage_services.yml", __FILE__), Rails.root.join("config")
     puts "Copied default configuration to config/storage_services.yml"
 
     migration_file_path = "db/migrate/#{Time.now.utc.strftime("%Y%m%d%H%M%S")}_active_storage_create_tables.rb"