Resolve remaining review cleanups

Author: vipulnsward

lib/uploadcare/api/rest/document_conversions.rb

@@ -35,6 +35,7 @@ def convert(paths:, options: {}, request_options: {})
     body = { paths: paths }
     body[:store] = normalize_bool_param(options[:store]) if options.key?(:store)
     body[:save_in_group] = normalize_bool_param(options[:save_in_group]) if options.key?(:save_in_group)
+    body.compact!
 
     rest.post(path: '/convert/document/', params: body, headers: {}, request_options: request_options)
   end
@@ -53,10 +54,11 @@ def status(token:, request_options: {})
   private
 
   def normalize_bool_param(value)
-    case value
+    normalized = value.is_a?(String) ? value.strip.downcase : value
+
+    case normalized
     when true, 1, '1', 'true' then '1'
     when false, 0, '0', 'false' then '0'
-    else value ? '1' : '0'
     end
   end
 end

lib/uploadcare/internal/upload_io.rb

@@ -18,7 +18,10 @@ def self.wrap(source, filename: nil)
   end
 
   def self.path_backed?(source)
-    source.respond_to?(:path) && source.path && ::File.exist?(source.path)
+    source.respond_to?(:path) &&
+      source.path &&
+      ::File.file?(source.path) &&
+      ::File.readable?(source.path)
   end
 
   def self.extract_filename(source)

lib/uploadcare/resources/file.rb

@@ -6,11 +6,13 @@
 # and instance methods (store, delete, reload, convert) for working with files.
 #
 # @example Finding a file
-#   file = Uploadcare::File.find(uuid: "file-uuid")
+#   client = Uploadcare.client
+#   file = client.files.find(uuid: "file-uuid")
 #   file.original_filename  # => "photo.jpg"
 #
 # @example Uploading
-#   file = Uploadcare::File.upload(File.open("photo.jpg"), store: true)
+#   client = Uploadcare.client
+#   file = client.files.upload(::File.open("photo.jpg"), store: true)
 #
 # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/File
 class Uploadcare::Resources::File < Uploadcare::Resources::BaseResource
@@ -288,7 +290,12 @@ def convert_to_video(params: {}, options: {}, request_options: {})
   def uuid
     return @uuid if @uuid
 
-    source = @url || @original_file_url
+    source =
+      if @url
+        @url
+      elsif uploadcare_cdn_url?(@original_file_url)
+        @original_file_url
+      end
     return @uuid unless source
 
     @uuid = source[/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}/]
@@ -298,13 +305,18 @@ def uuid
   #
   # @return [String]
   def cdn_url
-    return @url if @url
+    return @url if uploadcare_cdn_url?(@url)
+    return @original_file_url if uploadcare_cdn_url?(@original_file_url)
 
     "#{config.cdn_base}#{uuid}/"
   end
 
   private
 
+  def uploadcare_cdn_url?(value)
+    value.to_s.start_with?(config.cdn_base.to_s)
+  end
+
   def convert_file(params, converter, options = {}, request_options: {})
     raise ArgumentError, 'The first argument must be a Hash' unless params.is_a?(Hash)
 

lib/uploadcare/resources/file_metadata.rb

@@ -20,7 +20,7 @@ def index(uuid: nil, request_options: {})
     response = Uploadcare::Result.unwrap(
       client.api.rest.file_metadata.index(uuid: uuid || @uuid, request_options: request_options)
     )
-    @metadata = response if response.is_a?(Hash)
+    @metadata = response.is_a?(Hash) ? response.transform_keys(&:to_s) : {}
     self
   end
 
@@ -72,9 +72,12 @@ def update(key:, value:, uuid: nil, request_options: {})
   # @param request_options [Hash] Request options
   # @return [String] Metadata value
   def show(key:, uuid: nil, request_options: {})
