16 files changed, 325 insertions, 191 deletions
diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb
index 4bc61d13f3..a723b4d56a 100644
--- a/activestorage/test/controllers/disk_controller_test.rb
+++ b/activestorage/test/controllers/disk_controller_test.rb
@@ -5,11 +5,12 @@ require "database/setup"
 
 class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
   test "showing blob inline" do
-    blob = create_blob
+    blob = create_blob(filename: "hello.jpg", content_type: "image/jpg")
+
     get blob.service_url
     assert_response :ok
-    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.jpg\"; filename*=UTF-8''hello.jpg", response.headers["Content-Disposition"]
+    assert_equal "image/jpg", response.headers["Content-Type"]
     assert_equal "Hello world!", response.body
   end
 
@@ -22,11 +23,11 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     assert_equal "Hello world!", response.body
   end
 
-  test "showing blob range inline" do
+  test "showing blob range" do
     blob = create_blob
     get blob.service_url, headers: { "Range" => "bytes=5-9" }
     assert_response :partial_content
-    assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", response.headers["Content-Disposition"]
+    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 " worl", response.body
   end
@@ -36,6 +37,10 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     blob.delete
 
     get blob.service_url
+  end
+
+  test "showing blob with invalid key" do
+    get rails_disk_service_url(encoded_key: "Invalid key", filename: "hello.txt")
     assert_response :not_found
   end
 
@@ -67,6 +72,16 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     assert_not blob.service.exist?(blob.key)
   end
 
+  test "directly uploading blob with different but equivalent content type" do
+    data = "Something else entirely!"
+    blob = create_blob_before_direct_upload(
+      byte_size: data.size, checksum: Digest::MD5.base64digest(data), content_type: "application/x-gzip")
+
+    put blob.service_url_for_direct_upload, params: data, headers: { "Content-Type" => "application/x-gzip" }
+    assert_response :no_content
+    assert_equal data, blob.download
+  end
+
   test "directly uploading blob with mismatched content length" do
     data = "Something else entirely!"
     blob = create_blob_before_direct_upload byte_size: data.size - 1, checksum: Digest::MD5.base64digest(data)
diff --git a/activestorage/test/dummy/app/assets/config/manifest.js b/activestorage/test/dummy/app/assets/config/manifest.js
index a8adebe722..bb109908b2 100644
--- a/activestorage/test/dummy/app/assets/config/manifest.js
+++ b/activestorage/test/dummy/app/assets/config/manifest.js
@@ -1,5 +1,3 @@
 
 //= link_tree ../images
-//= link_directory ../javascripts .js
 //= link_directory ../stylesheets .css
-//= link active_storage_manifest.js
diff --git a/activestorage/test/dummy/bin/yarn b/activestorage/test/dummy/bin/yarn
index c9b7498378..d0dd7c27ac 100755
--- a/activestorage/test/dummy/bin/yarn
+++ b/activestorage/test/dummy/bin/yarn
@@ -3,11 +3,9 @@
 
 VENDOR_PATH = File.expand_path("..", __dir__)
 Dir.chdir(VENDOR_PATH) do
-  begin
-    exec "yarnpkg #{ARGV.join(" ")}"
-  rescue Errno::ENOENT
-    $stderr.puts "Yarn executable was not detected in the system."
-    $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install"
-    exit 1
-  end
+  exec "yarnpkg #{ARGV.join(" ")}"
+rescue Errno::ENOENT
+  $stderr.puts "Yarn executable was not detected in the system."
+  $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install"
+  exit 1
 end
diff --git a/activestorage/test/dummy/config/application.rb b/activestorage/test/dummy/config/application.rb
index bd14ac0b1a..151c8ade4b 100644
--- a/activestorage/test/dummy/config/application.rb
+++ b/activestorage/test/dummy/config/application.rb
@@ -15,7 +15,7 @@ Bundler.require(*Rails.groups)
 
 module Dummy
   class Application < Rails::Application
-    config.load_defaults 5.2
+    config.load_defaults 6.0
 
     config.active_storage.service = :local
   end
diff --git a/activestorage/test/dummy/config/environments/production.rb b/activestorage/test/dummy/config/environments/production.rb
index 34ac3bc561..be7f5b80d4 100644
--- a/activestorage/test/dummy/config/environments/production.rb
+++ b/activestorage/test/dummy/config/environments/production.rb
@@ -25,8 +25,7 @@ Rails.application.configure do
   # Apache or NGINX already handles this.
   config.public_file_server.enabled = ENV["RAILS_SERVE_STATIC_FILES"].present?
 
