From 605484079d297d1ba6835628465be81f03c052ee Mon Sep 17 00:00:00 2001
From: George Claghorn <george.claghorn@gmail.com>
Date: Sun, 22 Oct 2017 13:16:59 -0400
Subject: Extract metadata from images and videos

---
 .../app/jobs/active_storage/analyze_job.rb         |   8 +++
 activestorage/app/jobs/active_storage/purge_job.rb |   2 +-
 .../app/models/active_storage/attachment.rb        |   7 ++
 activestorage/app/models/active_storage/blob.rb    |  57 ++++++++++++++-
 activestorage/lib/active_storage.rb                |   3 +
 activestorage/lib/active_storage/analyzer.rb       |  33 +++++++++
 .../lib/active_storage/analyzer/image_analyzer.rb  |  36 ++++++++++
 .../lib/active_storage/analyzer/null_analyzer.rb   |  13 ++++
 .../lib/active_storage/analyzer/video_analyzer.rb  |  79 +++++++++++++++++++++
 activestorage/lib/active_storage/attached/one.rb   |   6 +-
 activestorage/lib/active_storage/downloading.rb    |  21 ++++++
 activestorage/lib/active_storage/engine.rb         |  14 +++-
 activestorage/lib/active_storage/log_subscriber.rb |   2 +-
 activestorage/lib/active_storage/previewer.rb      |  27 ++-----
 .../lib/active_storage/previewer/pdf_previewer.rb  |   2 +-
 .../active_storage/previewer/video_previewer.rb    |  10 ++-
 activestorage/lib/active_storage/service.rb        |   2 -
 activestorage/test/analyzer/image_analyzer_test.rb |  16 +++++
 activestorage/test/analyzer/video_analyzer_test.rb |  35 +++++++++
 .../test/fixtures/files/rotated_video.mp4          | Bin 0 -> 275090 bytes
 .../fixtures/files/video_without_video_stream.mp4  | Bin 0 -> 16252 bytes
 activestorage/test/models/attachments_test.rb      |  59 ++++++++++++++-
 activestorage/test/previewer/pdf_previewer_test.rb |   3 +
 .../test/previewer/video_previewer_test.rb         |   3 +
 activestorage/test/test_helper.rb                  |   6 +-
 25 files changed, 407 insertions(+), 37 deletions(-)
 create mode 100644 activestorage/app/jobs/active_storage/analyze_job.rb
 create mode 100644 activestorage/lib/active_storage/analyzer.rb
 create mode 100644 activestorage/lib/active_storage/analyzer/image_analyzer.rb
 create mode 100644 activestorage/lib/active_storage/analyzer/null_analyzer.rb
 create mode 100644 activestorage/lib/active_storage/analyzer/video_analyzer.rb
 create mode 100644 activestorage/lib/active_storage/downloading.rb
 create mode 100644 activestorage/test/analyzer/image_analyzer_test.rb
 create mode 100644 activestorage/test/analyzer/video_analyzer_test.rb
 create mode 100644 activestorage/test/fixtures/files/rotated_video.mp4
 create mode 100644 activestorage/test/fixtures/files/video_without_video_stream.mp4

(limited to 'activestorage')

diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb
new file mode 100644
index 0000000000..a11a73d030
--- /dev/null
+++ b/activestorage/app/jobs/active_storage/analyze_job.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later.
+class ActiveStorage::AnalyzeJob < ActiveJob::Base
+  def perform(blob)
+    blob.analyze
+  end
+end
diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb
index 990ab27c9f..188840f702 100644
--- a/activestorage/app/jobs/active_storage/purge_job.rb
+++ b/activestorage/app/jobs/active_storage/purge_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Provides delayed purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later.
+# Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later.
 class ActiveStorage::PurgeJob < ActiveJob::Base
   # FIXME: Limit this to a custom ActiveStorage error
   retry_on StandardError
diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb
index 29226e8ee9..9f61a5dbf3 100644
--- a/activestorage/app/models/active_storage/attachment.rb
+++ b/activestorage/app/models/active_storage/attachment.rb
@@ -14,6 +14,8 @@ class ActiveStorage::Attachment < ActiveRecord::Base
 
   delegate_missing_to :blob
 
+  after_create_commit :analyze_blob_later
+
   # Synchronously purges the blob (deletes it from the configured service) and destroys the attachment.
   def purge
     blob.purge
