diff --git a/Gemfile b/Gemfile index 1797d20194..8e2031ed85 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ gem 'rake' gem 'byebug' gem 'sqlite3' +gem 'httparty' gem 'aws-sdk', require: false gem 'google-cloud-storage', require: false diff --git a/Gemfile.lock b/Gemfile.lock index c4f3d9fd89..797996cdc3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -83,6 +83,8 @@ GEM multi_json (~> 1.11) os (~> 0.9) signet (~> 0.7) + httparty (0.15.5) + multi_xml (>= 0.5.2) httpclient (2.8.3) i18n (0.8.4) jmespath (1.3.1) @@ -100,6 +102,7 @@ GEM mini_portile2 (2.1.0) minitest (5.10.2) multi_json (1.12.1) + multi_xml (0.6.0) multipart-post (2.0.0) nokogiri (1.7.2) mini_portile2 (~> 2.1.0) @@ -139,6 +142,7 @@ DEPENDENCIES bundler (~> 1.15) byebug google-cloud-storage + httparty rake sqlite3 diff --git a/lib/active_storage/blob.rb b/lib/active_storage/blob.rb index 26c116712b..3336c4ebc3 100644 --- a/lib/active_storage/blob.rb +++ b/lib/active_storage/blob.rb @@ -25,6 +25,10 @@ class ActiveStorage::Blob < ActiveRecord::Base def create_after_upload!(io:, filename:, content_type: nil, metadata: nil) build_after_upload(io: io, filename: filename, content_type: content_type, metadata: metadata).tap(&:save!) end + + def create_before_direct_upload!(filename:, byte_size:, checksum:, content_type: nil, metadata: nil) + create! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type, metadata: metadata + end end # We can't wait until the record is first saved to have a key for it @@ -40,6 +44,10 @@ class ActiveStorage::Blob < ActiveRecord::Base service.url key, expires_in: expires_in, disposition: disposition, filename: filename end + def url_for_direct_upload(expires_in: 5.minutes) + service.url_for_direct_upload key, expires_in: expires_in, content_type: content_type, content_length: byte_size + end + def upload(io) self.checksum = compute_checksum_in_chunks(io) diff --git a/lib/active_storage/direct_uploads_controller.rb b/lib/active_storage/direct_uploads_controller.rb new file mode 100644 index 0000000000..99ff27f903 --- /dev/null +++ b/lib/active_storage/direct_uploads_controller.rb @@ -0,0 +1,14 @@ +require "action_controller" +require "active_storage/blob" + +class ActiveStorage::DirectUploadsController < ActionController::Base + def create + blob = ActiveStorage::Blob.create_before_direct_upload!(blob_args) + render json: { url: blob.url_for_direct_upload, sgid: blob.to_sgid.to_param } + end + + private + def blob_args + params.require(:blob).permit(:filename, :byte_size, :checksum, :content_type, :metadata).to_h.symbolize_keys + end +end diff --git a/lib/active_storage/engine.rb b/lib/active_storage/engine.rb index adcf42ee58..c251f522c6 100644 --- a/lib/active_storage/engine.rb +++ b/lib/active_storage/engine.rb @@ -16,10 +16,11 @@ module ActiveStorage initializer "active_storage.routes" do require "active_storage/disk_controller" + require "active_storage/direct_uploads_controller" config.after_initialize do |app| app.routes.prepend do - get "/rails/blobs/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_blob + eval(File.read(File.expand_path("../routes.rb", __FILE__))) end end end diff --git a/lib/active_storage/routes.rb b/lib/active_storage/routes.rb new file mode 100644 index 0000000000..748427a776 --- /dev/null +++ b/lib/active_storage/routes.rb @@ -0,0 +1,2 @@ +get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_blob +post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads diff --git a/lib/active_storage/service.rb b/lib/active_storage/service.rb index f50849b694..d0d4362006 100644 --- a/lib/active_storage/service.rb +++ b/lib/active_storage/service.rb @@ -78,6 +78,10 @@ class ActiveStorage::Service raise NotImplementedError end + def url_for_direct_upload(key, expires_in:, content_type:, content_length:) + raise NotImplementedError + end + private def instrument(operation, key, payload = {}, &block) ActiveSupport::Notifications.instrument( diff --git a/lib/active_storage/service/disk_service.rb b/lib/active_storage/service/disk_service.rb index e2d9191189..87fc06c799 100644 --- a/lib/active_storage/service/disk_service.rb +++ b/lib/active_storage/service/disk_service.rb @@ -59,7 +59,7 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service if defined?(Rails) && defined?(Rails.application) Rails.application.routes.url_helpers.rails_disk_blob_path(verified_key_with_expiration, disposition: disposition, filename: filename) else - "/rails/blobs/#{verified_key_with_expiration}/#{filename}?disposition=#{disposition}" + "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?disposition=#{disposition}" end payload[:url] = generated_url diff --git a/lib/active_storage/service/s3_service.rb b/lib/active_storage/service/s3_service.rb index 53890751ee..c3b6688bb9 100644 --- a/lib/active_storage/service/s3_service.rb +++ b/lib/active_storage/service/s3_service.rb @@ -56,6 +56,17 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service end end + def url_for_direct_upload(key, expires_in:, content_type:, content_length:) + 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 + private def object_for(key) bucket.object(key) diff --git a/test/blob_test.rb b/test/blob_test.rb index ac9ca37487..60cf5426a8 100644 --- a/test/blob_test.rb +++ b/test/blob_test.rb @@ -23,6 +23,6 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase private def expected_url_for(blob, disposition: :inline) - "/rails/blobs/#{ActiveStorage::VerifiedKeyWithExpiration.encode(blob.key, expires_in: 5.minutes)}/#{blob.filename}?disposition=#{disposition}" + "/rails/active_storage/disk/#{ActiveStorage::VerifiedKeyWithExpiration.encode(blob.key, expires_in: 5.minutes)}/#{blob.filename}?disposition=#{disposition}" end end diff --git a/test/direct_uploads_controller_test.rb b/test/direct_uploads_controller_test.rb new file mode 100644 index 0000000000..bed985148e --- /dev/null +++ b/test/direct_uploads_controller_test.rb @@ -0,0 +1,36 @@ +require "test_helper" +require "database/setup" + +require "action_controller" +require "action_controller/test_case" + +require "active_storage/direct_uploads_controller" + +if SERVICE_CONFIGURATIONS[:s3] + class ActiveStorage::DirectUploadsControllerTest < ActionController::TestCase + setup do + @blob = create_blob + @routes = Routes + @controller = ActiveStorage::DirectUploadsController.new + + @old_service = ActiveStorage::Blob.service + ActiveStorage::Blob.service = ActiveStorage::Service.configure(:s3, SERVICE_CONFIGURATIONS) + end + + teardown do + ActiveStorage::Blob.service = @old_service + end + + test "creating new direct upload" do + post :create, params: { blob: { + filename: "hello.txt", byte_size: 6, checksum: Digest::MD5.base64digest("Hello"), content_type: "text/plain" } } + + details = JSON.parse(@response.body) + + assert_match /rails-activestorage\.s3.amazonaws\.com/, details["url"] + assert_equal "hello.txt", GlobalID::Locator.locate_signed(details["sgid"]).filename.to_s + end + end +else + puts "Skipping Direct Upload tests because no S3 configuration was supplied" +end diff --git a/test/disk_controller_test.rb b/test/disk_controller_test.rb index 119ee5828f..834ad1bfd9 100644 --- a/test/disk_controller_test.rb +++ b/test/disk_controller_test.rb @@ -1,19 +1,10 @@ require "test_helper" require "database/setup" -require "action_controller" -require "action_controller/test_case" - require "active_storage/disk_controller" require "active_storage/verified_key_with_expiration" class ActiveStorage::DiskControllerTest < ActionController::TestCase - Routes = ActionDispatch::Routing::RouteSet.new.tap do |routes| - routes.draw do - get "/rails/blobs/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_blob - end - end - setup do @blob = create_blob @routes = Routes diff --git a/test/service/disk_service_test.rb b/test/service/disk_service_test.rb index c5404f55e6..565acbf516 100644 --- a/test/service/disk_service_test.rb +++ b/test/service/disk_service_test.rb @@ -7,7 +7,7 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests test "url generation" do - assert_match /rails\/blobs\/.*\/avatar\.png\?disposition=inline/, + assert_match /rails\/active_storage\/disk\/.*\/avatar\.png\?disposition=inline/, @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png") end end diff --git a/test/service/s3_service_test.rb b/test/service/s3_service_test.rb index 3e1838e393..167aa78a17 100644 --- a/test/service/s3_service_test.rb +++ b/test/service/s3_service_test.rb @@ -1,4 +1,6 @@ require "service/shared_service_tests" +require "httparty" +require "uri" if SERVICE_CONFIGURATIONS[:s3] class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase @@ -6,6 +8,35 @@ if SERVICE_CONFIGURATIONS[:s3] include ActiveStorage::Service::SharedServiceTests + test "direct upload" do + # FIXME: This test is failing because of a mismatched request signature, but it works in the browser. + skip + + begin + key = SecureRandom.base58(24) + data = "Something else entirely!" + direct_upload_url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size) + + url = URI.parse(direct_upload_url).to_s.split("?").first + query = CGI::parse(URI.parse(direct_upload_url).query).collect { |(k, v)| [ k, v.first ] }.to_h + + HTTParty.post( + url, + query: query, + body: data, + headers: { + "Content-Type": "text/plain", + "Origin": "http://localhost:3000" + }, + debug_output: STDOUT + ) + + assert_equal data, @service.download(key) + ensure + @service.delete key + end + end + test "signed URL generation" do assert_match /rails-activestorage\.s3\.amazonaws\.com.*response-content-disposition=inline.*avatar\.png/, @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png") diff --git a/test/service/shared_service_tests.rb b/test/service/shared_service_tests.rb index e799c24c35..ad6a9dea7f 100644 --- a/test/service/shared_service_tests.rb +++ b/test/service/shared_service_tests.rb @@ -1,13 +1,5 @@ require "test_helper" require "active_support/core_ext/securerandom" -require "yaml" - -SERVICE_CONFIGURATIONS = begin - YAML.load_file(File.expand_path("../configurations.yml", __FILE__)).deep_symbolize_keys -rescue Errno::ENOENT - puts "Missing service configuration file in test/service/configurations.yml" - {} -end module ActiveStorage::Service::SharedServiceTests extend ActiveSupport::Concern diff --git a/test/test_helper.rb b/test/test_helper.rb index b67296a659..ca1e0cad7e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -5,6 +5,17 @@ require "active_support/testing/autorun" require "byebug" require "active_storage" + +require "active_storage/service" +require "yaml" +SERVICE_CONFIGURATIONS = begin + YAML.load_file(File.expand_path("../service/configurations.yml", __FILE__)).deep_symbolize_keys +rescue Errno::ENOENT + puts "Missing service configuration file in test/service/configurations.yml" + {} +end + + require "active_storage/service/disk_service" ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: File.join(Dir.tmpdir, "active_storage")) ActiveStorage::Service.logger = ActiveSupport::Logger.new(STDOUT) @@ -19,6 +30,16 @@ class ActiveSupport::TestCase end end +require "action_controller" +require "action_controller/test_case" + +class ActionController::TestCase + Routes = ActionDispatch::Routing::RouteSet.new.tap do |routes| + routes.draw do + eval(File.read(File.expand_path("../../lib/active_storage/routes.rb", __FILE__))) + end + end +end require "active_storage/attached" ActiveRecord::Base.send :extend, ActiveStorage::Attached::Macros @@ -26,3 +47,4 @@ ActiveRecord::Base.send :extend, ActiveStorage::Attached::Macros require "global_id" GlobalID.app = "ActiveStorageExampleApp" ActiveRecord::Base.send :include, GlobalID::Identification +SignedGlobalID.verifier = ActiveStorage::VerifiedKeyWithExpiration.verifier \ No newline at end of file