Address code review feedback on examples and resources

vipulnsward · vipulnsward · commit f22d44bcfbdc · 2026-02-09T17:09:18.000+05:30
diff --git a/api_examples/rest_api/delete_files_storage.rb b/api_examples/rest_api/delete_files_storage.rb
@@ -4,5 +4,8 @@
 Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY')
 
 uuids = ENV.fetch('UPLOADCARE_FILE_UUIDS',
-                  '21975c81-7f57-4c7a-aef9-acfe28779f78,cbaf2d73-5169-4b2b-a543-496cf2813dff').split(',')
+                  '21975c81-7f57-4c7a-aef9-acfe28779f78,cbaf2d73-5169-4b2b-a543-496cf2813dff')
+           .split(',')
+           .map(&:strip)
+           .reject(&:empty?)
 puts Uploadcare::File.batch_delete(uuids: uuids)
diff --git a/api_examples/rest_api/post_addons_remove_bg_execute.rb b/api_examples/rest_api/post_addons_remove_bg_execute.rb
@@ -4,4 +4,4 @@
 Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY')
 
 uuid = ENV.fetch('UPLOADCARE_FILE_UUID', '1bac376c-aa7e-4356-861b-dd2657b5bfd2')
-Uploadcare::Addons.remove_bg(uuid: uuid, params: { crop: true })
+puts Uploadcare::Addons.remove_bg(uuid: uuid, params: { crop: true })
diff --git a/api_examples/rest_api/post_files_remote_copy.rb b/api_examples/rest_api/post_files_remote_copy.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 # Copy a file to remote/custom storage
 # NOTE: Custom storage must be configured in your Uploadcare Dashboard first:
 #   Dashboard → Project Settings → Custom Storage
diff --git a/api_examples/rest_api/put_webhooks_id.rb b/api_examples/rest_api/put_webhooks_id.rb
@@ -10,4 +10,5 @@
   is_active: true,
   signing_secret: 'webhook-secret'
 }
-Uploadcare::Webhook.update(id: webhook_id, **options)
+updated_webhook = Uploadcare::Webhook.update(id: webhook_id, **options)
+puts updated_webhook.inspect
diff --git a/api_examples/upload_api/get_from_url_status.rb b/api_examples/upload_api/get_from_url_status.rb
@@ -5,7 +5,7 @@
 
 # Configure Uploadcare
 Uploadcare.configure do |config|
-  config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil)
+  config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')
 end
 
 # Start an async upload
@@ -15,8 +15,13 @@
 source_url = ENV.fetch('UPLOADCARE_TEST_URL', 'https://upload.wikimedia.org/wikipedia/commons/thumb/4/4d/Cat_November_2010-1a.jpg/1200px-Cat_November_2010-1a.jpg')
 
 client = Uploadcare::UploadClient.new
-result = client.upload_from_url(source_url: source_url, async: true).success
-token = result['token']
+result = client.upload_from_url(source_url: source_url, async: true)
+unless result.success?
+  warn "Upload failed: #{result.error_message}"
+  exit 1
+end
+
+token = result.success['token']
 
 puts "Upload token: #{token}"
 puts
@@ -26,7 +31,13 @@
 puts '=' * 50
 
 5.times do |i|
-  status = client.upload_from_url_status(token: token).success
+  status_result = client.upload_from_url_status(token: token)
+  unless status_result.success?
+    warn "Status check failed: #{status_result.error_message}"
+    break
+  end
+
+  status = status_result.success
 
   puts "Poll #{i + 1}:"
   puts "  Status: #{status['status']}"
diff --git a/api_examples/upload_api/get_group_info.rb b/api_examples/upload_api/get_group_info.rb
@@ -1,4 +1,5 @@
 require 'uploadcare'
+require 'dotenv/load'
 Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')
 Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY')
 
diff --git a/api_examples/upload_api/post_base.rb b/api_examples/upload_api/post_base.rb
@@ -14,7 +14,8 @@
 
 puts 'Uploading file...'
 result = nil
-File.open('spec/fixtures/kitten.jpeg', 'rb') do |file|
+fixture_path = File.expand_path('../../spec/fixtures/kitten.jpeg', __dir__)
+File.open(fixture_path, 'rb') do |file|
   result = client.upload_file(file: file, store: true)
 end
 
diff --git a/api_examples/upload_api/post_from_url.rb b/api_examples/upload_api/post_from_url.rb
@@ -15,20 +15,31 @@
 source_url = ENV.fetch('UPLOADCARE_TEST_URL', 'https://upload.wikimedia.org/wikipedia/commons/thumb/4/4d/Cat_November_2010-1a.jpg/1200px-Cat_November_2010-1a.jpg')
 
 client = Uploadcare::UploadClient.new
