13 files changed, 99 insertions, 20 deletions
diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb
index b3b9c97fe4..2612006551 100644
--- a/activestorage/test/analyzer/video_analyzer_test.rb
+++ b/activestorage/test/analyzer/video_analyzer_test.rb
@@ -8,28 +8,52 @@ require "active_storage/analyzer/video_analyzer"
 class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase
   test "analyzing a video" do
     blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4")
-    metadata = blob.tap(&:analyze).metadata
+    metadata = extract_metadata_from(blob)
 
     assert_equal 640, metadata[:width]
     assert_equal 480, metadata[:height]
-    assert_equal [4, 3], metadata[:aspect_ratio]
+    assert_equal [4, 3], metadata[:display_aspect_ratio]
     assert_equal 5.166648, metadata[:duration]
     assert_not_includes metadata, :angle
   end
 
   test "analyzing a rotated video" do
     blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4")
-    metadata = blob.tap(&:analyze).metadata
+    metadata = extract_metadata_from(blob)
 
     assert_equal 480, metadata[:width]
     assert_equal 640, metadata[:height]
-    assert_equal [4, 3], metadata[:aspect_ratio]
+    assert_equal [4, 3], metadata[:display_aspect_ratio]
     assert_equal 5.227975, metadata[:duration]
     assert_equal 90, metadata[:angle]
   end
 
+  test "analyzing a video with rectangular samples" do
+    blob = create_file_blob(filename: "video_with_rectangular_samples.mp4", content_type: "video/mp4")
+    metadata = extract_metadata_from(blob)
+
+    assert_equal 1280, metadata[:width]
+    assert_equal 720, metadata[:height]
+    assert_equal [16, 9], metadata[:display_aspect_ratio]
+  end
+
+  test "analyzing a video with an undefined display aspect ratio" do
+    blob = create_file_blob(filename: "video_with_undefined_display_aspect_ratio.mp4", content_type: "video/mp4")
+    metadata = extract_metadata_from(blob)
+
+    assert_equal 640, metadata[:width]
+    assert_equal 480, metadata[:height]
+    assert_nil metadata[:display_aspect_ratio]
+  end
+
   test "analyzing a video without a video stream" do
     blob = create_file_blob(filename: "video_without_video_stream.mp4", content_type: "video/mp4")
-    assert_equal({ "analyzed" => true }, blob.tap(&:analyze).metadata)
+    metadata = extract_metadata_from(blob)
+    assert_equal({ "analyzed" => true, "identified" => true }, metadata)
   end
+
+  private
+    def extract_metadata_from(blob)
+      blob.tap(&:analyze).metadata
+    end
 end
diff --git a/activestorage/test/database/setup.rb b/activestorage/test/database/setup.rb
index 705650a25d..daeeb5695b 100644
--- a/activestorage/test/database/setup.rb
+++ b/activestorage/test/database/setup.rb
@@ -3,5 +3,5 @@
 require_relative "create_users_migration"
 
 ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
-ActiveRecord::Migrator.migrate File.expand_path("../../db/migrate", __dir__)
+ActiveRecord::Base.connection.migration_context.migrate
 ActiveStorageCreateUsers.migrate(:up)
diff --git a/activestorage/test/dummy/config/boot.rb b/activestorage/test/dummy/config/boot.rb
index 72516d9255..59459d4ae3 100644
--- a/activestorage/test/dummy/config/boot.rb
+++ b/activestorage/test/dummy/config/boot.rb
@@ -5,7 +5,3 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../../Gemfile", __dir__)
 
 require "bundler/setup" if File.exist?(ENV["BUNDLE_GEMFILE"])
 $LOAD_PATH.unshift File.expand_path("../../../lib", __dir__)
-
-if %w[s server c console].any? { |a| ARGV.include?(a) }
-  puts "=> Booting Rails"
-end
diff --git a/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4
new file mode 100644
index 0000000000..12b04afc87
--- /dev/null
+++ b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4
diff --git a/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4
new file mode 100644
index 0000000000..eb354e756f
--- /dev/null
+++ b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4
diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb
index 20eec3c220..f0aa96b411 100644
--- a/activestorage/test/models/attachments_test.rb
+++ b/activestorage/test/models/attachments_test.rb
@@ -97,6 +97,29 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     assert_equal "funky.jpg", @user.avatar_attachment.blob.filename.to_s
   end
 
+  test "identify newly-attached, directly-uploaded blob" do
+    # Simulate a direct upload.
+    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
+    assert @user.avatar.identified?
+  end
+
+  test "identify newly-attached blob only once" do
+    blob = create_file_blob
+    assert blob.identified?
+
+    # The blob's backing file is a PNG image. Fudge its content type so we can tell if it's identified when we attach it.
+    blob.update! content_type: "application/octet-stream"
+
+    @user.avatar.attach blob
+    assert_equal "application/octet-stream", blob.content_type
+  end
+
   test "analyze newly-attached blob" do
     perform_enqueued_jobs do
       @user.avatar.attach create_file_blob
