Skip to content

Commit 684e760

Browse files
skipkayhilzzak
authored andcommitted
Load configured Active Storage plugins during boot
Previously, the parts of Active Storage using ruby-vips, mini_magick, or image_processing would try to load gems only when used. This strategy works well because it allows apps that don't need these features to easily ignore them and not have to depend on gems they don't need. However, the downsides to this strategy are that it requires loading code during requests and that it moves potential error messages into request logs instead of those errors being immediately visible on boot. This commit addresses these issues by restructing how the Vips and Image Magick transformers/analyzers are organized so that they will be loaded (if configured) on boot along with whatever dependencies they need. For now, if :vips or :mini_magick is specified as the Active Storage :variant_processor, not having the required gem in the Gemfile will print a deprecation warning instead of erroring because it is likely many apps are currently configured this way.
1 parent 1edb808 commit 684e760

File tree

13 files changed

+168
-104
lines changed

13 files changed

+168
-104
lines changed

activestorage/app/models/active_storage/variation.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,6 @@ def digest
8282

8383
private
8484
def transformer
85-
ActiveStorage::Transformers::ImageProcessingTransformer.new(transformations.except(:format))
85+
ActiveStorage.variant_transformer.new(transformations.except(:format))
8686
end
8787
end

activestorage/lib/active_storage.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ module ActiveStorage
4949
mattr_accessor :verifier
5050
mattr_accessor :variant_processor, default: :mini_magick
5151

52+
mattr_accessor :variant_transformer
53+
5254
mattr_accessor :queues, default: {}
5355

5456
mattr_accessor :previewers, default: []
@@ -379,5 +381,7 @@ module Transformers
379381

380382
autoload :Transformer
381383
autoload :ImageProcessingTransformer
384+
autoload :Vips
385+
autoload :ImageMagick
382386
end
383387
end

activestorage/lib/active_storage/analyzer/image_analyzer.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ module ActiveStorage
1212
# ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick.new(blob).metadata
1313
# # => { width: 4104, height: 2736 }
1414
class Analyzer::ImageAnalyzer < Analyzer
15+
extend ActiveSupport::Autoload
16+
17+
autoload :Vips
18+
autoload :ImageMagick
19+
1520
def self.accept?(blob)
1621
blob.image?
1722
end

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
11
# frozen_string_literal: true
22

3+
begin
4+
gem "mini_magick"
5+
rescue LoadError => error
6+
raise error unless error.message.include?("ruby-vips")
7+
end
8+
39
module ActiveStorage
410
# This analyzer relies on the third-party {MiniMagick}[https://github.com/minimagick/minimagick] gem. MiniMagick requires
511
# the {ImageMagick}[http://www.imagemagick.org] system library.
612
class Analyzer::ImageAnalyzer::ImageMagick < Analyzer::ImageAnalyzer
7-
def self.accept?(blob)
8-
super && ActiveStorage.variant_processor == :mini_magick
9-
end
10-
1113
private
1214
def read_image
13-
begin
14-
require "mini_magick"
15-
rescue LoadError
16-
logger.info "Skipping image analysis because the mini_magick gem isn't installed"
17-
return {}
18-
end
19-
2015
download_blob_to_tempfile do |file|
2116
image = instrument("mini_magick") do
2217
MiniMagick::Image.new(file.path)

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
11
# frozen_string_literal: true
22

3+
begin
4+
gem "ruby-vips"
5+
require "ruby-vips"
6+
rescue LoadError => error
7+
raise error unless error.message.include?("ruby-vips")
8+
end
9+
310
module ActiveStorage
411
# This analyzer relies on the third-party {ruby-vips}[https://github.com/libvips/ruby-vips] gem. Ruby-vips requires
512
# the {libvips}[https://libvips.github.io/libvips/] system library.
613
class Analyzer::ImageAnalyzer::Vips < Analyzer::ImageAnalyzer
7-
def self.accept?(blob)
8-
super && ActiveStorage.variant_processor == :vips
9-
end
10-
1114
private
1215
def read_image
13-
begin
14-
require "ruby-vips"
15-
rescue LoadError
16-
logger.info "Skipping image analysis because the ruby-vips gem isn't installed"
17-
return {}
18-
end
19-
2016
download_blob_to_tempfile do |file|
2117
image = instrument("vips") do
2218
# ruby-vips will raise Vips::Error if it can't find an appropriate loader for the file
@@ -35,6 +31,9 @@ def read_image
3531
logger.error "Skipping image analysis due to a Vips error: #{error.message}"
3632
{}
3733
end
34+
rescue ::Vips::Error => error
35+
logger.error "Skipping image analysis due to an Vips error: #{error.message}"
36+
{}
3837
end
3938

