[Metadata] Handle OpenUri large file case for Vips (#189)

igorkasyanchuk · Jul 17, 2024 · 31d92a4 · 31d92a4
1 parent ab27760
commit 31d92a4
Show file tree

Hide file tree

Showing 16 changed files with 144 additions and 4 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -44,20 +44,27 @@ GEM
       minitest (>= 5.1)
       tzinfo (~> 2.0)
       zeitwerk (~> 2.3)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.1)
+    bigdecimal (3.1.8)
     builder (3.2.4)
     coderay (1.1.3)
     combustion (1.3.1)
       activesupport (>= 3.0.0)
       railties (>= 3.0.0)
       thor (>= 0.14.6)
     concurrent-ruby (1.1.7)
+    crack (1.0.0)
+      bigdecimal
+      rexml
     crass (1.0.6)
     docile (1.4.0)
     erubi (1.10.0)
     ffi (1.15.5)
     globalid (1.0.0)
       activesupport (>= 5.0)
+    hashdiff (1.1.0)
     i18n (1.8.7)
       concurrent-ruby (~> 1.0)
     loofah (2.8.0)
@@ -83,6 +90,7 @@ GEM
     pry (0.13.1)
       coderay (~> 1.1)
       method_source (~> 1.0)
+    public_suffix (6.0.0)
     racc (1.5.2)
     rack (2.2.3)
     rack-test (1.1.0)
@@ -127,6 +135,10 @@ GEM
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.0.0)
+    webmock (3.23.0)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
     zeitwerk (2.4.2)
 
 PLATFORMS
@@ -146,6 +158,7 @@ DEPENDENCIES
   ruby-vips (>= 2.1.0)
   simplecov
   sqlite3
+  webmock (~> 3.23)
 
 BUNDLED WITH
    2.3.7
diff --git a/active_storage_validations.gemspec b/active_storage_validations.gemspec
@@ -22,6 +22,7 @@ Gem::Specification.new do |s|
 
   %w[activejob activemodel activestorage activesupport].each { |rails_subcomponent| s.add_dependency rails_subcomponent, '>= 5.2.0' }
   s.add_development_dependency 'minitest-focus', "~> 1.4"
+  s.add_development_dependency 'webmock', "~> 3.23"
   s.add_development_dependency 'combustion', "~> 1.3"
   s.add_development_dependency 'mini_magick', ">= 4.9.5"
   s.add_development_dependency 'ruby-vips', ">= 2.1.0"

diff --git a/gemfiles/rails_6_1_3_1.gemfile.lock b/gemfiles/rails_6_1_3_1.gemfile.lock
@@ -44,20 +44,27 @@ GEM
       minitest (>= 5.1)
       tzinfo (~> 2.0)
       zeitwerk (~> 2.3)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.2)
+    bigdecimal (3.1.8)
     builder (3.2.4)
     coderay (1.1.3)
     combustion (1.3.7)
       activesupport (>= 3.0.0)
       railties (>= 3.0.0)
       thor (>= 0.14.6)
     concurrent-ruby (1.2.2)
+    crack (1.0.0)
+      bigdecimal
+      rexml
     crass (1.0.6)
     docile (1.4.0)
     erubi (1.12.0)
     ffi (1.16.3)
     globalid (1.2.1)
       activesupport (>= 6.1)
+    hashdiff (1.1.0)
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
     json (2.6.3)
@@ -87,6 +94,7 @@ GEM
     pry (0.14.2)
       coderay (~> 1.1)
       method_source (~> 1.0)
+    public_suffix (6.0.0)
     racc (1.7.3)
     rack (2.2.8)
     rack-test (2.1.0)
@@ -138,6 +146,10 @@ GEM
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.5.0)
+    webmock (3.23.1)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
     zeitwerk (2.6.12)
 
 PLATFORMS
@@ -161,6 +173,7 @@ DEPENDENCIES
   ruby-vips (>= 2.1.0)
   simplecov
   sqlite3
+  webmock (~> 3.23)
 
 BUNDLED WITH
    2.3.7
diff --git a/gemfiles/rails_7_0.gemfile.lock b/gemfiles/rails_7_0.gemfile.lock
@@ -43,20 +43,27 @@ GEM
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
       tzinfo (~> 2.0)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.2)
+    bigdecimal (3.1.8)
     builder (3.2.4)
     coderay (1.1.3)
     combustion (1.3.5)
       activesupport (>= 3.0.0)
       railties (>= 3.0.0)
       thor (>= 0.14.6)
     concurrent-ruby (1.1.10)
