Merge pull request rails#45100 from skipkayhil/load-as-gems-in-initializer

rafaelfranca · web-flow · commit e978ef011f11 · 2025-01-07T08:09:07.000-08:00
Load configured Active Storage plugins during boot
diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb
@@ -82,6 +82,6 @@ def digest
 
   private
     def transformer
-      ActiveStorage::Transformers::ImageProcessingTransformer.new(transformations.except(:format))
+      ActiveStorage.variant_transformer.new(transformations.except(:format))
     end
 end
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
@@ -49,6 +49,8 @@ module ActiveStorage
   mattr_accessor :verifier
   mattr_accessor :variant_processor, default: :mini_magick
 
+  mattr_accessor :variant_transformer
+
   mattr_accessor :queues, default: {}
 
   mattr_accessor :previewers, default: []
@@ -379,5 +381,7 @@ module Transformers
 
     autoload :Transformer
     autoload :ImageProcessingTransformer
+    autoload :Vips
+    autoload :ImageMagick
   end
 end
diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb
@@ -12,6 +12,11 @@ module ActiveStorage
   #   ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick.new(blob).metadata
   #   # => { width: 4104, height: 2736 }
   class Analyzer::ImageAnalyzer < Analyzer
+    extend ActiveSupport::Autoload
+
+    autoload :Vips
+    autoload :ImageMagick
+
     def self.accept?(blob)
       blob.image?
     end
diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb b/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb
@@ -1,22 +1,17 @@
 # frozen_string_literal: true
 
+begin
+  gem "mini_magick"
+rescue LoadError => error
+  raise error unless error.message.include?("ruby-vips")
+end
+
 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
-        begin
-          require "mini_magick"
-        rescue LoadError
-          logger.info "Skipping image analysis because the mini_magick gem isn't installed"
-          return {}
-        end
-
         download_blob_to_tempfile do |file|
           image = instrument("mini_magick") do
             MiniMagick::Image.new(file.path)
diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb b/activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb
@@ -1,22 +1,18 @@
 # frozen_string_literal: true
 
+begin
+  gem "ruby-vips"
+  require "ruby-vips"
+rescue LoadError => error
+  raise error unless error.message.include?("ruby-vips")
+end
+
 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
-        begin
-          require "ruby-vips"
-        rescue LoadError
-          logger.info "Skipping image analysis because the ruby-vips gem isn't installed"
-          return {}
-        end
-
         download_blob_to_tempfile do |file|
           image = instrument("vips") do
             # ruby-vips will raise Vips::Error if it can't find an appropriate loader for the file
@@ -35,6 +31,9 @@ def read_image
           logger.error "Skipping image analysis due to a Vips error: #{error.message}"
           {}
         end
+      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/
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
@@ -12,8 +12,6 @@
 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"
 