4039
ROTATIONS = /Right-top|Left-bottom|Top-right|Bottom-left/

activestorage/lib/active_storage/engine.rb

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
require "active_storage/previewer/video_previewer"
1313

1414
require "active_storage/analyzer/image_analyzer"
15-
require "active_storage/analyzer/image_analyzer/image_magick"
16-
require "active_storage/analyzer/image_analyzer/vips"
1715
require "active_storage/analyzer/video_analyzer"
1816
require "active_storage/analyzer/audio_analyzer"
1917

@@ -27,7 +25,7 @@ class Engine < Rails::Engine # :nodoc:
2725

2826
config.active_storage = ActiveSupport::OrderedOptions.new
2927
config.active_storage.previewers = [ ActiveStorage::Previewer::PopplerPDFPreviewer, ActiveStorage::Previewer::MuPDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ]
30-
config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
28+
config.active_storage.analyzers = [ ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ]
3129
config.active_storage.paths = ActiveSupport::OrderedOptions.new
3230
config.active_storage.queues = ActiveSupport::InheritableOptions.new
3331
config.active_storage.precompile_assets = true
@@ -86,9 +84,43 @@ class Engine < Rails::Engine # :nodoc:
8684
initializer "active_storage.configs" do
8785
config.after_initialize do |app|
8886
ActiveStorage.logger = app.config.active_storage.logger || Rails.logger
89-
ActiveStorage.variant_processor = app.config.active_storage.variant_processor || :mini_magick
87+
ActiveStorage.variant_processor = app.config.active_storage.variant_processor
9088
ActiveStorage.previewers = app.config.active_storage.previewers || []
91-
ActiveStorage.analyzers = app.config.active_storage.analyzers || []
89+
90+
begin
91+
analyzer, transformer =
92+
case ActiveStorage.variant_processor
93+
when :vips
94+
[
95+
ActiveStorage::Analyzer::ImageAnalyzer::Vips,
96+
ActiveStorage::Transformers::Vips
97+
]
98+
when :mini_magick
99+
[
100+
ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick,
101+
ActiveStorage::Transformers::ImageMagick
102+
]
103+
end
104+
105+
ActiveStorage.analyzers = [analyzer].concat(app.config.active_storage.analyzers || [])
106+
ActiveStorage.variant_transformer = transformer
107+
rescue LoadError => error
108+
case error.message
109+
when /libvips/
110+
ActiveStorage.logger.warn <<~WARNING.squish
111+
Using vips to process variants requires the libvips library.
112+
Please install libvips using the instructions on the libvips website.
113+
WARNING
114+
when /image_processing/
115+
ActiveStorage.logger.warn <<~WARNING.squish
116+
Generating image variants require the image_processing gem.
117+
Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.
118+
WARNING
119+
else
120+
raise
121+
end
122+
end
123+
92124
ActiveStorage.paths = app.config.active_storage.paths || {}
93125
ActiveStorage.routes_prefix = app.config.active_storage.routes_prefix || "/rails/active_storage"
94126
ActiveStorage.draw_routes = app.config.active_storage.draw_routes != false
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# frozen_string_literal: true
2+
3+
module ActiveStorage
4+
module Transformers
5+
class ImageMagick < ImageProcessingTransformer
6+
private
7+
def processor
8+
ImageProcessing::MiniMagick
9+
end
10+
11+
def validate_transformation(name, argument)
12+
method_name = name.to_s.tr("-", "_")
13+
14+
unless ActiveStorage.supported_image_processing_methods.include?(method_name)
15+
raise UnsupportedImageProcessingMethod, <<~ERROR.squish
16+
The provided transformation method is not supported: #{method_name}.
17+
ERROR
18+
end
19+
20+
if argument.present?
21+
if argument.is_a?(String) || argument.is_a?(Symbol)
22+
validate_arg_string(argument)
23+
elsif argument.is_a?(Array)
24+
validate_arg_array(argument)
25+
elsif argument.is_a?(Hash)
26+
validate_arg_hash(argument)
27+
end
28+
end
29+
30+
super
31+
end
32+
33+
def validate_arg_string(argument)
34+
unsupported_arguments = ActiveStorage.unsupported_image_processing_arguments.any? do |bad_arg|
35+
argument.to_s.downcase.include?(bad_arg)
36+
end
37+
38+
raise UnsupportedImageProcessingArgument if unsupported_arguments
39+
end
40+
41+
def validate_arg_array(argument)
42+
argument.each do |arg|
43+
if arg.is_a?(Integer) || arg.is_a?(Float)
44+
next
45+
elsif arg.is_a?(String) || arg.is_a?(Symbol)
46+
validate_arg_string(arg)
47+
elsif arg.is_a?(Array)
48+
validate_arg_array(arg)
49+
elsif arg.is_a?(Hash)
50+
validate_arg_hash(arg)
51+
end
52+
end
53+
end
54+
55+
def validate_arg_hash(argument)
56+
argument.each do |key, value|
57+
validate_arg_string(key)
58+
59+
if value.is_a?(Integer) || value.is_a?(Float)
60+
next
61+
elsif value.is_a?(String) || value.is_a?(Symbol)
62+
validate_arg_string(value)
63+
elsif value.is_a?(Array)
64+
validate_arg_array(value)
65+
elsif value.is_a?(Hash)
66+
validate_arg_hash(value)
67+
end
68+
end
69+
end
70+
end
71+
end
72+
end