+    crack (1.0.0)
+      bigdecimal
+      rexml
     crass (1.0.6)
     docile (1.4.0)
     erubi (1.10.0)
     ffi (1.15.5)
     globalid (1.0.0)
       activesupport (>= 5.0)
+    hashdiff (1.1.0)
     i18n (1.12.0)
       concurrent-ruby (~> 1.0)
     loofah (2.14.0)
@@ -79,6 +86,7 @@ GEM
     pry (0.14.1)
       coderay (~> 1.1)
       method_source (~> 1.0)
+    public_suffix (6.0.0)
     racc (1.6.0)
     rack (2.2.3)
     rack-test (1.1.0)
@@ -124,6 +132,10 @@ GEM
     tzinfo (2.0.5)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.1.0)
+    webmock (3.23.1)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
     zeitwerk (2.5.4)
 
 PLATFORMS
@@ -145,6 +157,7 @@ DEPENDENCIES
   ruby-vips (>= 2.1.0)
   simplecov
   sqlite3
+  webmock (~> 3.23)
 
 BUNDLED WITH
    2.3.7
diff --git a/gemfiles/rails_7_1.gemfile.lock b/gemfiles/rails_7_1.gemfile.lock
@@ -51,6 +51,8 @@ GEM
       minitest (>= 5.1)
       mutex_m
       tzinfo (~> 2.0)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.2)
     base64 (0.2.0)
     bigdecimal (3.1.4)
@@ -62,6 +64,9 @@ GEM
       thor (>= 0.14.6)
     concurrent-ruby (1.2.2)
     connection_pool (2.4.1)
+    crack (1.0.0)
+      bigdecimal
+      rexml
     crass (1.0.6)
     docile (1.4.0)
     drb (2.2.0)
@@ -70,6 +75,7 @@ GEM
     ffi (1.16.3)
     globalid (1.2.1)
       activesupport (>= 6.1)
+    hashdiff (1.1.0)
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
     io-console (0.6.0)
@@ -103,6 +109,7 @@ GEM
       method_source (~> 1.0)
     psych (5.1.1.1)
       stringio
+    public_suffix (6.0.0)
     racc (1.7.3)
     rack (3.0.8)
     rack-session (2.0.0)
@@ -167,6 +174,10 @@ GEM
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.5.0)
+    webmock (3.23.1)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
     webrick (1.8.1)
     zeitwerk (2.6.12)
 
@@ -191,6 +202,7 @@ DEPENDENCIES
   ruby-vips (>= 2.1.0)
   simplecov
   sqlite3
+  webmock (~> 3.23)
 
 BUNDLED WITH
    2.4.22
diff --git a/gemfiles/rails_next.gemfile.lock b/gemfiles/rails_next.gemfile.lock
@@ -101,21 +101,28 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.2)
+    bigdecimal (3.1.8)
     builder (3.2.4)
     coderay (1.1.3)
     combustion (1.3.5)
       activesupport (>= 3.0.0)
       railties (>= 3.0.0)
       thor (>= 0.14.6)
     concurrent-ruby (1.1.9)
+    crack (1.0.0)
+      bigdecimal
+      rexml
     crass (1.0.6)
     digest (3.1.0)
     docile (1.4.0)
     erubi (1.10.0)
     ffi (1.15.5)
     globalid (1.0.0)
       activesupport (>= 5.0)
+    hashdiff (1.1.0)
     i18n (1.10.0)
       concurrent-ruby (~> 1.0)
     io-wait (0.2.1)
@@ -161,6 +168,7 @@ GEM
     pry (0.14.1)
       coderay (~> 1.1)
       method_source (~> 1.0)
+    public_suffix (6.0.0)
     racc (1.7.0)
     rack (2.2.3)
     rack-test (1.1.0)
@@ -201,6 +209,10 @@ GEM
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.1.0)
+    webmock (3.23.1)
+      addressable (>= 2.8.0)
+      crack (>= 0.3.2)
+      hashdiff (>= 0.4.0, < 2.0.0)
     websocket-driver (0.7.5)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
@@ -224,6 +236,7 @@ DEPENDENCIES
   ruby-vips (>= 2.1.0)
   simplecov
   sqlite3
+  webmock (~> 3.23)
 
 BUNDLED WITH
    2.3.7