-  # Compress JavaScripts and CSS.
-  config.assets.js_compressor = :uglifier
+  # Compress CSS using a preprocessor.
   # config.assets.css_compressor = :sass
 
   # Do not fallback to assets pipeline if a precompiled asset is missed.
diff --git a/activestorage/test/dummy/config/webpacker.yml b/activestorage/test/dummy/config/webpacker.yml
new file mode 100644
index 0000000000..c1515a2e95
--- /dev/null
+++ b/activestorage/test/dummy/config/webpacker.yml
@@ -0,0 +1,72 @@
+# Note: You must restart bin/webpack-dev-server for changes to take effect
+
+default: &default
+  source_path: app/javascript
+  source_entry_path: packs
+  public_output_path: packs
+  cache_path: tmp/cache/webpacker
+  check_yarn_integrity: false
+
+  # Additional paths webpack should lookup modules
+  # ['app/assets', 'engine/foo/app/assets']
+  resolved_paths: []
+
+  # Reload manifest.json on all requests so we reload latest compiled packs
+  cache_manifest: false
+
+  extensions:
+    - .js
+    - .sass
+    - .scss
+    - .css
+    - .module.sass
+    - .module.scss
+    - .module.css
+    - .png
+    - .svg
+    - .gif
+    - .jpeg
+    - .jpg
+
+development:
+  <<: *default
+  compile: true
+
+  # Verifies that versions and hashed value of the package contents in the project's package.json
+  check_yarn_integrity: true
+
+  # Reference: https://webpack.js.org/configuration/dev-server/
+  dev_server:
+    https: false
+    host: localhost
+    port: 3035
+    public: localhost:3035
+    hmr: false
+    # Inline should be set to true if using HMR
+    inline: true
+    overlay: true
+    compress: true
+    disable_host_check: true
+    use_local_ip: false
+    quiet: false
+    headers:
+      'Access-Control-Allow-Origin': '*'
+    watch_options:
+      ignored: /node_modules/
+
+
+test:
+  <<: *default
+  compile: true
+
+  # Compile test packs to a separate directory
+  public_output_path: packs-test
+
+production:
+  <<: *default
+
+  # Production depends on precompilation of packs prior to booting for performance.
+  compile: false
+
+  # Cache manifest.json for performance
+  cache_manifest: true
diff --git a/activestorage/test/fixtures/files/racecar.tif b/activestorage/test/fixtures/files/racecar.tif
new file mode 100644
index 0000000000..0a11b22896
--- /dev/null
+++ b/activestorage/test/fixtures/files/racecar.tif
diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb
index 3b563b3fc8..8fede0e682 100644
--- a/activestorage/test/models/attached/many_test.rb
+++ b/activestorage/test/models/attached/many_test.rb
@@ -590,7 +590,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
       assert_equal "town.jpg", @user.highlights.first.filename.to_s
       assert_equal "funky.jpg", @user.highlights.second.filename.to_s
     ensure
-      User.send(:remove_method, :highlights)
+      User.remove_method :highlights
     end
   end
 end
diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb
index 561c3e9d23..7fb3262781 100644
--- a/activestorage/test/models/attached/one_test.rb
+++ b/activestorage/test/models/attached/one_test.rb
@@ -507,7 +507,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
 
       assert_equal "gpj.yknuf", @user.avatar
     ensure
-      User.send(:remove_method, :avatar)
+      User.remove_method :avatar
     end
   end
 end
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index 1a6a89de56..54cf9e2b8a 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -47,6 +47,10 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     assert_equal "text/csv", blob.content_type
   end
 
+  test "create after upload generates a 28-character base36 key" do
+    assert_match(/^[a-z0-9]{28}$/, create_blob.key)
+  end
+
   test "image?" do
     blob = create_file_blob filename: "racecar.jpg"
     assert_predicate blob, :image?
@@ -121,12 +125,21 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     end
   end
 
-  test "urls force attachment as content disposition for content types served as binary" do
+  test "urls force content_type to binary and attachment as content disposition for content types served as binary" do
     blob = create_blob(content_type: "text/html")
 
     freeze_time do
-      assert_equal expected_url_for(blob, disposition: :attachment), blob.service_url
-      assert_equal expected_url_for(blob, disposition: :attachment), blob.service_url(disposition: :inline)
+      assert_equal expected_url_for(blob, disposition: :attachment, content_type: "application/octet-stream"), blob.service_url
+      assert_equal expected_url_for(blob, disposition: :attachment, content_type: "application/octet-stream"), blob.service_url(disposition: :inline)
+    end
+  end
+
+  test "urls force attachment as content disposition when the content type is not allowed inline" do
+    blob = create_blob(content_type: "application/zip")
+
+    freeze_time do
+      assert_equal expected_url_for(blob, disposition: :attachment, content_type: "application/zip"), blob.service_url
+      assert_equal expected_url_for(blob, disposition: :attachment, content_type: "application/zip"), blob.service_url(disposition: :inline)
     end
   end
 
