Address review findings for uploader and mapping helpers

Author: vipulnsward

lib/active_storage/service/uploadcare_service/uuid_mapping.rb

@@ -41,11 +41,22 @@ def persist_uuid_to_blob(key, uuid)
         end
 
         def keys_for_prefix(prefix)
-          return ActiveStorage::Blob.where('key LIKE ?', "#{prefix}%").pluck(:key) if defined?(ActiveStorage::Blob)
+          if defined?(ActiveStorage::Blob)
+            sanitized_prefix = sanitize_sql_like_prefix(prefix)
+            return ActiveStorage::Blob.where('key LIKE ?', "#{sanitized_prefix}%").pluck(:key)
+          end
 
           @key_uuid_map.keys.select { |key| key.start_with?(prefix) }
         end
 
+        def sanitize_sql_like_prefix(prefix)
+          if defined?(ActiveRecord::Base) && ActiveRecord::Base.respond_to?(:sanitize_sql_like)
+            ActiveRecord::Base.sanitize_sql_like(prefix.to_s)
+          else
+            prefix.to_s.gsub(/[\\%_]/) { |char| "\\#{char}" }
+          end
+        end
+
         def key_if_uuid(key)
           key if key.to_s.match?(/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i)
         end

lib/uploadcare/rails/action_view/uploadcare_uploader_tags.rb

@@ -43,7 +43,9 @@ def uploadcare_uploader_field(object_name, method_name, options = {})
         # @param options [Hash]
         # @return [String]
         def uploadcare_uploader_field_tag(object_name, options = {})
-          hidden_field_tag(object_name, options[:value], uploadcare_uploader_options(options))
+          options = options.dup
+          value = options.delete(:value)
+          hidden_field_tag(object_name, value, uploadcare_uploader_options(options))
         end
 
         # Converts uploader options into HTML data attributes.

lib/uploadcare/rails/objects/concerns/loadable.rb

@@ -27,6 +27,9 @@ def uploadcare_configuration
         # @param new_attrs [Hash]
         # @return [Object]
         def update_attrs(new_attrs)
+          return self if new_attrs.nil?
+          raise ArgumentError, 'new_attrs must be a Hash' unless new_attrs.is_a?(Hash)
+
           new_attrs.each do |key, value|
             setter = "#{key}="
             public_send(setter, value) if respond_to?(setter)

spec/uploadcare/rails/action_view/uploadcare_uploader_tags_spec.rb

@@ -49,6 +49,14 @@
       expect(tag).to include(fragment)
     end
   end
+
+  it 'does not duplicate value as a data attribute' do
+    tag = uploadcare_uploader_field_tag(:title, value: 'https://ucarecdn.com/file/', multiple: true)
+
+    expect(tag).to include('value="https://ucarecdn.com/file/"')
+    expect(tag).to include('data-multiple="true"')
+    expect(tag).not_to include('data-value=')
+  end
 end
 
 RSpec.configure do |c|

spec/uploadcare/rails/active_storage/uploadcare_service_spec.rb

@@ -141,6 +141,14 @@
     expect(service).to have_received(:delete).with('prefix-2')
   end
 
+  it 'escapes SQL wildcard characters in prefix when deleting prefixed keys' do
+    relation = double(pluck: [])
+    allow(ActiveStorage::Blob).to receive(:where).with('key LIKE ?',
+                                                       'pre\%fix\_%').and_return(relation)
+
+    service.delete_prefixed('pre%fix_')
+  end
+
   it 'supports existence check using mapped uuid' do
     blob = double(metadata: { 'uploadcare_uuid' => uuid }, update!: true)
     allow(ActiveStorage::Blob).to receive(:find_by).with(key: 'blob-key').and_return(blob)

spec/uploadcare/rails/objects/concerns/loadable_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'uploadcare/rails/objects/concerns/loadable'
+
+RSpec.describe Uploadcare::Rails::Objects::Loadable do
+  let(:klass) do
+    Class.new do
+      include Uploadcare::Rails::Objects::Loadable
+
+      attr_accessor :name
+
+      def self.uploadcare_configuration
+        Struct.new(:cache_namespace, :cache_expires_in, :cache_files).new(nil, 1.minute, false)
+      end
+    end
+  end
+  let(:instance) { klass.new }
+
+  describe '#update_attrs' do
+    it 'returns self when attrs are nil' do
+      expect(instance.update_attrs(nil)).to eq(instance)
+    end
+
+    it 'raises for non-hash attrs' do
+      expect { instance.update_attrs('name') }.to raise_error(ArgumentError, 'new_attrs must be a Hash')
+    end
+
+    it 'assigns known attributes from hash' do
+      instance.update_attrs('name' => 'Uploadcare')
+
+      expect(instance.name).to eq('Uploadcare')
+    end
+  end
+end