Merge pull request rails#42005 from FestaLab/activestorage/vips-analyzer

Add support for vips in the image analyzer

Author: pixeltrix

activestorage/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Use libvips instead of ImageMagick to analyze images when `active_storage.variant_processor = vips`  
+
+    *Breno Gazzola*
+
 *   Add metadata value for presence of video channel in video blobs
 
     The `metadata` attribute of video blobs has a new boolean key named `video` that is set to

activestorage/lib/active_storage/analyzer.rb

@@ -2,7 +2,7 @@
 
 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.
+  # ActiveStorage::Analyzer::VideoAnalyzer for an example of a concrete subclass.
   class Analyzer
     attr_reader :blob
 

activestorage/lib/active_storage/analyzer/image_analyzer.rb

@@ -1,17 +1,14 @@
 # frozen_string_literal: true
 
 module ActiveStorage
-  # Extracts width and height in pixels from an image blob.
+  # This is an abstract base class for image analyzers, which extract width and height from an image blob.
   #
   # If the image contains EXIF data indicating its angle is 90 or 270 degrees, its width and height are swapped for convenience.
   #
   # Example:
   #
-  #   ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata
+  #   ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick.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.
   class Analyzer::ImageAnalyzer < Analyzer
     def self.accept?(blob)
       blob.image?
@@ -26,30 +23,5 @@ def metadata
         end
       end
     end
-
-    private
-      def read_image
-        download_blob_to_tempfile do |file|
-          require "mini_magick"
-          image = MiniMagick::Image.new(file.path)
-
-          if image.valid?
-            yield image
-          else
-            logger.info "Skipping image analysis because ImageMagick doesn't support the file"
-            {}
-          end
-        end
-      rescue LoadError
-        logger.info "Skipping image analysis because the mini_magick gem isn't installed"
-        {}
-      rescue MiniMagick::Error => error
-        logger.error "Skipping image analysis due to an ImageMagick error: #{error.message}"
-        {}
-      end
-
-      def rotated_image?(image)
-        %w[ RightTop LeftBottom ].include?(image["%[orientation]"])
-      end
   end
 end

activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  # This analyzer relies on the third-party {MiniMagick}[https://github.com/minimagick/minimagick] gem. MiniMagick requires
+  # the {ImageMagick}[http://www.imagemagick.org] system library.
+  class Analyzer::ImageAnalyzer::ImageMagick < Analyzer::ImageAnalyzer
+    def self.accept?(blob)
+      super && ActiveStorage.variant_processor == :mini_magick
+    end
+
+    private
+      def read_image
+        download_blob_to_tempfile do |file|
+          require "mini_magick"
+          image = MiniMagick::Image.new(file.path)
+
+          if image.valid?
+            yield image
+          else
+            logger.info "Skipping image analysis because ImageMagick doesn't support the file"
+            {}
+          end
+        end
+      rescue LoadError
+        logger.info "Skipping image analysis because the mini_magick gem isn't installed"
+        {}
+      rescue MiniMagick::Error => error
+        logger.error "Skipping image analysis due to an ImageMagick error: #{error.message}"
+        {}
+      end
+
+      def rotated_image?(image)
+        %w[ RightTop LeftBottom TopRight BottomLeft ].include?(image["%[orientation]"])
+      end
+  end
+end

activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  # This analyzer relies on the third-party {ruby-vips}[https://github.com/libvips/ruby-vips] gem. Ruby-vips requires
+  # the {libvips}[https://libvips.github.io/libvips/] system library.
+  class Analyzer::ImageAnalyzer::Vips < Analyzer::ImageAnalyzer
+    def self.accept?(blob)
+      super && ActiveStorage.variant_processor == :vips
+    end
+
+    private
+      def read_image
+        download_blob_to_tempfile do |file|
+          require "ruby-vips"
+          image = ::Vips::Image.new_from_file(file.path, access: :sequential)
+
+          if valid_image?(image)
+            yield image
+          else
+            logger.info "Skipping image analysis because Vips doesn't support the file"
+            {}
+          end
+        end
+      rescue LoadError
+        logger.info "Skipping image analysis because the ruby-vips gem isn't installed"
+        {}
+      rescue ::Vips::Error => error
+        logger.error "Skipping image analysis due to an Vips error: #{error.message}"
+        {}
+      end
+
+      ROTATIONS = /Right-top|Left-bottom|Top-right|Bottom-left/
+      def rotated_image?(image)
+        ROTATIONS === image.get("exif-ifd0-Orientation")
+      rescue ::Vips::Error
+        false
+      end
+
+      def valid_image?(image)
+        image.avg
+        true
+      rescue ::Vips::Error
+        false
+      end
+  end
+end