@@ -148,7 +161,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     arguments = [
       blob.key,
       expires_in: ActiveStorage.service_urls_expire_in,
-      disposition: :inline,
+      disposition: :attachment,
       content_type: blob.content_type,
       filename: blob.filename,
       thumb_size: "300x300",
@@ -183,9 +196,13 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
   end
 
   private
-    def expected_url_for(blob, disposition: :inline, filename: nil)
+    def expected_url_for(blob, disposition: :attachment, filename: nil, content_type: nil)
       filename ||= blob.filename
-      query_string = { content_type: blob.content_type, disposition: ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized) }.to_param
-      "https://example.com/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}"
+      content_type ||= blob.content_type
+
+      query = { disposition: ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized), content_type: content_type }
+      key_params = { key: blob.key }.merge(query)
+
+      "https://example.com/rails/active_storage/disk/#{ActiveStorage.verifier.generate(key_params, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query.to_param}"
     end
 end
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 6577f1cd9f..d98935eb9f 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -26,16 +26,14 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   end
 
   test "monochrome with default variant_processor" do
-    begin
-      ActiveStorage.variant_processor = nil
-
-      blob = create_file_blob(filename: "racecar.jpg")
-      variant = blob.variant(monochrome: true).processed
-      image = read_image(variant)
-      assert_match(/Gray/, image.colorspace)
-    ensure
-      ActiveStorage.variant_processor = :mini_magick
-    end
+    ActiveStorage.variant_processor = nil
+
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = blob.variant(monochrome: true).processed
+    image = read_image(variant)
+    assert_match(/Gray/, image.colorspace)
+  ensure
+    ActiveStorage.variant_processor = :mini_magick
   end
 
   test "disabled variation of JPEG blob" do
@@ -66,45 +64,41 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   end
 
   test "disabled variation using :combine_options" do
-    begin
-      ActiveStorage.variant_processor = nil
-      blob = create_file_blob(filename: "racecar.jpg")
-      variant = ActiveSupport::Deprecation.silence do
-        blob.variant(combine_options: {
-          crop: "100x100+0+0",
-          monochrome: false
-        }).processed
-      end
-      assert_match(/racecar\.jpg/, variant.service_url)
-
-      image = read_image(variant)
-      assert_equal 100, image.width
-      assert_equal 100, image.height
-      assert_match(/RGB/, image.colorspace)
-    ensure
-      ActiveStorage.variant_processor = :mini_magick
+    ActiveStorage.variant_processor = nil
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = ActiveSupport::Deprecation.silence do
+      blob.variant(combine_options: {
+        crop: "100x100+0+0",
+        monochrome: false
+      }).processed
     end
+    assert_match(/racecar\.jpg/, variant.service_url)
+
+    image = read_image(variant)
+    assert_equal 100, image.width
+    assert_equal 100, image.height
+    assert_match(/RGB/, image.colorspace)
+  ensure
+    ActiveStorage.variant_processor = :mini_magick
   end
 
   test "center-weighted crop of JPEG blob using :combine_options" do
-    begin
-      ActiveStorage.variant_processor = nil
-      blob = create_file_blob(filename: "racecar.jpg")
-      variant = ActiveSupport::Deprecation.silence do
-        blob.variant(combine_options: {
-          gravity: "center",
-          resize: "100x100^",
-          crop: "100x100+0+0",
-        }).processed
-      end
-      assert_match(/racecar\.jpg/, variant.service_url)
-
-      image = read_image(variant)
-      assert_equal 100, image.width
-      assert_equal 100, image.height
-    ensure
-      ActiveStorage.variant_processor = :mini_magick
+    ActiveStorage.variant_processor = nil
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = ActiveSupport::Deprecation.silence do
+      blob.variant(combine_options: {
+        gravity: "center",
+        resize: "100x100^",
+        crop: "100x100+0+0",
+      }).processed
     end
+    assert_match(/racecar\.jpg/, variant.service_url)
+
+    image = read_image(variant)
+    assert_equal 100, image.width
+    assert_equal 100, image.height
+  ensure
+    ActiveStorage.variant_processor = :mini_magick
   end
 
   test "center-weighted crop of JPEG blob using :resize_to_fill" do
@@ -139,6 +133,17 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     assert_equal 20, image.height
   end
 