-    Uploadcare::Result.unwrap(
-      client.api.rest.file_metadata.show(uuid: uuid || @uuid, key: key, request_options: request_options)
+    target_uuid = uuid || @uuid
+    result = Uploadcare::Result.unwrap(
+      client.api.rest.file_metadata.show(uuid: target_uuid, key: key, request_options: request_options)
     )
+    @metadata[key.to_s] = result if target_uuid == @uuid
+    result
   end
 
   # Delete a metadata key.

spec/uploadcare/api/rest/document_conversions_spec.rb

@@ -154,6 +154,37 @@
 
       expect(stub).to have_been_requested
     end
+
+    it 'normalizes case-insensitive string booleans' do
+      stub = stub_request(:post, 'https://api.uploadcare.com/convert/document/')
+             .with(body: hash_including('store' => '0', 'save_in_group' => '1'))
+             .to_return(
+               status: 200,
+               body: { result: [], problems: {} }.to_json,
+               headers: { 'Content-Type' => 'application/json' }
+             )
+
+      document_conversions.convert(paths: [conversion_path], options: { store: 'False', save_in_group: 'TRUE' })
+
+      expect(stub).to have_been_requested
+    end
+
+    it 'omits unsupported boolean-like values' do
+      stub = stub_request(:post, 'https://api.uploadcare.com/convert/document/')
+             .with do |request|
+               body = JSON.parse(request.body)
+               body['paths'] == [conversion_path] && !body.key?('store')
+             end
+             .to_return(
+               status: 200,
+               body: { result: [], problems: {} }.to_json,
+               headers: { 'Content-Type' => 'application/json' }
+             )
+
+      document_conversions.convert(paths: [conversion_path], options: { store: 'no' })
+
+      expect(stub).to have_been_requested
+    end
   end
 
   describe '#status' do

spec/uploadcare/internal/upload_io_spec.rb

@@ -50,5 +50,18 @@
         described_class.wrap('not-io')
       end.to raise_error(ArgumentError, /readable IO/)
     end
+
+    it 'does not treat directories as path-backed files' do
+      Dir.mktmpdir do |directory|
+        source = StringIO.new('stream-data')
+        source.define_singleton_method(:path) { directory }
+
+        wrapped = described_class.wrap(source, filename: 'directory.txt')
+
+        expect(File.file?(wrapped.path)).to eq(true)
+        expect(wrapped.original_filename).to eq('directory.txt')
+        wrapped.close!
+      end
+    end
   end
 end

spec/uploadcare/resources/file_metadata_spec.rb

@@ -94,6 +94,17 @@
         expect(metadata_instance['key1']).to eq('value1')
         expect(metadata_instance.to_h).to eq({ 'key1' => 'value1' })
       end
+
+      it 'normalizes metadata keys to strings' do
+        allow(rest_metadata).to receive(:index)
+          .with(uuid: file_uuid, request_options: {})
+          .and_return(Uploadcare::Result.success({ key1: 'value1' }))
+
+        metadata_instance.index
+
+        expect(metadata_instance['key1']).to eq('value1')
+        expect(metadata_instance.to_h).to eq({ 'key1' => 'value1' })
+      end
     end
 
     describe '#[] and #[]=' do
@@ -150,6 +161,17 @@
 
         result = metadata_instance.show(key: 'color')
         expect(result).to eq('green')
+        expect(metadata_instance['color']).to eq('green')
+      end
+
+      it 'does not update local metadata when uuid differs' do
+        allow(rest_metadata).to receive(:show)
+          .with(uuid: 'other-uuid', key: 'color', request_options: {})
+          .and_return(Uploadcare::Result.success('green'))
+
+        result = metadata_instance.show(key: 'color', uuid: 'other-uuid')
+        expect(result).to eq('green')
+        expect(metadata_instance['color']).to be_nil
       end
     end
 

spec/uploadcare/resources/file_spec.rb

@@ -404,14 +404,22 @@
       expect(file.uuid).to eq(file_uuid)
     end
 
+    it 'does not extract uuid from non-Uploadcare original_file_url' do
+      file = described_class.new(
+        { 'original_file_url' => "https://example.com/#{file_uuid}/photo.jpg" },
+        client
+      )
+      expect(file.uuid).to be_nil
+    end
+
     it 'returns nil when no uuid source is available' do
       file = described_class.new({}, client)
       expect(file.uuid).to be_nil
     end
   end
 
   describe '#cdn_url' do
-    it 'returns the url attribute when set' do
+    it 'returns the CDN url attribute when set' do
       file = described_class.new(file_attrs, client)
       expect(file.cdn_url).to eq("https://ucarecdn.com/#{file_uuid}/")
     end
@@ -420,5 +428,24 @@
       file = described_class.new({ 'uuid' => file_uuid }, client)
       expect(file.cdn_url).to eq("https://ucarecdn.com/#{file_uuid}/")
     end
+
+    it 'falls back to Uploadcare original_file_url when url is not a CDN URL' do
+      file = described_class.new(
+        {
+          'url' => "https://api.uploadcare.com/files/#{file_uuid}/",
+          'original_file_url' => "https://ucarecdn.com/#{file_uuid}/photo.jpg"
+        },
+        client
+      )
+      expect(file.cdn_url).to eq("https://ucarecdn.com/#{file_uuid}/photo.jpg")
+    end
+
+    it 'ignores non-Uploadcare original_file_url values' do
+      file = described_class.new(
+        { 'uuid' => file_uuid, 'original_file_url' => 'https://example.com/photo.jpg' },
+        client
+      )
+      expect(file.cdn_url).to eq("https://ucarecdn.com/#{file_uuid}/")
+    end
   end
 end