@@ -25,4 +27,9 @@ class ActiveStorage::Attachment < ActiveRecord::Base
     blob.purge_later
     destroy
   end
+
+  private
+    def analyze_blob_later
+      blob.analyze_later unless blob.analyzed?
+    end
 end
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index 84b8f3827b..99823e14c6 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "active_storage/analyzer/null_analyzer"
+
 # A blob is a record that contains the metadata about a file and a key for where that file resides on the service.
 # Blobs can be created in two ways:
 #
@@ -20,7 +22,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
   self.table_name = "active_storage_blobs"
 
   has_secure_token :key
-  store :metadata, coder: JSON
+  store :metadata, accessors: [ :analyzed ], coder: JSON
 
   class_attribute :service
 
@@ -224,6 +226,46 @@ class ActiveStorage::Blob < ActiveRecord::Base
   end
 
 
+  # Extracts and stores metadata from the file associated with this blob using a relevant analyzer. Active Storage comes
+  # with built-in analyzers for images and videos. See ActiveStorage::Analyzer::ImageAnalyzer and
+  # ActiveStorage::Analyzer::VideoAnalyzer for information about the specific attributes they extract and the third-party
+  # libraries they require.
+  #
+  # To choose the analyzer for a blob, Active Storage calls +accept?+ on each registered analyzer in order. It uses the
+  # first analyzer for which +accept?+ returns true when given the blob. If no registered analyzer accepts the blob, no
+  # metadata is extracted from it.
+  #
+  # In a Rails application, add or remove analyzers by manipulating +Rails.application.config.active_storage.analyzers+
+  # in an initializer:
+  #
+  #   # Add a custom analyzer for Microsoft Office documents:
+  #   Rails.application.config.active_storage.analyzers.append DOCXAnalyzer
+  #
+  #   # Remove the built-in video analyzer:
+  #   Rails.application.config.active_storage.analyzers.delete ActiveStorage::Analyzer::VideoAnalyzer
+  #
+  # Outside of a Rails application, manipulate +ActiveStorage.analyzers+ instead.
+  #
+  # You won't ordinarily need to call this method from a Rails application. New blobs are automatically and asynchronously
+  # analyzed via #analyze_later when they're attached for the first time.
+  def analyze
+    update! metadata: extract_metadata_via_analyzer
+  end
+
+  # Enqueues an ActiveStorage::AnalyzeJob which calls #analyze.
+  #
+  # This method is automatically called for a blob when it's attached for the first time. You can call it to analyze a blob
+  # again (e.g. if you add a new analyzer or modify an existing one).
+  def analyze_later
+    ActiveStorage::AnalyzeJob.perform_later(self)
+  end
+
+  # Returns true if the blob has been analyzed.
+  def analyzed?
+    analyzed
+  end
+
+
   # Deletes the file on the service that's associated with this blob. This should only be done if the blob is going to be
   # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+
   # methods in most circumstances.
@@ -255,4 +297,17 @@ class ActiveStorage::Blob < ActiveRecord::Base
         io.rewind
       end.base64digest
     end
+
+
+    def extract_metadata_via_analyzer
+      analyzer.metadata.merge(analyzed: true)
+    end
+
+    def analyzer
+      analyzer_class.new(self)
+    end
+
+    def analyzer_class
+      ActiveStorage.analyzers.detect { |klass| klass.accept?(self) } || ActiveStorage::Analyzer::NullAnalyzer
+    end
 end
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
index 9ac1e7ab54..cfdb2a8acd 100644
--- a/activestorage/lib/active_storage.rb
+++ b/activestorage/lib/active_storage.rb
@@ -34,7 +34,10 @@ module ActiveStorage
   autoload :Attached
   autoload :Service
   autoload :Previewer
+  autoload :Analyzer
 
+  mattr_accessor :logger
   mattr_accessor :verifier
   mattr_accessor :previewers, default: []
+  mattr_accessor :analyzers, default: []
 end
diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb
new file mode 100644
index 0000000000..837785a12b
--- /dev/null
+++ b/activestorage/lib/active_storage/analyzer.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require "active_storage/downloading"
+
+module ActiveStorage
+  # This is an abstract base class for analyzers, which extract metadata from blobs. See
+  # ActiveStorage::Analyzer::ImageAnalyzer for an example of a concrete subclass.
+  class Analyzer
+    include Downloading
+
+    attr_reader :blob
+
+    # Implement this method in a concrete subclass. Have it return true when given a blob from which
+    # the analyzer can extract metadata.
+    def self.accept?(blob)
+      false
+    end
+
+    def initialize(blob)
+      @blob = blob
+    end
+
+    # Override this method in a concrete subclass. Have it return a Hash of metadata.
+    def metadata
+      raise NotImplementedError
+    end
+
+    private
+      def logger
+        ActiveStorage.logger
+      end
+  end
+end
diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb
new file mode 100644
index 0000000000..b25d2092b9
--- /dev/null
+++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  # Extracts width and height in pixels from an image blob.
+  #
+  # Example:
+  #
+  #   ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata
+  #   # => { width: 4104, height: 2736 }
+  #
+  # This analyzer relies on the third-party [MiniMagick]{https://github.com/minimagick/minimagick} gem. MiniMagick requires
+  # the [ImageMagick]{http://www.imagemagick.org} system library. These libraries are not provided by Rails; you must
+  # install them yourself to use this analyzer.
+  class Analyzer::ImageAnalyzer < Analyzer
+    def self.accept?(blob)
+      blob.image?
+    end
+
+    def metadata
+      read_image do |image|
+        { width: image.width, height: image.height }
+      end
+    rescue LoadError
+      logger.info "Skipping image analysis because the mini_magick gem isn't installed"
+      {}
+    end
+
+    private
+      def read_image
+        download_blob_to_tempfile do |file|
+          require "mini_magick"
+          yield MiniMagick::Image.new(file.path)
+        end
+      end
+  end
+end
diff --git a/activestorage/lib/active_storage/analyzer/null_analyzer.rb b/activestorage/lib/active_storage/analyzer/null_analyzer.rb
new file mode 100644
index 0000000000..8ff7ce48e5
--- /dev/null
+++ b/activestorage/lib/active_storage/analyzer/null_analyzer.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  class Analyzer::NullAnalyzer < Analyzer # :nodoc:
+    def self.accept?(blob)
+      true
+    end
+
+    def metadata
+      {}
+    end
+  end
+end
diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb
new file mode 100644
index 0000000000..408b5e58e9
--- /dev/null
+++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require "active_support/core_ext/hash/compact"
+
+module ActiveStorage
+  # Extracts the following from a video blob:
+  #
+  # * Width (pixels)
+  # * Height (pixels)
+  # * Duration (seconds)
+  # * Angle (degrees)
+  # * Aspect ratio
+  #
+  # Example:
+  #
+  #   ActiveStorage::VideoAnalyzer.new(blob).metadata
+  #   # => { width: 640, height: 480, duration: 5.0, angle: 0, aspect_ratio: [4, 3] }
+  #
+  # This analyzer requires the {ffmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. You must
+  # install ffmpeg yourself to use this analyzer.
+  class Analyzer::VideoAnalyzer < Analyzer
+    def self.accept?(blob)
+      blob.video?
+    end
+
+    def metadata
+      { width: width, height: height, duration: duration, angle: angle, aspect_ratio: aspect_ratio }.compact
+    end
+
+    private
+      def width
+        Integer(video_stream["width"]) if video_stream["width"]
+      end
+
+      def height
+        Integer(video_stream["height"]) if video_stream["height"]
+      end
+
+      def duration
+        Float(video_stream["duration"]) if video_stream["duration"]
+      end
+
+      def angle
+        Integer(tags["rotate"]) if tags["rotate"]
+      end
+
+      def aspect_ratio
+        if descriptor = video_stream["display_aspect_ratio"]
+          descriptor.split(":", 2).collect(&:to_i)
+        end
+      end
+
+
+      def tags
+        @tags ||= video_stream["tags"] || {}
+      end
+
+      def video_stream
+        @video_stream ||= streams.detect { |stream| stream["codec_type"] == "video" } || {}
+      end
+
+      def streams
+        probe["streams"] || []
+      end
+
+      def probe
+        download_blob_to_tempfile { |file| probe_from(file) }
+      end
+
+      def probe_from(file)
+        IO.popen([ "ffprobe", "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output|
+          JSON.parse(output.read)
+        end
+      rescue Errno::ENOENT
+        logger.info "Skipping video analysis because ffmpeg isn't installed"
+        {}
+      end
+  end
+end
diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb
index c66be08f58..dc19512484 100644
--- a/activestorage/lib/active_storage/attached/one.rb
+++ b/activestorage/lib/active_storage/attached/one.rb
@@ -59,12 +59,16 @@ module ActiveStorage
       def replace(attachable)
         blob.tap do
           transaction do
-            destroy
+            destroy_attachment
             write_attachment create_attachment_from(attachable)
           end
         end.purge_later
       end
 
+      def destroy_attachment
+        attachment.destroy
+      end
+
       def create_attachment_from(attachable)
         ActiveStorage::Attachment.create!(record: record, name: name, blob: create_blob_from(attachable))
       end
diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb
new file mode 100644
index 0000000000..bcf42f610e
--- /dev/null
+++ b/activestorage/lib/active_storage/downloading.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  module Downloading
+    private
+      # Opens a new tempfile and copies blob data into it. Yields the tempfile.
+      def download_blob_to_tempfile # :doc:
+        Tempfile.open("ActiveStorage") do |file|
+          download_blob_to file
+          yield file
+        end
+      end
+
+      # Efficiently download blob data into the given file.
+      def download_blob_to(file) # :doc:
+        file.binmode
+        blob.download { |chunk| file.write(chunk) }
+        file.rewind
+      end
+  end
+end
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
index 335eae8dd8..a01a14cd83 100644
--- a/activestorage/lib/active_storage/engine.rb
+++ b/activestorage/lib/active_storage/engine.rb
@@ -6,20 +6,22 @@ require "active_storage"
 require "active_storage/previewer/pdf_previewer"
 require "active_storage/previewer/video_previewer"
 
+require "active_storage/analyzer/image_analyzer"
+require "active_storage/analyzer/video_analyzer"
+
 module ActiveStorage
   class Engine < Rails::Engine # :nodoc:
     isolate_namespace ActiveStorage
 
     config.active_storage = ActiveSupport::OrderedOptions.new
     config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ]
+    config.active_storage.analyzers  = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ]
 
     config.eager_load_namespaces << ActiveStorage
 
     initializer "active_storage.logger" do