+  test "resized variation of TIFF blob" do
+    blob = create_file_blob(filename: "racecar.tif")
+    variant = blob.variant(resize: "50x50").processed
+    assert_match(/racecar\.png/, variant.service_url)
+
+    image = read_image(variant)
+    assert_equal "PNG", image.type
+    assert_equal 50, image.width
+    assert_equal 33, image.height
+  end
+
   test "optimized variation of GIF blob" do
     blob = create_file_blob(filename: "image.gif", content_type: "image/gif")
 
@@ -156,22 +161,20 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "service_url doesn't grow in length despite long variant options" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(font: "a" * 10_000).processed
-    assert_operator variant.service_url.length, :<, 525
+    assert_operator variant.service_url.length, :<, 730
   end
 
   test "works for vips processor" do
-    begin
-      ActiveStorage.variant_processor = :vips
-      blob = create_file_blob(filename: "racecar.jpg")
-      variant = blob.variant(thumbnail_image: 100).processed
-
-      image = read_image(variant)
-      assert_equal 100, image.width
-      assert_equal 67, image.height
-    rescue LoadError
-      # libvips not installed
-    ensure
-      ActiveStorage.variant_processor = :mini_magick
-    end
+    ActiveStorage.variant_processor = :vips
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = blob.variant(thumbnail_image: 100).processed
+
+    image = read_image(variant)
+    assert_equal 100, image.width
+    assert_equal 67, image.height
+  rescue LoadError
+    # libvips not installed
+  ensure
+    ActiveStorage.variant_processor = :mini_magick
   end
 end
diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb
index 09c2e7f99c..2b07902d07 100644
--- a/activestorage/test/service/azure_storage_service_test.rb
+++ b/activestorage/test/service/azure_storage_service_test.rb
@@ -18,20 +18,18 @@ if SERVICE_CONFIGURATIONS[:azure]
     end
 
     test "uploading a tempfile" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-
-        Tempfile.open do |file|
-          file.write(data)
-          file.rewind
-          @service.upload(key, file)
-        end
-
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete(key)
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+
+      Tempfile.open do |file|
+        file.write(data)
+        file.rewind
+        @service.upload(key, file)
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete(key)
     end
   end
 else
diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb
index 2ba2f8b346..6bca428f50 100644
--- a/activestorage/test/service/gcs_service_test.rb
+++ b/activestorage/test/service/gcs_service_test.rb
@@ -10,45 +10,72 @@ if SERVICE_CONFIGURATIONS[:gcs]
     include ActiveStorage::Service::SharedServiceTests
 
     test "direct upload" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-        checksum = Digest::MD5.base64digest(data)
