diff options
Diffstat (limited to 'activestorage')
| -rw-r--r-- | activestorage/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | activestorage/app/controllers/active_storage/disk_controller.rb | 15 | ||||
| -rw-r--r-- | activestorage/lib/active_storage/service/gcs_service.rb | 44 | ||||
| -rw-r--r-- | activestorage/test/controllers/disk_controller_test.rb | 10 | ||||
| -rw-r--r-- | activestorage/test/service/shared_service_tests.rb | 18 |
5 files changed, 62 insertions, 30 deletions
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 60d7d19540..4c12adae56 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,8 @@ +* The Google Cloud Storage service properly supports streaming downloads. + It now requires version 1.11 or newer of the google-cloud-storage gem. + + *George Claghorn* + * Use the [ImageProcessing](https://github.com/janko-m/image_processing) gem for Active Storage variants, and deprecate the MiniMagick backend. diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index 7bc5eb3fdb..63918eb6f4 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -5,21 +5,30 @@ # Always go through the BlobsController, or your own authenticated controller, rather than directly # to the service url. class ActiveStorage::DiskController < ActiveStorage::BaseController + include ActionController::Live + skip_forgery_protection def show if key = decode_verified_key - send_data disk_service.download(key), - disposition: params[:disposition], content_type: params[:content_type] + response.headers["Content-Type"] = params[:content_type] || DEFAULT_SEND_FILE_TYPE + response.headers["Content-Disposition"] = params[:disposition] || DEFAULT_SEND_FILE_DISPOSITION + + disk_service.download key do |chunk| + response.stream.write chunk + end else head :not_found end + ensure + response.stream.close end def update if token = decode_verified_token if acceptable_content?(token) disk_service.upload token[:key], request.body, checksum: token[:checksum] + head :no_content else head :unprocessable_entity end @@ -28,6 +37,8 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController end rescue ActiveStorage::IntegrityError head :unprocessable_entity + ensure + response.stream.close end private diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 7a1839a1aa..38acef81f4 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -gem "google-cloud-storage", "~> 1.8" +gem "google-cloud-storage", "~> 1.11" require "google/cloud/storage" require "net/http" @@ -32,28 +32,21 @@ module ActiveStorage end end - # FIXME: Download in chunks when given a block. - def download(key) - instrument :download, key: key do - io = file_for(key).download - io.rewind - - if block_given? - yield io.string - else - io.string + def download(key, &block) + if block_given? + instrument :streaming_download, key: key do + stream(key, &block) + end + else + instrument :download, key: key do + file_for(key).download.string end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - file = file_for(key) - uri = URI(file.signed_url(expires: 30.seconds)) - - Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client| - client.get(uri, "Range" => "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body - end + file_for(key).download(range: range).string end end @@ -111,8 +104,21 @@ module ActiveStorage private attr_reader :config - def file_for(key) - bucket.file(key, skip_lookup: true) + def file_for(key, skip_lookup: true) + bucket.file(key, skip_lookup: skip_lookup) + end + + # Reads the file for the given key in chunks, yielding each to the block. + def stream(key) + file = file_for(key, skip_lookup: false) + + chunk_size = 5.megabytes + offset = 0 + + while offset < file.size + yield file.download(range: offset..(offset + chunk_size - 1)).string + offset += chunk_size + end end def bucket diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb index 940dbf5918..9af7c83bdf 100644 --- a/activestorage/test/controllers/disk_controller_test.rb +++ b/activestorage/test/controllers/disk_controller_test.rb @@ -8,16 +8,18 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest blob = create_blob get blob.service_url - assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", @response.headers["Content-Disposition"] - assert_equal "text/plain", @response.headers["Content-Type"] + assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"] + assert_equal "text/plain", response.headers["Content-Type"] + assert_equal "Hello world!", response.body end test "showing blob as attachment" do blob = create_blob get blob.service_url(disposition: :attachment) - assert_equal "attachment; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", @response.headers["Content-Disposition"] - assert_equal "text/plain", @response.headers["Content-Type"] + assert_equal "attachment; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"] + assert_equal "text/plain", response.headers["Content-Type"] + assert_equal "Hello world!", response.body end diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index 24debe7f47..b9f352e460 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -51,13 +51,21 @@ module ActiveStorage::Service::SharedServiceTests end test "downloading in chunks" do - chunks = [] + key = SecureRandom.base58(24) + expected_chunks = [ "a" * 5.megabytes, "b" ] + actual_chunks = [] - @service.download(FIXTURE_KEY) do |chunk| - chunks << chunk - end + begin + @service.upload key, StringIO.new(expected_chunks.join) + + @service.download key do |chunk| + actual_chunks << chunk + end - assert_equal [ FIXTURE_DATA ], chunks + assert_equal expected_chunks, actual_chunks, "Downloaded chunks did not match uploaded data" + ensure + @service.delete key + end end test "downloading partially" do |