Detect images properly in FileUpload and MultipleFileUpload field

Closes #3633

Author: mshibuya

lib/rails_admin/config/fields/types/file_upload.rb

@@ -52,7 +52,7 @@ class FileUpload < RailsAdmin::Config::Fields::Base
           end
 
           register_instance_option :image? do
-            mime_type = Mime::Type.lookup_by_extension(resource_url.to_s.split('.').last)
+            mime_type = Mime::Type.lookup_by_extension(extension)
             mime_type.to_s.match?(/^image/)
           end
 
@@ -66,6 +66,12 @@ class FileUpload < RailsAdmin::Config::Fields::Base
             }
           end
 
+          def extension
+            URI.parse(resource_url).path.split('.').last
+          rescue URI::InvalidURIError
+            nil
+          end
+
           # virtual class
           def resource_url
             raise 'not implemented'

lib/rails_admin/config/fields/types/multiple_file_upload.rb

@@ -47,13 +47,19 @@ def initialize(value)
             end
 
             register_instance_option :image? do
-              mime_type = Mime::Type.lookup_by_extension(resource_url.to_s.split('.').last)
+              mime_type = Mime::Type.lookup_by_extension(extension)
               mime_type.to_s.match?(/^image/)
             end
 
             def resource_url(_thumb = false)
               raise 'not implemented'
             end
+
+            def extension
+              URI.parse(resource_url).path.split('.').last
+            rescue URI::InvalidURIError
+              nil
+            end
           end
 
           def initialize(*args)

spec/rails_admin/config/fields/types/file_upload_spec.rb

@@ -83,4 +83,57 @@ def resource_url
       end
     end
   end
+
+  describe '#image?' do
+    let(:filename) { 'dummy.txt' }
+    let :rails_admin_field do
+      RailsAdmin.config('FieldTest').fields.detect do |f|
+        f.name == :string_field
+      end.with(
+        object: FieldTest.new(string_field: filename),
+        view: ApplicationController.new.view_context,
+      )
+    end
+    before do
+      RailsAdmin.config FieldTest do
+        field :string_field, :file_upload do
+          def resource_url
+            "http://example.com/#{value}"
+          end
+        end
+      end
+    end
+
+    context 'when the file is not an image' do
+      let(:filename) { 'dummy.txt' }
+
+      it 'returns false' do
+        expect(rails_admin_field.image?).to be false
+      end
+    end
+
+    context 'when the file is an image' do
+      let(:filename) { 'dummy.jpg' }
+
+      it 'returns true' do
+        expect(rails_admin_field.image?).to be true
+      end
+    end
+
+    context 'when the file is an image but suffixed with a query string' do
+      let(:filename) { 'dummy.jpg?foo=bar' }
+
+      it 'returns true' do
+        expect(rails_admin_field.image?).to be true
+      end
+    end
+
+    context "when the filename can't be represented as a valid URI" do
+      let(:filename) { 'du mmy.jpg' }
+
+      it 'returns false' do
+        expect(rails_admin_field.image?).to be false
+      end
+    end
+  end
 end

spec/rails_admin/config/fields/types/multiple_file_upload_spec.rb

@@ -146,4 +146,59 @@ def value
       expect(rails_admin_field.with(object: FieldTest.new(string_field: 'dummy.txt')).attachments.map(&:value)).to eq ['dummy.txt']
     end
   end
+
+  describe '#image?' do
+    let(:filename) { 'dummy.txt' }
+    let :rails_admin_field do
+      RailsAdmin.config('FieldTest').fields.detect do |f|
+        f.name == :string_field
+      end.with(
+        object: FieldTest.new(string_field: filename),
+        view: ApplicationController.new.view_context,
+      )
+    end
+    before do
+      RailsAdmin.config FieldTest do
+        field :string_field, :multiple_file_upload do
+          attachment do
+            def resource_url
+              "http://example.com/#{value}"
+            end
+          end
+        end
+      end
+    end
+
+    context 'when the file is not an image' do
+      let(:filename) { 'dummy.txt' }
+
+      it 'returns false' do
+        expect(rails_admin_field.attachments.first.image?).to be false
+      end
+    end
+
+    context 'when the file is an image' do
+      let(:filename) { 'dummy.jpg' }
+
+      it 'returns true' do
+        expect(rails_admin_field.attachments.first.image?).to be true
+      end
+    end
+
+    context 'when the file is an image but suffixed with a query string' do
+      let(:filename) { 'dummy.jpg?foo=bar' }
+
+      it 'returns true' do
+        expect(rails_admin_field.attachments.first.image?).to be true
+      end
+    end
+
+    context "when the filename can't be represented as a valid URI" do
+      let(:filename) { 'du mmy.jpg' }
+
+      it 'returns false' do
+        expect(rails_admin_field.attachments.first.image?).to be false
+      end
+    end
+  end
 end