-      require "active_storage/service"
-
       config.after_initialize do |app|
-        ActiveStorage::Service.logger = app.config.active_storage.logger || Rails.logger
+        ActiveStorage.logger = app.config.active_storage.logger || Rails.logger
       end
     end
 
@@ -69,5 +71,11 @@ module ActiveStorage
         ActiveStorage.previewers = app.config.active_storage.previewers || []
       end
     end
+
+    initializer "active_storage.analyzers" do
+      config.after_initialize do |app|
+        ActiveStorage.analyzers = app.config.active_storage.analyzers || []
+      end
+    end
   end
 end
diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb
index 0d00a75c0e..5cbf4bd1a5 100644
--- a/activestorage/lib/active_storage/log_subscriber.rb
+++ b/activestorage/lib/active_storage/log_subscriber.rb
@@ -27,7 +27,7 @@ module ActiveStorage
     end
 
     def logger
-      ActiveStorage::Service.logger
+      ActiveStorage.logger
     end
 
     private
diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb
index 930b376067..460f6d5678 100644
--- a/activestorage/lib/active_storage/previewer.rb
+++ b/activestorage/lib/active_storage/previewer.rb
@@ -1,10 +1,14 @@
 # frozen_string_literal: true
 
+require "active_storage/downloading"
+
 module ActiveStorage
   # This is an abstract base class for previewers, which generate images from blobs. See
   # ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of
   # concrete subclasses.
   class Previewer
+    include Downloading
+
     attr_reader :blob
 
     # Implement this method in a concrete subclass. Have it return true when given a blob from which
@@ -24,37 +28,20 @@ module ActiveStorage
     end
 
     private
-      # Downloads the blob to a new tempfile. Yields the tempfile.
-      #
-      # Use this method to get a tempfile that you can provide to a drawing command.
-      def open # :doc:
-        Tempfile.open("input") do |file|
-          download_blob_to file
-          yield file
-        end
-      end
-
-      def download_blob_to(file)
-        file.binmode
-        blob.download { |chunk| file.write(chunk) }
-        file.rewind
-      end
-
-
       # Executes a system command, capturing its binary output in a tempfile. Yields the tempfile.
       #
-      # Use this method to shell out to system libraries (e.g. mupdf or ffmpeg) for preview image
+      # Use this method to shell out to a system library (e.g. mupdf or ffmpeg) for preview image
       # generation. The resulting tempfile can be used as the +:io+ value in an attachable Hash:
       #
       #   def preview
