diff options
-rw-r--r-- | Gemfile | 1 | ||||
-rw-r--r-- | Gemfile.lock | 9 | ||||
-rw-r--r-- | activestorage/activestorage.gemspec | 2 | ||||
-rw-r--r-- | activestorage/app/models/active_storage/blob/identifiable.rb | 10 | ||||
-rw-r--r-- | activestorage/app/models/active_storage/identification.rb | 35 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service.rb | 5 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service/azure_storage_service.rb | 7 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service/disk_service.rb | 28 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service/gcs_service.rb | 13 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service/mirror_service.rb | 2 | ||||
-rw-r--r-- | activestorage/lib/active_storage/service/s3_service.rb | 6 | ||||
-rw-r--r-- | activestorage/test/models/attachments_test.rb | 1 | ||||
-rw-r--r-- | activestorage/test/models/blob_test.rb | 2 | ||||
-rw-r--r-- | activestorage/test/service/configurator_test.rb | 2 | ||||
-rw-r--r-- | activestorage/test/service/shared_service_tests.rb | 5 | ||||
-rw-r--r-- | activestorage/test/test_helper.rb | 3 | ||||
-rw-r--r-- | guides/source/active_storage_overview.md | 9 |
17 files changed, 66 insertions, 74 deletions
@@ -19,7 +19,6 @@ gem "rack-cache", "~> 1.2" gem "coffee-rails" gem "sass-rails" gem "turbolinks", "~> 5" -gem "webmock" # require: false so bcrypt is loaded only when has_secure_password is used. # This is to avoid Active Model (and by extension the entire framework) diff --git a/Gemfile.lock b/Gemfile.lock index baac1cee7e..a5d946bf0c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -187,8 +187,6 @@ GEM concurrent-ruby (1.0.5-java) connection_pool (2.2.1) cookiejar (0.3.3) - crack (0.4.3) - safe_yaml (~> 1.0.0) crass (1.0.3) curses (1.0.2) daemons (1.2.4) @@ -261,7 +259,6 @@ GEM multi_json (~> 1.11) os (~> 0.9) signet (~> 0.7) - hashdiff (0.3.7) hiredis (0.6.1) hiredis (0.6.1-java) http_parser.rb (0.6.0) @@ -400,7 +397,6 @@ GEM rubyzip (1.2.1) rufus-scheduler (3.4.2) et-orbi (~> 1.0) - safe_yaml (1.0.4) sass (3.5.3) sass-listen (~> 4.0.0) sass-listen (4.0.0) @@ -482,10 +478,6 @@ GEM json (>= 1.8) nokogiri (~> 1.6) wdm (0.1.1) - webmock (3.2.1) - addressable (>= 2.3.6) - crack (>= 0.3.2) - hashdiff websocket (1.2.4) websocket-driver (0.6.5) websocket-extensions (>= 0.1.0) @@ -563,7 +555,6 @@ DEPENDENCIES uglifier (>= 1.3.0) w3c_validators wdm (>= 0.1.0) - webmock websocket-client-simple! BUNDLED WITH diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index df7801a671..cb1bb00a25 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -29,6 +29,4 @@ Gem::Specification.new do |s| s.add_dependency "activerecord", version s.add_dependency "marcel", "~> 0.3.1" - - s.add_development_dependency "webmock", "~> 3.2.1" end diff --git a/activestorage/app/models/active_storage/blob/identifiable.rb b/activestorage/app/models/active_storage/blob/identifiable.rb index dbe03cfa6c..049e45dc3e 100644 --- a/activestorage/app/models/active_storage/blob/identifiable.rb +++ b/activestorage/app/models/active_storage/blob/identifiable.rb @@ -2,7 +2,7 @@ module ActiveStorage::Blob::Identifiable def identify - update!(content_type: identification.content_type, identified: true) unless identified? + update! content_type: identify_content_type, identified: true unless identified? end def identified? @@ -10,7 +10,11 @@ module ActiveStorage::Blob::Identifiable end private - def identification - ActiveStorage::Identification.new self + def identify_content_type + Marcel::MimeType.for download_identifiable_chunk, name: filename.to_s, declared_type: content_type + end + + def download_identifiable_chunk + service.download_chunk key, 0...4.kilobytes end end diff --git a/activestorage/app/models/active_storage/identification.rb b/activestorage/app/models/active_storage/identification.rb deleted file mode 100644 index 8d334ae1ea..0000000000 --- a/activestorage/app/models/active_storage/identification.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require "net/http" - -class ActiveStorage::Identification #:nodoc: - attr_reader :blob - - def initialize(blob) - @blob = blob - end - - def content_type - Marcel::MimeType.for(identifiable_chunk, name: filename, declared_type: declared_content_type) - end - - private - def identifiable_chunk - Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client| - client.get(uri, "Range" => "bytes=0-4095").body - end - end - - def uri - @uri ||= URI.parse(blob.service_url) - end - - - def filename - blob.filename.to_s - end - - def declared_content_type - blob.content_type - end -end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index f2e1269f27..f2fda2d5cf 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -73,6 +73,11 @@ module ActiveStorage raise NotImplementedError end + # Return the partial content of the file at the +key+ between the +start+ and +stop+ byte offsets. + def download_chunk(key, range) + raise NotImplementedError + end + # Delete the file at the +key+. def delete(key) raise NotImplementedError diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 0a9eb7f23d..5682087469 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -41,6 +41,13 @@ module ActiveStorage end end + def download_chunk(key, range) + instrument :download_chunk, key: key, range: range do + _, io = blobs.get_blob(container, key, start_range: range.begin, end_range: range.exclude_end? ? range.end - 1 : range.end) + io.force_encoding(Encoding::BINARY) + end + end + def delete(key) instrument :delete, key: key do begin diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index d17eea9046..75b66081c3 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -9,10 +9,10 @@ module ActiveStorage # Wraps a local disk path as an Active Storage service. See ActiveStorage::Service for the generic API # documentation that applies to all services. class Service::DiskService < Service - attr_reader :root, :host + attr_reader :root - def initialize(root:, host: "http://localhost:3000") - @root, @host = root, host + def initialize(root:) + @root = root end def upload(key, io, checksum: nil) @@ -38,6 +38,15 @@ module ActiveStorage end end + def download_chunk(key, range) + instrument :download_chunk, key: key, range: range do + File.open(path_for(key), "rb") do |file| + file.seek range.begin + file.read range.size + end + end + end + def delete(key) instrument :delete, key: key do begin @@ -69,12 +78,11 @@ module ActiveStorage verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key) generated_url = - Rails.application.routes.url_helpers.rails_disk_service_url( + url_helpers.rails_disk_service_path( verified_key_with_expiration, filename: filename, disposition: content_disposition_with(type: disposition, filename: filename), - content_type: content_type, - host: host + content_type: content_type ) payload[:url] = generated_url @@ -96,7 +104,7 @@ module ActiveStorage purpose: :blob_token } ) - generated_url = Rails.application.routes.url_helpers.update_rails_disk_service_url(verified_token_with_expiration, host: host) + generated_url = url_helpers.update_rails_disk_service_path(verified_token_with_expiration) payload[:url] = generated_url @@ -121,11 +129,17 @@ module ActiveStorage path_for(key).tap { |path| FileUtils.mkdir_p File.dirname(path) } end + def ensure_integrity_of(key, checksum) unless Digest::MD5.file(path_for(key)).base64digest == checksum delete key raise ActiveStorage::IntegrityError end end + + + def url_helpers + @url_helpers ||= Rails.application.routes.url_helpers + end end end diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index d163605754..83e9bbb305 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -3,7 +3,10 @@ gem "google-cloud-storage", "~> 1.8" require "google/cloud/storage" +require "net/http" + require "active_support/core_ext/object/to_query" +require "active_storage/filename" module ActiveStorage # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API @@ -43,6 +46,16 @@ module ActiveStorage end end + def download_chunk(key, range) + instrument :download_chunk, key: key, range: range do + uri = URI(url(key, expires_in: 30.seconds, filename: ActiveStorage::Filename.new(""), content_type: "application/octet-stream", disposition: "inline")) + + 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 + end + end + def delete(key) instrument :delete, key: key do begin diff --git a/activestorage/lib/active_storage/service/mirror_service.rb b/activestorage/lib/active_storage/service/mirror_service.rb index 7eca8ce7f4..6002ef5a00 100644 --- a/activestorage/lib/active_storage/service/mirror_service.rb +++ b/activestorage/lib/active_storage/service/mirror_service.rb @@ -9,7 +9,7 @@ module ActiveStorage class Service::MirrorService < Service attr_reader :primary, :mirrors - delegate :download, :exist?, :url, to: :primary + delegate :download, :download_chunk, :exist?, :url, to: :primary # Stitch together from named services. def self.build(primary:, mirrors:, configurator:, **options) #:nodoc: diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index c95672f338..8ab7a44131 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -38,6 +38,12 @@ module ActiveStorage end end + def download_chunk(key, range) + instrument :download_chunk, key: key, range: range do + object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY) + end + end + def delete(key) instrument :delete, key: key do object_for(key).delete diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 25e0352eca..cb52a57b33 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -126,7 +126,6 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase blob = create_blob_before_direct_upload(filename: "racecar.jpg", content_type: "application/octet-stream", byte_size: 1124062, checksum: "7GjDDNEQb4mzMzsW+MS0JQ==") ActiveStorage::Blob.service.upload(blob.key, file_fixture("racecar.jpg").open) - stub_request(:get, %r{localhost:3000/rails/active_storage/disk/.*}).to_return(body: file_fixture("racecar.jpg")) @user.avatar.attach(blob) assert_equal "image/jpeg", @user.avatar.reload.content_type diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 779e47ffb6..202d0fb093 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -140,6 +140,6 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase def expected_url_for(blob, disposition: :inline, filename: nil) filename ||= blob.filename query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{filename.parameters}" }.to_param - "http://localhost:3000/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}" + "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}" end end diff --git a/activestorage/test/service/configurator_test.rb b/activestorage/test/service/configurator_test.rb index fe8a637ad0..1c9c5c3aa0 100644 --- a/activestorage/test/service/configurator_test.rb +++ b/activestorage/test/service/configurator_test.rb @@ -5,10 +5,8 @@ require "service/shared_service_tests" class ActiveStorage::Service::ConfiguratorTest < ActiveSupport::TestCase test "builds correct service instance based on service name" do service = ActiveStorage::Service::Configurator.build(:foo, foo: { service: "Disk", root: "path" }) - assert_instance_of ActiveStorage::Service::DiskService, service assert_equal "path", service.root - assert_equal "http://localhost:3000", service.host end test "raises error when passing non-existent service name" do diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index ce28c4393a..24debe7f47 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -60,6 +60,11 @@ module ActiveStorage::Service::SharedServiceTests assert_equal [ FIXTURE_DATA ], chunks end + test "downloading partially" do + assert_equal "\x10\x00\x00", @service.download_chunk(FIXTURE_KEY, 19..21) + assert_equal "\x10\x00\x00", @service.download_chunk(FIXTURE_KEY, 19...22) + end + test "existing" do assert @service.exist?(FIXTURE_KEY) assert_not @service.exist?(FIXTURE_KEY + "nonsense") diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 98fa44a604..aaf1d452ea 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -7,7 +7,6 @@ require "bundler/setup" require "active_support" require "active_support/test_case" require "active_support/testing/autorun" -require "webmock/minitest" require "mini_magick" begin @@ -42,8 +41,6 @@ ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing") class ActiveSupport::TestCase self.file_fixture_path = File.expand_path("fixtures/files", __dir__) - setup { WebMock.allow_net_connect! } - private def create_blob(data: "Hello world!", filename: "hello.txt", content_type: "text/plain") ActiveStorage::Blob.create_after_upload! io: StringIO.new(data), filename: filename, content_type: content_type diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index a7cb14c52a..014fd57052 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -93,15 +93,6 @@ local: root: <%= Rails.root.join("storage") %> ``` -Optionally specify a host for generating URLs (the default is `http://localhost:3000`): - -```yaml -local: - service: Disk - root: <%= Rails.root.join("storage") %> - host: http://myapp.test -``` - ### Amazon S3 Service Declare an S3 service in `config/storage.yml`: |