@@ -27,7 +25,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::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
+    config.active_storage.analyzers = [ ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
     config.active_storage.paths = ActiveSupport::OrderedOptions.new
     config.active_storage.queues = ActiveSupport::InheritableOptions.new
     config.active_storage.precompile_assets = true
@@ -86,9 +84,43 @@ class Engine < Rails::Engine # :nodoc:
     initializer "active_storage.configs" do
       config.after_initialize do |app|
         ActiveStorage.logger            = app.config.active_storage.logger || Rails.logger
-        ActiveStorage.variant_processor = app.config.active_storage.variant_processor || :mini_magick
+        ActiveStorage.variant_processor = app.config.active_storage.variant_processor
         ActiveStorage.previewers        = app.config.active_storage.previewers || []
-        ActiveStorage.analyzers         = app.config.active_storage.analyzers || []
+
+        begin
+          analyzer, transformer =
+            case ActiveStorage.variant_processor
+            when :vips
+              [
+                ActiveStorage::Analyzer::ImageAnalyzer::Vips,
+                ActiveStorage::Transformers::Vips
+              ]
+            when :mini_magick
+              [
+                ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick,
+                ActiveStorage::Transformers::ImageMagick
+              ]
+            end
+
+          ActiveStorage.analyzers = [analyzer].concat(app.config.active_storage.analyzers || [])
+          ActiveStorage.variant_transformer = transformer
+        rescue LoadError => error
+          case error.message
+          when /libvips/
+            ActiveStorage.logger.warn <<~WARNING.squish
+              Using vips to process variants requires the libvips library.
+              Please install libvips using the instructions on the libvips website.
+            WARNING
+          when /image_processing/
+            ActiveStorage.logger.warn <<~WARNING.squish
+              Generating image variants require the image_processing gem.
+              Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.
+            WARNING
+          else
+            raise
+          end
+        end
+
         ActiveStorage.paths             = app.config.active_storage.paths || {}
         ActiveStorage.routes_prefix     = app.config.active_storage.routes_prefix || "/rails/active_storage"
         ActiveStorage.draw_routes       = app.config.active_storage.draw_routes != false
diff --git a/activestorage/lib/active_storage/transformers/image_magick.rb b/activestorage/lib/active_storage/transformers/image_magick.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  module Transformers
+    class ImageMagick < ImageProcessingTransformer
+      private
+        def processor
+          ImageProcessing::MiniMagick
+        end
+
+        def validate_transformation(name, argument)
+          method_name = name.to_s.tr("-", "_")
+
+          unless ActiveStorage.supported_image_processing_methods.include?(method_name)
+            raise UnsupportedImageProcessingMethod, <<~ERROR.squish
+              The provided transformation method is not supported: #{method_name}.
+            ERROR
+          end
+
+          if argument.present?
+            if argument.is_a?(String) || argument.is_a?(Symbol)
+              validate_arg_string(argument)
+            elsif argument.is_a?(Array)
+              validate_arg_array(argument)
+            elsif argument.is_a?(Hash)
+              validate_arg_hash(argument)
+            end
+          end
+
+          super
+        end
+
+        def validate_arg_string(argument)
+          unsupported_arguments = ActiveStorage.unsupported_image_processing_arguments.any? do |bad_arg|
+            argument.to_s.downcase.include?(bad_arg)
+          end
+
+          raise UnsupportedImageProcessingArgument if unsupported_arguments
+        end
+
+        def validate_arg_array(argument)
+          argument.each do |arg|
+            if arg.is_a?(Integer) || arg.is_a?(Float)
+              next
+            elsif arg.is_a?(String) || arg.is_a?(Symbol)
+              validate_arg_string(arg)
+            elsif arg.is_a?(Array)
+              validate_arg_array(arg)
+            elsif arg.is_a?(Hash)
+              validate_arg_hash(arg)
+            end
+          end
+        end
+
+        def validate_arg_hash(argument)
+          argument.each do |key, value|
+            validate_arg_string(key)
+
+            if value.is_a?(Integer) || value.is_a?(Float)
+              next
+            elsif value.is_a?(String) || value.is_a?(Symbol)
+              validate_arg_string(value)
+            elsif value.is_a?(Array)
+              validate_arg_array(value)
+            elsif value.is_a?(Hash)
+              validate_arg_hash(value)
+            end
+          end
+        end
+    end
+  end
+end
diff --git a/activestorage/lib/active_storage/transformers/image_processing_transformer.rb b/activestorage/lib/active_storage/transformers/image_processing_transformer.rb
@@ -25,22 +25,9 @@ def process(file, format:)
             call
         end
 
-        def processor
-          ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize)
-        end
-
         def operations
           transformations.each_with_object([]) do |(name, argument), list|
-            if ActiveStorage.variant_processor == :mini_magick
-              validate_transformation(name, argument)
-            end
-
-            if name.to_s == "combine_options"
-              raise ArgumentError, <<~ERROR.squish
-                Active Storage's ImageProcessing transformer doesn't support :combine_options,
-                as it always generates a single command.
-              ERROR
-            end
+            validate_transformation(name, argument)
 
             if argument.present?
               list << [ name, argument ]
@@ -49,61 +36,12 @@ def operations
         end
 
         def validate_transformation(name, argument)