-      #     open do |input|
+      #     download_blob_to_tempfile do |input|
       #       draw "my-drawing-command", input.path, "--format", "png", "-" do |output|
       #         yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png"
       #       end
       #     end
       #   end
       def draw(*argv) # :doc:
-        Tempfile.open("output") do |file|
+        Tempfile.open("ActiveStorage") do |file|
           capture(*argv, to: file)
           yield file
         end
diff --git a/activestorage/lib/active_storage/previewer/pdf_previewer.rb b/activestorage/lib/active_storage/previewer/pdf_previewer.rb
index 31a2a8f120..a2f05c74a6 100644
--- a/activestorage/lib/active_storage/previewer/pdf_previewer.rb
+++ b/activestorage/lib/active_storage/previewer/pdf_previewer.rb
@@ -7,7 +7,7 @@ module ActiveStorage
     end
 
     def preview
-      open do |input|
+      download_blob_to_tempfile do |input|
         draw "mutool", "draw", "-F", "png", "-o", "-", input.path, "1" do |output|
           yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png"
         end
diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb
index 840d87f100..49f128d142 100644
--- a/activestorage/lib/active_storage/previewer/video_previewer.rb
+++ b/activestorage/lib/active_storage/previewer/video_previewer.rb
@@ -7,11 +7,17 @@ module ActiveStorage
     end
 
     def preview
-      open do |input|
-        draw "ffmpeg", "-i", input.path, "-y", "-vcodec", "png", "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-" do |output|
+      download_blob_to_tempfile do |input|
+        draw_relevant_frame_from input do |output|
           yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png"
         end
       end
     end
+
+    private
+      def draw_relevant_frame_from(file, &block)
+        draw "ffmpeg", "-i", file.path, "-y", "-vcodec", "png",
+          "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-", &block
+      end
   end
 end
diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb
index 1f012da1e7..aa150e4d8a 100644
--- a/activestorage/lib/active_storage/service.rb
+++ b/activestorage/lib/active_storage/service.rb
@@ -41,8 +41,6 @@ module ActiveStorage
     extend ActiveSupport::Autoload
     autoload :Configurator
 
-    class_attribute :logger
-
     class_attribute :url_expires_in, default: 5.minutes
 
     class << self
diff --git a/activestorage/test/analyzer/image_analyzer_test.rb b/activestorage/test/analyzer/image_analyzer_test.rb
new file mode 100644
index 0000000000..9087072215
--- /dev/null
+++ b/activestorage/test/analyzer/image_analyzer_test.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+require "active_storage/analyzer/image_analyzer"
+
+class ActiveStorage::Analyzer::ImageAnalyzerTest < ActiveSupport::TestCase
+  test "analyzing an image" do
+    blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
+    metadata = blob.tap(&:analyze).metadata
+
+    assert_equal 4104, metadata[:width]
+    assert_equal 2736, metadata[:height]
+  end
+end
diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb
new file mode 100644
index 0000000000..4a3c4a8bfc
--- /dev/null
+++ b/activestorage/test/analyzer/video_analyzer_test.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+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
+
+    assert_equal 640, metadata[:width]
+    assert_equal 480, metadata[:height]
+    assert_equal [4, 3], metadata[: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
+
+    assert_equal 640, metadata[:width]
+    assert_equal 480, metadata[:height]
+    assert_equal [4, 3], metadata[:aspect_ratio]
+    assert_equal 5.227975, metadata[:duration]
+    assert_equal 90, metadata[:angle]
+  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)
+  end
+end
diff --git a/activestorage/test/fixtures/files/rotated_video.mp4 b/activestorage/test/fixtures/files/rotated_video.mp4
new file mode 100644
index 0000000000..4c7a4e9e57
Binary files /dev/null and b/activestorage/test/fixtures/files/rotated_video.mp4 differ
diff --git a/activestorage/test/fixtures/files/video_without_video_stream.mp4 b/activestorage/test/fixtures/files/video_without_video_stream.mp4
new file mode 100644
index 0000000000..e6a55f868b
Binary files /dev/null and b/activestorage/test/fixtures/files/video_without_video_stream.mp4 differ
diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb
index 379ae0a416..47f2bd7911 100644
--- a/activestorage/test/models/attachments_test.rb
+++ b/activestorage/test/models/attachments_test.rb
@@ -15,7 +15,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     assert_equal "funky.jpg", @user.avatar.filename.to_s
   end
 