-result = client.upload_from_url(source_url: source_url, store: true).success
+result = client.upload_from_url(source_url: source_url, store: true)
+unless result.success?
+  warn "Upload failed: #{result.error_message}"
+  exit 1
+end
+
+payload = result.success
 
 puts 'Upload complete!'
-puts "File UUID: #{result['uuid']}"
-puts "Original filename: #{result['original_filename']}"
-puts "File size: #{result['size']} bytes"
+puts "File UUID: #{payload['uuid']}"
+puts "Original filename: #{payload['original_filename']}"
+puts "File size: #{payload['size']} bytes"
 puts
 
 # Example 2: Upload from URL (async mode - returns immediately)
 puts 'Example 2: Upload from URL (async mode)'
 puts '=' * 50
 
-result = client.upload_from_url(source_url: source_url, async: true).success
-token = result['token']
+result = client.upload_from_url(source_url: source_url, async: true)
+unless result.success?
+  warn "Async upload failed: #{result.error_message}"
+  exit 1
+end
+
+token = result.success['token']
 
 puts 'Upload started asynchronously'
 puts "Token: #{token}"
@@ -38,7 +49,13 @@
 puts 'Example 3: Check upload status'
 puts '=' * 50
 
-status = client.upload_from_url_status(token: token).success
+status_result = client.upload_from_url_status(token: token)
+unless status_result.success?
+  warn "Status check failed: #{status_result.error_message}"
+  exit 1
+end
+
+status = status_result.success
 
 case status['status']
 when 'success'
diff --git a/examples/group_creation.rb b/examples/group_creation.rb
@@ -43,7 +43,13 @@
   file_paths.each_with_index do |path, index|
     File.open(path, 'rb') do |file|
       response = upload_client.upload_file(file: file, store: true)
-      uuid = response.values.first
+      if response.failure?
+        puts "✗ Upload failed: #{response.error_message}"
+        exit 1
+      end
+
+      payload = response.success
+      uuid = payload[File.basename(path)] || payload.values.first
       uuids << uuid
       puts "  #{index + 1}. #{File.basename(path)} → #{uuid}"
     end
diff --git a/lib/uploadcare/clients/file_client.rb b/lib/uploadcare/clients/file_client.rb
@@ -45,9 +45,7 @@ def batch_store(uuids:, request_options: {})
     # @return [Hash] The response body containing 'result' and 'problems'
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/File/operation/filesDelete
     def batch_delete(uuids:, request_options: {})
-      # Call parent class delete method directly
-      RestClient.instance_method(:delete).bind(self).call(path: '/files/storage/', params: uuids, headers: {},
-                                                          request_options: request_options)
+      request(method: :delete, path: '/files/storage/', params: uuids, headers: {}, request_options: request_options)
     end
 
     # Copies a file to local storage
diff --git a/lib/uploadcare/param/upload/signature_generator.rb b/lib/uploadcare/param/upload/signature_generator.rb
@@ -7,8 +7,15 @@ module Param
     module Upload
       class SignatureGenerator
         def self.call(config: Uploadcare.configuration)
-          expires_at = Time.now.to_i + config.upload_signature_lifetime
-          to_sign = config.secret_key.to_s + expires_at.to_s
+          secret_key = config.secret_key.to_s
+          lifetime = config.upload_signature_lifetime
+          raise ArgumentError, 'secret_key is required for upload signature' if secret_key.empty?
+          unless lifetime.is_a?(Integer) && lifetime.positive?
+            raise ArgumentError, 'upload_signature_lifetime must be a positive Integer'
+          end
+
+          expires_at = Time.now.to_i + lifetime
+          to_sign = secret_key + expires_at.to_s
           signature = Digest::MD5.hexdigest(to_sign)
           { signature: signature, expire: expires_at }
         end
diff --git a/lib/uploadcare/resources/file.rb b/lib/uploadcare/resources/file.rb
@@ -253,6 +253,10 @@ def process_convert_result(result, request_options: {})
                                    request_options: request_options)
       end
 
+      if result.respond_to?(:result) && result.result.is_a?(Array) && result.result.first.is_a?(Hash)
+        return process_hash_result({ 'result' => result.result }, request_options: request_options)
+      end
+
       result
     end
 
