Merge pull request #32471 from janko-m/use-image_processing-gem

Use ImageProcessing gem for ActiveStorage variants

10 files changed, 171 insertions, 74 deletions

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index d794afb0e6..60d7d19540 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,22 @@
+*   Use the [ImageProcessing](https://github.com/janko-m/image_processing) gem
+    for Active Storage variants, and deprecate the MiniMagick backend.
+
+    This means that variants are now automatically oriented if the original
+    image was rotated. Also, in addition to the existing ImageMagick
+    operations, variants can now use `:resize_to_fit`, `:resize_to_fill`, and
+    other ImageProcessing macros. These are now recommended over raw `:resize`,
+    as they also sharpen the thumbnail after resizing.
+
+    The ImageProcessing gem also comes with a backend implemented on
+    [libvips](http://jcupitt.github.io/libvips/), an alternative to
+    ImageMagick which has significantly better performance than
+    ImageMagick in most cases, both in terms of speed and memory usage. In
+    Active Storage it's now possible to switch to the libvips backend by
+    changing `Rails.application.config.active_storage.variant_processor` to
+    `:vips`.
+
+    *Janko Marohnić*
+
 *   Rails 6 requires Ruby 2.4.1 or newer.
 
     *Jeremy Daer*
diff --git a/activestorage/README.md b/activestorage/README.md
index 85ab70dac6..b677721d95 100644
--- a/activestorage/README.md
+++ b/activestorage/README.md
@@ -4,7 +4,7 @@ Active Storage makes it simple to upload and reference files in cloud services l
 
 Files can be uploaded from the server to the cloud or directly from the client to the cloud.
 
-Image files can furthermore be transformed using on-demand variants for quality, aspect ratio, size, or any other [MiniMagick](https://github.com/minimagick/minimagick) supported transformation.
+Image files can furthermore be transformed using on-demand variants for quality, aspect ratio, size, or any other [MiniMagick](https://github.com/minimagick/minimagick) or [Vips](http://www.rubydoc.info/gems/ruby-vips/Vips/Image) supported transformation.
 
 ## Compared to other storage solutions
 
@@ -99,7 +99,7 @@ Variation of image attachment:
 
 ```erb
 <%# Hitting the variant URL will lazy transform the original blob and then redirect to its new service location %>
-<%= image_tag user.avatar.variant(resize: "100x100") %>
+<%= image_tag user.avatar.variant(resize_to_fit: [100, 100]) %>
 ```
 
 ## Direct uploads
diff --git a/activestorage/app/models/active_storage/blob/representable.rb b/activestorage/app/models/active_storage/blob/representable.rb
index fea62e62de..03d5511481 100644
--- a/activestorage/app/models/active_storage/blob/representable.rb
+++ b/activestorage/app/models/active_storage/blob/representable.rb
@@ -10,7 +10,7 @@ module ActiveStorage::Blob::Representable
   # Returns an ActiveStorage::Variant instance with the set of +transformations+ provided. This is only relevant for image
   # files, and it allows any image to be transformed for size, colors, and the like. Example:
   #
-  #   avatar.variant(resize: "100x100").processed.service_url
+  #   avatar.variant(resize_to_fit: [100, 100]).processed.service_url
   #
   # This will create and process a variant of the avatar blob that's constrained to a height and width of 100px.
   # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations.
@@ -18,7 +18,7 @@ module ActiveStorage::Blob::Representable
   # Frequently, though, you don't actually want to transform the variant right away. But rather simply refer to a
   # specific variant that can be created by a controller on-demand. Like so:
   #
-  #   <%= image_tag Current.user.avatar.variant(resize: "100x100") %>
+  #   <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %>
   #
   # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController
   # can then produce on-demand.
@@ -43,13 +43,13 @@ module ActiveStorage::Blob::Representable
   # from a non-image blob. Active Storage comes with built-in previewers for videos and PDF documents. The video previewer
   # extracts the first frame from a video and the PDF previewer extracts the first page from a PDF document.
   #
-  #   blob.preview(resize: "100x100").processed.service_url
+  #   blob.preview(resize_to_fit: [100, 100]).processed.service_url
   #
   # Avoid processing previews synchronously in views. Instead, link to a controller action that processes them on demand.
   # Active Storage provides one, but you may want to create your own (for example, if you need authentication). Here’s
   # how to use the built-in version:
   #
-  #   <%= image_tag video.preview(resize: "100x100") %>
+  #   <%= image_tag video.preview(resize_to_fit: [100, 100]) %>
   #
   # This method raises ActiveStorage::UnpreviewableError if no previewer accepts the receiving blob. To determine
   # whether a blob is accepted by any previewer, call ActiveStorage::Blob#previewable?.
@@ -69,7 +69,7 @@ module ActiveStorage::Blob::Representable
 
   # Returns an ActiveStorage::Preview for a previewable blob or an ActiveStorage::Variant for a variable image blob.
   #
-  #   blob.representation(resize: "100x100").processed.service_url
+  #   blob.representation(resize_to_fit: [100, 100]).processed.service_url
   #
   # Raises ActiveStorage::UnrepresentableError if the receiving blob is neither variable nor previewable. Call
   # ActiveStorage::Blob#representable? to determine whether a blob is representable.
diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb
index 2b87897183..de58763399 100644
--- a/activestorage/app/models/active_storage/preview.rb
+++ b/activestorage/app/models/active_storage/preview.rb
@@ -38,7 +38,7 @@ class ActiveStorage::Preview
 
   # Processes the preview if it has not been processed yet. Returns the receiving Preview instance for convenience:
   #
-  #   blob.preview(resize: "100x100").processed.service_url
+  #   blob.preview(resize_to_fit: [100, 100]).processed.service_url
   #
   # Processing a preview generates an image from its blob and attaches the preview image to the blob. Because the preview
   # image is stored with the blob, it is only generated once.
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index d84208419c..b782489a92 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -6,19 +6,28 @@ require "active_storage/downloading"
 # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the
 # original.
 #
-# Variants rely on {MiniMagick}[https://github.com/minimagick/minimagick] for the actual transformations
-# of the file, so you must add <tt>gem "mini_magick"</tt> to your Gemfile if you wish to use variants.
+# Variants rely on {ImageProcessing}[https://github.com/janko-m/image_processing] gem for the actual transformations
+# of the file, so you must add <tt>gem "image_processing"</tt> to your Gemfile if you wish to use variants. By
+# default, images will be processed with {ImageMagick}[http://imagemagick.org] using the
+# {MiniMagick}[https://github.com/minimagick/minimagick] gem, but you can also switch to the
+# {libvips}[http://jcupitt.github.io/libvips/] processor operated by the {ruby-vips}[https://github.com/jcupitt/ruby-vips]
+# gem).
 #
-# Note that to create a variant it's necessary to download the entire blob file from the service and load it
-# into memory. The larger the image, the more memory is used. Because of this process, you also want to be
-# considerate about when the variant is actually processed. You shouldn't be processing variants inline in a
-# template, for example. Delay the processing to an on-demand controller, like the one provided in
+#   Rails.application.config.active_storage.variant_processor
+#   # => :mini_magick
+#
+#   Rails.application.config.active_storage.variant_processor = :vips
+#   # => :vips
+#
+# Note that to create a variant it's necessary to download the entire blob file from the service. Because of this process,
+# you also want to be considerate about when the variant is actually processed. You shouldn't be processing variants inline
+# in a template, for example. Delay the processing to an on-demand controller, like the one provided in
 # ActiveStorage::RepresentationsController.
 #
 # To refer to such a delayed on-demand variant, simply link to the variant through the resolved route provided
 # by Active Storage like so:
 #
-#   <%= image_tag Current.user.avatar.variant(resize: "100x100") %>
+#   <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %>
 #
 # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController
 # can then produce on-demand.
@@ -27,15 +36,22 @@ require "active_storage/downloading"
 # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform
 # the transformations, upload the variant to the service, and return itself again. Example:
 #
-#   avatar.variant(resize: "100x100").processed.service_url
+#   avatar.variant(resize_to_fit: [100, 100]).processed.service_url
 #
 # This will create and process a variant of the avatar blob that's constrained to a height and width of 100.
 # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations.
 #
-# A list of all possible transformations is available at https://www.imagemagick.org/script/mogrify.php. You can
-# combine as many as you like freely:
+# You can combine any number of ImageMagick/libvips operations into a variant, as well as any macros provided by the
+# ImageProcessing gem (such as +resize_to_fit+):
+#
+#   avatar.variant(resize_to_fit: [800, 800], monochrome: true, flip: "-90")
+#
+# Visit the following links for a list of available ImageProcessing commands and ImageMagick/libvips operations:
 #
-#   avatar.variant(resize: "100x100", monochrome: true, flip: "-90")
+# * {ImageProcessing::MiniMagick}[https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#methods]
+# * {ImageMagick reference}[https://www.imagemagick.org/script/mogrify.php]
+# * {ImageProcessing::Vips}[https://github.com/janko-m/image_processing/blob/master/doc/vips.md#methods]
+# * {ruby-vips reference}[http://www.rubydoc.info/gems/ruby-vips/Vips/Image]
 class ActiveStorage::Variant
   include ActiveStorage::Downloading
 
@@ -82,10 +98,10 @@ class ActiveStorage::Variant
     end
 
     def process
-      open_image do |image|
-        transform image
-        format image
-        upload image
+      download_blob_to_tempfile do |image|
+        transform image do |output|
+          upload output
+        end
       end
     end
 
@@ -102,31 +118,18 @@ class ActiveStorage::Variant
       blob.content_type.presence_in(WEB_IMAGE_CONTENT_TYPES) || "image/png"
     end
 
-
-    def open_image(&block)
-      image = download_image
+    def transform(image)
+      format = "png" unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type)
+      result = variation.transform(image, format: format)
 
       begin
-        yield image
+        yield result
       ensure
-        image.destroy!
+        result.close!
       end
     end
 
-    def download_image
-      require "mini_magick"
-      MiniMagick::Image.create(blob.filename.extension_with_delimiter) { |file| download_blob_to(file) }
-    end
-
-    def transform(image)
-      variation.transform(image)
-    end
-
-    def format(image)
-      image.format("PNG") unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type)
-    end
-
-    def upload(image)
-      File.open(image.path, "r") { |file| service.upload(key, file) }
+    def upload(file)
+      service.upload(key, file)
     end
 end
diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb
index 12e7f9f0b5..42f00beb82 100644
--- a/activestorage/app/models/active_storage/variation.rb
+++ b/activestorage/app/models/active_storage/variation.rb
@@ -6,17 +6,9 @@
 # In case you do need to use this directly, it's instantiated using a hash of transformations where
 # the key is the command and the value is the arguments. Example:
 #
-#   ActiveStorage::Variation.new(resize: "100x100", monochrome: true, trim: true, rotate: "-90")
+#   ActiveStorage::Variation.new(resize_to_fit: [100, 100], monochrome: true, trim: true, rotate: "-90")
 #
-# You can also combine multiple transformations in one step, e.g. for center-weighted cropping:
-#
-#   ActiveStorage::Variation.new(combine_options: {
-#     resize: "100x100^",
-#     gravity: "center",
-#     crop: "100x100+0+0",
-#   })
-#
-# A list of all possible transformations is available at https://www.imagemagick.org/script/mogrify.php.
+# The options map directly to {ImageProcessing}[https://github.com/janko-m/image_processing] commands.
 class ActiveStorage::Variation
   attr_reader :transformations
 
@@ -51,10 +43,49 @@ class ActiveStorage::Variation
     @transformations = transformations
   end
 
-  # Accepts an open MiniMagick image instance, like what's returned by <tt>MiniMagick::Image.read(io)</tt>,
-  # and performs the +transformations+ against it. The transformed image instance is then returned.
-  def transform(image)
+  # Accepts a File object, performs the +transformations+ against it, and
+  # saves the transformed image into a temporary file. If +format+ is specified
+  # it will be the format of the result image, otherwise the result image
+  # retains the source format.
+  def transform(file, format: nil)
     ActiveSupport::Notifications.instrument("transform.active_storage") do
+      if processor
+        image_processing_transform(file, format)
+      else
+        mini_magick_transform(file, format)
+      end
+    end
+  end
+
+  # Returns a signed key for all the +transformations+ that this variation was instantiated with.
+  def key
+    self.class.encode(transformations)
+  end
+
+  private
+    # Applies image transformations using the ImageProcessing gem.
+    def image_processing_transform(file, format)
+      operations = transformations.inject([]) do |list, (name, argument)|
+        if name.to_s == "combine_options"
+          ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.")
+          list.concat argument.to_a
+        else
+          list << [name, argument]
+        end
+      end
+
+      processor
+        .source(file)
+        .loader(page: 0)
+        .convert(format)
+        .apply(operations)
+        .call
+    end
+
+    # Applies image transformations using the MiniMagick gem.
+    def mini_magick_transform(file, format)
+      image = MiniMagick::Image.new(file.path, file)
+
       transformations.each do |name, argument_or_subtransformations|
         image.mogrify do |command|
           if name.to_s == "combine_options"
@@ -66,15 +97,24 @@ class ActiveStorage::Variation
           end
         end
       end
+
+      image.format(format) if format
+
+      image.tempfile.tap(&:open)
     end
-  end
 
-  # Returns a signed key for all the +transformations+ that this variation was instantiated with.
-  def key
-    self.class.encode(transformations)
-  end
+    # Returns the ImageProcessing processor class specified by `ActiveStorage.variant_processor`.
+    def processor
+      begin
+        require "image_processing"
+      rescue LoadError
+        ActiveSupport::Deprecation.warn("Using mini_magick gem directly is deprecated and will be removed in Rails 6.1. Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.")
+        return nil
+      end
+
+      ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize) if ActiveStorage.variant_processor
+    end
 
-  private
     def pass_transform_argument(command, method, argument)
       if eligible_argument?(argument)
         command.public_send(method, argument)
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
index e1bd974853..e1deee1d82 100644
--- a/activestorage/lib/active_storage.rb
+++ b/activestorage/lib/active_storage.rb
@@ -45,6 +45,7 @@ module ActiveStorage
   mattr_accessor :queue
   mattr_accessor :previewers, default: []
   mattr_accessor :analyzers, default: []
+  mattr_accessor :variant_processor, default: :mini_magick
   mattr_accessor :paths, default: {}
   mattr_accessor :variable_content_types, default: []
   mattr_accessor :content_types_to_serve_as_binary, default: []
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
index 1385e2aa84..99588cdd4b 100644
--- a/activestorage/lib/active_storage/engine.rb
+++ b/activestorage/lib/active_storage/engine.rb
@@ -43,11 +43,12 @@ module ActiveStorage
 
     initializer "active_storage.configs" do
       config.after_initialize do |app|
-        ActiveStorage.logger     = app.config.active_storage.logger || Rails.logger
-        ActiveStorage.queue      = app.config.active_storage.queue
-        ActiveStorage.previewers = app.config.active_storage.previewers || []
-        ActiveStorage.analyzers  = app.config.active_storage.analyzers || []
-        ActiveStorage.paths      = app.config.active_storage.paths || {}
+        ActiveStorage.logger            = app.config.active_storage.logger || Rails.logger
+        ActiveStorage.queue             = app.config.active_storage.queue
+        ActiveStorage.variant_processor = app.config.active_storage.variant_processor || :mini_magick
+        ActiveStorage.previewers        = app.config.active_storage.previewers || []
+        ActiveStorage.analyzers         = app.config.active_storage.analyzers || []
+        ActiveStorage.paths             = app.config.active_storage.paths || {}
 
         ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || []
         ActiveStorage.content_types_to_serve_as_binary = app.config.active_storage.content_types_to_serve_as_binary || []
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 0f3ada25c0..e74bbc9ab4 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -25,13 +25,30 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     assert_match(/Gray/, image.colorspace)
   end
 
-  test "center-weighted crop of JPEG blob" do
+  test "center-weighted crop of JPEG blob using :combine_options" do
+    begin
+      ActiveStorage.variant_processor = nil
+      blob = create_file_blob(filename: "racecar.jpg")
+      variant = ActiveSupport::Deprecation.silence do
+        blob.variant(combine_options: {
+          gravity: "center",
+          resize: "100x100^",
+          crop: "100x100+0+0",
+        }).processed
+      end
+      assert_match(/racecar\.jpg/, variant.service_url)
+
+      image = read_image(variant)
+      assert_equal 100, image.width
+      assert_equal 100, image.height
+    ensure
+      ActiveStorage.variant_processor = :mini_magick
+    end
+  end
+
+  test "center-weighted crop of JPEG blob using :resize_to_fill" do
     blob = create_file_blob(filename: "racecar.jpg")
-    variant = blob.variant(combine_options: {
-      gravity: "center",
-      resize: "100x100^",
-      crop: "100x100+0+0",
-    }).processed
+    variant = blob.variant(resize_to_fill: [100, 100]).processed
     assert_match(/racecar\.jpg/, variant.service_url)
 
     image = read_image(variant)
@@ -80,4 +97,20 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     variant = blob.variant(font: "a" * 10_000).processed
     assert_operator variant.service_url.length, :<, 525
   end
+
+  test "works for vips processor" do
+    begin
+      ActiveStorage.variant_processor = :vips
+      blob = create_file_blob(filename: "racecar.jpg")
+      variant = blob.variant(thumbnail_image: 100).processed
+
+      image = read_image(variant)
+      assert_equal 100, image.width
+      assert_equal 67, image.height
+    rescue LoadError
+      # libvips not installed
+    ensure
+      ActiveStorage.variant_processor = :mini_magick
+    end
+  end
 end
diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb
index 028874f374..4787eccd09 100644
--- a/activestorage/test/test_helper.rb
+++ b/activestorage/test/test_helper.rb
@@ -7,7 +7,7 @@ require "bundler/setup"
 require "active_support"
 require "active_support/test_case"
 require "active_support/testing/autorun"
-require "mini_magick"
+require "image_processing/mini_magick"
 
 begin
   require "byebug"