-  test "attach existing sgid blob" do
+  test "attach existing blob from a signed ID" do
     @user.avatar.attach create_blob(filename: "funky.jpg").signed_id
     assert_equal "funky.jpg", @user.avatar.filename.to_s
   end
@@ -63,6 +63,31 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     assert_equal "funky.jpg", @user.avatar_attachment.blob.filename.to_s
   end
 
+  test "analyze newly-attached blob" do
+    perform_enqueued_jobs do
+      @user.avatar.attach create_file_blob
+    end
+
+    assert_equal 4104, @user.avatar.reload.metadata[:width]
+    assert_equal 2736, @user.avatar.metadata[:height]
+  end
+
+  test "analyze attached blob only once" do
+    blob = create_file_blob
+
+    perform_enqueued_jobs do
+      @user.avatar.attach blob
+    end
+
+    assert blob.reload.analyzed?
+
+    @user.avatar.attachment.destroy
+
+    assert_no_enqueued_jobs do
+      @user.reload.avatar.attach blob
+    end
+  end
+
   test "purge attached blob" do
     @user.avatar.attach create_blob(filename: "funky.jpg")
     avatar_key = @user.avatar.key
@@ -135,6 +160,38 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     assert_equal "town.jpg", @user.highlights_attachments.first.blob.filename.to_s
   end
 
+  test "analyze newly-attached blobs" do
+    perform_enqueued_jobs do
+      @user.highlights.attach(
+        create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg"),
+        create_file_blob(filename: "video.mp4", content_type: "video/mp4"))
+    end
+
+    assert_equal 4104, @user.highlights.first.metadata[:width]
+    assert_equal 2736, @user.highlights.first.metadata[:height]
+
+    assert_equal 640, @user.highlights.second.metadata[:width]
+    assert_equal 480, @user.highlights.second.metadata[:height]
+  end
+
+  test "analyze attached blobs only once" do
+    blobs = [
+      create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg"),
+      create_file_blob(filename: "video.mp4", content_type: "video/mp4")
+    ]
+
+    perform_enqueued_jobs do
+      @user.highlights.attach(blobs)
+    end
+
+    assert blobs.each(&:reload).all?(&:analyzed?)
+
+    @user.highlights.attachments.destroy_all
+
+    assert_no_enqueued_jobs do
+      @user.highlights.attach(blobs)
+    end
+  end
 
   test "purge attached blobs" do
     @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
diff --git a/activestorage/test/previewer/pdf_previewer_test.rb b/activestorage/test/previewer/pdf_previewer_test.rb
index 60f075d1b2..fe32f39be4 100644
--- a/activestorage/test/previewer/pdf_previewer_test.rb
+++ b/activestorage/test/previewer/pdf_previewer_test.rb
@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+require "test_helper"
+require "database/setup"
+
 require "active_storage/previewer/pdf_previewer"
 
 class ActiveStorage::Previewer::PDFPreviewerTest < ActiveSupport::TestCase
diff --git a/activestorage/test/previewer/video_previewer_test.rb b/activestorage/test/previewer/video_previewer_test.rb
index 967d5d5ba9..dba9b0d7e2 100644
--- a/activestorage/test/previewer/video_previewer_test.rb
+++ b/activestorage/test/previewer/video_previewer_test.rb
@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+require "test_helper"
+require "database/setup"
+
 require "active_storage/previewer/video_previewer"
 
 class ActiveStorage::Previewer::VideoPreviewerTest < ActiveSupport::TestCase
diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb
index 60656feb80..38408cdad3 100644
--- a/activestorage/test/test_helper.rb
+++ b/activestorage/test/test_helper.rb
@@ -33,8 +33,8 @@ end
 
 require "tmpdir"
 ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: Dir.mktmpdir("active_storage_tests"))
-ActiveStorage::Service.logger = ActiveSupport::Logger.new(nil)
 
+ActiveStorage.logger = ActiveSupport::Logger.new(nil)
 ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing")
 
 class ActiveSupport::TestCase
@@ -46,9 +46,7 @@ class ActiveSupport::TestCase
     end
 
     def create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
-      ActiveStorage::Blob.create_after_upload! \
-        io: file_fixture(filename).open,
-        filename: filename, content_type: content_type
+      ActiveStorage::Blob.create_after_upload! io: file_fixture(filename).open, filename: filename, content_type: content_type
     end
 
     def create_blob_before_direct_upload(filename: "hello.txt", byte_size:, checksum:, content_type: "text/plain")
-- 
cgit v1.2.3