Extract transformers

6 files changed, 146 insertions, 70 deletions

diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index 239df34d81..ea57fa5f78 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -101,14 +101,8 @@ class ActiveStorage::Variant
       end
     end
 
-    def transform(image)
-      result = variation.transform(image, format: format)
-
-      begin
-        yield result
-      ensure
-        result.close!
-      end
+    def transform(image, &block)
+      variation.transform(image, format: format, &block)
     end
 
     def upload(file)
diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb
index ae1376a6cf..3adc2407e5 100644
--- a/activestorage/app/models/active_storage/variation.rb
+++ b/activestorage/app/models/active_storage/variation.rb
@@ -47,13 +47,9 @@ class ActiveStorage::Variation
   # 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)
+  def transform(file, format: nil, &block)
     ActiveSupport::Notifications.instrument("transform.active_storage") do
-      if processor
-        image_processing_transform(file, format)
-      else
-        mini_magick_transform(file, format)
-      end
+      transformer.transform(file, format: format, &block)
     end
   end
 
@@ -63,63 +59,22 @@ class ActiveStorage::Variation
   end
 
   private
-    # Applies image transformations using the ImageProcessing gem.
-    def image_processing_transform(file, format)
-      operations = transformations.each_with_object([]) do |(name, argument), list|
-        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.keep_if { |key, value| value.present? }.to_a
-        elsif argument.present?
-          list << [name, argument]
+    def transformer
+      if ActiveStorage.variant_processor
+        begin
+          require "image_processing"
+        rescue LoadError
+          ActiveSupport::Deprecation.warn <<~WARNING
+            Generating image variants will require the image_processing gem in Rails 6.1.
+            Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.
+          WARNING
+
+          ActiveStorage::Transformers::MiniMagickTransformer.new(transformations)
+        else
+          ActiveStorage::Transformers::ImageProcessingTransformer.new(transformations)
         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"
-            argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument|
-              pass_transform_argument(command, subtransformation_name, subtransformation_argument)
-            end
-          else
-            pass_transform_argument(command, name, argument_or_subtransformations)
-          end
-        end
-      end
-
-      image.format(format) if format
-
-      image.tempfile.tap(&:open)
-    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
-
-    def pass_transform_argument(command, method, argument)
-      if argument == true
-        command.public_send(method)
-      elsif argument.present?
-        command.public_send(method, argument)
+      else
+        ActiveStorage::Transformers::MiniMagickTransformer.new(transformations)
       end
     end
 end
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
index 928db32691..d3e3a2f49b 100644
--- a/activestorage/lib/active_storage.rb
+++ b/activestorage/lib/active_storage.rb
@@ -50,4 +50,12 @@ module ActiveStorage
   mattr_accessor :variable_content_types, default: []
   mattr_accessor :content_types_to_serve_as_binary, default: []
   mattr_accessor :service_urls_expire_in, default: 5.minutes
+
+  module Transformers
+    extend ActiveSupport::Autoload
+
+    autoload :Transformer
+    autoload :ImageProcessingTransformer
+    autoload :MiniMagickTransformer
+  end
 end
diff --git a/activestorage/lib/active_storage/transformers/image_processing_transformer.rb b/activestorage/lib/active_storage/transformers/image_processing_transformer.rb
new file mode 100644
index 0000000000..7f8685b72d
--- /dev/null
+++ b/activestorage/lib/active_storage/transformers/image_processing_transformer.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require "image_processing"
+
+module ActiveStorage
+  module Transformers
+    class ImageProcessingTransformer < Transformer
+      private
+        def process(file, format:)
+          processor.
+            source(file).
+            loader(page: 0).
+            convert(format).
+            apply(operations).
+            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 name.to_s == "combine_options"
+              ActiveSupport::Deprecation.warn <<~WARNING
+                Active Storage's ImageProcessing transformer doesn't support :combine_options,
+                as it always generates a single ImageMagick command. Passing :combine_options will
+                not be supported in Rails 6.1.
+              WARNING
+
+              list.concat argument.keep_if { |key, value| value.present? }.to_a
+            elsif argument.present?
+              list << [ name, argument ]
+            end
+          end
+        end
+    end
+  end
+end
diff --git a/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb b/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb
new file mode 100644
index 0000000000..e8e99cea9e
--- /dev/null
+++ b/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require "mini_magick"
+
+module ActiveStorage
+  module Transformers
+    class MiniMagickTransformer < Transformer
+      private
+        def process(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"
+                argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument|
+                  pass_transform_argument(command, subtransformation_name, subtransformation_argument)
+                end
+              else
+                pass_transform_argument(command, name, argument_or_subtransformations)
+              end
+            end
+          end
+
+          image.format(format) if format
+
+          image.tempfile.tap(&:open)
+        end
+
+        def pass_transform_argument(command, method, argument)
+          if argument == true
+            command.public_send(method)
+          elsif argument.present?
+            command.public_send(method, argument)
+          end
+        end
+    end
+  end
+end
diff --git a/activestorage/lib/active_storage/transformers/transformer.rb b/activestorage/lib/active_storage/transformers/transformer.rb
new file mode 100644
index 0000000000..2e21201004
--- /dev/null
+++ b/activestorage/lib/active_storage/transformers/transformer.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  module Transformers
+    # A Transformer applies a set of transformations to an image.
+    #
+    # The following concrete subclasses are included in Active Storage:
+    #
+    # * ActiveStorage::Transformers::ImageProcessingTransformer:
+    #   backed by ImageProcessing, a common interface for MiniMagick and ruby-vips
+    #
+    # * ActiveStorage::Transformers::MiniMagickTransformer:
+    #   backed by MiniMagick, a wrapper around the ImageMagick CLI
+    class Transformer
+      attr_reader :transformations
+
+      def initialize(transformations)
+        @transformations = transformations
+      end
+
+      # Applies the transformations to the source image in +file+, producing a target image in the
+      # specified +format+. Yields an open Tempfile containing the target image. Closes and unlinks
+      # the output tempfile after yielding to the given block. Returns the result of the block.
+      def transform(file, format:)
+        output = process(file, format: format)
+
+        begin
+          yield output
+        ensure
+          output.close!
+        end
+      end
+
+      private
+        # Returns an open Tempfile containing a transformed image in the given +format+.
+        # All subclasses implement this method.
+        def process(file, format:) #:doc:
+          raise NotImplementedError
+        end
+    end
+  end
+end