Scope caches by config, tighten error handling#186
Conversation
WalkthroughMajor v5.0.0 overhaul. Replaces entity-based architecture with resource/client layers, introduces new configuration, REST/Upload clients (Faraday), authenticator, error/throttle handling, signed URL/webhook verification, and result wrapper. Extensive public API renames to keyword args. Adds examples and updates docs/CI. Removes legacy clients/entities/params and many old VCR cassettes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant U as Uploader (Resources)
participant UC as UploadClient
participant S as Upload API
participant R as RestClient
Dev->>U: upload(object:, store:, metadata:)
alt small file or URL
U->>UC: upload_file / upload_from_url(...)
UC->>S: POST /base or /from_url
S-->>UC: 200 {uuid or token}
alt async URL
UC->>S: GET /from_url/status?token=...
S-->>UC: 200 {status: success, uuid}
end
UC-->>U: result (uuid/attrs)
U-->>Dev: Uploadcare::File
else large file (multipart)
U->>UC: multipart_start(filename,size,content_type,...)
UC->>S: POST /multipart/start
S-->>UC: 200 {uuid, parts[links]}
loop for each part
UC->>S: PUT presigned S3 URL (part)
S-->>UC: 200 OK
end
UC->>S: POST /multipart/complete {uuid}
S-->>UC: 200 {file_id}
UC-->>U: result (uuid/attrs)
U-->>Dev: Uploadcare::File
end
note over U,R: R used by resource loaders (e.g., File.info)
sequenceDiagram
autonumber
actor Dev as Developer
participant A as Addons (Resources)
participant AC as AddonsClient
participant REST as REST API
Dev->>A: aws_rekognition_detect_labels(uuid:)
A->>AC: aws_rekognition_detect_labels(uuid:)
AC->>REST: POST /addons/aws_rekognition_detect_labels/execute/
REST-->>AC: 200 {request_id}
AC-->>A: Result
Dev->>A: aws_rekognition_detect_labels_status(request_id:)
A->>AC: aws_rekognition_detect_labels_status(request_id:)
AC->>REST: GET /addons/.../status/?request_id=...
REST-->>AC: 200 {status, result}
AC-->>A: Result
A-->>Dev: Addons(resource)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ruby.yml (1)
24-46:⚠️ Potential issue | 🟠 MajorUpdate actions/checkout to v4 to address actionlint warnings.
actionlint flags
actions/checkout@v3as outdated because it runs on Node 16, which is no longer supported on GitHub-hosted runners. Upgrade both occurrences (lines 24 and 45) to v4, which uses Node 20.🛠️ Proposed fix
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4 @@ - - uses: actions/checkout@v3 + - uses: actions/checkout@v4README.md (1)
1165-1170:⚠️ Potential issue | 🟡 MinorFix variable name mismatch in the group creation example.
@files_aryis defined, but Line 1169 uses@files.✍️ Suggested doc fix
- `@group` = Uploadcare::Group.create(uuids: `@files`) + `@group` = Uploadcare::Group.create(uuids: `@files_ary`)
🤖 Fix all issues with AI agents
In `@examples/large_file_upload.rb`:
- Around line 86-93: The code treats result from multipart_upload as a Hash but
multipart_upload returns a Result object; change the prints to first unwrap the
Result (e.g., call result.success or result.value!) into a response_hash and
then index response_hash['uuid'] and other keys (or use the unwrapped object's
symbol/key access) so lines referencing result['uuid'], result[...] etc. operate
on the actual response Hash rather than the Result wrapper.
In `@lib/uploadcare/authenticator.rb`:
- Around line 30-34: Default headers hardcode 'Content-Type' to
'application/json' causing signature mismatch when a different content_type is
signed; update the authenticator to set 'Content-Type' from the
signed/content_type parameter (use the provided content_type when present,
otherwise fall back to 'application/json') wherever headers are built (e.g., the
`@default_headers` initialization and the headers-building logic around lines
65-70), ensuring the header value used in the HTTP request matches the value
used to compute the signature.
- Around line 45-48: In headers(http_method, uri, body = '', content_type =
'application/json') in authenticator.rb, don't silently return `@default_headers`
when `@config.auth_type` is not 'Uploadcare.Simple' but `@config.secret_key` is
nil/empty; instead raise an authentication error (e.g., raise
AuthenticationError.new("missing secret_key for secure auth") or a
RuntimeError/ArgumentError if AuthenticationError doesn't exist) so callers fail
fast; update the check around `@config.secret_key` to raise with a clear message
and ensure any tests or callers expecting headers are adjusted accordingly.
In `@lib/uploadcare/clients/document_converter_client.rb`:
- Around line 18-25: The convert_document method currently coerces
options[:store] and options[:save_in_group] using Ruby truthiness which treats
strings like "0" and "false" as true; add a small normalization helper (e.g.,
normalize_flag(value) or normalize_bool_param) and use it inside
convert_document to convert accepted inputs (true/false, "1"/"0",
"true"/"false", 1/0) into exact API strings "1" or "0" before building body;
update the body assignment for store and save_in_group to call this helper and
then call post(path: '/convert/document/', params: body, ...) as before.
In `@lib/uploadcare/clients/multipart_uploader_client.rb`:
- Around line 61-64: In process_chunk (multipart_uploader_client.rb) the call
::File.read(file, CHUNK_SIZE, offset) treats file as a path but file is an open
File object; fix by either using file.path with ::File.read (e.g.,
::File.read(file.path, CHUNK_SIZE, offset)) or, preferably, read from the File
object directly: seek to offset on the File object and call
file.read(CHUNK_SIZE) to obtain the chunk, then pass that chunk to
Uploadcare::Result.unwrap(put(links[link_index], chunk)).
In `@lib/uploadcare/clients/upload_client.rb`:
- Around line 621-686: In upload_parts_parallel, avoid preloading all parts into
memory (the parts array); instead enqueue lightweight descriptors (offset and
length and presigned_url) and have worker threads read the slice from disk
inside the thread (use a shared file mutex and file.seek/read or open
independent File objects per worker) before calling multipart_upload_part;
update the queue population to push descriptors rather than part_data, adjust
the worker to perform the seek/read using the descriptor, keep the existing
uploaded accounting and error handling, and ensure total_size and part indexing
still derive from presigned_urls/part_size.
- Around line 790-807: The upload methods currently assume file.path exists and
break for IOs like StringIO; update the initial validations in both
multipart_upload and upload_file to require file.respond_to?(:read) AND
file.respond_to?(:path) (i.e., reject or raise a clear error if path is missing)
so form_data_for can safely call file.path; reference the multipart_upload and
upload_file validation checks and the form_data_for helper to ensure they all
consistently validate/respond to IO objects lacking `#path`.
In `@lib/uploadcare/clients/uploader_client.rb`:
- Around line 94-97: The upload_options_to_params method currently skips setting
'UPLOADCARE_STORE' when options[:store] is false because it uses a truthy check;
change the condition to detect the presence of the key instead (e.g., use
options.key?(:store) or check for nil) so that upload_options_to_params will
call store_value(options[:store]) and include 'UPLOADCARE_STORE' even when
store: false is passed.
In `@lib/uploadcare/clients/webhook_client.rb`:
- Around line 20-30: The create_webhook method builds a payload but ignores
options[:version]; update create_webhook to include the version when provided by
merging { version: options[:version] } (using compact to drop nil) into payload
(similar to the signing_secret merge) so callers can set the webhook payload
version before the post call.
In `@lib/uploadcare/configuration.rb`:
- Around line 104-106: The custom_cname method currently calls
CnameGenerator.generate_cname with no context causing it to read global
Uploadcare.configuration; change custom_cname to pass the instance configuration
(or its public_key) into CnameGenerator.generate_cname and update
CnameGenerator.generate_cname to accept and use that configuration/public_key
parameter instead of referencing Uploadcare.configuration directly so cache
scoping uses the Configuration instance's public key.
In `@lib/uploadcare/resources/file.rb`:
- Around line 191-208: The uuid method currently hardcodes
'https://ucarecdn.com/' causing wrong extraction for custom CDN/API hosts;
update uuid to extract the UUID from `@url` using a host-agnostic path regex (e.g.
capture the first path segment after the domain via something like
%r{\Ahttps?://[^/]+/([^/?#]+)}), assign and return that value (set `@uuid` =
captured_value) so cdn_url (which calls uuid and `@config.cdn_base.call`) gets a
correct identifier regardless of CDN domain or subdomain; ensure the regex
handles optional trailing slash and query fragments.
- Around line 120-129: The batch_store response is being accessed with symbol
keys (response[:status], response[:result]) but Faraday returns string-keyed
hashes; update Uploadcare::File.batch_store to normalize response keys (e.g.,
call transform_keys(&:to_s) or otherwise convert to string keys) and then pass
normalized['status'], normalized['result'], and normalized['problems'] || {}
into BatchFileResult; apply the same normalization approach in the analogous
batch_delete handling so BatchFileResult is initialized with the correct
string-keyed fields.
In
`@spec/fixtures/vcr_cassettes/Upload_API_Integration/Complete_Upload_Workflow/when_uploading_from_URL/handles_async_URL_upload_with_status_checking.yml`:
- Around line 54-61: Replace the literal async upload token present in the
cassette body and status URL with a scrubbed placeholder: find the JSON body
string containing "type":"token","token":"79926202-9c03-4bb7-ac9c-458fbb407c08"
and replace the token value with a neutral marker (e.g.
"<SCRUBBED_UPLOAD_TOKEN>"); also update the recorded request/uri that contains
"/from_url/status/?token=79926202-9c03-4bb7-ac9c-458fbb407c08" to use the same
placeholder so both the response body and the status check URL are sanitized
consistently. Ensure you only replace the token value (the characters after
"token": and after "token=") and leave surrounding JSON and URL structure
intact.
🟡 Minor comments (22)
lib/uploadcare/error_handler.rb-55-62 (1)
55-62:⚠️ Potential issue | 🟡 MinorGuard JSON parsing in
catch_upload_errors.A non‑JSON 200 body will currently raise
JSON::ParserErrorand mask the original failure.🔧 Suggested fix
def catch_upload_errors(response) return unless response[:status] == 200 - parsed_response = JSON.parse(response[:body].to_s) - error = parsed_response['error'] if parsed_response.is_a?(Hash) - raise Exception::RequestError, error if error + parsed_response = JSON.parse(response[:body].to_s) + error = parsed_response['error'] if parsed_response.is_a?(Hash) + raise Exception::RequestError, error if error +rescue JSON::ParserError + nil endlib/uploadcare/clients/multipart_uploader_client.rb-48-55 (1)
48-55:⚠️ Potential issue | 🟡 MinorComment mentions "multiple threads" but implementation is sequential.
The comment on line 48 says "In multiple threads, split file into chunks..." but the implementation uses a simple sequential loop with
.times. If parallel uploads are intended for performance, this would needThreadorConcurrent::Future. If sequential is intentional, update the comment.📝 Proposed comment fix (if sequential is intentional)
- # In multiple threads, split file into chunks and upload those chunks into respective Amazon links + # Split file into chunks and upload those chunks sequentially into respective Amazon links # `@param` object [File]lib/uploadcare/clients/multipart_uploader_client.rb-7-9 (1)
7-9:⚠️ Potential issue | 🟡 MinorComment states "10MB" but constant is 5MB.
The comment on line 7 says "Default chunk size for multipart uploads (10MB)" but
CHUNK_SIZEis actually 5,242,880 bytes (5MB).📝 Proposed fix
# `@see` https://uploadcare.com/api-refs/upload-api/#tag/Upload - # Default chunk size for multipart uploads (10MB) + # Default chunk size for multipart uploads (5MB) class MultipartUploaderClient < UploadClientapi_examples/upload_api/get_file_info_example.rb-8-11 (1)
8-11:⚠️ Potential issue | 🟡 MinorFail fast when required keys are missing.
ENV.fetch(..., nil)quietly sets keys to nil, which then fails later with an auth error that’s harder to understand. A clear upfront check improves UX.✅ Suggested fix
-Uploadcare.configure do |config| - config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil) - config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', nil) -end +public_key = ENV['UPLOADCARE_PUBLIC_KEY'] +secret_key = ENV['UPLOADCARE_SECRET_KEY'] +unless public_key && secret_key + abort 'Set UPLOADCARE_PUBLIC_KEY and UPLOADCARE_SECRET_KEY' +end + +Uploadcare.configure do |config| + config.public_key = public_key + config.secret_key = secret_key +endexamples/simple_upload.rb-10-14 (1)
10-14:⚠️ Potential issue | 🟡 MinorAdd a friendly error when API keys are missing.
ENV.fetchwill raise before your error handling, leading to a stack trace. A preflight check keeps this example user-friendly.✅ Suggested fix
-Uploadcare.configure do |config| - config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') - config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY') -end +public_key = ENV['UPLOADCARE_PUBLIC_KEY'] +secret_key = ENV['UPLOADCARE_SECRET_KEY'] +unless public_key && secret_key + abort 'Set UPLOADCARE_PUBLIC_KEY and UPLOADCARE_SECRET_KEY' +end + +Uploadcare.configure do |config| + config.public_key = public_key + config.secret_key = secret_key +endapi_examples/upload_api/create_group.rb-22-33 (1)
22-33:⚠️ Potential issue | 🟡 MinorAdd error handling for upload results.
Lines 23 and 27 call.successdirectly without checking for failures. If an upload fails,.successreturnsnil, causing.values.firston lines 31–32 to crash. Follow the established pattern in other examples (e.g.,post_base.rb): store the result, checkresult.failure?, then accessresult.success.examples/batch_upload.rb-37-63 (1)
37-63:⚠️ Potential issue | 🟡 MinorEnsure file handles are closed even if opening fails.
Line 38 opens files before the
beginblock; if anyFile.openraises (permissions, symbolic links, transient IO errors), files already opened remain open and the rescue handler won't run. Move the open insidebeginand close inensureto guarantee closure.🔧 Proposed fix
-# Open all files -files = file_paths.map { |path| File.open(path, 'rb') } - -begin +files = [] +begin + # Open all files + files = file_paths.map { |path| File.open(path, 'rb') } # Upload all files results = Uploadcare::Uploader.upload(object: files, store: true) - # Close files - files.each(&:close) - # Display results puts '✓ Batch upload complete!' puts puts 'Results:' @@ -rescue StandardError => e - files.each(&:close) +rescue StandardError => e puts "✗ Batch upload failed: #{e.message}" exit 1 +ensure + files.each(&:close) endapi_examples/upload_api/test_url_upload.rb-8-11 (1)
8-11:⚠️ Potential issue | 🟡 MinorUse fail-fast for missing credentials instead of silently defaulting to nil.
Lines 9-10 use
ENV.fetch(..., nil)which silently sets credentials to nil if environment variables are missing. This defers the error to downstream API calls, resulting in confusing failures. Remove the nil default to raise an error immediately at startup:Proposed fix
- config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil) - config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', nil) + config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') + config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY')lib/uploadcare/resources/batch_file_result.rb-36-39 (1)
36-39:⚠️ Potential issue | 🟡 MinorAdd a default for
problemsto ensure API safety.Line 39 assigns
@problems = problemsdirectly. While all current callers provide a default withproblems: response[:problems] || {}, adding a defensive default in the initializer itself prevents potential issues if the class is instantiated directly. The documented example showsresult.problems.any?, which assumes@problemsis a Hash.- `@problems` = problems + `@problems` = problems || {}lib/uploadcare/clients/video_converter_client.rb-15-17 (1)
15-17:⚠️ Potential issue | 🟡 MinorIncorrect type documentation for
tokenparameter.The
@param tokenis documented asInteger, but conversion job tokens are typically UUID strings (e.g., similar to"97b15c86-c405-41c0-86a3-f8064ce1a01d"seen in other API responses).📝 Proposed fix
# 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 statusGemfile-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorRemove the unused
tsortgem.The
tsortgem is not referenced anywhere in the codebase (norequirestatements orTSortclass usage detected), so it should be removed from the Gemfile.api_examples/upload_api/comprehensive_demo.rb-8-11 (1)
8-11:⚠️ Potential issue | 🟡 MinorFail fast when credentials are missing.
Using
ENV.fetch(..., nil)silently sets nil keys, which leads to confusing auth errors later. Prefer required env vars or a clear exception.🛠️ Suggested fix
- config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil) - config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', nil) + config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') + config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY')examples/upload_with_progress.rb-11-14 (1)
11-14:⚠️ Potential issue | 🟡 MinorFail fast when API keys are missing.
Nil defaults defer failure with less clear errors; consider requiring env values up-front.Suggested fix
- config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil) - config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', nil) + config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') + config.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY')examples/large_file_upload.rb-17-26 (1)
17-26:⚠️ Potential issue | 🟡 MinorValidate thread count before starting upload.
Guard against 0/negative values so multipart uploads always use a valid parallelism setting.Suggested fix
file_path = ARGV[0] threads = (ARGV[1] || 4).to_i +if threads < 1 + puts 'threads must be >= 1' + exit 1 +endexamples/upload_with_progress.rb-39-72 (1)
39-72:⚠️ Potential issue | 🟡 MinorEnsure the file handle closes on errors.
If the upload raises,file.closeis skipped. AFile.openblock or ensure avoids leaks.Suggested fix
- file = File.open(file_path, 'rb') - start_time = Time.now - - result = Uploadcare::Uploader.upload(object: file, store: true) do |progress| - # Calculate progress metrics - uploaded_mb = (progress[:uploaded] / 1024.0 / 1024.0).round(2) - total_mb = (progress[:total] / 1024.0 / 1024.0).round(2) - percentage = progress[:percentage].to_i - part = progress[:part] - total_parts = progress[:total_parts] - - # Calculate speed and ETA - elapsed = Time.now - start_time - speed_mbps = uploaded_mb / elapsed - remaining_mb = total_mb - uploaded_mb - eta_seconds = remaining_mb / speed_mbps if speed_mbps.positive? - - # Create progress bar - bar_length = 40 - filled = (bar_length * percentage / 100).to_i - bar = ('█' * filled) + ('░' * (bar_length - filled)) - - # Display progress - print "\r#{bar} #{percentage}% | " - print "#{uploaded_mb}/#{total_mb} MB | " - print "Part #{part}/#{total_parts} | " - print "Speed: #{speed_mbps.round(2)} MB/s" - print " | ETA: #{eta_seconds.to_i}s" if eta_seconds - $stdout.flush - end - - file.close + start_time = Time.now + result = File.open(file_path, 'rb') do |file| + Uploadcare::Uploader.upload(object: file, store: true) do |progress| + # Calculate progress metrics + uploaded_mb = (progress[:uploaded] / 1024.0 / 1024.0).round(2) + total_mb = (progress[:total] / 1024.0 / 1024.0).round(2) + percentage = progress[:percentage].to_i + part = progress[:part] + total_parts = progress[:total_parts] + + # Calculate speed and ETA + elapsed = Time.now - start_time + speed_mbps = uploaded_mb / elapsed + remaining_mb = total_mb - uploaded_mb + eta_seconds = remaining_mb / speed_mbps if speed_mbps.positive? + + # Create progress bar + bar_length = 40 + filled = (bar_length * percentage / 100).to_i + bar = ('█' * filled) + ('░' * (bar_length - filled)) + + # Display progress + print "\r#{bar} #{percentage}% | " + print "#{uploaded_mb}/#{total_mb} MB | " + print "Part #{part}/#{total_parts} | " + print "Speed: #{speed_mbps.round(2)} MB/s" + print " | ETA: #{eta_seconds.to_i}s" if eta_seconds + $stdout.flush + end + endlib/uploadcare/clients/file_client.rb-7-9 (1)
7-9:⚠️ Potential issue | 🟡 MinorInconsistent path formatting:
'files/'vs'/files/...'.Line 8 uses
'files/'(no leading slash), while other methods use paths with leading slashes (e.g.,'/files/#{uuid}/storage/'). This inconsistency could cause URL construction issues depending on howRestClienthandles path joining.🔧 Suggested fix for consistency
def list(params: {}, request_options: {}) - get(path: 'files/', params: params, headers: {}, request_options: request_options) + get(path: '/files/', params: params, headers: {}, request_options: request_options) endlib/uploadcare/configuration.rb-95-100 (1)
95-100:⚠️ Potential issue | 🟡 MinorLogger assignment overrides intentional
nilsetting.If a user explicitly passes
logger: nilto disable logging, the@logger ||= Logger.new($stdout)on line 99 will override it with a default logger. Consider checking if the option was explicitly provided.🛠️ Proposed fix
def initialize(**options) - DEFAULTS.merge(options).each do |attribute, value| + merged = DEFAULTS.merge(options) + merged.each do |attribute, value| send("#{attribute}=", value) end - `@logger` ||= Logger.new($stdout) + `@logger` = Logger.new($stdout) if `@logger.nil`? && !options.key?(:logger) endlib/uploadcare/resources/group.rb-42-43 (1)
42-43:⚠️ Potential issue | 🟡 MinorFix typo in TODO comments: "opeartion" → "operation".
Same typo as in
file_metadata.rb.Also applies to: 54-55
lib/uploadcare/resources/file_metadata.rb-17-18 (1)
17-18:⚠️ Potential issue | 🟡 MinorFix typo in TODO comments: "opeartion" → "operation".
Multiple TODO comments contain the typo "opeartion" instead of "operation".
✏️ Fix typos
- # TODO - Remove uuid if the opeartion is being perfomed on same file + # TODO - Remove uuid if the operation is being performed on same fileAlso applies to: 42-43, 52-53, 60-61
lib/uploadcare/resources/group.rb-92-99 (1)
92-99:⚠️ Potential issue | 🟡 Minor
loadcopies all instance variables including the client reference.When copying instance variables from the freshly loaded group, this also overwrites
@group_clientwith the client from the loaded instance. If the original instance was created with a different config, this could cause unexpected behavior.🛡️ Preserve original client reference
def load + original_client = `@group_client` group_with_info = self.class.info(group_id: id, config: `@config`) # Copy attributes from the loaded group group_with_info.instance_variables.each do |var| instance_variable_set(var, group_with_info.instance_variable_get(var)) end + `@group_client` = original_client self endlib/uploadcare/clients/upload_client.rb-742-748 (1)
742-748:⚠️ Potential issue | 🟡 MinorValidate metadata type instead of silently ignoring it.
Line 743 returns{}for non-hash metadata, which conflicts with the stricter validation goal and can hide input bugs for URL/multipart uploads.🔧 Suggested fix
- def generate_metadata_params(metadata = nil) - return {} if metadata.nil? || !metadata.is_a?(Hash) + def generate_metadata_params(metadata = nil) + return {} if metadata.nil? + raise ArgumentError, 'metadata must be a Hash' unless metadata.is_a?(Hash) metadata.each_with_object({}) do |(key, value), result| result["metadata[#{key}]"] = value.to_s end endREADME.md-123-131 (1)
123-131:⚠️ Potential issue | 🟡 MinorUpdate README examples to use keyword arguments for v5 APIs.
Ruby 3 keyword-argument separation means positional calls in the examples at lines 127, 969, 1007, 1015, 1024, 1040, 1052, 1087, and 1111 will raise
ArgumentErroragainst the current method signatures.All methods—
batch_store,batch_delete,local_copy,remote_copy,convert_document,convert_video, andlist—require keyword arguments:Suggested doc fixes
- result = Uploadcare::File.batch_store(uuids) + result = Uploadcare::File.batch_store(uuids: uuids) - batch_result = Uploadcare::File.batch_store(uuids) + batch_result = Uploadcare::File.batch_store(uuids: uuids) - result = Uploadcare::File.batch_delete(uuids) + result = Uploadcare::File.batch_delete(uuids: uuids) - file = Uploadcare::File.local_copy(source, store: true) + file = Uploadcare::File.local_copy(source: source, options: { store: true }) - file = Uploadcare::File.remote_copy(source_object, target, make_public: true) + file = Uploadcare::File.remote_copy(source: source_object, target: target, options: { make_public: true }) - `@converted_file` = `@file.convert_document`({ format: "png", page: "1" }, store: true) + `@converted_file` = `@file.convert_document`(params: { format: "png", page: "1" }, options: { store: true }) - `@converted_file` = `@file.convert_video`( - { + `@converted_file` = `@file.convert_video`( + params: { format: "ogg", quality: "best", cut: { start_time: "0:0:0.1", length: "end" }, size: { resize_mode: "change_ratio", width: "600", height: "400" }, thumb: { N: 1, number: 2 } - }, - store: true + }, + options: { store: true } ) - `@file_list` = Uploadcare::File.list(options) + `@file_list` = Uploadcare::File.list(options: options) - `@list` = Uploadcare::File.list(options) + `@list` = Uploadcare::File.list(options: options)
🧹 Nitpick comments (33)
api_examples/rest_api/post_files_local_copy.rb (1)
1-8: Consider addingfrozen_string_literalpragma for consistency.Other API example files in this PR (e.g.,
put_multipart_part.rb) include the# frozen_string_literal: truepragma. Adding it here would maintain consistency across example files.♻️ Suggested fix
+# frozen_string_literal: true + require_relative '../../lib/uploadcare' require 'dotenv/load'spec/TESTING.md (1)
85-97: Add language specifier to fenced code block.The directory structure code block lacks a language identifier. Adding one improves markdown rendering and satisfies linting rules.
📝 Suggested fix
-``` +```text spec/ ├── fixtures/ │ └── vcr_cassettes/ # VCR recorded cassettesapi_examples/upload_api/put_multipart_part.rb (1)
35-42: Consider checking for failure before calling.success.Line 40 calls
.successdirectly without first verifying the result isn't a failure. Ifmultipart_startfails, calling.successon a failed result may produce unexpected behavior. For consistency with the error handling on line 58, consider checking for failure first.♻️ Suggested fix
response = client.multipart_start( filename: filename, size: file_size, content_type: content_type, store: true - ).success + ) + raise response.error if response.failure? + + response = response.success upload_uuid = response['uuid'] presigned_urls = response['parts']lib/uploadcare/webhook_signature_verifier.rb (2)
9-11: Unnecessary variable assignment.
webhook_body_jsonis simply an alias forwebhook_bodyand adds no value. Consider usingwebhook_bodydirectly throughout the method.♻️ Proposed simplification
def self.valid?(webhook_body: nil, signing_secret: nil, x_uc_signature_header: nil) - webhook_body_json = webhook_body signing_secret ||= ENV.fetch('UC_SIGNING_SECRET', nil) - return false unless valid_parameters?(signing_secret, x_uc_signature_header, webhook_body_json) + return false unless valid_parameters?(signing_secret, x_uc_signature_header, webhook_body) - calculated_signature = calculate_signature(signing_secret, webhook_body_json) + calculated_signature = calculate_signature(signing_secret, webhook_body) # Use constant-time comparison to prevent timing attacks secure_compare?(calculated_signature, x_uc_signature_header) end
47-54: PreferOpenSSL.fixed_length_secure_compareover a custom implementation.The custom constant-time comparison logic is correct, but Ruby's built-in
OpenSSL.fixed_length_secure_compare(available since Ruby 2.5, fully supported in Ruby 3.3+) is more reliable and battle-tested for cryptographic operations.♻️ Proposed fix
def self.secure_compare?(first, second) return false unless first.bytesize == second.bytesize - left = first.unpack('C*') - res = 0 - second.each_byte { |byte| res |= byte ^ left.shift } - res.zero? + OpenSSL.fixed_length_secure_compare(first, second) endlib/uploadcare/result.rb (1)
38-44: Redundant accessor methods.The
successandfailureinstance methods simply return@valueand@errorrespectively, which are already exposed viaattr_reader :value, :error. Consider removing these duplicates unless the naming distinction is intentional for API clarity.api_examples/rest_api/delete_files_storage.rb (1)
6-7: Consider trimming and filtering UUIDs from ENV.ENV values often include spaces (e.g., "uuid1, uuid2"), which can yield invalid UUIDs. A small normalization helps avoid confusing API errors.
♻️ Suggested tweak
-uuids = ENV.fetch('UPLOADCARE_FILE_UUIDS', - '21975c81-7f57-4c7a-aef9-acfe28779f78,cbaf2d73-5169-4b2b-a543-496cf2813dff').split(',') +uuids = ENV.fetch('UPLOADCARE_FILE_UUIDS', + '21975c81-7f57-4c7a-aef9-acfe28779f78,cbaf2d73-5169-4b2b-a543-496cf2813dff') + .split(',') + .map(&:strip) + .reject(&:empty?)api_examples/upload_api/get_group_info.rb (1)
1-3: Load dotenv for .env parity with other examples.This script reads ENV but doesn’t load
.env, unlike most other updated examples. Adding dotenv keeps behavior consistent and avoids confusion for users following .env-based setup.🔧 Proposed change
require 'uploadcare' +require 'dotenv/load' Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')lib/uploadcare/param/upload/signature_generator.rb (1)
9-13: Add explicit validation forsecret_keyand signature lifetime.
Line 10-11 will fail with aTypeErrorifupload_signature_lifetimeis nil, andsecret_key.to_squietly generates invalid signatures if empty. Consider validating and raising a clear error.🔧 Proposed guard clauses
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 } endapi_examples/rest_api/post_files_remote_copy.rb (1)
1-14: Missingfrozen_string_literalpragma for consistency.Other example files in this PR (e.g.,
get_from_url_status.rb,group_creation.rb) include the# frozen_string_literal: truepragma. Consider adding it here for consistency across the codebase.Suggested fix
+# frozen_string_literal: true + # Copy a file to remote/custom storage # NOTE: Custom storage must be configured in your Uploadcare Dashboard first:api_examples/upload_api/get_from_url_status.rb (2)
7-9: Consider a meaningful default or clearer error for missing public key.Using
nilas the default means the script will silently proceed with a nil public key if the environment variable isn't set, likely causing a confusing error later. Consider either raising explicitly or providing a placeholder default like other examples.Suggested fix
Uploadcare.configure do |config| - config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', nil) + config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') # Raises KeyError if not set endAlternatively, match other examples:
config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')
17-19: Unchecked.successcall may fail unexpectedly if the upload fails.If the async upload request fails, calling
.successon the result may raise an exception or return unexpected data. For a robust example, consider checking if the result succeeded first.Suggested defensive pattern
client = Uploadcare::UploadClient.new result = client.upload_from_url(source_url: source_url, async: true) -token = result.success['token'] + +unless result.success? + puts "Upload failed: #{result.error}" + exit 1 +end + +token = result.value['token']Note: Adjust method names based on the actual Result wrapper API.
api_examples/rest_api/put_webhooks_id.rb (1)
13-13: Consider capturing and displaying the update result.Unlike other examples that print results (e.g.,
get_project.rb), this example doesn't display the webhook update response, making it harder to verify success.Suggested fix
-Uploadcare::Webhook.update(id: webhook_id, **options) +updated_webhook = Uploadcare::Webhook.update(id: webhook_id, **options) +puts updated_webhook.inspectlib/uploadcare/resources/project.rb (1)
7-11: Unused@project_clientinstance variable.The
@project_clientis instantiated in the constructor but never used. The class methodshowcreates its own client instance. If there are no planned instance methods that will use this client, consider removing it to avoid confusion.Suggested fix if not needed
def initialize(attributes = {}, config = Uploadcare.configuration) super - `@project_client` = Uploadcare::ProjectClient.new(config: config) assign_attributes(attributes) endIf instance methods are planned that will use this client, consider adding a comment or keeping it.
examples/group_creation.rb (1)
43-50: Assumption about response structure may be fragile.
response.values.firstassumes the upload response is a hash where the first value is the UUID. If the response structure changes or varies, this could fail silently or produce incorrect results. Consider adding a brief comment documenting the expected response format, or using a more explicit key if available.Suggested documentation
file_paths.each_with_index do |path, index| File.open(path, 'rb') do |file| response = upload_client.upload_file(file: file, store: true) + # Response format: { "<filename>" => "<uuid>" } uuid = response.values.first uuids << uuid puts " #{index + 1}. #{File.basename(path)} → #{uuid}" end endapi_examples/upload_api/post_from_url.rb (1)
17-18: Consider adding error handling for robustness in this example.Calling
.successdirectly assumes the API call will always succeed. For a more robust example, consider checking the result first or demonstrating error handling:result = client.upload_from_url(source_url: source_url, store: true) if result.success? # handle success else puts "Error: #{result.failure}" endThis would better demonstrate production-ready usage patterns to users following this example.
api_examples/rest_api/post_addons_remove_bg_execute.rb (1)
7-7: Consider adding output to show the result.Unlike other examples in this directory (e.g.,
delete_files_uuid_metadata_key.rb), this example doesn't output the result, making it less useful for demonstration purposes.📝 Proposed fix
-Uploadcare::Addons.remove_bg(uuid: uuid, params: { crop: true }) +puts Uploadcare::Addons.remove_bg(uuid: uuid, params: { crop: true }).rubocop.yml (1)
92-159: Consider enablingRSpec/VerifiedDoublesfor better test reliability.The extensive disabling of RSpec cops is understandable for adopting rubocop-rspec incrementally. However,
RSpec/VerifiedDoubles(line 149-150) helps catch interface mismatches between mocks and real objects. Consider enabling it in the future to improve test reliability.api_examples/upload_api/post_base.rb (1)
15-19: Make the fixture path resilient to the current working directory.Right now the example assumes it’s run from the repo root. Use a path relative to the script location to avoid “file not found” when run elsewhere.
Proposed change
-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) endlib/uploadcare/resources/video_converter.rb (2)
19-27: Consider returning aVideoConverterinstance for API consistency.The
convertclass method returns the raw unwrapped result, while other resource classes (e.g.,Addons) return instances of the resource class. This inconsistency could confuse users of the API.Additionally, the method doesn't validate that
paramscontains required keys (:uuid,:format,:quality), which could lead to malformed paths.♻️ Suggested improvement
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
29-32: Documentation type mismatch:@param token [Integer]but tokens are typically strings.The YARD doc states
tokenis anInteger, but conversion job tokens from the Uploadcare API are typically strings (UUIDs or similar). Consider updating the documentation to reflect the actual type.📝 Fix documentation
# 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 statuslib/uploadcare/configuration.rb (1)
66-68: ENV values in DEFAULTS are captured at class load time.The
DEFAULTShash is evaluated when the class is loaded, meaningENV.fetch('UPLOADCARE_PUBLIC_KEY', '')captures the environment value at that moment. If the environment variables change after the class loads, newConfigurationinstances will still use the originally captured values.This is likely acceptable for most use cases, but worth documenting or noting if dynamic ENV reloading is expected.
lib/uploadcare/clients/file_client.rb (1)
47-51: Unusual pattern usinginstance_method(:delete).bind(self).call.This workaround exists because
FileClient#deleteis overridden to acceptuuid:while the parent'sdeleteacceptspath:. Consider renaming the instance method todelete_fileor similar to avoid the need for this binding workaround, improving readability.♻️ Alternative approach
Rename
deletetodelete_fileto avoid shadowing the parent method:- def delete(uuid:, request_options: {}) - super(path: "/files/#{uuid}/storage/", params: {}, headers: {}, request_options: request_options) + def delete_file(uuid:, request_options: {}) + delete(path: "/files/#{uuid}/storage/", params: {}, headers: {}, request_options: request_options) endThen
batch_deletebecomes simpler: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) + delete(path: '/files/storage/', params: uuids, headers: {}, request_options: request_options) endlib/uploadcare/resources/webhook.rb (2)
7-9: Redundantinitializemethod.This initializer only calls
superwith no additional logic. Unless you plan to add Webhook-specific initialization later, it can be removed to rely on the inheritedBaseResource#initialize.♻️ Remove redundant initializer
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
14-19: Consider memoizingWebhookClientinstances per configuration.Unlike
Addonswhich memoizes clients viaaddons_client(config), eachWebhookmethod creates a newWebhookClientinstance. For consistency and potential performance benefits (connection reuse), consider using a similar memoization pattern.♻️ Add client memoization
+ class << self + private + + def webhook_client(config) + `@webhook_clients` ||= {} + `@webhook_clients`[config] ||= Uploadcare::WebhookClient.new(config: config) + end + end + 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) } endApply similar changes to
create,update, anddeletemethods.lib/uploadcare/resources/paginated_collection.rb (2)
91-91: TODO: Add#allmethod.I can help implement an
#allmethod that collects all resources across pages.Do you want me to generate an implementation that lazily or eagerly fetches all pages and returns a combined array of resources?
107-110: Handle edge case where URL has no query string.
URI.decode_www_form(uri.query.to_s)safely handles nil, but ifpage_urlis malformed (e.g., just a path without a query), this returns an empty hash. The subsequentclient.list({})call should still work, but consider whether this is the intended behavior when the API returns URLs without query parameters.lib/uploadcare/resources/uploader.rb (3)
7-10: Instance variable@uploader_clientis initialized but never used.The constructor initializes
@uploader_client, but all public methods are class methods that create their own client instances viauploader_client(config:). Either remove the instance variable or add instance methods that use it.♻️ Option 1: Remove unused instance initialization
- def initialize(attributes = {}, config = Uploadcare.configuration) - super - `@uploader_client` = Uploadcare::UploaderClient.new(config: config) - end + def initialize(attributes = {}, config = Uploadcare.configuration) + super + end
72-80: Inconsistent return type frommultipart_upload.The method returns either an
Uploadcare::Fileinstance or the raw response hash/value when the response doesn't contain auuidkey. This inconsistency may surprise callers.Consider documenting this behavior in the
@returnannotation or ensuring a consistent return type.
105-107: Class method creates a new client on every call.
uploader_clientinstantiates a newUploaderClienteach time it's called. If this method is called frequently in tight loops (e.g., batch operations), consider caching per config or documenting that callers should manage their own client instance for performance-sensitive scenarios.lib/uploadcare/param/upload/upload_params_generator.rb (1)
47-48: Potentially unreachable code path.Based on
SignatureGenerator.call(fromlib/uploadcare/param/upload/signature_generator.rb), it always returns a Hash{ signature: ..., expire: ... }. Theunless signature_data.is_a?(Hash)check appears defensive but may be unreachable under normal operation.If this is intentional defensive coding for potential future changes to SignatureGenerator, consider adding a comment explaining when this branch would execute.
lib/uploadcare/resources/group.rb (2)
83-90: Fragile URL parsing for ID extraction.The regex-based ID extraction assumes
cdn_urlalways follows the formathttps://ucarecdn.com/<id>/.... Consider usingURI.parsefor more robust parsing.♻️ More robust ID extraction
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{/.*}, '') - `@id` + # Extract ID from CDN URL path + uri = URI.parse(`@cdn_url`) + `@id` = uri.path.split('/').reject(&:empty?).first end
109-116: PreferInteger#timesover range iteration.Using
files_count.timesis more idiomatic Ruby for simple counting iterations.♻️ Simplify iteration
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 { |i| "#{cdn_url}nth/#{i}/" } end
| puts "UUID: #{result['uuid']}" | ||
| puts "Size: #{file_size_mb} MB" | ||
| puts "Time: #{elapsed.round(2)} seconds" | ||
| puts "Speed: #{(file_size_mb / elapsed).round(2)} MB/s" | ||
| puts "Threads: #{threads}" | ||
| puts 'Method: Multipart upload' | ||
| puts | ||
| puts "CDN URL: https://ucarecdn.com/#{result['uuid']}/" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the multipart_upload method definition
rg -n "def multipart_upload" -C 10 libRepository: uploadcare/uploadcare-ruby
Length of output: 3963
🏁 Script executed:
#!/bin/bash
# Read the complete multipart_upload method implementation
sed -n '306,400p' lib/uploadcare/clients/upload_client.rbRepository: uploadcare/uploadcare-ruby
Length of output: 4043
🏁 Script executed:
#!/bin/bash
# Find the Result class definition
rg -n "class Result" libRepository: uploadcare/uploadcare-ruby
Length of output: 111
🏁 Script executed:
#!/bin/bash
# Read the Result class implementation
cat -n lib/uploadcare/result.rbRepository: uploadcare/uploadcare-ruby
Length of output: 1340
🏁 Script executed:
#!/bin/bash
# Find the multipart_complete method
rg -n "def multipart_complete" -A 20 libRepository: uploadcare/uploadcare-ruby
Length of output: 1760
🏁 Script executed:
#!/bin/bash
# Check the example file to see how the result is actually used
cat -n examples/large_file_upload.rb | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 3746
🏁 Script executed:
#!/bin/bash
# Check if Result class has a [] method defined or responds to hash access
rg -n "\[\]|dig|fetch" lib/uploadcare/result.rbRepository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check what the post method returns
rg -n "def post" lib/uploadcare/clients/upload_client.rb -A 5Repository: uploadcare/uploadcare-ruby
Length of output: 276
🏁 Script executed:
#!/bin/bash
# Check what make_request returns
rg -n "def make_request" lib/uploadcare/clients/upload_client.rb -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 541
🏁 Script executed:
#!/bin/bash
# Check what handle_response returns
rg -n "def handle_response" lib/uploadcare/clients/upload_client.rb -A 15Repository: uploadcare/uploadcare-ruby
Length of output: 562
🏁 Script executed:
#!/bin/bash
# Check what parse_success_response returns
rg -n "def parse_success_response" lib/uploadcare/clients/upload_client.rb -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 514
🏁 Script executed:
#!/bin/bash
# Check if there are other examples or specs that show how multipart_upload result is used
rg -n "multipart_upload" --type rb -B 2 -A 5 | head -50Repository: uploadcare/uploadcare-ruby
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check if there are other examples or specs that show how multipart_upload result is used
rg -n "multipart_upload" -B 2 -A 5 | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 8571
🏁 Script executed:
#!/bin/bash
# Check the Uploader class to see if it wraps the UploadClient result
rg -n "def multipart_upload" lib/uploadcare/resources/uploader.rb -A 15Repository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Find files that contain multipart_upload method definition
rg -l "def multipart_upload" libRepository: uploadcare/uploadcare-ruby
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Search for Uploader class and multipart_upload
rg -n "multipart_upload" lib/uploadcare/resources/ -B 2 -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 2552
Unwrap the Result object before hash access.
multipart_upload returns a Result object wrapping the response hash, not the hash directly. Hash indexing with result['uuid'] will fail with NoMethodError. Either unwrap the result first or access the wrapped value via .success or .value!:
Recommended fix
puts 'Upload Details:'
puts '-' * 50
- puts "UUID: #{result['uuid']}"
+ response = result.is_a?(Uploadcare::Result) ? result.value! : result
+ puts "UUID: #{response['uuid']}"
puts "Size: #{file_size_mb} MB"
puts "Time: #{elapsed.round(2)} seconds"
puts "Speed: #{(file_size_mb / elapsed).round(2)} MB/s"
puts "Threads: #{threads}"
puts 'Method: Multipart upload'
puts
- puts "CDN URL: https://ucarecdn.com/#{result['uuid']}/"
+ puts "CDN URL: https://ucarecdn.com/#{response['uuid']}/"🤖 Prompt for AI Agents
In `@examples/large_file_upload.rb` around lines 86 - 93, The code treats result
from multipart_upload as a Hash but multipart_upload returns a Result object;
change the prints to first unwrap the Result (e.g., call result.success or
result.value!) into a response_hash and then index response_hash['uuid'] and
other keys (or use the unwrapped object's symbol/key access) so lines
referencing result['uuid'], result[...] etc. operate on the actual response Hash
rather than the Result wrapper.
| @default_headers = { | ||
| 'Accept' => 'application/vnd.uploadcare-v0.7+json', | ||
| 'Content-Type' => 'application/json', | ||
| 'User-Agent' => Uploadcare::Param::UserAgent.call(config: config) | ||
| } |
There was a problem hiding this comment.
Ensure Content-Type header matches the signed content_type.
Signature uses content_type, but headers always return the default application/json. This can break auth for non-JSON bodies.
🛠️ Suggested fix
- def headers(http_method, uri, body = '', content_type = 'application/json')
- return simple_auth_headers if `@config.auth_type` == 'Uploadcare.Simple'
+ def headers(http_method, uri, body = '', content_type = 'application/json')
+ return simple_auth_headers(content_type) if `@config.auth_type` == 'Uploadcare.Simple'
return `@default_headers` if `@config.secret_key.nil`? || `@config.secret_key.empty`?
validate_public_key
- secure_auth_headers(http_method, uri, body, content_type)
+ secure_auth_headers(http_method, uri, body, content_type)
end
private
- def simple_auth_headers
- `@default_headers.merge`({ 'Authorization' => "#{`@config.auth_type`} #{`@config.public_key`}:#{`@config.secret_key`}" })
+ def simple_auth_headers(content_type)
+ `@default_headers.merge`({
+ 'Content-Type' => content_type,
+ 'Authorization' => "#{`@config.auth_type`} #{`@config.public_key`}:#{`@config.secret_key`}"
+ })
end
@@
- def secure_auth_headers(http_method, uri, body, content_type)
+ def secure_auth_headers(http_method, uri, body, content_type)
date = Time.now.gmtime.strftime('%a, %d %b %Y %H:%M:%S GMT')
signature = generate_signature(http_method, uri, body, content_type, date)
auth_headers = { 'Authorization' => "Uploadcare #{`@config.public_key`}:#{signature}", 'Date' => date }
- `@default_headers.merge`(auth_headers)
+ `@default_headers.merge`({ 'Content-Type' => content_type }).merge(auth_headers)
endAlso applies to: 65-70
🤖 Prompt for AI Agents
In `@lib/uploadcare/authenticator.rb` around lines 30 - 34, Default headers
hardcode 'Content-Type' to 'application/json' causing signature mismatch when a
different content_type is signed; update the authenticator to set 'Content-Type'
from the signed/content_type parameter (use the provided content_type when
present, otherwise fall back to 'application/json') wherever headers are built
(e.g., the `@default_headers` initialization and the headers-building logic around
lines 65-70), ensuring the header value used in the HTTP request matches the
value used to compute the signature.
| def headers(http_method, uri, body = '', content_type = 'application/json') | ||
| return simple_auth_headers if @config.auth_type == 'Uploadcare.Simple' | ||
| return @default_headers if @config.secret_key.nil? || @config.secret_key.empty? | ||
|
|
There was a problem hiding this comment.
Fail fast when secret_key is missing in secure auth.
Silently returning default headers can lead to confusing 401s and unauthenticated calls. Raise an auth error instead.
🛠️ Suggested fix
- return `@default_headers` if `@config.secret_key.nil`? || `@config.secret_key.empty`?
+ if `@config.secret_key.nil`? || `@config.secret_key.empty`?
+ raise Uploadcare::Exception::AuthError, 'Secret Key is blank.'
+ end🤖 Prompt for AI Agents
In `@lib/uploadcare/authenticator.rb` around lines 45 - 48, In
headers(http_method, uri, body = '', content_type = 'application/json') in
authenticator.rb, don't silently return `@default_headers` when `@config.auth_type`
is not 'Uploadcare.Simple' but `@config.secret_key` is nil/empty; instead raise an
authentication error (e.g., raise AuthenticationError.new("missing secret_key
for secure auth") or a RuntimeError/ArgumentError if AuthenticationError doesn't
exist) so callers fail fast; update the check around `@config.secret_key` to raise
with a clear message and ensure any tests or callers expecting headers are
adjusted accordingly.
| def convert_document(paths:, options: {}, request_options: {}) | ||
| body = { | ||
| paths: paths, | ||
| store: options[:store] ? '1' : '0', | ||
| save_in_group: options[:save_in_group] ? '1' : '0' | ||
| } | ||
|
|
||
| post(path: '/convert/document/', params: body, headers: {}, request_options: request_options) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's verify the exact code in the repository
cat -n lib/uploadcare/clients/document_converter_client.rb | sed -n '18,30p'Repository: uploadcare/uploadcare-ruby
Length of output: 651
🏁 Script executed:
# Let's also check if there are any similar patterns or existing normalization functions
rg "normalize.*option|bool.*option" --type rbRepository: uploadcare/uploadcare-ruby
Length of output: 96
🏁 Script executed:
# Check the context around the method to understand the class structure
cat -n lib/uploadcare/clients/document_converter_client.rbRepository: uploadcare/uploadcare-ruby
Length of output: 2022
🌐 Web query:
Uploadcare REST API document conversion store save_in_group parameter types v0.7.0
💡 Result:
In Uploadcare REST API v0.7, the save_in_group parameter for POST /convert/document/ is typed as a string, with an enum of:
"0","false","1","true"(default:"0") (uploadcare.com)
Behavior:
"1"/"true": for multi-page documents, converted pages are also saved as a file group."0"/"false": no file group is created (you’ll receive the multi-page result as an archive rather than separate files). (uploadcare.com)
Citations:
- 1: https://uploadcare.com/docs/api/rest/?utm_source=openai
- 2: https://uploadcare.com/docs/api/rest/?utm_source=openai
🏁 Script executed:
# Search for existing normalization patterns in the codebase
rg "normalize|bool" --type ruby -B 2 -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 674
🏁 Script executed:
# Check other client methods to see how they handle options
rg "def.*options:" lib/uploadcare/clients/ --type ruby -A 5Repository: uploadcare/uploadcare-ruby
Length of output: 31989
🏁 Script executed:
# Look for similar boolean parameter handling patterns
rg "options\[.*\].*\?" lib/uploadcare/clients/ --type ruby -B 1 -A 1Repository: uploadcare/uploadcare-ruby
Length of output: 662
Validate and normalize store / save_in_group before coercion.
String values '0' and 'false' are truthy in Ruby, so callers cannot explicitly disable these options using API-compliant values. When options[:store] = '0' is passed, it evaluates as true and sends '1' to the API—the opposite of intent.
Use a helper to explicitly handle boolean, string, and integer inputs:
Suggested approach
def convert_document(paths:, options: {}, request_options: {})
- body = {
- paths: paths,
- store: options[:store] ? '1' : '0',
- save_in_group: options[:save_in_group] ? '1' : '0'
- }
+ body = { paths: paths }
+ body[:store] = normalize_bool_option(options[:store]) if options.key?(:store)
+ body[:save_in_group] = normalize_bool_option(options[:save_in_group]) if options.key?(:save_in_group)
post(path: '/convert/document/', params: body, headers: {}, request_options: request_options)
end
+
+private
+
+def normalize_bool_option(value)
+ case value
+ when true, '1', 1 then '1'
+ when false, '0', 0, nil then '0'
+ else
+ raise ArgumentError, 'Expected boolean or "1"/"0"'
+ end
+end🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/document_converter_client.rb` around lines 18 - 25,
The convert_document method currently coerces options[:store] and
options[:save_in_group] using Ruby truthiness which treats strings like "0" and
"false" as true; add a small normalization helper (e.g., normalize_flag(value)
or normalize_bool_param) and use it inside convert_document to convert accepted
inputs (true/false, "1"/"0", "true"/"false", 1/0) into exact API strings "1" or
"0" before building body; update the body assignment for store and save_in_group
to call this helper and then call post(path: '/convert/document/', params: body,
...) as before.
| def process_chunk(file, links, link_index, &chunk_block) | ||
| offset = link_index * CHUNK_SIZE | ||
| chunk = ::File.read(file, CHUNK_SIZE, offset) | ||
| Uploadcare::Result.unwrap(put(links[link_index], chunk)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the upload method and how it calls process_chunk
rg -n 'def.*upload|process_chunk' lib/uploadcare/clients/multipart_uploader_client.rb -B2 -A10Repository: uploadcare/uploadcare-ruby
Length of output: 2885
🏁 Script executed:
#!/bin/bash
# Check the method calls to process_chunk to understand what file type is passed
rg -n 'process_chunk' lib/uploadcare/clients/multipart_uploader_client.rb -B5 -A2Repository: uploadcare/uploadcare-ruby
Length of output: 827
🏁 Script executed:
#!/bin/bash
# Search for where MultipartUploaderClient.upload is called in tests or other files
rg -n 'MultipartUploaderClient.*upload|\.upload\s*\(' --type=ruby | head -30Repository: uploadcare/uploadcare-ruby
Length of output: 3664
🏁 Script executed:
#!/bin/bash
# Let's also check the Ruby File.read documentation behavior
# First, find if there are any tests that demonstrate the usage
fd -e rb 'test|spec' --type f | xargs rg -l 'process_chunk|MultipartUploaderClient' | head -5Repository: uploadcare/uploadcare-ruby
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check the multipart uploader client test to see what's passed as file
cat -n spec/uploadcare/clients/multipart_uploader_client_spec.rb | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 4235
🏁 Script executed:
#!/bin/bash
# Also check if there's any indication of what file type is used in tests
rg -n 'let.*file|File\.open|\.new.*file' spec/uploadcare/clients/multipart_uploader_client_spec.rb -B2 -A2Repository: uploadcare/uploadcare-ruby
Length of output: 326
Use file.path with File.read() or read directly from the File object.
The ::File.read(file, CHUNK_SIZE, offset) call is incorrect—file is already an open File object (confirmed in tests: let(:file) { ::File.open(file_path, 'rb') }), but File.read() expects a file path string as the first argument. Either pass file.path:
chunk = ::File.read(file.path, CHUNK_SIZE, offset)or read directly from the file object:
file.seek(offset)
chunk = file.read(CHUNK_SIZE)🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/multipart_uploader_client.rb` around lines 61 - 64, In
process_chunk (multipart_uploader_client.rb) the call ::File.read(file,
CHUNK_SIZE, offset) treats file as a path but file is an open File object; fix
by either using file.path with ::File.read (e.g., ::File.read(file.path,
CHUNK_SIZE, offset)) or, preferably, read from the File object directly: seek to
offset on the File object and call file.read(CHUNK_SIZE) to obtain the chunk,
then pass that chunk to Uploadcare::Result.unwrap(put(links[link_index],
chunk)).
| def create_webhook(options: {}, request_options: {}) | ||
| payload = { | ||
| target_url: options[:target_url], | ||
| event: options[:event] || 'file.uploaded', | ||
| is_active: options[:is_active].nil? || options[:is_active] | ||
| } | ||
|
|
||
| # Add signing_secret if provided | ||
| payload.merge!({ signing_secret: options[:signing_secret] }.compact) | ||
|
|
||
| post(path: '/webhooks/', params: payload, headers: {}, request_options: request_options) |
There was a problem hiding this comment.
version option is documented but ignored in payload.
Passing options[:version] currently has no effect, so callers can’t set the webhook payload version.
🛠️ Suggested fix
- payload.merge!({ signing_secret: options[:signing_secret] }.compact)
+ payload.merge!({ signing_secret: options[:signing_secret], version: options[:version] }.compact)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def create_webhook(options: {}, request_options: {}) | |
| payload = { | |
| target_url: options[:target_url], | |
| event: options[:event] || 'file.uploaded', | |
| is_active: options[:is_active].nil? || options[:is_active] | |
| } | |
| # Add signing_secret if provided | |
| payload.merge!({ signing_secret: options[:signing_secret] }.compact) | |
| post(path: '/webhooks/', params: payload, headers: {}, request_options: request_options) | |
| def create_webhook(options: {}, request_options: {}) | |
| payload = { | |
| target_url: options[:target_url], | |
| event: options[:event] || 'file.uploaded', | |
| is_active: options[:is_active].nil? || options[:is_active] | |
| } | |
| # Add signing_secret if provided | |
| payload.merge!({ signing_secret: options[:signing_secret], version: options[:version] }.compact) | |
| post(path: '/webhooks/', params: payload, headers: {}, request_options: request_options) |
🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/webhook_client.rb` around lines 20 - 30, The
create_webhook method builds a payload but ignores options[:version]; update
create_webhook to include the version when provided by merging { version:
options[:version] } (using compact to drop nil) into payload (similar to the
signing_secret merge) so callers can set the webhook payload version before the
post call.
| def custom_cname | ||
| CnameGenerator.generate_cname | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check CnameGenerator implementation to verify how it handles config scoping
ast-grep --pattern 'def generate_cname($$$)'
rg -n "def generate_cname" --type ruby
rg -n "def cdn_base_postfix" --type rubyRepository: uploadcare/uploadcare-ruby
Length of output: 195
🏁 Script executed:
cat -n lib/uploadcare/cname_generator.rbRepository: uploadcare/uploadcare-ruby
Length of output: 1788
🏁 Script executed:
cat -n lib/uploadcare/configuration.rb | head -150Repository: uploadcare/uploadcare-ruby
Length of output: 5986
🏁 Script executed:
# Check how Uploadcare.configuration is set up and if multiple instances are expected
rg -n "Uploadcare\.configuration" --type ruby -A 2 -B 2 | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 8899
🏁 Script executed:
# Check the Uploadcare module setup to see how configuration is managed
rg -n "def self.configuration" --type ruby -A 5 -B 2Repository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
# Check the main Uploadcare module to understand configuration setup
rg -n "module Uploadcare" --type ruby -A 20 | head -80Repository: uploadcare/uploadcare-ruby
Length of output: 5993
🏁 Script executed:
# Find the main Uploadcare module definition file
find . -name "*.rb" -type f -exec grep -l "^module Uploadcare$" {} \; | grep -v spec | head -5Repository: uploadcare/uploadcare-ruby
Length of output: 259
🏁 Script executed:
cat -n lib/uploadcare.rb | head -50Repository: uploadcare/uploadcare-ruby
Length of output: 905
🏁 Script executed:
# Check CnameGenerator class methods to see if they support configuration parameters
cat -n lib/uploadcare/cname_generator.rbRepository: uploadcare/uploadcare-ruby
Length of output: 1788
Pass the configuration context to CnameGenerator.generate_cname.
The custom_cname instance method calls CnameGenerator.generate_cname without passing context, causing it to always use the global Uploadcare.configuration instead of the instance's public key. This breaks cache scoping when multiple Configuration instances with different public keys exist. CnameGenerator should accept and use a configuration or public_key parameter rather than hardcoding access to the global configuration.
🤖 Prompt for AI Agents
In `@lib/uploadcare/configuration.rb` around lines 104 - 106, The custom_cname
method currently calls CnameGenerator.generate_cname with no context causing it
to read global Uploadcare.configuration; change custom_cname to pass the
instance configuration (or its public_key) into CnameGenerator.generate_cname
and update CnameGenerator.generate_cname to accept and use that
configuration/public_key parameter instead of referencing
Uploadcare.configuration directly so cache scoping uses the Configuration
instance's public key.
| def self.batch_store(uuids:, config: Uploadcare.configuration, request_options: {}) | ||
| file_client = Uploadcare::FileClient.new(config: config) | ||
| response = Uploadcare::Result.unwrap(file_client.batch_store(uuids: uuids, request_options: request_options)) | ||
|
|
||
| BatchFileResult.new( | ||
| status: response[:status], | ||
| result: response[:result], | ||
| problems: response[:problems] || {}, | ||
| config: config | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the file structure and the specific lines mentioned
fd "file.rb" lib/uploadcare/resources/ -x cat -n {}Repository: uploadcare/uploadcare-ruby
Length of output: 13265
🏁 Script executed:
#!/bin/bash
# Also check the FileClient to understand batch_store implementation
fd "file_client.rb" lib/uploadcare/clients/ -x head -200 {}Repository: uploadcare/uploadcare-ruby
Length of output: 4185
🏁 Script executed:
#!/bin/bash
# Find Result class and unwrap method
fd "result.rb" lib/uploadcare/ -x cat -n {}Repository: uploadcare/uploadcare-ruby
Length of output: 3171
🏁 Script executed:
#!/bin/bash
# Also check RestClient to see what format responses are in
fd "rest_client.rb" lib/uploadcare/ -x cat -n {}Repository: uploadcare/uploadcare-ruby
Length of output: 7761
🏁 Script executed:
#!/bin/bash
# And check BatchFileResult to see what keys it expects
fd "batch_file_result.rb" lib/uploadcare/ -x cat -n {}Repository: uploadcare/uploadcare-ruby
Length of output: 1900
Use string keys to access batch response fields, or normalize them.
The API response from file_client.batch_store() and file_client.batch_delete() is parsed by Faraday's JSON middleware, which returns a hash with string keys. Lines 125–126 and 142–143 incorrectly access these with symbol keys (response[:status], response[:result]), which will return nil. This breaks BatchFileResult initialization and is inconsistent with other handlers like line 49 that correctly use string keys.
Fix by normalizing the response keys:
Suggested fix
BatchFileResult.new(
status: response[:status],
result: response[:result],
problems: response[:problems] || {},
config: config
)Replace with:
normalized = response.transform_keys(&:to_s)
BatchFileResult.new(
status: normalized['status'],
result: normalized['result'],
problems: normalized['problems'] || {},
config: config
)🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/file.rb` around lines 120 - 129, The batch_store
response is being accessed with symbol keys (response[:status],
response[:result]) but Faraday returns string-keyed hashes; update
Uploadcare::File.batch_store to normalize response keys (e.g., call
transform_keys(&:to_s) or otherwise convert to string keys) and then pass
normalized['status'], normalized['result'], and normalized['problems'] || {}
into BatchFileResult; apply the same normalization approach in the analogous
batch_delete handling so BatchFileResult is initialized with the correct
string-keyed fields.
| def uuid | ||
| return @uuid if @uuid | ||
|
|
||
| # If initialized from URL, extract UUID | ||
| if @url | ||
| extracted_uuid = @url.gsub('https://ucarecdn.com/', '') | ||
| extracted_uuid.gsub(%r{/.*}, '') | ||
| else | ||
| @uuid | ||
| end | ||
| end | ||
|
|
||
| def cdn_url | ||
| return @url if @url | ||
|
|
||
| # Generate CDN URL from uuid and config | ||
| "#{@config.cdn_base.call}#{uuid}/" | ||
| end |
There was a problem hiding this comment.
Fix UUID extraction for non-default CDN or API URLs.
Line 196 hardcodes ucarecdn.com; API URLs or custom CDN/subdomains will yield an incorrect UUID (e.g., https:) and break cdn_url.
🔧 Suggested fix (extract UUID by pattern)
def uuid
return `@uuid` if `@uuid`
- # If initialized from URL, extract UUID
- if `@url`
- extracted_uuid = `@url.gsub`('https://ucarecdn.com/', '')
- extracted_uuid.gsub(%r{/.*}, '')
- else
- `@uuid`
- end
+ source = `@url` || `@original_file_url`
+ 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}/]
end🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/file.rb` around lines 191 - 208, The uuid method
currently hardcodes 'https://ucarecdn.com/' causing wrong extraction for custom
CDN/API hosts; update uuid to extract the UUID from `@url` using a host-agnostic
path regex (e.g. capture the first path segment after the domain via something
like %r{\Ahttps?://[^/]+/([^/?#]+)}), assign and return that value (set `@uuid` =
captured_value) so cdn_url (which calls uuid and `@config.cdn_base.call`) gets a
correct identifier regardless of CDN domain or subdomain; ensure the regex
handles optional trailing slash and query fragments.
| body: | ||
| encoding: ASCII-8BIT | ||
| string: '{"type":"token","token":"79926202-9c03-4bb7-ac9c-458fbb407c08"}' | ||
| recorded_at: Sun, 08 Feb 2026 06:32:41 GMT | ||
| - request: | ||
| method: get | ||
| uri: https://upload.uploadcare.com/from_url/status/?token=79926202-9c03-4bb7-ac9c-458fbb407c08 | ||
| body: |
There was a problem hiding this comment.
Scrub upload token from the cassette.
The async upload token is stored verbatim in the response body and reused in the status URL. Even if short‑lived, it’s safer to filter/sanitize tokens in fixtures to avoid accidental leakage.
🔒 Suggested scrub in the cassette
- string: '{"type":"token","token":"79926202-9c03-4bb7-ac9c-458fbb407c08"}'
+ string: '{"type":"token","token":"<uploadcare_upload_token>"}'
...
- uri: https://upload.uploadcare.com/from_url/status/?token=79926202-9c03-4bb7-ac9c-458fbb407c08
+ uri: https://upload.uploadcare.com/from_url/status/?token=<uploadcare_upload_token>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body: | |
| encoding: ASCII-8BIT | |
| string: '{"type":"token","token":"79926202-9c03-4bb7-ac9c-458fbb407c08"}' | |
| recorded_at: Sun, 08 Feb 2026 06:32:41 GMT | |
| - request: | |
| method: get | |
| uri: https://upload.uploadcare.com/from_url/status/?token=79926202-9c03-4bb7-ac9c-458fbb407c08 | |
| body: | |
| body: | |
| encoding: ASCII-8BIT | |
| string: '{"type":"token","token":"<uploadcare_upload_token>"}' | |
| recorded_at: Sun, 08 Feb 2026 06:32:41 GMT | |
| - request: | |
| method: get | |
| uri: https://upload.uploadcare.com/from_url/status/?token=<uploadcare_upload_token> | |
| body: |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 56-56: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 60-60: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In
`@spec/fixtures/vcr_cassettes/Upload_API_Integration/Complete_Upload_Workflow/when_uploading_from_URL/handles_async_URL_upload_with_status_checking.yml`
around lines 54 - 61, Replace the literal async upload token present in the
cassette body and status URL with a scrubbed placeholder: find the JSON body
string containing "type":"token","token":"79926202-9c03-4bb7-ac9c-458fbb407c08"
and replace the token value with a neutral marker (e.g.
"<SCRUBBED_UPLOAD_TOKEN>"); also update the recorded request/uri that contains
"/from_url/status/?token=79926202-9c03-4bb7-ac9c-458fbb407c08" to use the same
placeholder so both the response body and the status check URL are sanitized
consistently. Ensure you only replace the token value (the characters after
"token": and after "token=") and leave surrounding JSON and URL structure
intact.
1 participant
Summary
Testing
Release Plan