diff --git a/lib/uploadcare/resources/group.rb b/lib/uploadcare/resources/group.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'uri'
+
 module Uploadcare
   class Group < BaseResource
     ATTRIBUTES = %i[
@@ -84,8 +86,8 @@ def id
       return @id if @id
       return unless @cdn_url
 
-      # If initialized from URL, extract ID
-      @id = @cdn_url.gsub('https://ucarecdn.com/', '').gsub(%r{/.*}, '')
+      uri = URI.parse(@cdn_url)
+      @id = uri.path.split('/').reject(&:empty?).first
       @id
     end
 
@@ -107,12 +109,7 @@ def cdn_url
     # Returns CDN URLs of all files from group without API requesting
     # @return [Array<String>] Array of CDN URLs for all files in the group
     def file_cdn_urls
-      file_cdn_urls = []
-      (0...files_count).each do |file_index|
-        file_cdn_url = "#{cdn_url}nth/#{file_index}/"
-        file_cdn_urls << file_cdn_url
-      end
-      file_cdn_urls
+      files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" }
     end
   end
 end
diff --git a/lib/uploadcare/resources/paginated_collection.rb b/lib/uploadcare/resources/paginated_collection.rb
@@ -88,7 +88,17 @@ def previous_page
       fetch_page(@previous_page_url)
     end
 
-    # TODO: Add #all method which return an array of resource
+    def all
+      collection = self
+      items = []
+
+      while collection
+        items.concat(collection.resources)
+        collection = collection.next_page
+      end
+
+      items
+    end
 
     private
 
diff --git a/lib/uploadcare/resources/project.rb b/lib/uploadcare/resources/project.rb
@@ -6,8 +6,6 @@ class Project < BaseResource
 
     def initialize(attributes = {}, config = Uploadcare.configuration)
       super
-      @project_client = Uploadcare::ProjectClient.new(config: config)
-      assign_attributes(attributes)
     end
 
     # Fetches project information
diff --git a/lib/uploadcare/resources/uploader.rb b/lib/uploadcare/resources/uploader.rb
@@ -6,7 +6,6 @@ module Uploadcare
   class Uploader < BaseResource
     def initialize(attributes = {}, config = Uploadcare.configuration)
       super
-      @uploader_client = Uploadcare::UploaderClient.new(config: config)
     end
 
     # Upload file or group of files from array, File, or url
@@ -103,7 +102,8 @@ def self.file_info(uuid:, config: Uploadcare.configuration, request_options: {})
     end
 
     def self.uploader_client(config: Uploadcare.configuration)
-      Uploadcare::UploaderClient.new(config: config)
+      @uploader_clients ||= {}
+      @uploader_clients[config] ||= Uploadcare::UploaderClient.new(config: config)
     end
 
     # @param object [File]
diff --git a/lib/uploadcare/resources/video_converter.rb b/lib/uploadcare/resources/video_converter.rb
@@ -17,17 +17,22 @@ def initialize(attributes = {}, config = Uploadcare.configuration)
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Video/operation/convertVideo
 
     def self.convert(params:, options: {}, config: Uploadcare.configuration, request_options: {})
+      raise ArgumentError, 'params must include :uuid' unless params[:uuid]
+      raise ArgumentError, 'params must include :format' unless params[:format]
+      raise ArgumentError, 'params must include :quality' unless params[:quality]
+
       paths = Array(params[:uuid]).map do |uuid|
         "#{uuid}/video/-/format/#{params[:format]}/-/quality/#{params[:quality]}/"
       end
 
       video_converter_client = Uploadcare::VideoConverterClient.new(config: config)
-      Uploadcare::Result.unwrap(video_converter_client.convert_video(paths: paths, options: options,
-                                                                     request_options: request_options))
+      response = Uploadcare::Result.unwrap(video_converter_client.convert_video(paths: paths, options: options,
+                                                                                request_options: request_options))
+      new(response, config)
     end
 
     # Fetches the status of a video conversion job by token
-    # @param token [Integer] The job token
+    # @param token [String] The job token
     # @return [Hash] The response containing the job status
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Conversion/operation/videoConvertStatus
     def fetch_status(token:, request_options: {})
diff --git a/lib/uploadcare/resources/webhook.rb b/lib/uploadcare/resources/webhook.rb
@@ -4,16 +4,11 @@ module Uploadcare
   class Webhook < BaseResource
     attr_accessor :id, :project, :created, :updated, :event, :target_url, :is_active, :signing_secret, :version
 
-    def initialize(attributes = {}, config = Uploadcare.configuration)
-      super
-    end
-
     # Class method to list all project webhooks
     # @return [Array<Uploadcare::Webhook>] Array of Webhook instances
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Webhook/operation/webhooksList
     def self.list(config: Uploadcare.configuration, request_options: {})
-      webhook_client = Uploadcare::WebhookClient.new(config: config)
-      response = Uploadcare::Result.unwrap(webhook_client.list_webhooks(request_options: request_options))
+      response = Uploadcare::Result.unwrap(webhook_client(config).list_webhooks(request_options: request_options))
 
       response.map { |webhook_data| new(webhook_data, config) }
     end
@@ -27,7 +22,6 @@ def self.list(config: Uploadcare.configuration, request_options: {})
     # @return [Uploadcare::Webhook] The created webhook as an object
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Webhook/operation/webhookCreate
     def self.create(target_url:, config: Uploadcare.configuration, request_options: {}, **options)
-      client = Uploadcare::WebhookClient.new(config: config)
       event = options.fetch(:event, 'file.uploaded')
       is_active = options.key?(:is_active) ? options[:is_active] : true
       signing_secret = options[:signing_secret]
@@ -40,7 +34,9 @@ def self.create(target_url:, config: Uploadcare.configuration, request_options:
       payload[:signing_secret] = signing_secret if signing_secret
       payload[:version] = version if version
 
-      response = Uploadcare::Result.unwrap(client.create_webhook(options: payload, request_options: request_options))
+      response = Uploadcare::Result.unwrap(
+        webhook_client(config).create_webhook(options: payload, request_options: request_options)
+      )
       new(response, config)
     end
 
@@ -53,11 +49,10 @@ def self.create(target_url:, config: Uploadcare.configuration, request_options:
     # @return [Uploadcare::Webhook] The updated webhook as an object
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Webhook/operation/updateWebhook
     def self.update(id:, config: Uploadcare.configuration, request_options: {}, **options)
-      client = Uploadcare::WebhookClient.new(config: config)
       payload = update_payload(options)
 
       response = Uploadcare::Result.unwrap(
-        client.update_webhook(id: id, options: payload, request_options: request_options)
+        webhook_client(config).update_webhook(id: id, options: payload, request_options: request_options)
       )
       new(response, config)
     end
@@ -68,8 +63,9 @@ def self.update(id:, config: Uploadcare.configuration, request_options: {}, **op
     # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Webhook/operation/webhookUnsubscribe
 
     def self.delete(target_url:, config: Uploadcare.configuration, request_options: {})
-      client = Uploadcare::WebhookClient.new(config: config)
-      Uploadcare::Result.unwrap(client.delete_webhook(target_url: target_url, request_options: request_options))
+      Uploadcare::Result.unwrap(
+        webhook_client(config).delete_webhook(target_url: target_url, request_options: request_options)
+      )
     end
 
     def self.update_payload(options)
@@ -78,5 +74,11 @@ def self.update_payload(options)
       payload
     end
     private_class_method :update_payload
+
+    def self.webhook_client(config)
+      @webhook_clients ||= {}
+      @webhook_clients[config] ||= Uploadcare::WebhookClient.new(config: config)
+    end
+    private_class_method :webhook_client
   end
 end
diff --git a/spec/uploadcare/param/upload/signature_generator_spec.rb b/spec/uploadcare/param/upload/signature_generator_spec.rb
@@ -14,14 +14,17 @@
     expect(result[:signature]).to eq(expected_signature)
   end
 
-  it 'handles nil secret key' do
-    allow(Time).to receive(:now).and_return(Time.at(1000))
+  it 'raises when secret key is missing' do
     config = Uploadcare::Configuration.new(secret_key: nil, upload_signature_lifetime: 30)
 
-    result = described_class.call(config: config)
+    expect { described_class.call(config: config) }
+      .to raise_error(ArgumentError, 'secret_key is required for upload signature')
+  end
 
-    expect(result[:expire]).to eq(1030)
-    expected_signature = Digest::MD5.hexdigest('1030')
-    expect(result[:signature]).to eq(expected_signature)
+  it 'raises when upload_signature_lifetime is invalid' do
+    config = Uploadcare::Configuration.new(secret_key: 'secret', upload_signature_lifetime: nil)
+
+    expect { described_class.call(config: config) }
+      .to raise_error(ArgumentError, 'upload_signature_lifetime must be a positive Integer')
   end
 end
diff --git a/spec/uploadcare/resources/uploader_spec.rb b/spec/uploadcare/resources/uploader_spec.rb
diff --git a/spec/uploadcare/resources/video_converter_spec.rb b/spec/uploadcare/resources/video_converter_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+# frozen_string_literal: true`
	`2`	`+`
`1`	`3`	`# Copy a file to remote/custom storage`
`2`	`4`	`# NOTE: Custom storage must be configured in your Uploadcare Dashboard first:`
`3`	`5`	`# Dashboard → Project Settings → Custom Storage`
Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,5 @@`
`10`	`10`	`is_active: true,`
`11`	`11`	`signing_secret: 'webhook-secret'`
`12`	`12`	`}`
`13`		`-Uploadcare::Webhook.update(id: webhook_id, **options)`
	`13`	`+updated_webhook = Uploadcare::Webhook.update(id: webhook_id, **options)`
	`14`	`+puts updated_webhook.inspect`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`require 'uploadcare'`
	`2`	`+require 'dotenv/load'`
`2`	`3`	`Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')`
`3`	`4`	`Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY')`
`4`	`5`