@@ -115,7 +138,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
 
     assert blob.reload.analyzed?
 
-    @user.avatar.attachment.destroy
+    @user.avatar.detach
 
     assert_no_enqueued_jobs do
       @user.reload.avatar.attach blob
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index f94e65ed77..b5daee2b57 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -13,6 +13,16 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     assert_equal Digest::MD5.base64digest(data), blob.checksum
   end
 
+  test "create after upload extracts content type from data" do
+    blob = create_file_blob content_type: "application/octet-stream"
+    assert_equal "image/jpeg", blob.content_type
+  end
+
+  test "create after upload extracts content type from filename" do
+    blob = create_blob content_type: "application/octet-stream"
+    assert_equal "text/plain", blob.content_type
+  end
+
   test "text?" do
     blob = create_blob data: "Hello world!"
     assert blob.text?
@@ -41,6 +51,25 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     end
   end
 
+  test "urls force 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)
+    end
+  end
+
+  test "urls allow for custom filename" do
+    blob = create_blob(filename: "original.txt")
+    new_filename = ActiveStorage::Filename.new("new.txt")
+
+    freeze_time do
+      assert_equal expected_url_for(blob), blob.service_url
+      assert_equal expected_url_for(blob, filename: new_filename), blob.service_url(filename: new_filename)
+    end
+  end
+
   test "purge deletes file from external service" do
     blob = create_blob
 
@@ -57,8 +86,9 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
   end
 
   private
-    def expected_url_for(blob, disposition: :inline)
-      query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param
-      "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{blob.filename}?#{query_string}"
+    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}"
     end
 end
diff --git a/activestorage/test/models/preview_test.rb b/activestorage/test/models/preview_test.rb
index bcd8442f4b..b0c6d2c45b 100644
--- a/activestorage/test/models/preview_test.rb
+++ b/activestorage/test/models/preview_test.rb
@@ -33,7 +33,7 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase
   test "previewing an unpreviewable blob" do
     blob = create_file_blob
 
-    assert_raises ActiveStorage::Blob::UnpreviewableError do
+    assert_raises ActiveStorage::UnpreviewableError do
       blob.preview resize: "640x280"
     end
   end
diff --git a/activestorage/test/models/representation_test.rb b/activestorage/test/models/representation_test.rb
index 29fe61aee4..2a06b31c77 100644
--- a/activestorage/test/models/representation_test.rb
+++ b/activestorage/test/models/representation_test.rb
@@ -34,7 +34,7 @@ class ActiveStorage::RepresentationTest < ActiveSupport::TestCase
   test "representing an unrepresentable blob" do
     blob = create_blob
 
-    assert_raises ActiveStorage::Blob::UnrepresentableError do
+    assert_raises ActiveStorage::UnrepresentableError do
       blob.representation resize: "100x100"
     end
   end
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 7157bfac3a..0cf8a583bd 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -59,7 +59,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   end
 
   test "variation of invariable blob" do
-    assert_raises ActiveStorage::Blob::InvariableError do
+    assert_raises ActiveStorage::InvariableError do
       create_file_blob(filename: "report.pdf", content_type: "application/pdf").variant(resize: "100x100")
     end
   end
@@ -67,6 +67,6 @@ 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, :<, 500
+    assert_operator variant.service_url.length, :<, 525
   end
 end
diff --git a/activestorage/test/service/configurator_test.rb b/activestorage/test/service/configurator_test.rb
index a2fd035e02..fe8a637ad0 100644
--- a/activestorage/test/service/configurator_test.rb
+++ b/activestorage/test/service/configurator_test.rb
@@ -5,7 +5,10 @@ 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/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb
index 92101b1282..08efb095bc 100644
--- a/activestorage/test/service/mirror_service_test.rb
+++ b/activestorage/test/service/mirror_service_test.rb
@@ -10,8 +10,8 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   end.to_h
 
   config = mirror_config.merge \
-    mirror:   { service: "Mirror", primary: "primary", mirrors: mirror_config.keys },
-    primary:  { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary") }
+    mirror:  { service: "Mirror", primary: "primary", mirrors: mirror_config.keys },
+    primary: { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary") }
 
   SERVICE = ActiveStorage::Service.configure :mirror, config
 
diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb
index aaf1d452ea..98fa44a604 100644
--- a/activestorage/test/test_helper.rb
+++ b/activestorage/test/test_helper.rb
@@ -7,6 +7,7 @@ require "bundler/setup"
 require "active_support"
 require "active_support/test_case"
 require "active_support/testing/autorun"
+require "webmock/minitest"
 require "mini_magick"
 
 begin
@@ -41,6 +42,8 @@ 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