activestorage/lib/active_storage/engine.rb

@@ -12,6 +12,8 @@
 require "active_storage/previewer/video_previewer"
 
 require "active_storage/analyzer/image_analyzer"
+require "active_storage/analyzer/image_analyzer/image_magick"
+require "active_storage/analyzer/image_analyzer/vips"
 require "active_storage/analyzer/video_analyzer"
 require "active_storage/analyzer/audio_analyzer"
 
@@ -25,7 +27,7 @@ class Engine < Rails::Engine # :nodoc:
 
     config.active_storage = ActiveSupport::OrderedOptions.new
     config.active_storage.previewers = [ ActiveStorage::Previewer::PopplerPDFPreviewer, ActiveStorage::Previewer::MuPDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ]
-    config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
+    config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
     config.active_storage.paths = ActiveSupport::OrderedOptions.new
     config.active_storage.queues = ActiveSupport::InheritableOptions.new
 

activestorage/test/analyzer/image_analyzer/image_magick_test.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+require "active_storage/analyzer/image_analyzer"
+
+class ActiveStorage::Analyzer::ImageAnalyzer::ImageMagickTest < ActiveSupport::TestCase
+  test "analyzing a JPEG image" do
+    analyze_with_image_magick do
+      blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 4104, metadata[:width]
+      assert_equal 2736, metadata[:height]
+    end
+  end
+
+  test "analyzing a rotated JPEG image" do
+    analyze_with_image_magick do
+      blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 2736, metadata[:width]
+      assert_equal 4104, metadata[:height]
+    end
+  end
+
+  test "analyzing an SVG image without an XML declaration" do
+    analyze_with_image_magick do
+      blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 792, metadata[:width]
+      assert_equal 584, metadata[:height]
+    end
+  end
+
+  test "analyzing an unsupported image type" do
+    analyze_with_image_magick do
+      blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type")
+      metadata = extract_metadata_from(blob)
+
+      assert_nil metadata[:width]
+      assert_nil metadata[:height]
+    end
+  end
+
+  private
+    def analyze_with_image_magick
+      previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :mini_magick
+      require "mini_magick"
+
+      yield
+    rescue LoadError
+      ENV["CI"] ? raise : skip("Variant processor image_magick is not installed")
+    ensure
+      ActiveStorage.variant_processor = previous_processor
+    end
+end

activestorage/test/analyzer/image_analyzer/vips_test.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+require "active_storage/analyzer/image_analyzer"
+
+class ActiveStorage::Analyzer::ImageAnalyzer::VipsTest < ActiveSupport::TestCase
+  test "analyzing a JPEG image" do
+    analyze_with_vips do
+      blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 4104, metadata[:width]
+      assert_equal 2736, metadata[:height]
+    end
+  end
+
+  test "analyzing a rotated JPEG image" do
+    analyze_with_vips do
+      blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 2736, metadata[:width]
+      assert_equal 4104, metadata[:height]
+    end
+  end
+
+  test "analyzing an SVG image without an XML declaration" do
+    analyze_with_vips do
+      blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml")
+      metadata = extract_metadata_from(blob)
+
+      assert_equal 792, metadata[:width]
+      assert_equal 584, metadata[:height]
+    end
+  end
+
+  test "analyzing an unsupported image type" do
+    analyze_with_vips do
+      blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type")
+      metadata = extract_metadata_from(blob)
+
+      assert_nil metadata[:width]
+      assert_nil metadata[:height]
+    end
+  end
+
+  private
+    def analyze_with_vips
+      previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :vips
+      require "ruby-vips"
+
+      yield
+    rescue LoadError
+      ENV["CI"] ? raise : skip("Variant processor vips is not installed")
+    ensure
+      ActiveStorage.variant_processor = previous_processor
+    end
+end

activestorage/test/analyzer/image_analyzer_test.rb

@@ -206,7 +206,7 @@ def process_variants_with(processor)
       previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, processor
       yield
     rescue LoadError
-      skip "Variant processor #{processor.inspect} is not installed"
+      ENV["CI"] ? raise : skip("Variant processor #{processor.inspect} is not installed")
     ensure
       ActiveStorage.variant_processor = previous_processor
     end