activestorage/lib/active_storage/transformers/image_processing_transformer.rb

Lines changed: 5 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,9 @@ def process(file, format:)
2525
call
2626
end
2727

28-
def processor
29-
ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize)
30-
end
31-
3228
def operations
3329
transformations.each_with_object([]) do |(name, argument), list|
34-
if ActiveStorage.variant_processor == :mini_magick
35-
validate_transformation(name, argument)
36-
end
37-
38-
if name.to_s == "combine_options"
39-
raise ArgumentError, <<~ERROR.squish
40-
Active Storage's ImageProcessing transformer doesn't support :combine_options,
41-
as it always generates a single command.
42-
ERROR
43-
end
30+
validate_transformation(name, argument)
4431

4532
if argument.present?
4633
list << [ name, argument ]
@@ -49,61 +36,12 @@ def operations
4936
end
5037

5138
def validate_transformation(name, argument)
52-
method_name = name.to_s.tr("-", "_")
53-
54-
unless ActiveStorage.supported_image_processing_methods.any? { |method| method_name == method }
55-
raise UnsupportedImageProcessingMethod, <<~ERROR.squish
56-
One or more of the provided transformation methods is not supported.
39+
if name.to_s == "combine_options"
40+
raise ArgumentError, <<~ERROR.squish
41+
Active Storage's ImageProcessing transformer doesn't support :combine_options,
42+
as it always generates a single command.
5743
ERROR
5844
end
59-
60-
if argument.present?
61-
if argument.is_a?(String) || argument.is_a?(Symbol)
62-
validate_arg_string(argument)
63-
elsif argument.is_a?(Array)
64-
validate_arg_array(argument)
65-
elsif argument.is_a?(Hash)
66-
validate_arg_hash(argument)
67-
end
68-
end
69-
end
70-
71-
def validate_arg_string(argument)
72-
unsupported_arguments = ActiveStorage.unsupported_image_processing_arguments.any? do |bad_arg|
73-
argument.to_s.downcase.include?(bad_arg)
74-
end
75-
76-
raise UnsupportedImageProcessingArgument if unsupported_arguments
77-
end
78-
79-
def validate_arg_array(argument)
80-
argument.each do |arg|
81-
if arg.is_a?(Integer) || arg.is_a?(Float)
82-
next
83-
elsif arg.is_a?(String) || arg.is_a?(Symbol)
84-
validate_arg_string(arg)
85-
elsif arg.is_a?(Array)
86-
validate_arg_array(arg)
87-
elsif arg.is_a?(Hash)
88-
validate_arg_hash(arg)
89-
end
90-
end
91-
end
92-
93-
def validate_arg_hash(argument)
94-
argument.each do |key, value|
95-
validate_arg_string(key)
96-
97-
if value.is_a?(Integer) || value.is_a?(Float)
98-
next
99-
elsif value.is_a?(String) || value.is_a?(Symbol)
100-
validate_arg_string(value)
101-
elsif value.is_a?(Array)
102-
validate_arg_array(value)
103-
elsif value.is_a?(Hash)
104-
validate_arg_hash(value)
105-
end
106-
end
10745
end
10846
end
10947
end
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module ActiveStorage
4+
module Transformers
5+
class Vips < ImageProcessingTransformer
6+
def processor
7+
ImageProcessing::Vips
8+
end
9+
end
10+
end
11+
end

activestorage/test/analyzer/image_analyzer/image_magick_test.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,12 @@ class ActiveStorage::Analyzer::ImageAnalyzer::ImageMagickTest < ActiveSupport::T
6060

6161
private
6262
def analyze_with_image_magick
63-
previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :mini_magick
64-
require "mini_magick"
63+
previous_analyzers, ActiveStorage.analyzers = ActiveStorage.analyzers, [ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick]
6564

6665
yield
6766
rescue LoadError
6867
ENV["BUILDKITE"] ? raise : skip("Variant processor image_magick is not installed")
6968
ensure
70-
ActiveStorage.variant_processor = previous_processor
69+
ActiveStorage.analyzers = previous_analyzers
7170
end
7271
end

0 commit comments

Comments
 (0)