Add ActiveStorage::Blob#open

[David Robertson & George Claghorn]

8 files changed, 81 insertions, 12 deletions

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 679ca0df03..dff4cb4912 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Add `ActiveStorage::Blob#open`, which downloads a blob to a tempfile on disk
+    and yields the tempfile. Deprecate `ActiveStorage::Downloading`.
+
+    *David Robertson, George Claghorn*
+
 *   Pass in `identify: false` as an argument when providing a `content_type` for
     `ActiveStorage::Attached::{One,Many}#attach` to bypass automatic content
     type inference. For example:
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index b8b3b62f22..9bf0766843 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "active_storage/downloader"
+
 # A blob is a record that contains the metadata about a file and a key for where that file resides on the service.
 # Blobs can be created in two ways:
 #
@@ -164,6 +166,11 @@ class ActiveStorage::Blob < ActiveRecord::Base
     service.download key, &block
   end
 
+  # Downloads the blob to a tempfile on disk. Yields the tempfile.
+  def open(&block)
+    ActiveStorage::Downloader.new(self).download_blob_to_tempfile(&block)
+  end
+
 
   # Deletes the file on the service that's associated with this blob. This should only be done if the blob is going to be
   # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index b782489a92..2a7006553c 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require "active_storage/downloading"
-
 # Image blobs can have variants that are the result of a set of transformations applied to the original.
 # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the
 # original.
@@ -53,8 +51,6 @@ require "active_storage/downloading"
 # * {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
-
   WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif )
 
   attr_reader :blob, :variation
@@ -98,7 +94,7 @@ class ActiveStorage::Variant
     end
 
     def process
-      download_blob_to_tempfile do |image|
+      blob.open do |image|
         transform image do |output|
           upload output
         end
diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb
index 7c4168c1a0..87e9e42aa2 100644
--- a/activestorage/lib/active_storage/analyzer.rb
+++ b/activestorage/lib/active_storage/analyzer.rb
@@ -1,13 +1,9 @@
 # frozen_string_literal: true
 
-require "active_storage/downloading"
-
 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.
   class Analyzer
-    include Downloading
-
     attr_reader :blob
 
     # Implement this method in a concrete subclass. Have it return true when given a blob from which
@@ -26,6 +22,11 @@ module ActiveStorage
     end
 
     private
+      # Downloads the blob to a tempfile on disk. Yields the tempfile.
+      def download_blob_to_tempfile #:doc:
+        blob.open(&block)
+      end
+
       def logger #:doc:
         ActiveStorage.logger
       end
diff --git a/activestorage/lib/active_storage/downloader.rb b/activestorage/lib/active_storage/downloader.rb
new file mode 100644
index 0000000000..7f5b4936a5
--- /dev/null
+++ b/activestorage/lib/active_storage/downloader.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+  class Downloader
+    def initialize(blob)
+      @blob = blob
+    end
+
+    def download_blob_to_tempfile
+      open_tempfile do |file|
+        download_blob_to file
+        yield file
+      end
+    end
+
+    private
+      attr_reader :blob
+
+      def open_tempfile
+        file = Tempfile.open([ "ActiveStorage", tempfile_extension_with_delimiter ])
+
+        begin
+          yield file
+        ensure
+          file.close!
+        end
+      end
+
+      def download_blob_to(file)
+        file.binmode
+        blob.download { |chunk| file.write(chunk) }
+        file.flush
+        file.rewind
+      end
+
+      def tempfile_extension_with_delimiter
+        blob.filename.extension_with_delimiter
+      end
+  end
+end
diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb
index 7c3d20ade0..df820bc088 100644
--- a/activestorage/lib/active_storage/downloading.rb
+++ b/activestorage/lib/active_storage/downloading.rb
@@ -1,9 +1,17 @@
 # frozen_string_literal: true
 
 require "tmpdir"
+require "active_support/core_ext/string/filters"
 
 module ActiveStorage
   module Downloading
+    def self.included(klass)
+      ActiveSupport::Deprecation.warn <<~MESSAGE.squish, caller_locations(2)
+        ActiveStorage::Downloading is deprecated and will be removed in Active Storage 6.1.
+        Use ActiveStorage::Blob#open instead.
+      MESSAGE
+    end
+
     private
       # Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile.
       def download_blob_to_tempfile #:doc:
diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb
index cf19987d72..2053ed4511 100644
--- a/activestorage/lib/active_storage/previewer.rb
+++ b/activestorage/lib/active_storage/previewer.rb
@@ -1,14 +1,12 @@
 # frozen_string_literal: true
 
-require "active_storage/downloading"
+require "active_support/core_ext/string/filters"
 
 module ActiveStorage
   # This is an abstract base class for previewers, which generate images from blobs. See
   # ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of
   # concrete subclasses.
   class Previewer
-    include Downloading
-
     attr_reader :blob
 
     # Implement this method in a concrete subclass. Have it return true when given a blob from which
@@ -28,6 +26,11 @@ module ActiveStorage
     end
 
     private
+      # Downloads the blob to a tempfile on disk. Yields the tempfile.
+      def download_blob_to_tempfile #:doc:
+        blob.open(&block)
+      end
+
       # Executes a system command, capturing its binary output in a tempfile. Yields the tempfile.
       #
       # Use this method to shell out to a system library (e.g. mupdf or ffmpeg) for preview image
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index 17151117db..1bc5951d79 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -84,6 +84,15 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     assert_equal "a" * 64.kilobytes, chunks.second
   end
 
+  test "open" do
+    create_file_blob(filename: "racecar.jpg").open do |file|
+      assert file.binmode?
+      assert_equal 0, file.pos
+      assert_match(/\.jpg\z/, file.path)
+      assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file"
+    end
+  end
+
   test "urls expiring in 5 minutes" do
     blob = create_blob