-          method_name = name.to_s.tr("-", "_")
-
-          unless ActiveStorage.supported_image_processing_methods.any? { |method| method_name == method }
-            raise UnsupportedImageProcessingMethod, <<~ERROR.squish
-              One or more of the provided transformation methods is not supported.
+          if name.to_s == "combine_options"
+            raise ArgumentError, <<~ERROR.squish
+              Active Storage's ImageProcessing transformer doesn't support :combine_options,
+              as it always generates a single command.
             ERROR
           end
-
-          if argument.present?
-            if argument.is_a?(String) || argument.is_a?(Symbol)
-              validate_arg_string(argument)
-            elsif argument.is_a?(Array)
-              validate_arg_array(argument)
-            elsif argument.is_a?(Hash)
-              validate_arg_hash(argument)
-            end
-          end
-        end
-
-        def validate_arg_string(argument)
-          unsupported_arguments = ActiveStorage.unsupported_image_processing_arguments.any? do |bad_arg|
-            argument.to_s.downcase.include?(bad_arg)
-          end
-
-          raise UnsupportedImageProcessingArgument if unsupported_arguments
-        end
-
-        def validate_arg_array(argument)
-          argument.each do |arg|
-            if arg.is_a?(Integer) || arg.is_a?(Float)
-              next
-            elsif arg.is_a?(String) || arg.is_a?(Symbol)
-              validate_arg_string(arg)
-            elsif arg.is_a?(Array)
-              validate_arg_array(arg)
-            elsif arg.is_a?(Hash)
-              validate_arg_hash(arg)
-            end
-          end
-        end
-
-        def validate_arg_hash(argument)
-          argument.each do |key, value|
-            validate_arg_string(key)
-
-            if value.is_a?(Integer) || value.is_a?(Float)
-              next
-            elsif value.is_a?(String) || value.is_a?(Symbol)
-              validate_arg_string(value)
-            elsif value.is_a?(Array)
-              validate_arg_array(value)
-            elsif value.is_a?(Hash)
-              validate_arg_hash(value)
-            end
-          end
         end
     end
   end
diff --git a/activestorage/lib/active_storage/transformers/vips.rb b/activestorage/lib/active_storage/transformers/vips.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  module Transformers
+    class Vips < ImageProcessingTransformer
+      def processor
+        ImageProcessing::Vips
+      end
+    end
+  end
+end
diff --git a/activestorage/test/analyzer/image_analyzer/image_magick_test.rb b/activestorage/test/analyzer/image_analyzer/image_magick_test.rb
@@ -60,13 +60,12 @@ class ActiveStorage::Analyzer::ImageAnalyzer::ImageMagickTest < ActiveSupport::T
 
   private
     def analyze_with_image_magick
-      previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :mini_magick
-      require "mini_magick"
+      previous_analyzers, ActiveStorage.analyzers = ActiveStorage.analyzers, [ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick]
 
       yield
     rescue LoadError
       ENV["BUILDKITE"] ? raise : skip("Variant processor image_magick is not installed")
     ensure
-      ActiveStorage.variant_processor = previous_processor
+      ActiveStorage.analyzers = previous_analyzers
     end
 end
diff --git a/activestorage/test/analyzer/image_analyzer/vips_test.rb b/activestorage/test/analyzer/image_analyzer/vips_test.rb
@@ -60,13 +60,12 @@ class ActiveStorage::Analyzer::ImageAnalyzer::VipsTest < ActiveSupport::TestCase
 
   private
     def analyze_with_vips
-      previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :vips
-      require "ruby-vips"
+      previous_analyzers, ActiveStorage.analyzers = ActiveStorage.analyzers, [ActiveStorage::Analyzer::ImageAnalyzer::Vips]
 
       yield
     rescue LoadError
       ENV["BUILDKITE"] ? raise : skip("Variant processor vips is not installed")
     ensure
-      ActiveStorage.variant_processor = previous_processor
+      ActiveStorage.analyzers = previous_analyzers
     end
 end
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