-        url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-        uri = URI.parse url
-        request = Net::HTTP::Put.new uri.request_uri
-        request.body = data
-        request.add_field "Content-Type", ""
-        request.add_field "Content-MD5", checksum
-        Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-          http.request request
-        end
-
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      checksum = Digest::MD5.base64digest(data)
+      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
+
+      uri = URI.parse url
+      request = Net::HTTP::Put.new uri.request_uri
+      request.body = data
+      request.add_field "Content-Type", ""
+      request.add_field "Content-MD5", checksum
+      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+        http.request request
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
+    end
+
+    test "upload with content_type and content_disposition" do
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, 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 "text/plain", response.content_type
+      assert_match(/attachment;.*test.txt/, response["Content-Disposition"])
+    ensure
+      @service.delete key
+    end
+
+    test "upload with content_type" do
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), 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 "text/plain", response.content_type
+      assert_match(/inline;.*test.html/, response["Content-Disposition"])
+    ensure
+      @service.delete key
+    end
+
+    test "update metadata" do
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html")
+
+      @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: :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"])
+    ensure
+      @service.delete key
     end
 
     test "signed URL generation" do
       assert_match(/storage\.googleapis\.com\/.*response-content-disposition=inline.*test\.txt.*response-content-type=text%2Fplain/,
         @service.url(@key, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain"))
     end
-
-    test "signed URL response headers" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data))
-
-        url = @service.url(key, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
-        response = Net::HTTP.get_response(URI(url))
-        assert_equal "text/plain", response.content_type
-      ensure
-        @service.delete key
-      end
-    end
   end
 else
   puts "Skipping GCS Service tests because no GCS configuration was supplied"
diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb
index bb502dde60..94c751a4ff 100644
--- a/activestorage/test/service/mirror_service_test.rb
+++ b/activestorage/test/service/mirror_service_test.rb
@@ -18,22 +18,20 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   include ActiveStorage::Service::SharedServiceTests
 
   test "uploading to all services" do
-    begin
-      key      = SecureRandom.base58(24)
-      data     = "Something else entirely!"
-      io       = StringIO.new(data)
-      checksum = Digest::MD5.base64digest(data)
+    key      = SecureRandom.base58(24)
+    data     = "Something else entirely!"
+    io       = StringIO.new(data)
+    checksum = Digest::MD5.base64digest(data)
 
-      @service.upload key, io.tap(&:read), checksum: checksum
-      assert_predicate io, :eof?
+    @service.upload key, io.tap(&:read), checksum: checksum
+    assert_predicate io, :eof?
 
-      assert_equal data, @service.primary.download(key)
-      @service.mirrors.each do |mirror|
-        assert_equal data, mirror.download(key)
-      end
-    ensure
-      @service.delete key
+    assert_equal data, @service.primary.download(key)
+    @service.mirrors.each do |mirror|
+      assert_equal data, mirror.download(key)
     end
+  ensure
+    @service.delete key
   end
 
   test "downloading from primary service" do
diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb
index 559aa028f2..0a6004267f 100644
--- a/activestorage/test/service/s3_service_test.rb
+++ b/activestorage/test/service/s3_service_test.rb
@@ -2,6 +2,7 @@
 
 require "service/shared_service_tests"
 require "net/http"
+require "database/setup"
 
 if SERVICE_CONFIGURATIONS[:s3]
   class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase
@@ -10,25 +11,23 @@ if SERVICE_CONFIGURATIONS[:s3]
     include ActiveStorage::Service::SharedServiceTests
 
     test "direct upload" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-        checksum = Digest::MD5.base64digest(data)
-        url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-        uri = URI.parse url
-        request = Net::HTTP::Put.new uri.request_uri
-        request.body = data
-        request.add_field "Content-Type", "text/plain"
-        request.add_field "Content-MD5", checksum
-        Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-          http.request request
-        end
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      checksum = Digest::MD5.base64digest(data)
+      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
+      uri = URI.parse url
+      request = Net::HTTP::Put.new uri.request_uri
+      request.body = data
+      request.add_field "Content-Type", "text/plain"
+      request.add_field "Content-MD5", checksum
+      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+        http.request request
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "upload a zero byte file" do
diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb
index ca2490f2bc..17f3736056 100644
--- a/activestorage/test/service/shared_service_tests.rb
+++ b/activestorage/test/service/shared_service_tests.rb
@@ -20,30 +20,42 @@ module ActiveStorage::Service::SharedServiceTests
     end
 
     test "uploading with integrity" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data))
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data))
 
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
-      end
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "uploading without integrity" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-
-        assert_raises(ActiveStorage::IntegrityError) do
-          @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data"))
-        end
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
 
-        assert_not @service.exist?(key)
-      ensure
-        @service.delete key
+      assert_raises(ActiveStorage::IntegrityError) do
+        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data"))
       end
+
+      assert_not @service.exist?(key)
+    ensure
+      @service.delete key
+    end
+
+    test "uploading with integrity and multiple keys" do
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+      @service.upload(
+        key,
+        StringIO.new(data),
+        checksum: Digest::MD5.base64digest(data),
+        filename: "racecar.jpg",
+        content_type: "image/jpg"
+      )
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "downloading" do
@@ -111,20 +123,18 @@ module ActiveStorage::Service::SharedServiceTests
     end
 
     test "deleting by prefix" do
-      begin
-        @service.upload("a/a/a", StringIO.new(FIXTURE_DATA))
-        @service.upload("a/a/b", StringIO.new(FIXTURE_DATA))
-        @service.upload("a/b/a", StringIO.new(FIXTURE_DATA))
-
-        @service.delete_prefixed("a/a/")
-        assert_not @service.exist?("a/a/a")
-        assert_not @service.exist?("a/a/b")
-        assert @service.exist?("a/b/a")
-      ensure
-        @service.delete("a/a/a")
-        @service.delete("a/a/b")
-        @service.delete("a/b/a")
-      end
+      @service.upload("a/a/a", StringIO.new(FIXTURE_DATA))
+      @service.upload("a/a/b", StringIO.new(FIXTURE_DATA))
+      @service.upload("a/b/a", StringIO.new(FIXTURE_DATA))
+
+      @service.delete_prefixed("a/a/")
+      assert_not @service.exist?("a/a/a")
+      assert_not @service.exist?("a/a/b")
+      assert @service.exist?("a/b/a")
+    ensure
+      @service.delete("a/a/a")
+      @service.delete("a/a/b")
+      @service.delete("a/b/a")
     end
   end
 end