diff --git a/lib/active_storage_validations/metadata.rb b/lib/active_storage_validations/metadata.rb
@@ -101,7 +101,7 @@ def read_image
     end
 
     def new_image_from_path(path)
-      if vips_image_processor? && (Vips::get_suffixes.include?(File.extname(path).downcase) || !Vips::respond_to?(:vips_foreign_get_suffixes))
+      if vips_image_processor? && (supported_vips_suffix?(path) || vips_version_below_8_8? || open_uri_tempfile?(path))
         begin
           Vips::Image.new_from_file(path)
         rescue exception_class
@@ -116,6 +116,24 @@ def new_image_from_path(path)
       end
     end
 
+    def supported_vips_suffix?(path)
+      Vips::get_suffixes.include?(File.extname(path).downcase)
+    end
+
+    def vips_version_below_8_8?
+      # FYI, Vips 8.8 was released in 2019
+      # https://github.com/libvips/libvips/releases/tag/v8.8.0
+      !Vips::respond_to?(:vips_foreign_get_suffixes)
+    end
+
+    def open_uri_tempfile?(path)
+      # When trying to open urls for 'large' images, OpenURI will return a
+      # tempfile. That tempfile does not have an extension indicating the type
+      # of file. However, Vips will be able to process it anyway.
+      # The 'large' file value is derived from OpenUri::Buffer class (> 10ko)
+      path.split('/').last.starts_with?("open-uri")
+    end
+
     def valid_image?(image)
       return false unless image
 

diff --git a/test/dummy/app/models/metadata.rb b/test/dummy/app/models/metadata.rb
@@ -0,0 +1,5 @@
+class Metadata < ApplicationRecord
+  has_one_attached :large_image
+
+  validates :large_image, processable_image: true
+end
diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb
@@ -135,6 +135,11 @@
     t.datetime :updated_at, precision: 6, null: false
   end
 
+  create_table :metadata, force: :cascade do |t|
+    t.datetime :created_at, precision: 6, null: false
+    t.datetime :updated_at, precision: 6, null: false
+  end
+
   create_table :projects, force: :cascade do |t|
     t.string :title
     t.datetime :created_at, null: false

diff --git a/test/dummy/public/file_10ko b/test/dummy/public/file_10ko
diff --git a/test/dummy/public/file_10ko.png b/test/dummy/public/file_10ko.png
diff --git a/test/dummy/public/file_3ko.png b/test/dummy/public/file_3ko.png
diff --git a/test/metadata_test.rb b/test/metadata_test.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "open-uri"
+
+describe ActiveStorageValidations::Metadata do
+  include ValidatorHelpers
+
+  let(:instance) { Metadata.new }
+
+  describe "Vips" do
+    # Uncomment these lines in development, or launch test with ENV['IMAGE_PROCESSOR'] = :vips
+    # before do
+    #   @original_variant_processor = Rails.application.config.active_storage.variant_processor
+    #   Rails.application.config.active_storage.variant_processor = :vips
+    # end
+
+    # after do
+    #   Rails.application.config.active_storage.variant_processor = @original_variant_processor
+    # end
+
+    describe "OpenURI" do
+      before do
+        stub_request(:get, url)
+          .to_return(body: File.open(Rails.root.join('public', fetched_file)), status: 200)
+      end
+
+      subject { instance.large_image.attach(io: remote_image, filename: fetched_file, content_type: "image/png") and instance }
+
+      let(:url) { "https://example_image.jpg" }
+      let(:remote_image) { URI.parse(url).open }
+
+      describe "Opening small images (< 10ko) resulting in OpenUri returning a StringIO" do
+        let(:fetched_file) { 'file_3ko.png' }
+
+        it { is_expected_to_be_valid }
+      end
+
+      describe "Opening large images (>= 10ko) resulting in OpenUri returning a Tempfile" do
+        let(:fetched_file) { 'file_10ko.png' }
+
+        it { is_expected_to_be_valid }
+      end
+    end
+  end
+end
diff --git a/test/support/files.rb b/test/support/files.rb
@@ -229,9 +229,9 @@ def file_7ko_and_jpg
 
 def file_10ko
   {
-    io: File.open(Rails.root.join('public', 'file_10ko')),
+    io: File.open(Rails.root.join('public', 'file_10ko.png')),
     filename: 'file_10ko',
-    content_type: 'text/html'
+    content_type: 'image/png'
   }
 end