v5.0: Complete gem rewrite with modern architecture #186
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRewrites library to v5.0.0: replaces ApiStruct entities with resource/client architecture, centralizes configuration, adds Rest/Upload clients, authenticator, error/throttle handling, multipart & URL upload flows, webhook verifier, result wrapper, many examples and fixtures; removes legacy clients/entities/params and many old VCR cassettes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant U as Uploadcare::Uploader (resource)
participant UC as Uploadcare::UploadClient
participant S as Upload API
Dev->>U: upload(object:, store:, ...)
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 token
UC->>S: GET /from_url/status?token=...
S-->>UC: 200 {status: success, uuid}
end
UC-->>U: result (uuid, attrs)
U-->>Dev: Uploadcare::File instance
else large file (multipart)
U->>UC: multipart_start(filename,size,content_type)
UC->>S: POST /multipart/start
S-->>UC: 200 {uuid, parts[links]}
loop per 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 instance
end
sequenceDiagram
autonumber
participant Dev as Developer
participant A as Uploadcare::Addons (resource)
participant AC as Uploadcare::AddonsClient
participant R as REST API
Dev->>A: aws_rekognition_detect_labels(uuid:)
A->>AC: aws_rekognition_detect_labels(uuid:, ...)
AC->>R: POST /addons/aws_rekognition_detect_labels/execute/
R-->>AC: 200 {request_id}
AC-->>A: result (request_id)
Dev->>A: aws_rekognition_detect_labels_status(request_id:)
A->>AC: aws_rekognition_detect_labels_status(request_id:)
AC->>R: GET /addons/.../status/?request_id=...
R-->>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
lib/uploadcare/authenticator.rb
Outdated
| @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.
lib/uploadcare/authenticator.rb
Outdated
| 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.
lib/uploadcare/configuration.rb
Outdated
| 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.
lib/uploadcare/resources/file.rb
Outdated
| 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.
lib/uploadcare/resources/file.rb
Outdated
| 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.
vipulnsward
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@api_examples/upload_api/post_base.rb`:
- Around line 27-32: The payload is a Hash with string keys, so replace the
incorrect destructuring of payload.first with direct key lookups: read
payload['uuid'] into file_uuid and payload['original_filename'] into file_name
(use the same variable names payload and result.success already present); update
the prints to use those variables. Locate this in the upload handling around
result.success / payload in post_base.rb where payload.first is used and change
it to the direct-key access approach (see post_from_url.rb for the correct
pattern).
In `@lib/uploadcare/clients/file_client.rb`:
- Around line 7-9: In FileClient#list, update the path argument passed to get
from 'files/' to '/files/' so it matches the leading-slash convention used by
other methods (e.g., the get calls in methods that reference
'/files/#{uuid}/storage/' and '/files/storage/'); locate the list method and
modify the get(path: 'files/', ...) call to get(path: '/files/', ...).
In `@lib/uploadcare/param/upload/signature_generator.rb`:
- Around line 18-19: Replace the MD5-based signing with HMAC-SHA256: in the
SignatureGenerator (or wherever signature is computed) replace
Digest::MD5.hexdigest(secret_key + expires_at.to_s) with an HMAC-SHA256 using
the secret_key and expires_at.to_s (e.g. via OpenSSL::HMAC.hexdigest('sha256',
secret_key, expires_at.to_s)); also ensure OpenSSL is required at the top so the
OpenSSL::HMAC call is available, and keep the result assigned to the existing
signature variable.
In `@lib/uploadcare/resources/group.rb`:
- Around line 109-113: The file_cdn_urls method can raise when files_count is
nil; update file_cdn_urls to guard files_count (in the Group class) before
mapping: if files_count is nil or zero return an empty array (or coerce
files_count to an integer via files_count.to_i) and only build
"#{cdn_url}nth/#{file_index}/" when cdn_url and a non-nil files_count exist;
reference the file_cdn_urls method and the files_count and cdn_url symbols when
making the change.
In `@lib/uploadcare/resources/paginated_collection.rb`:
- Around line 122-124: The fetch_response method is calling client.list with a
positional arg causing ArgumentError; change the call in
PaginatedCollection#fetch_response to pass the params as a keyword (e.g.,
client.list(params: params)) and include request_options if applicable so it
matches FileClient#list(params: {}, request_options: {}); update any similar
callers in PaginatedCollection to use keyword args.
- Around line 126-138: The paginated response is still a Result wrapper from
client.list and build_paginated_collection is treating it like a hash (accessing
response['results']), breaking pagination; update the fetch_response
implementation used by PaginatedCollection to unwrap the Result before use by
calling Uploadcare::Result.unwrap(client.list(params)) (mirror the pattern in
File.list and Group.list) so build_paginated_collection receives a plain hash
and can safely access 'results', 'next', 'previous', 'per_page', and 'total'.
🧹 Nitpick comments (12)
api_examples/rest_api/post_addons_remove_bg_execute.rb (1)
3-6: Fail fast when required env vars are missing.Default placeholders for keys/UUID can mask misconfiguration and lead to confusing auth errors. Prefer explicit
ENV.fetchwith no defaults (or a clear error message).✅ Suggested change
-Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY') -Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY') +Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') +Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY') -uuid = ENV.fetch('UPLOADCARE_FILE_UUID', '1bac376c-aa7e-4356-861b-dd2657b5bfd2') +uuid = ENV.fetch('UPLOADCARE_FILE_UUID')lib/uploadcare/resources/project.rb (1)
7-9: Redundant initializer can be removed.The initializer simply delegates to
superwith the same signature. IfBaseResource#initializeaccepts the same parameters, this method can be omitted entirely—Ruby will automatically call the parent's initializer.♻️ Suggested simplification
module Uploadcare class Project < BaseResource attr_accessor :name, :pub_key, :autostore_enabled, :collaborators - - def initialize(attributes = {}, config = Uploadcare.configuration) - super - end # Fetches project informationapi_examples/upload_api/post_from_url.rb (1)
7-9: Inconsistent environment variable handling across examples.This file uses
ENV.fetch('UPLOADCARE_PUBLIC_KEY')without a default, which raisesKeyErrorif the variable is missing. However,get_from_url_status.rbusesENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY')with a fallback, andpost_base.rbusesENV['...'] || 'your_public_key'.Consider using a consistent approach across all example files for better developer experience.
♻️ Option A: Add fallback for consistency
Uploadcare.configure do |config| - config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') + config.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'your_public_key') endapi_examples/upload_api/post_base.rb (1)
3-5: Missingdotenv/loadrequire.Other upload API examples (
get_from_url_status.rb,post_from_url.rb) includerequire 'dotenv/load'to load environment variables from.envfiles. This example is missing it, which could cause confusion when the environment variables aren't picked up.♻️ Add dotenv require for consistency
require_relative '../../lib/uploadcare' +require 'dotenv/load' # Configure Uploadcarelib/uploadcare/resources/paginated_collection.rb (1)
91-101: Consider adding documentation for theallmethod.The
allmethod collects resources across all pages but lacks YARD documentation unlike other public methods. Also, for very large collections, this could consume significant memory. Consider adding a warning in the docs.📝 Add documentation
+ # Fetch all resources across all pages + # + # `@note` This loads all pages into memory. Use with caution for large collections. + # `@return` [Array] Array of all resource objects def all collection = self items = []lib/uploadcare/resources/webhook.rb (1)
78-82: Client cache may cause memory retention if many configs are used.The
@webhook_clientshash usesconfigobjects as keys. If the application creates many distinct configuration objects, they'll all be retained in this class-level cache indefinitely. This is likely fine for typical usage (single global config), but worth noting.For most applications this is not an issue, but if dynamic config creation becomes a pattern, consider using
ObjectSpace::WeakMapor limiting cache size.api_examples/rest_api/put_webhooks_id.rb (1)
1-4: Fail fast on missing credentials in example scripts.Using default placeholders can lead to confusing 401s against the API. Consider requiring env vars so users see an immediate, clear error.
💡 Suggested change
-Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY') -Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY') +Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') +Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY')api_examples/upload_api/get_group_info.rb (1)
2-7: Prefer explicit configuration and group ID for clearer example behavior.Default placeholders can trigger confusing API errors (401/404). Consider requiring env values (or exiting with a usage message).
💡 Suggested change
-Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY', 'YOUR_PUBLIC_KEY') -Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY', 'YOUR_SECRET_KEY') +Uploadcare.configuration.public_key = ENV.fetch('UPLOADCARE_PUBLIC_KEY') +Uploadcare.configuration.secret_key = ENV.fetch('UPLOADCARE_SECRET_KEY') -uuid = ENV.fetch('UPLOADCARE_GROUP_UUID', '0d712319-b970-4602-850c-bae1ced521a6~1') +uuid = ENV.fetch('UPLOADCARE_GROUP_UUID')examples/group_creation.rb (1)
19-31: Validate files withFile.file?to avoid directories.
File.exist?allows directories;File.file?is stricter and avoids confusing upload errors.💡 Suggested change
- unless File.exist?(path) + unless File.file?(path) puts "Error: File not found: #{path}" exit 1 endlib/uploadcare/resources/video_converter.rb (1)
19-32: Consider normalizingparamskeys for user input.Allowing string keys makes the API friendlier without changing behavior.
💡 Suggested change
- 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] + def self.convert(params:, options: {}, config: Uploadcare.configuration, request_options: {}) + params = params.transform_keys(&:to_sym) + 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]lib/uploadcare/resources/uploader.rb (1)
33-38: Reusecreate_basic_fileto avoid duplication.This keeps file creation consistent with
upload_files.💡 Suggested change
- Uploadcare::File.new({ uuid: uuid, original_filename: file_name }, config) + create_basic_file(uuid: uuid, file_name: file_name, config: config)lib/uploadcare/resources/group.rb (1)
40-60: Resolve TODO: allow instance methods to default to the current group ID.This removes extra boilerplate and matches expected OO usage. I can help implement if desired.
| payload = result.success | ||
| file_name, file_uuid = payload.first | ||
|
|
||
| puts 'File uploaded successfully!' | ||
| puts "UUID: #{file_uuid}" | ||
| puts "Original filename: #{file_name}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the upload_file method
ast-grep --pattern $'def upload_file($$$)
$$$
end'Repository: uploadcare/uploadcare-ruby
Length of output: 779
🏁 Script executed:
# Also search for the UploadClient class and related response handling
rg 'class UploadClient|def upload_file' -B 2 -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 2094
🏁 Script executed:
# Search for where result.success is used and how payload is constructed
rg 'result\.success|payload' -B 3 -A 3 api_examples/upload_api/post_base.rbRepository: uploadcare/uploadcare-ruby
Length of output: 207
🏁 Script executed:
# Check the file structure to understand the codebase layout
git ls-files api_examples/upload_api/Repository: uploadcare/uploadcare-ruby
Length of output: 870
🏁 Script executed:
# Find the Result class and examine its structure
rg 'class Result|def success|def unwrap' lib/uploadcare/ -B 2 -A 5Repository: uploadcare/uploadcare-ruby
Length of output: 1560
🏁 Script executed:
# Look at the post method implementation in UploadClient or its parent RestClient
rg 'def post' lib/uploadcare/clients/ -B 2 -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 2128
🏁 Script executed:
# Check tests or examples to see actual API response structure
rg 'upload_file.*result|payload|first' api_examples/ -B 3 -A 3Repository: uploadcare/uploadcare-ruby
Length of output: 4599
🏁 Script executed:
# Look for the API response structure - check what post returns
rg 'def post' lib/uploadcare/ -A 15 | head -60Repository: uploadcare/uploadcare-ruby
Length of output: 2675
Fix incorrect payload destructuring.
The payload returned by upload_file is a Hash with string keys ('uuid', 'original_filename', etc.), not an array. Calling payload.first destructures the first key-value pair, assigning the key to file_name and value to file_uuid—which is incorrect. Access the values directly:
file_uuid = payload['uuid']
file_name = payload['original_filename']See api_examples/upload_api/post_from_url.rb for the correct approach.
🤖 Prompt for AI Agents
In `@api_examples/upload_api/post_base.rb` around lines 27 - 32, The payload is a
Hash with string keys, so replace the incorrect destructuring of payload.first
with direct key lookups: read payload['uuid'] into file_uuid and
payload['original_filename'] into file_name (use the same variable names payload
and result.success already present); update the prints to use those variables.
Locate this in the upload handling around result.success / payload in
post_base.rb where payload.first is used and change it to the direct-key access
approach (see post_from_url.rb for the correct pattern).
| def list(params: {}, request_options: {}) | ||
| get(path: 'files/', params: params, headers: {}, request_options: request_options) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual content of file_client.rb to see all path usages
cat -n lib/uploadcare/clients/file_client.rbRepository: uploadcare/uploadcare-ruby
Length of output: 4583
🏁 Script executed:
# Check the parent RestClient class to understand how paths are handled
find . -name "*.rb" -type f | xargs grep -l "class RestClient" | head -5Repository: uploadcare/uploadcare-ruby
Length of output: 109
🏁 Script executed:
# Search for all path: usages across clients to see the pattern
rg "path:\s*['\"]" lib/uploadcare/clients/ -A 1 -B 1Repository: uploadcare/uploadcare-ruby
Length of output: 14685
🏁 Script executed:
cat -n lib/uploadcare/clients/rest_client.rb | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 4549
🏁 Script executed:
# Check if there are any tests that verify the correct path format
find . -name "*test*" -o -name "*spec*" | grep -i file_clientRepository: uploadcare/uploadcare-ruby
Length of output: 115
🏁 Script executed:
# Search for base_url or URL construction in RestClient
rg -n "base.*url|join.*path|request.*path" lib/uploadcare/clients/rest_client.rb -A 2 -B 2Repository: uploadcare/uploadcare-ruby
Length of output: 2340
🏁 Script executed:
cat -n lib/uploadcare/clients/rest_client.rb | sed -n '128,160p'Repository: uploadcare/uploadcare-ruby
Length of output: 1504
🏁 Script executed:
# Check the file_client_spec.rb to see what expected behavior is
cat -n ./spec/uploadcare/clients/file_client_spec.rb | head -80Repository: uploadcare/uploadcare-ruby
Length of output: 2752
🏁 Script executed:
# Check the rest_api_root configuration
rg -n "rest_api_root" lib/uploadcare/ -A 2 -B 2Repository: uploadcare/uploadcare-ruby
Length of output: 1928
🏁 Script executed:
# Let's see if there are any other tests for file_client to confirm path expectations
grep -n "path.*files" ./spec/uploadcare/clients/file_client_spec.rbRepository: uploadcare/uploadcare-ruby
Length of output: 410
🏁 Script executed:
# Let's verify the exact test expectation - check the full_url construction
sed -n '9,13p' ./spec/uploadcare/clients/file_client_spec.rbRepository: uploadcare/uploadcare-ruby
Length of output: 245
🏁 Script executed:
# Check how Faraday (the HTTP client) handles path joining
# Let's look for any path normalization or joining logic
rg -n "build_uri|Addressable" lib/uploadcare/clients/rest_client.rb -A 3 -B 1Repository: uploadcare/uploadcare-ruby
Length of output: 485
Add leading slash for consistency with other methods in this class.
The list method uses 'files/' while all other FileClient methods use paths with a leading slash (e.g., '/files/#{uuid}/storage/', '/files/storage/'). For consistency within the class, add the leading slash.
🔧 Suggested fix
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)
end📝 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 list(params: {}, request_options: {}) | |
| get(path: 'files/', params: params, headers: {}, request_options: request_options) | |
| end | |
| def list(params: {}, request_options: {}) | |
| get(path: '/files/', params: params, headers: {}, request_options: request_options) | |
| end |
🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/file_client.rb` around lines 7 - 9, In
FileClient#list, update the path argument passed to get from 'files/' to
'/files/' so it matches the leading-slash convention used by other methods
(e.g., the get calls in methods that reference '/files/#{uuid}/storage/' and
'/files/storage/'); locate the list method and modify the get(path: 'files/',
...) call to get(path: '/files/', ...).
| to_sign = secret_key + expires_at.to_s | ||
| signature = Digest::MD5.hexdigest(to_sign) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Uploadcare upload API signed uploads signature format MD5
💡 Result:
For Upload API “signed uploads” (the signature form field you send with /base/, multipart start, from_url, etc.) the signature is not MD5.
Signature format (signed uploads):
expire: a Unix timestamp (seconds) (sent as a string/message when signing). (uploadcare.com)signature: HMAC-SHA256 of theexpirevalue, using your project Secret key, typically sent as a hex digest. (uploadcare.com)- You then upload with
UPLOADCARE_PUB_KEY,signature, andexpire. (uploadcare.com)
If you’re thinking of MD5, that’s from REST API request signing, where the signature string includes the hex MD5 of the request body, and then it’s signed with HMAC-SHA1. (uploadcare.com)
Citations:
- 1: https://uploadcare.com/docs/security/secure-uploads/
- 2: https://uploadcare.com/docs/security/secure-uploads/
- 3: https://uploadcare.com/docs/security/secure-uploads/
- 4: https://uploadcare.com/docs/api/rest/?utm_source=openai
🏁 Script executed:
# Let's also check the context of the file to understand the implementation better
cd lib/uploadcare/param/upload || true
if [ -f signature_generator.rb ]; then
echo "=== signature_generator.rb content ==="
cat -n signature_generator.rb
fiRepository: uploadcare/uploadcare-ruby
Length of output: 1079
🏁 Script executed:
# Check if there are any other references to signature formats or algorithms in the codebase
rg -i "hmac|sha256|md5|signature" --type rb -A 2 -B 2 | head -100Repository: uploadcare/uploadcare-ruby
Length of output: 96
Replace MD5-based signing with HMAC-SHA256 per Uploadcare's upload API specification.
The Uploadcare Upload API requires HMAC-SHA256 for signed uploads, not MD5. According to their official documentation, the signature must be an HMAC-SHA256 digest of the expire value using your Secret key.
The current implementation incorrectly uses Digest::MD5.hexdigest(secret_key + expires_at.to_s). Update it to use HMAC-SHA256:
Suggested fix
signature = OpenSSL::HMAC.hexdigest('sha256', secret_key, expires_at.to_s)(Note: MD5 is used in the REST API for signing request bodies combined with HMAC-SHA1, which is a different endpoint.)
🤖 Prompt for AI Agents
In `@lib/uploadcare/param/upload/signature_generator.rb` around lines 18 - 19,
Replace the MD5-based signing with HMAC-SHA256: in the SignatureGenerator (or
wherever signature is computed) replace Digest::MD5.hexdigest(secret_key +
expires_at.to_s) with an HMAC-SHA256 using the secret_key and expires_at.to_s
(e.g. via OpenSSL::HMAC.hexdigest('sha256', secret_key, expires_at.to_s)); also
ensure OpenSSL is required at the top so the OpenSSL::HMAC call is available,
and keep the result assigned to the existing signature variable.
lib/uploadcare/resources/group.rb
Outdated
| # 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 | ||
| files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" } | ||
| end |
There was a problem hiding this comment.
Guard files_count when building file_cdn_urls.
If files_count is nil (e.g., unloaded group), this raises.
💡 Suggested change
- def file_cdn_urls
- files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" }
- end
+ def file_cdn_urls
+ return [] unless files_count
+ files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" }
+ end📝 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.
| # 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 | |
| files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" } | |
| end | |
| # 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 | |
| return [] unless files_count | |
| files_count.times.map { |file_index| "#{cdn_url}nth/#{file_index}/" } | |
| end |
🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/group.rb` around lines 109 - 113, The file_cdn_urls
method can raise when files_count is nil; update file_cdn_urls to guard
files_count (in the Group class) before mapping: if files_count is nil or zero
return an empty array (or coerce files_count to an integer via files_count.to_i)
and only build "#{cdn_url}nth/#{file_index}/" when cdn_url and a non-nil
files_count exist; reference the file_cdn_urls method and the files_count and
cdn_url symbols when making the change.
| def fetch_response(params) | ||
| client.list(params) | ||
| end |
There was a problem hiding this comment.
Incorrect method call: positional argument instead of keyword argument.
The fetch_response method passes params as a positional argument, but FileClient#list (and likely other clients) expects keyword arguments: list(params: {}, request_options: {}). This will cause ArgumentError when navigating to next/previous pages.
🐛 Fix the method call
def fetch_response(params)
- client.list(params)
+ client.list(params: params)
end🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/paginated_collection.rb` around lines 122 - 124, The
fetch_response method is calling client.list with a positional arg causing
ArgumentError; change the call in PaginatedCollection#fetch_response to pass the
params as a keyword (e.g., client.list(params: params)) and include
request_options if applicable so it matches FileClient#list(params: {},
request_options: {}); update any similar callers in PaginatedCollection to use
keyword args.
| def build_paginated_collection(response) | ||
| new_resources = build_resources(response['results']) | ||
|
|
||
| self.class.new( | ||
| resources: new_resources, | ||
| next_page: response['next'], | ||
| previous_page: response['previous'], | ||
| per_page: response['per_page'], | ||
| total: response['total'], | ||
| client: client, | ||
| resource_class: resource_class | ||
| ) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, execute the requested verification - check what client.list returns
rg -n "def list" lib/uploadcare/clients/ -A 5Repository: uploadcare/uploadcare-ruby
Length of output: 1564
🏁 Script executed:
#!/bin/bash
# Check how build_paginated_collection is called
rg -n "build_paginated_collection" lib/uploadcare/ -B 3 -A 3Repository: uploadcare/uploadcare-ruby
Length of output: 1160
🏁 Script executed:
#!/bin/bash
# Check for Result wrapper pattern in the codebase
rg -n "class Result|\.ok\?|\.error" lib/uploadcare/ | head -30Repository: uploadcare/uploadcare-ruby
Length of output: 370
🏁 Script executed:
#!/bin/bash
# Look at the overall structure of paginated_collection.rb
cat -n lib/uploadcare/resources/paginated_collection.rb | head -50Repository: uploadcare/uploadcare-ruby
Length of output: 1861
🏁 Script executed:
#!/bin/bash
# Check the get method implementation in the client base
rg -n "def get" lib/uploadcare/clients/ -A 10 | head -50Repository: uploadcare/uploadcare-ruby
Length of output: 2771
🏁 Script executed:
#!/bin/bash
# Look at the full fetch_response method
cat -n lib/uploadcare/resources/paginated_collection.rb | sed -n '120,130p'Repository: uploadcare/uploadcare-ruby
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Examine the Result class
cat -n lib/uploadcare/result.rbRepository: uploadcare/uploadcare-ruby
Length of output: 1340
🏁 Script executed:
#!/antml/bash
# Check how responses are handled elsewhere - look for Result unwrapping patterns
rg -n "\.ok\?" lib/uploadcare/ -B 2 -A 2 | head -40Repository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if there's error handling or Result checking in similar methods
rg -n "response\[" lib/uploadcare/resources/ -B 2 -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 6208
🏁 Script executed:
#!/bin/bash
# Check where build_paginated_collection is called
rg -n "build_paginated_collection\|fetch_response\|\.next_page\|\.previous_page" lib/uploadcare/resources/paginated_collection.rb -B 3 -A 3Repository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check who calls PaginatedCollection.new and with what
rg -n "PaginatedCollection.new" lib/uploadcare/resources/ -B 5 -A 10Repository: uploadcare/uploadcare-ruby
Length of output: 2245
🏁 Script executed:
#!/bin/bash
# Check the next_page and previous_page methods in paginated_collection.rb
cat -n lib/uploadcare/resources/paginated_collection.rb | sed -n '100,140p'Repository: uploadcare/uploadcare-ruby
Length of output: 1408
🏁 Script executed:
#!/an/bash
# Check if build_paginated_collection is actually called anywhere
rg -n "build_paginated_collection" lib/uploadcare/Repository: uploadcare/uploadcare-ruby
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Check the next_page and previous_page methods
cat -n lib/uploadcare/resources/paginated_collection.rb | sed -n '70,115p'Repository: uploadcare/uploadcare-ruby
Length of output: 1581
🏁 Script executed:
#!/bin/bash
# Let me verify the Result.unwrap is not being called in fetch_response path
cat -n lib/uploadcare/resources/paginated_collection.rb | sed -n '100,140p'Repository: uploadcare/uploadcare-ruby
Length of output: 1408
🏁 Script executed:
#!/bin/bash
# Confirm that client in paginated_collection is the same type as in file.rb/group.rb
rg -n "@client|client =" lib/uploadcare/resources/paginated_collection.rb -B 2 -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 238
🏁 Script executed:
#!/an/bash
# Verify that file_client.list returns Result
rg -n "class FileClient|class GroupClient" lib/uploadcare/clients/ -A 3Repository: uploadcare/uploadcare-ruby
Length of output: 842
🏁 Script executed:
#!/bin/bash
# Let me check what RestClient looks like to confirm get() wraps in Result
cat -n lib/uploadcare/clients/rest_client.rb | sed -n '80,100p'Repository: uploadcare/uploadcare-ruby
Length of output: 1032
🏁 Script executed:
#!/bin/bash
# Test accessing [] on Result object to confirm it fails
cat -n lib/uploadcare/result.rbRepository: uploadcare/uploadcare-ruby
Length of output: 1340
🏁 Script executed:
#!/bin/bash
# Check RestClient.request method to see if it wraps in Result
cat -n lib/uploadcare/clients/rest_client.rb | sed -n '1,80p'Repository: uploadcare/uploadcare-ruby
Length of output: 3594
🏁 Script executed:
#!/bin/bash
# Double check - does FileClient or GroupClient override get()?
rg -n "def get" lib/uploadcare/clients/file_client.rb lib/uploadcare/clients/group_client.rbRepository: uploadcare/uploadcare-ruby
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check the request method in RestClient
cat -n lib/uploadcare/clients/rest_client.rb | sed -n '101,120p'Repository: uploadcare/uploadcare-ruby
Length of output: 1015
🏁 Script executed:
#!/bin/bash
# Verify which client base classes use - check RestClient and any intermediate classes
rg -n "def request|class RestClient|class FileClient|class GroupClient" lib/uploadcare/clients/ -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 1233
Add Result.unwrap() to handle the Result wrapper returned by client.list().
The fetch_response method returns a Result object (from client.list), but build_paginated_collection treats it as a plain hash when accessing response['results'] and other keys. This breaks pagination when next_page or previous_page is called.
Match the pattern used in File.list and Group.list by unwrapping the Result before hash access:
def fetch_response(params)
Uploadcare::Result.unwrap(client.list(params))
end🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/paginated_collection.rb` around lines 126 - 138, The
paginated response is still a Result wrapper from client.list and
build_paginated_collection is treating it like a hash (accessing
response['results']), breaking pagination; update the fetch_response
implementation used by PaginatedCollection to unwrap the Result before use by
calling Uploadcare::Result.unwrap(client.list(params)) (mirror the pattern in
File.list and Group.list) so build_paginated_collection receives a plain hash
and can safely access 'results', 'next', 'previous', 'per_page', and 'total'.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@lib/uploadcare/clients/group_client.rb`:
- Around line 17-19: The info method builds a path by interpolating uuid
directly; URL-encode the uuid before interpolation (e.g., use
URI.encode_www_form_component) so characters like '~' are percent-encoded, and
apply the same change to the other GroupClient methods that construct paths with
"/groups/#{uuid}/" (the other occurrences in this class) to ensure all UUIDs are
safely encoded in URLs.
In `@lib/uploadcare/clients/rest_client.rb`:
- Around line 45-49: The request body is being JSON-encoded twice because the
Faraday builder uses conn.request :json and prepare_body_or_params currently
calls params.to_json; to fix, stop manual serialization in
prepare_body_or_params (remove the params.to_json) and return/pass the Ruby Hash
(or IO/stream) so Faraday's :json middleware handles encoding, ensuring
prepare_body_or_params (and any callers) supply raw params when building
requests.
- Around line 61-70: The issue is that 429 responses are converted into a
RequestError after Faraday raises, bypassing handle_throttling in make_request;
update ErrorHandler#raise_status_error to detect HTTP 429 and raise
Uploadcare::Exception::ThrottleError (instead of RequestError) so
handle_throttling can catch and retry; ensure raise_status_error uses the
response status (429) when deciding between ThrottleError and other errors and
preserves original error details when raising the ThrottleError.
In `@lib/uploadcare/configuration.rb`:
- Around line 94-99: The Configuration class uses the standard Logger constant
in initialize (calling Logger.new($stdout)) but never requires the stdlib; add
an explicit require 'logger' at the top of the file (before the Configuration
class or the initialize method) so that Logger is defined when Configuration.new
is called; ensure the change references the initialize method and the Logger
constant to locate where the dependency is needed.
In `@lib/uploadcare/error_handler.rb`:
- Around line 54-61: catch_upload_errors currently calls JSON.parse on
response[:body] and will raise JSON::ParserError for invalid JSON; wrap the
JSON.parse call in a begin/rescue that catches JSON::ParserError (like
extract_error_message does) and treat the parsed_response as nil (or fall back
to the raw body) so the method does not bubble a parser exception; ensure the
method still returns early unless a valid parsed_response is a Hash with an
'error' key before raising Uploadcare::Exception::RequestError.
In `@lib/uploadcare/result.rb`:
- Around line 63-67: The value! method currently does raise `@error` which will
cause a TypeError if `@error` is not an Exception; update value! (in class Result
/ method value!) to detect the error object's type and raise appropriately: if
`@error` is an Exception or a String raise it directly, otherwise wrap it in a
RuntimeError (or RuntimeError.new(`@error.to_s`) / include `@error.inspect`) so
calling Result.failure(any_object).value! always raises a valid Exception
without causing TypeError.
In `@lib/uploadcare/signed_url_generators/akamai_generator.rb`:
- Around line 30-32: The valid? method currently calls uuid.match(UUID_REGEX)
which will raise NoMethodError for nil/non-string inputs; update the valid?
method to first validate the input type/value (e.g., explicitly raise
ArgumentError if uuid is nil or not a String) and only then apply UUID_REGEX
matching (using the UUID_REGEX constant) so callers receive the expected
ArgumentError flow instead of a NoMethodError.
🧹 Nitpick comments (5)
lib/uploadcare/result.rb (1)
41-43:unwrapraises on failure Results, which may be unexpected.The method is documented to "Unwrap a Result or return the value as-is" but it delegates to
value!which raises on failure. Consider whether this should silently returnnilor the error, or if the raising behavior should be documented.lib/uploadcare/clients/upload_group_client.rb (1)
62-63: Preferrespond_to?overmethods.include?for UUID detection.
This is more idiomatic and avoids building the full method list.♻️ Proposed refactor
- uuids.map { |file| file.methods.include?(:uuid) ? file.uuid : file } + uuids.map { |file| file.respond_to?(:uuid) ? file.uuid : file }lib/uploadcare/resources/paginated_collection.rb (1)
90-103: Consider adding a guard for empty resources inallmethod.If
resourcesisnil(e.g., from a malformed response),concatwill raiseTypeError. A defensive guard would improve robustness.💡 Suggested improvement
def all collection = self items = [] while collection - items.concat(collection.resources) + items.concat(collection.resources || []) collection = collection.next_page end items endlib/uploadcare/resources/group.rb (1)
106-113: Consider selective attribute copying inload.Copying all instance variables (including
@group_clientand@config) from the freshly loaded group overwrites the original client/config references. This is likely fine since they should be equivalent, but explicitly copying onlyATTRIBUTESwould be cleaner and avoid potential subtle bugs.♻️ Alternative approach
def load 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 + ATTRIBUTES.each do |attr| + instance_variable_set("@#{attr}", group_with_info.public_send(attr)) + end self endlib/uploadcare/clients/uploader_client.rb (1)
150-159: Consider including status in error message for debugging.When raising
RequestErrorfor error status, including the full response or status could aid debugging.💡 Optional enhancement
def handle_polling_response(response) case response['status'] when 'error' - raise Uploadcare::Exception::RequestError, response['error'] + raise Uploadcare::Exception::RequestError, "Upload failed: #{response['error']}" when 'progress', 'waiting', 'unknown'
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 1000-1005: The README shows incorrect usage of Group instance
methods: when you instantiate a Group with Group.new(uuid: "Group UUID"), do not
pass uuid again to the instance methods; update the examples so calls use the
instance directly (e.g., remove the uuid: param from `@group.info`(...) and
`@group.delete`(...)) and ensure class-level examples (if any) use
Uploadcare::Group.info(uuid: ...) or Uploadcare::Group.delete(uuid: ...) to
distinguish them from instance usage; search for Group.new, `@group.info`,
`@group.delete` in the doc to correct any other copy-paste instances.
- Line 822: Change the heading levels for the peer sections so they match the
other top-level subsections: replace the "### Deleting Files" and "### Copying
Files" headings with "## Deleting Files" and "## Copying Files" respectively
(i.e., update the headings that read "Deleting Files" and "Copying Files" to
level 2 so they are peers of "## Storing Files" and follow markdownlint MD001).
- Line 835: The README example calls Uploadcare::File.batch_delete with a
positional arg but the method is defined as def self.batch_delete(uuids:, ...);
update the example to use the keyword argument form by calling
Uploadcare::File.batch_delete(uuids: uuids) (mirroring batch_store) so the call
matches the batch_delete signature.
- Line 664: The call to multipart_upload_part is using positional args but the
method signature requires keyword args; update the call to
upload_client.multipart_upload_part to pass presigned_url: presigned_url and
part_data: part_data (i.e., use keyword arguments) so it matches the method def
multipart_upload_part(presigned_url:, part_data:, **options) and the example on
line 664.
- Line 854: The README example calls Uploadcare::File.remote_copy with
positional args and an invalid make_public parameter; update the example to call
remote_copy with keyword arguments (source: ..., target: ...) and pass
make_public inside an options hash (options: { make_public: true }) so the call
becomes Uploadcare::File.remote_copy(source: source_object, target: target,
options: { make_public: true }); ensure the referenced symbols are remote_copy,
Uploadcare::File, source, target, and options in the updated example.
- Line 844: The README example calls Uploadcare::File.local_copy with a
positional argument and misplaced store flag; update the example to call
Uploadcare::File.local_copy using the keyword parameter names to match the
method signature (def self.local_copy(source:, options: {}, ...)), i.e. pass
source: as a keyword and wrap store: inside the options: hash so the call uses
source: '...', options: { store: true } when referencing
Uploadcare::File.local_copy.
🧹 Nitpick comments (3)
README.md (3)
458-462: Inconsistent block parameter naming.The block parameter is named
optionshere but is calledprogressin other multipart upload examples (lines 420, 426, 512, 561, 608, 619). For consistency and clarity, consider standardizing onprogressthroughout the documentation since the hash contains progress-related data.📝 Suggested consistency fix
-Uploadcare::Uploader.multipart_upload(file: file, store: true) do |options| - progress += (100.0 / options[:links_count]) +Uploadcare::Uploader.multipart_upload(file: file, store: true) do |progress| + progress_percent += (100.0 / progress[:links_count]) puts "PROGRESS = #{progress}" end
81-86: Add blank lines around the table.The method renames table should be surrounded by blank lines for proper Markdown formatting.
📝 Proposed fix
#### Method Renames + | Old Method (v4.x) | New Method (v5.0) | |-------------------|-------------------| | `Addons.check_aws_rekognition_detect_labels_status` | `Addons.aws_rekognition_detect_labels_status` | | `Addons.check_aws_rekognition_detect_moderation_labels_status` | `Addons.aws_rekognition_detect_moderation_labels_status` | | `Addons.check_uc_clamav_virus_scan_status` | `Addons.uc_clamav_virus_scan_status` | | `Addons.check_remove_bg_status` | `Addons.remove_bg_status` | +As per static analysis, markdownlint rule MD058 requires tables to be surrounded by blank lines.
144-144: Use fenced code blocks instead of indented.The indented code block should be converted to a fenced code block for consistency with the rest of the document.
📝 Proposed fix
And then execute: - - $ bundle + +```bash +$ bundle +```As per static analysis, markdownlint rule MD046 requires consistent code block style (fenced).
| part_data = file.read(part_size) | ||
| break if part_data.nil? || part_data.empty? | ||
|
|
||
| upload_client.multipart_upload_part(presigned_url: url, part_data: part_data) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for multipart_upload_part method definition
echo "=== Searching for method definition ==="
rg -n "def multipart_upload_part" --type=rb -A5
echo -e "\n=== Searching for all usages ==="
rg -n "multipart_upload_part" --type=rb -B2 -A2
echo -e "\n=== Looking for the README.md lines in question ==="
sed -n '395,400p; 664p' README.mdRepository: uploadcare/uploadcare-ruby
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Search without type flag and look for library structure
echo "=== Finding Ruby files ==="
fd "\.rb$" lib/ --type f 2>/dev/null | head -20
echo -e "\n=== Search for multipart_upload_part in lib directory ==="
rg "multipart_upload_part" lib/ -B3 -A8 2>/dev/null || echo "Not found in lib/"
echo -e "\n=== Check for gem or module structure ==="
find . -name "*.rb" -type f 2>/dev/null | grep -E "(upload|client)" | head -10
echo -e "\n=== Look at repository structure ==="
ls -la lib/ 2>/dev/null || ls -laRepository: uploadcare/uploadcare-ruby
Length of output: 4911
Fix line 397 to use keyword arguments for multipart_upload_part.
Line 397 uses positional arguments: upload_client.multipart_upload_part(presigned_url, part_data), but the actual method signature requires keyword arguments: def multipart_upload_part(presigned_url:, part_data:, **options). Change to: upload_client.multipart_upload_part(presigned_url: presigned_url, part_data: part_data) to match line 664 and the library's implementation.
🤖 Prompt for AI Agents
In `@README.md` at line 664, The call to multipart_upload_part is using positional
args but the method signature requires keyword args; update the call to
upload_client.multipart_upload_part to pass presigned_url: presigned_url and
part_data: part_data (i.e., use keyword arguments) so it matches the method def
multipart_upload_part(presigned_url:, part_data:, **options) and the example on
line 664.
README.md
Outdated
| end | ||
| ``` | ||
|
|
||
| ### Deleting Files |
There was a problem hiding this comment.
Fix heading level hierarchy.
The heading "### Deleting Files" appears after "### Storing Files" but they are peer sections. Both should be ## (level 2) headings, not ### (level 3), since they are major subsections under "File management."
📝 Proposed fix
-### Deleting Files
+## Deleting FilesApply the same fix to line 839 ("### Copying Files").
As per static analysis, markdownlint rule MD001 requires heading levels to increment by one level at a time.
📝 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.
| ### Deleting Files | |
| ## Deleting Files |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 822-822: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In `@README.md` at line 822, Change the heading levels for the peer sections so
they match the other top-level subsections: replace the "### Deleting Files" and
"### Copying Files" headings with "## Deleting Files" and "## Copying Files"
respectively (i.e., update the headings that read "Deleting Files" and "Copying
Files" to level 2 so they are peers of "## Storing Files" and follow
markdownlint MD001).
README.md
Outdated
| # Batch delete multiple files | ||
| ```ruby | ||
| uuids = ['FILE_UUID_1', 'FILE_UUID_2'] | ||
| result = Uploadcare::File.batch_delete(uuids) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check batch_delete method signature definition
ast-grep --pattern $'def batch_delete($$$) { $$$ }'
# Also search with simpler pattern
ast-grep --pattern $'def batch_delete'
# Find batch_delete usages
rg -n "batch_delete" --type=rb -C 3Repository: uploadcare/uploadcare-ruby
Length of output: 406
🏁 Script executed:
#!/bin/bash
# Find Uploadcare::File class and batch_delete
rg -n "def batch_delete|def batch_store" -A 1 --type ruby
# Look for Uploadcare::File class
rg -n "class.*File|def.*batch_" --type ruby | head -30
# Check if there's a wrapper or delegation
rg -n "Uploadcare::File" --type ruby | grep -E "(batch_delete|batch_store)" | head -10Repository: uploadcare/uploadcare-ruby
Length of output: 2260
Fix batch_delete call to use keyword argument.
The method signature requires a keyword argument: def self.batch_delete(uuids:, ...). Update the example from Uploadcare::File.batch_delete(uuids) to Uploadcare::File.batch_delete(uuids: uuids) to match the actual method definition and align with batch_store.
🤖 Prompt for AI Agents
In `@README.md` at line 835, The README example calls
Uploadcare::File.batch_delete with a positional arg but the method is defined as
def self.batch_delete(uuids:, ...); update the example to use the keyword
argument form by calling Uploadcare::File.batch_delete(uuids: uuids) (mirroring
batch_store) so the call matches the batch_delete signature.
README.md
Outdated
| # Copy a file to local storage | ||
| ```ruby | ||
| source = '1bac376c-aa7e-4356-861b-dd2657b5bfd2' | ||
| file = Uploadcare::File.local_copy(source, store: true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the local_copy method definition
ast-grep --pattern $'def local_copy($$$)
$$$
end'Repository: uploadcare/uploadcare-ruby
Length of output: 995
🏁 Script executed:
# Search for local_copy usage and definition
rg -n "local_copy" --type=rb -C2 -A5Repository: uploadcare/uploadcare-ruby
Length of output: 96
🏁 Script executed:
# Look for v5.0 related changes or API design documentation
git log --oneline --all | grep -i "v5\|keyword\|api" | head -20Repository: uploadcare/uploadcare-ruby
Length of output: 1024
🏁 Script executed:
# Check README.md around line 844 for context
sed -n '835,850p' README.mdRepository: uploadcare/uploadcare-ruby
Length of output: 391
🏁 Script executed:
# Check if there's a class method wrapper for local_copy
rg "def.*local_copy" --type=ruby -B2 -A8Repository: uploadcare/uploadcare-ruby
Length of output: 2903
🏁 Script executed:
# Check the broader README context around line 844
sed -n '800,860p' README.mdRepository: uploadcare/uploadcare-ruby
Length of output: 1435
🏁 Script executed:
# Look for any class method definitions on Uploadcare::File
rg "class.*File" --type=ruby -A20 | head -80Repository: uploadcare/uploadcare-ruby
Length of output: 6238
Fix README example: local_copy requires keyword arguments.
The example calls Uploadcare::File.local_copy(source, store: true) with positional source, but the method signature is def self.local_copy(source:, options: {}, ...), requiring source: as a keyword argument. Additionally, store: true must be wrapped in the options: parameter. The correct usage is:
file = Uploadcare::File.local_copy(source: '1bac376c-aa7e-4356-861b-dd2657b5bfd2', options: { store: true })🤖 Prompt for AI Agents
In `@README.md` at line 844, The README example calls Uploadcare::File.local_copy
with a positional argument and misplaced store flag; update the example to call
Uploadcare::File.local_copy using the keyword parameter names to match the
method signature (def self.local_copy(source:, options: {}, ...)), i.e. pass
source: as a keyword and wrap store: inside the options: hash so the call uses
source: '...', options: { store: true } when referencing
Uploadcare::File.local_copy.
README.md
Outdated
| ```ruby | ||
| source_object = '1bac376c-aa7e-4356-861b-dd2657b5bfd2' | ||
| target = 'custom_storage_connected_to_the_project' | ||
| file = Uploadcare::File.remote_copy(source_object, target, make_public: true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for remote_copy method definition
# First, let's find all Ruby files and search for the remote_copy method definition
rg "def remote_copy" --type=rb -A 10
# Also search for remote_copy usages to understand the current usage patterns
rg "remote_copy" --type=rb -B 2 -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Search for remote_copy method definition in Ruby files
# Find all .rb files and search for remote_copy definition
rg "def remote_copy" --glob="*.rb" -A 10
# Also search for remote_copy usages
rg "remote_copy" --glob="*.rb" -B 2 -A 2Repository: uploadcare/uploadcare-ruby
Length of output: 7086
🏁 Script executed:
#!/bin/bash
# Check README.md around line 854
sed -n '850,860p' README.mdRepository: uploadcare/uploadcare-ruby
Length of output: 479
Fix the remote_copy example in README.md - uses invalid argument syntax.
The example on line 854 uses positional arguments where keyword arguments are required, and passes make_public directly instead of nesting it in the options hash. The correct syntax is:
Corrected example
source_object = '1bac376c-aa7e-4356-861b-dd2657b5bfd2'
target = 'custom_storage_connected_to_the_project'
file = Uploadcare::File.remote_copy(source: source_object, target: target, options: { make_public: true })
puts file
# => "https://my-storage.example.com/path/to/copied-file"🤖 Prompt for AI Agents
In `@README.md` at line 854, The README example calls Uploadcare::File.remote_copy
with positional args and an invalid make_public parameter; update the example to
call remote_copy with keyword arguments (source: ..., target: ...) and pass
make_public inside an options hash (options: { make_public: true }) so the call
becomes Uploadcare::File.remote_copy(source: source_object, target: target,
options: { make_public: true }); ensure the referenced symbols are remote_copy,
Uploadcare::File, source, target, and options in the updated example.
README.md
Outdated
| @group = Uploadcare::Group.new(uuid: "Group UUID") | ||
| @group.info(uuid: "Group UUID") | ||
|
|
||
| # group can be deleted by group ID. | ||
| Uploadcare::Group.delete(group.id) | ||
| @group = Uploadcare::Group.new(uuid: "Group UUID") | ||
| @group.delete(uuid: "Group UUID") |
There was a problem hiding this comment.
Incorrect instance method usage for Group.
Lines 1001 and 1005 show instance methods being called with a uuid parameter:
@group.info(uuid: "Group UUID")@group.delete(uuid: "Group UUID")
Since @group is already initialized with a UUID (line 1000), these instance methods should not require the uuid parameter again. This is likely a copy-paste error from class method examples.
📝 Proposed fix
# get a file group by its ID.
`@group` = Uploadcare::Group.new(uuid: "Group UUID")
-@group.info(uuid: "Group UUID")
+@group.info
# group can be deleted by group ID.
`@group` = Uploadcare::Group.new(uuid: "Group UUID")
-@group.delete(uuid: "Group UUID")
+@group.deleteAlternatively, if these are meant to be class method calls:
-@group = Uploadcare::Group.new(uuid: "Group UUID")
-@group.info(uuid: "Group UUID")
+Uploadcare::Group.info(uuid: "Group UUID")
-@group = Uploadcare::Group.new(uuid: "Group UUID")
-@group.delete(uuid: "Group UUID")
+Uploadcare::Group.delete(uuid: "Group UUID")📝 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.
| @group = Uploadcare::Group.new(uuid: "Group UUID") | |
| @group.info(uuid: "Group UUID") | |
| # group can be deleted by group ID. | |
| Uploadcare::Group.delete(group.id) | |
| @group = Uploadcare::Group.new(uuid: "Group UUID") | |
| @group.delete(uuid: "Group UUID") | |
| `@group` = Uploadcare::Group.new(uuid: "Group UUID") | |
| `@group.info` | |
| # group can be deleted by group ID. | |
| `@group` = Uploadcare::Group.new(uuid: "Group UUID") | |
| `@group.delete` |
🤖 Prompt for AI Agents
In `@README.md` around lines 1000 - 1005, The README shows incorrect usage of
Group instance methods: when you instantiate a Group with Group.new(uuid: "Group
UUID"), do not pass uuid again to the instance methods; update the examples so
calls use the instance directly (e.g., remove the uuid: param from
`@group.info`(...) and `@group.delete`(...)) and ensure class-level examples (if
any) use Uploadcare::Group.info(uuid: ...) or Uploadcare::Group.delete(uuid:
...) to distinguish them from instance usage; search for Group.new, `@group.info`,
`@group.delete` in the doc to correct any other copy-paste instances.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@lib/uploadcare/clients/rest_client.rb`:
- Around line 72-110: The YARD `@return` annotations for the HTTP verb helpers
post, get, put, and delete incorrectly state they return a Hash; update each
method's documentation to reflect that they return an Uploadcare::Result (the
wrapper returned by request). Edit the comments above def post, def get, def
put, and def delete to replace "@return [Hash] Parsed JSON response body" with
"@return [Uploadcare::Result] Result wrapper containing the parsed response" (or
similar wording) so callers see the correct return type.
In `@lib/uploadcare/clients/upload_group_client.rb`:
- Around line 12-15: The create_group method must validate the uuids input
before building the multipart body to avoid NoMethodError from parse_uuids and
to match UploadClient#create_group behavior; update create_group (or
group_body_hash) to assert that uuids is an Array and not empty (or raise a
clear ArgumentError) before calling group_body_hash/parse_uuids, and ensure any
nil/single-value inputs are normalized or rejected the same way
UploadClient#create_group does so downstream code will not call parse_uuids on
nil or non-array values.
In `@lib/uploadcare/configuration.rb`:
- Around line 109-118: The cdn_base method's lambda has inconsistent indentation
causing RuboCop Layout/IndentationWidth failures; open the cdn_base method and
reindent the block so the lambda body uses consistent two-space indentation: the
if, its branches (Uploadcare::CnameGenerator.cdn_base_postfix and
default_cdn_base) and the else keyword should be aligned as a normal Ruby block,
with the final end matching the lambda do; ensure the inner return expressions
are indented one level inside the lambda to satisfy RuboCop.
In `@lib/uploadcare/error_handler.rb`:
- Around line 46-58: The method raise_status_error is above the cyclomatic
complexity threshold due to the inlined 429 branch; extract the 429 handling
into a new helper (e.g., raise_throttle_error(response, message) or
parse_retry_timeout(response)) that reads headers (headers =
response.is_a?(Hash) ? response[:headers] : nil), computes retry_after/to_f with
the 10.0 fallback, and raises Uploadcare::Exception::ThrottleError.new(timeout,
message: message); then replace the 429 case in raise_status_error with a single
call to that helper (or use an early return) to reduce branching inside
raise_status_error.
In `@lib/uploadcare/resources/group.rb`:
- Around line 51-56: RuboCop flags missing blank line between method definitions
in the Group resource; add a single empty line before or after the def
delete(...) method (method name: delete in class Group, and surrounding methods
such as any prior or following def) so there is exactly one blank line
separating this method from the adjacent method definitions to satisfy
Layout/EmptyLineBetweenDefs.
In `@lib/uploadcare/signed_url_generators/akamai_generator.rb`:
- Around line 30-34: The UUID_REGEX constant is currently a String causing
valid?(uuid) to raise when calling uuid.match?; change UUID_REGEX to be a Regexp
(e.g., use /\A...\z/ to anchor start/end) and update the constant definition
accordingly so that AkamaiGenerator#valid? can call uuid.match?(UUID_REGEX)
without error; locate the UUID_REGEX constant near the
signed_url_generators/Akamai generator and ensure the pattern is properly
anchored and compiled as a Regexp.
🧹 Nitpick comments (1)
lib/uploadcare/resources/file_metadata.rb (1)
39-58: Keep the in-memory metadata cache in sync after update/delete.
@metadatais populated byindex, butupdate/deleteleave it stale, so[]/to_hcan return outdated data.♻️ Suggested cache sync
def update(key:, value:, uuid: nil, request_options: {}) - Uploadcare::Result.unwrap(`@file_metadata_client.update`(uuid: uuid || `@uuid`, key: key, value: value, - request_options: request_options)) + result = Uploadcare::Result.unwrap(`@file_metadata_client.update`(uuid: uuid || `@uuid`, key: key, value: value, + request_options: request_options)) + `@metadata`[key.to_s] = result + result end @@ def delete(uuid: nil, key:, request_options: {}) - Uploadcare::Result.unwrap(`@file_metadata_client.delete`(uuid: uuid || `@uuid`, key: key, - request_options: request_options)) + result = Uploadcare::Result.unwrap(`@file_metadata_client.delete`(uuid: uuid || `@uuid`, key: key, + request_options: request_options)) + `@metadata.delete`(key.to_s) + result end
| # Make a POST request | ||
| # | ||
| # @param path [String] API endpoint path | ||
| # @param params [Hash] Request body parameters | ||
| # @param headers [Hash] Additional request headers | ||
| # @return [Hash] Parsed JSON response body | ||
| def post(path:, params: {}, headers: {}, request_options: {}) | ||
| request(method: :post, path: path, params: params, headers: headers, request_options: request_options) | ||
| end | ||
|
|
||
| # Make a GET request | ||
| # | ||
| # @param path [String] API endpoint path | ||
| # @param params [Hash] Query parameters | ||
| # @param headers [Hash] Additional request headers | ||
| # @return [Hash] Parsed JSON response body | ||
| def get(path:, params: {}, headers: {}, request_options: {}) | ||
| request(method: :get, path: path, params: params, headers: headers, request_options: request_options) | ||
| end | ||
|
|
||
| # Make a PUT request | ||
| # | ||
| # @param path [String] API endpoint path | ||
| # @param params [Hash] Request body parameters | ||
| # @param headers [Hash] Additional request headers | ||
| # @return [Hash] Parsed JSON response body | ||
| def put(path:, params: {}, headers: {}, request_options: {}) | ||
| request(method: :put, path: path, params: params, headers: headers, request_options: request_options) | ||
| end | ||
|
|
||
| # Make a DELETE request | ||
| # | ||
| # @param path [String] API endpoint path | ||
| # @param params [Hash] Request body parameters | ||
| # @param headers [Hash] Additional request headers | ||
| # @return [Hash] Parsed JSON response body | ||
| def delete(path:, params: {}, headers: {}, request_options: {}) | ||
| request(method: :delete, path: path, params: params, headers: headers, request_options: request_options) | ||
| end |
There was a problem hiding this comment.
Update verb helper docs to return Uploadcare::Result.
These methods call request, which wraps responses in Uploadcare::Result, but the docs claim a Hash. This is misleading for callers.
📚 Proposed doc fix
- # `@return` [Hash] Parsed JSON response body
+ # `@return` [Uploadcare::Result] Result wrapper(Apply to post, get, put, delete.)
📝 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.
| # Make a POST request | |
| # | |
| # @param path [String] API endpoint path | |
| # @param params [Hash] Request body parameters | |
| # @param headers [Hash] Additional request headers | |
| # @return [Hash] Parsed JSON response body | |
| def post(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :post, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a GET request | |
| # | |
| # @param path [String] API endpoint path | |
| # @param params [Hash] Query parameters | |
| # @param headers [Hash] Additional request headers | |
| # @return [Hash] Parsed JSON response body | |
| def get(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :get, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a PUT request | |
| # | |
| # @param path [String] API endpoint path | |
| # @param params [Hash] Request body parameters | |
| # @param headers [Hash] Additional request headers | |
| # @return [Hash] Parsed JSON response body | |
| def put(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :put, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a DELETE request | |
| # | |
| # @param path [String] API endpoint path | |
| # @param params [Hash] Request body parameters | |
| # @param headers [Hash] Additional request headers | |
| # @return [Hash] Parsed JSON response body | |
| def delete(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :delete, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a POST request | |
| # | |
| # `@param` path [String] API endpoint path | |
| # `@param` params [Hash] Request body parameters | |
| # `@param` headers [Hash] Additional request headers | |
| # `@return` [Uploadcare::Result] Result wrapper | |
| def post(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :post, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a GET request | |
| # | |
| # `@param` path [String] API endpoint path | |
| # `@param` params [Hash] Query parameters | |
| # `@param` headers [Hash] Additional request headers | |
| # `@return` [Uploadcare::Result] Result wrapper | |
| def get(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :get, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a PUT request | |
| # | |
| # `@param` path [String] API endpoint path | |
| # `@param` params [Hash] Request body parameters | |
| # `@param` headers [Hash] Additional request headers | |
| # `@return` [Uploadcare::Result] Result wrapper | |
| def put(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :put, path: path, params: params, headers: headers, request_options: request_options) | |
| end | |
| # Make a DELETE request | |
| # | |
| # `@param` path [String] API endpoint path | |
| # `@param` params [Hash] Request body parameters | |
| # `@param` headers [Hash] Additional request headers | |
| # `@return` [Uploadcare::Result] Result wrapper | |
| def delete(path:, params: {}, headers: {}, request_options: {}) | |
| request(method: :delete, path: path, params: params, headers: headers, request_options: request_options) | |
| end |
🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/rest_client.rb` around lines 72 - 110, The YARD
`@return` annotations for the HTTP verb helpers post, get, put, and delete
incorrectly state they return a Hash; update each method's documentation to
reflect that they return an Uploadcare::Result (the wrapper returned by
request). Edit the comments above def post, def get, def put, and def delete to
replace "@return [Hash] Parsed JSON response body" with "@return
[Uploadcare::Result] Result wrapper containing the parsed response" (or similar
wording) so callers see the correct return type.
| def create_group(uuids:, request_options: {}, **options) | ||
| body_hash = group_body_hash(uuids, options) | ||
| post(path: 'group/', params: body_hash, headers: {}, request_options: request_options) | ||
| end |
There was a problem hiding this comment.
Validate uuids input before building multipart body.
Without array/empty checks, parse_uuids can raise NoMethodError, and behavior diverges from UploadClient#create_group.
🛠️ Suggested fix
def create_group(uuids:, request_options: {}, **options)
+ raise ArgumentError, 'uuids must be an array' unless uuids.is_a?(Array)
+ raise ArgumentError, 'uuids cannot be empty' if uuids.empty?
body_hash = group_body_hash(uuids, options)
post(path: 'group/', params: body_hash, headers: {}, request_options: request_options)
end🤖 Prompt for AI Agents
In `@lib/uploadcare/clients/upload_group_client.rb` around lines 12 - 15, The
create_group method must validate the uuids input before building the multipart
body to avoid NoMethodError from parse_uuids and to match
UploadClient#create_group behavior; update create_group (or group_body_hash) to
assert that uuids is an Array and not empty (or raise a clear ArgumentError)
before calling group_body_hash/parse_uuids, and ensure any nil/single-value
inputs are normalized or rejected the same way UploadClient#create_group does so
downstream code will not call parse_uuids on nil or non-array values.
| # Returns the CDN base URL based on subdomain configuration | ||
| # @return [Proc] A proc that returns the appropriate CDN base URL | ||
| def cdn_base | ||
| lambda do | ||
| if use_subdomains | ||
| Uploadcare::CnameGenerator.cdn_base_postfix | ||
| else | ||
| default_cdn_base | ||
| end | ||
| end |
There was a problem hiding this comment.
Fix indentation in cdn_base to satisfy RuboCop.
Current indentation violates Layout/IndentationWidth and fails CI.
🛠️ Suggested fix
def cdn_base
lambda do
if use_subdomains
- Uploadcare::CnameGenerator.cdn_base_postfix
- else
- default_cdn_base
- end
+ Uploadcare::CnameGenerator.cdn_base_postfix
+ else
+ default_cdn_base
+ end
end
end📝 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.
| # Returns the CDN base URL based on subdomain configuration | |
| # @return [Proc] A proc that returns the appropriate CDN base URL | |
| def cdn_base | |
| lambda do | |
| if use_subdomains | |
| Uploadcare::CnameGenerator.cdn_base_postfix | |
| else | |
| default_cdn_base | |
| end | |
| end | |
| # Returns the CDN base URL based on subdomain configuration | |
| # `@return` [Proc] A proc that returns the appropriate CDN base URL | |
| def cdn_base | |
| lambda do | |
| if use_subdomains | |
| Uploadcare::CnameGenerator.cdn_base_postfix | |
| else | |
| default_cdn_base | |
| end | |
| end | |
| end |
🧰 Tools
🪛 GitHub Actions: Ruby
[error] 114-114: Layout/IndentationWidth: Use 2 (not 0) spaces for indentation.
🤖 Prompt for AI Agents
In `@lib/uploadcare/configuration.rb` around lines 109 - 118, The cdn_base
method's lambda has inconsistent indentation causing RuboCop
Layout/IndentationWidth failures; open the cdn_base method and reindent the
block so the lambda body uses consistent two-space indentation: the if, its
branches (Uploadcare::CnameGenerator.cdn_base_postfix and default_cdn_base) and
the else keyword should be aligned as a normal Ruby block, with the final end
matching the lambda do; ensure the inner return expressions are indented one
level inside the lambda to satisfy RuboCop.
lib/uploadcare/error_handler.rb
Outdated
| def raise_status_error(response, message) | ||
| status = response.is_a?(Hash) ? response[:status] : response | ||
| case status | ||
| when 400 then raise Uploadcare::Exception::InvalidRequestError, message | ||
| when 404 then raise Uploadcare::Exception::NotFoundError, message | ||
| when 429 | ||
| headers = response.is_a?(Hash) ? response[:headers] : nil | ||
| retry_after = headers && (headers['retry-after'] || headers['Retry-After']) | ||
| timeout = retry_after.to_f | ||
| timeout = 10.0 if timeout <= 0 | ||
| raise Uploadcare::Exception::ThrottleError.new(timeout, message: message) | ||
| else raise Uploadcare::Exception::RequestError, message | ||
| end |
There was a problem hiding this comment.
Reduce cyclomatic complexity to unblock CI.
raise_status_error exceeds the configured complexity threshold. Extracting the 429 branch into a helper (or using early returns) should satisfy RuboCop.
🛠️ Suggested refactor to lower complexity
def raise_status_error(response, message)
status = response.is_a?(Hash) ? response[:status] : response
- case status
- when 400 then raise Uploadcare::Exception::InvalidRequestError, message
- when 404 then raise Uploadcare::Exception::NotFoundError, message
- when 429
- headers = response.is_a?(Hash) ? response[:headers] : nil
- retry_after = headers && (headers['retry-after'] || headers['Retry-After'])
- timeout = retry_after.to_f
- timeout = 10.0 if timeout <= 0
- raise Uploadcare::Exception::ThrottleError.new(timeout, message: message)
- else raise Uploadcare::Exception::RequestError, message
- end
+ return raise Uploadcare::Exception::InvalidRequestError, message if status == 400
+ return raise Uploadcare::Exception::NotFoundError, message if status == 404
+ return raise_throttle_error(response, message) if status == 429
+
+ raise Uploadcare::Exception::RequestError, message
end
+
+def raise_throttle_error(response, message)
+ headers = response.is_a?(Hash) ? response[:headers] : nil
+ retry_after = headers && (headers['retry-after'] || headers['Retry-After'])
+ timeout = retry_after.to_f
+ timeout = 10.0 if timeout <= 0
+ raise Uploadcare::Exception::ThrottleError.new(timeout, message: message)
+end🧰 Tools
🪛 GitHub Actions: Ruby
[error] 46-46: Metrics/CyclomaticComplexity: Cyclomatic complexity for raise_status_error is too high. [9/7]
🤖 Prompt for AI Agents
In `@lib/uploadcare/error_handler.rb` around lines 46 - 58, The method
raise_status_error is above the cyclomatic complexity threshold due to the
inlined 429 branch; extract the 429 handling into a new helper (e.g.,
raise_throttle_error(response, message) or parse_retry_timeout(response)) that
reads headers (headers = response.is_a?(Hash) ? response[:headers] : nil),
computes retry_after/to_f with the 10.0 fallback, and raises
Uploadcare::Exception::ThrottleError.new(timeout, message: message); then
replace the 429 case in raise_status_error with a single call to that helper (or
use an early return) to reduce branching inside raise_status_error.
| # Deletes a group by UUID. | ||
| # @param uuid [String] The UUID of the group to delete. | ||
| # @return [Nil] Returns nil on successful deletion. | ||
| # @see https://uploadcare.com/api-refs/rest-api/v0.7.0/#tag/Group/operation/deleteGroup | ||
| def delete(uuid: nil, request_options: {}) | ||
| Uploadcare::Result.unwrap(@group_client.delete(uuid: uuid || id, request_options: request_options)) |
There was a problem hiding this comment.
Add the required blank line between method definitions.
RuboCop is failing Layout/EmptyLineBetweenDefs here.
🧹 Proposed fix
def info(uuid: nil, request_options: {})
response = Uploadcare::Result.unwrap(`@group_client.info`(uuid: uuid || id, request_options: request_options))
assign_attributes(response)
self
end
+
# Deletes a group by UUID.
# `@param` uuid [String] The UUID of the group to delete.🧰 Tools
🪛 GitHub Actions: Ruby
[error] 55-55: Layout/EmptyLineBetweenDefs: Expected 1 empty line between method definitions; found 0.
🤖 Prompt for AI Agents
In `@lib/uploadcare/resources/group.rb` around lines 51 - 56, RuboCop flags
missing blank line between method definitions in the Group resource; add a
single empty line before or after the def delete(...) method (method name:
delete in class Group, and surrounding methods such as any prior or following
def) so there is exactly one blank line separating this method from the adjacent
method definitions to satisfy Layout/EmptyLineBetweenDefs.
| def valid?(uuid) | ||
| raise ArgumentError, 'Must contain valid UUID' unless uuid.is_a?(String) | ||
|
|
||
| def delimiter | ||
| '~' | ||
| end | ||
| uuid.match?(UUID_REGEX) | ||
| end |
There was a problem hiding this comment.
Fix UUID regex type: match? expects a Regexp.
UUID_REGEX is a String, so String#match? will raise a TypeError for valid inputs. Define it as a Regexp and anchor the match.
🛠️ Suggested fix
- UUID_REGEX = '[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12}'
+ UUID_REGEX = /\A[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}\z/i
@@
- uuid.match?(UUID_REGEX)
+ UUID_REGEX.match?(uuid)Does Ruby String#match? accept a String argument, or does it require a Regexp?
🤖 Prompt for AI Agents
In `@lib/uploadcare/signed_url_generators/akamai_generator.rb` around lines 30 -
34, The UUID_REGEX constant is currently a String causing valid?(uuid) to raise
when calling uuid.match?; change UUID_REGEX to be a Regexp (e.g., use /\A...\z/
to anchor start/end) and update the constant definition accordingly so that
AkamaiGenerator#valid? can call uuid.match?(UUID_REGEX) without error; locate
the UUID_REGEX constant near the signed_url_generators/Akamai generator and
ensure the pattern is properly anchored and compiled as a Regexp.
There was a problem hiding this comment.
Pull request overview
Modernizes the uploadcare-ruby gem for v5.0 with a new layered architecture (Resources/Clients/Concerns), updated dependencies, and Ruby 3.3+ support.
Changes:
- Adds new core building blocks (Result wrapper, throttle handling, webhook signature verification, new REST clients/resources).
- Refactors/renames namespaces toward a flatter public API and replaces legacy client/entity/concern implementations.
- Updates tooling/CI and refreshes/renames fixtures and runnable examples for the new API.
Reviewed changes
Copilot reviewed 273 out of 362 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/fixtures/vcr_cassettes/ws_rekognition_detect_labels_status_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old addon status error). |
| spec/fixtures/vcr_cassettes/ws_rekognition_detect_labels_status.yml | Removed legacy VCR cassette fixture (old addon status success). |
| spec/fixtures/vcr_cassettes/ws_rekognition_detect_labels_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old request_id validation). |
| spec/fixtures/vcr_cassettes/ws_rekognition_detect_labels.yml | Removed legacy VCR cassette fixture (old addon execute). |
| spec/fixtures/vcr_cassettes/video_convert_get_status.yml | Removed legacy VCR cassette fixture (old video status). |
| spec/fixtures/vcr_cassettes/video_convert_file_info.yml | Removed legacy VCR cassette fixture (old file info for video conversion). |
| spec/fixtures/vcr_cassettes/video_convert_convert_many_with_error.yml | Removed legacy VCR cassette fixture (old batch convert error). |
| spec/fixtures/vcr_cassettes/video_convert_convert_many.yml | Removed legacy VCR cassette fixture (old batch convert success). |
| spec/fixtures/vcr_cassettes/uploader_multipart_upload.yml | Removed legacy VCR cassette fixture (old multipart upload). |
| spec/fixtures/vcr_cassettes/upload_upload_from_url_timeout.yml | Added VCR cassette fixture for URL upload failure path (missing pub_key). |
| spec/fixtures/vcr_cassettes/upload_upload_from_url_async.yml | Removed legacy VCR cassette fixture (old async URL upload). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_start_small.yml | Removed legacy VCR cassette fixture (old multipart start small file). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_start.yml | Removed legacy VCR cassette fixture (old multipart start). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_small.yml | Removed legacy VCR cassette fixture (old multipart small upload). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_complete_wrong_id.yml | Removed legacy VCR cassette fixture (old multipart complete missing file). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_complete_unfinished.yml | Removed legacy VCR cassette fixture (old multipart unfinished). |
| spec/fixtures/vcr_cassettes/upload_multipart_upload_complete.yml | Removed legacy VCR cassette fixture (old multipart already uploaded). |
| spec/fixtures/vcr_cassettes/upload_group_delete.yml | Removed legacy VCR cassette fixture (old group delete). |
| spec/fixtures/vcr_cassettes/upload_from_url_basic.yml | Added VCR cassette fixture for URL upload failure path (missing pub_key). |
| spec/fixtures/vcr_cassettes/upload_file_info.yml | Removed legacy VCR cassette fixture (old Upload API info endpoint). |
| spec/fixtures/vcr_cassettes/uc_clamav_virus_scan_status_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old addon status error). |
| spec/fixtures/vcr_cassettes/uc_clamav_virus_scan_status.yml | Removed legacy VCR cassette fixture (old addon status success). |
| spec/fixtures/vcr_cassettes/uc_clamav_virus_scan_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old addon execute error). |
| spec/fixtures/vcr_cassettes/uc_clamav_virus_scan.yml | Removed legacy VCR cassette fixture (old addon execute success). |
| spec/fixtures/vcr_cassettes/rest_webhook_update.yml | Removed legacy VCR cassette fixture (old webhook update). |
| spec/fixtures/vcr_cassettes/rest_webhook_list_unpaid.yml | Removed legacy VCR cassette fixture (old webhook list unpaid). |
| spec/fixtures/vcr_cassettes/rest_webhook_list.yml | Removed legacy VCR cassette fixture (old webhook list). |
| spec/fixtures/vcr_cassettes/rest_webhook_destroy1.yml | Removed legacy VCR cassette fixture (old webhook destroy via POST). |
| spec/fixtures/vcr_cassettes/rest_webhook_destroy.yml | Removed legacy VCR cassette fixture (old webhook destroy via DELETE). |
| spec/fixtures/vcr_cassettes/rest_webhook_create.yml | Removed legacy VCR cassette fixture (old webhook create). |
| spec/fixtures/vcr_cassettes/rest_list_groups_limited.yml | Removed legacy VCR cassette fixture (old group list pagination). |
| spec/fixtures/vcr_cassettes/rest_file_store.yml | Removed legacy VCR cassette fixture (old file store). |
| spec/fixtures/vcr_cassettes/rest_file_info_no_connection.yml | Removed legacy VCR cassette fixture (old redirect/no connection scenario). |
| spec/fixtures/vcr_cassettes/rest_file_info_fail.yml | Removed legacy VCR cassette fixture (old file info 404). |
| spec/fixtures/vcr_cassettes/rest_file_delete_nonexistent.yml | Removed legacy VCR cassette fixture (old delete nonexistent). |
| spec/fixtures/vcr_cassettes/rest_file_delete.yml | Removed legacy VCR cassette fixture (old delete). |
| spec/fixtures/vcr_cassettes/rest_file_batch_store_fail.yml | Removed legacy VCR cassette fixture (old batch store fail). |
| spec/fixtures/vcr_cassettes/rest_file_batch_delete_fail.yml | Removed legacy VCR cassette fixture (old batch delete fail). |
| spec/fixtures/vcr_cassettes/remove_bg_status_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old addon status error). |
| spec/fixtures/vcr_cassettes/remove_bg_status.yml | Removed legacy VCR cassette fixture (old remove_bg status). |
| spec/fixtures/vcr_cassettes/remove_bg_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old request_id validation). |
| spec/fixtures/vcr_cassettes/remove_bg.yml | Removed legacy VCR cassette fixture (old remove_bg execute). |
| spec/fixtures/vcr_cassettes/project.yml | Removed legacy VCR cassette fixture (old project show). |
| spec/fixtures/vcr_cassettes/http_fail.yml | Removed legacy VCR cassette fixture (old insecure request warning). |
| spec/fixtures/vcr_cassettes/group_delete_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old group delete 404). |
| spec/fixtures/vcr_cassettes/file_metadata_update.yml | Removed legacy VCR cassette fixture (old metadata update). |
| spec/fixtures/vcr_cassettes/file_metadata_show_nonexistent_key.yml | Removed legacy VCR cassette fixture (old metadata show missing key). |
| spec/fixtures/vcr_cassettes/file_metadata_show.yml | Removed legacy VCR cassette fixture (old metadata show). |
| spec/fixtures/vcr_cassettes/file_metadata_index_nonexistent_uuid.yml | Removed legacy VCR cassette fixture (old metadata index 404). |
| spec/fixtures/vcr_cassettes/file_metadata_index.yml | Removed legacy VCR cassette fixture (old metadata index). |
| spec/fixtures/vcr_cassettes/file_metadata_delete.yml | Removed legacy VCR cassette fixture (old metadata delete). |
| spec/fixtures/vcr_cassettes/file_metadata_create.yml | Removed legacy VCR cassette fixture (old metadata create). |
| spec/fixtures/vcr_cassettes/file_info.yml | Removed legacy VCR cassette fixture (old file info). |
| spec/fixtures/vcr_cassettes/document_convert_to_multipage.yml | Removed legacy VCR cassette fixture (old doc convert multipage). |
| spec/fixtures/vcr_cassettes/document_convert_info.yml | Removed legacy VCR cassette fixture (old doc convert info). |
| spec/fixtures/vcr_cassettes/document_convert_get_status.yml | Removed legacy VCR cassette fixture (old doc convert status). |
| spec/fixtures/vcr_cassettes/document_convert_file_info.yml | Removed legacy VCR cassette fixture (old file info for doc conversion). |
| spec/fixtures/vcr_cassettes/document_convert_convert_many_with_error.yml | Removed legacy VCR cassette fixture (old doc convert error). |
| spec/fixtures/vcr_cassettes/document_convert_convert_many.yml | Removed legacy VCR cassette fixture (old doc convert success). |
| spec/fixtures/vcr_cassettes/Upload_API_Integration/Performance/Parallel_multipart_upload/parallel_upload_is_faster_than_sequential.yml | Added VCR cassette for multipart start failure path under performance suite. |
| spec/fixtures/vcr_cassettes/Upload_API_Integration/Complete_Upload_Workflow/Multipart_Upload_Complete_Verify/performs_complete_multipart_upload_workflow.yml | Added VCR cassette for multipart workflow failure path. |
| mise.toml | Adds mise toolchain pin for Ruby runtime. |
| lib/uploadcare/webhook_signature_verifier.rb | Adds webhook signature verification utility. |
| lib/uploadcare/version.rb | Bumps gem version constant to 5.0.0 with inline doc. |
| lib/uploadcare/throttle_handler.rb | Adds throttling retry helper module for clients. |
| lib/uploadcare/signed_url_generators/base_generator.rb | Refactors signed URL generator base class to flat constant and adds doc. |
| lib/uploadcare/signed_url_generators/akamai_generator.rb | Refactors Akamai generator with input checks and signature changes. |
| lib/uploadcare/result.rb | Introduces Result wrapper for success/failure and exception capture. |
| lib/uploadcare/resources/video_converter.rb | Adds new VideoConverter resource using Rest client. |
| lib/uploadcare/resources/project.rb | Adds new Project resource using Rest client. |
| lib/uploadcare/resources/document_converter.rb | Adds new DocumentConverter resource using Rest client. |
| lib/uploadcare/resources/batch_file_result.rb | Adds BatchFileResult to wrap batch store/delete results. |
| lib/uploadcare/resources/base_resource.rb | Adds BaseResource with config and attribute assignment. |
| lib/uploadcare/param/webhook_signature_verifier.rb | Removes legacy Param::WebhookSignatureVerifier implementation. |
| lib/uploadcare/param/user_agent.rb | Refactors User-Agent builder to accept config and use new VERSION constant. |
| lib/uploadcare/param/upload/upload_params_generator.rb | Refactors upload params generation (store/metadata/signature handling). |
| lib/uploadcare/param/upload/signature_generator.rb | Refactors signed upload signature generation with validation and config param. |
| lib/uploadcare/param/simple_auth_header.rb | Removes legacy simple auth header builder. |
| lib/uploadcare/param/secure_auth_header.rb | Removes legacy secure auth header builder. |
| lib/uploadcare/param/param.rb | Removes legacy Param include hook. |
| lib/uploadcare/param/conversion/video/processing_job_url_builder.rb | Removes legacy conversion URL builder. |
| lib/uploadcare/param/conversion/document/processing_job_url_builder.rb | Removes legacy conversion URL builder. |
| lib/uploadcare/param/authentication_header.rb | Removes legacy auth header dispatcher. |
| lib/uploadcare/exception/upload_error.rb | Adds upload-specific exception hierarchy. |
| lib/uploadcare/exception/throttle_error.rb | Refactors ThrottleError constant location and initializer. |
| lib/uploadcare/exception/retry_error.rb | Refactors RetryError constant location. |
| lib/uploadcare/exception/request_error.rb | Refactors RequestError constant location. |
| lib/uploadcare/exception/not_found_error.rb | Adds NotFoundError derived from RequestError. |
| lib/uploadcare/exception/invalid_request_error.rb | Adds InvalidRequestError derived from RequestError. |
| lib/uploadcare/exception/conversion_error.rb | Refactors ConversionError constant location. |
| lib/uploadcare/exception/configuration_error.rb | Refactors ConfigurationError constant location. |
| lib/uploadcare/exception/auth_error.rb | Refactors AuthError constant location. |
| lib/uploadcare/entity/webhook.rb | Removes legacy Entity::Webhook. |
| lib/uploadcare/entity/project.rb | Removes legacy Entity::Project. |
| lib/uploadcare/entity/group_list.rb | Removes legacy Entity::GroupList paginator wrapper. |
| lib/uploadcare/entity/group.rb | Removes legacy Entity::Group. |
| lib/uploadcare/entity/file_metadata.rb | Removes legacy Entity::FileMetadata. |
| lib/uploadcare/entity/file_list.rb | Removes legacy Entity::FileList. |
| lib/uploadcare/entity/entity.rb | Removes legacy Entity base loader. |
| lib/uploadcare/entity/decorator/paginator.rb | Removes legacy paginator decorator. |
| lib/uploadcare/entity/conversion/video_converter.rb | Removes legacy conversion entity. |
| lib/uploadcare/entity/conversion/document_converter.rb | Removes legacy conversion entity. |
| lib/uploadcare/entity/conversion/base_converter.rb | Removes legacy conversion base. |
| lib/uploadcare/entity/addons.rb | Removes legacy Entity::Addons. |
| lib/uploadcare/concern/upload_error_handler.rb | Removes legacy upload error handler concern. |
| lib/uploadcare/concern/throttle_handler.rb | Removes legacy throttle handler concern. |
| lib/uploadcare/concern/error_handler.rb | Removes legacy error handler concern. |
| lib/uploadcare/cname_generator.rb | Refactors CDN CNAME generation and adds caching. |
| lib/uploadcare/clients/video_converter_client.rb | Adds new REST VideoConverter client. |
| lib/uploadcare/clients/project_client.rb | Adds new REST Project client. |
| lib/uploadcare/clients/group_client.rb | Adds new REST Group client. |
| lib/uploadcare/clients/file_metadata_client.rb | Adds new REST FileMetadata client. |
| lib/uploadcare/clients/document_converter_client.rb | Adds new REST DocumentConverter client. |
| lib/uploadcare/client/webhook_client.rb | Removes legacy Client::WebhookClient. |
| lib/uploadcare/client/upload_client.rb | Removes legacy Client::UploadClient. |
| lib/uploadcare/client/rest_group_client.rb | Removes legacy Client::RestGroupClient. |
| lib/uploadcare/client/rest_client.rb | Removes legacy Client::RestClient. |
| lib/uploadcare/client/project_client.rb | Removes legacy Client::ProjectClient. |
| lib/uploadcare/client/multipart_upload_client.rb | Removes legacy multipart uploader client. |
| lib/uploadcare/client/multipart_upload/chunks_client.rb | Removes legacy Parallel-based chunk uploader. |
| lib/uploadcare/client/group_client.rb | Removes legacy Upload API group client. |
| lib/uploadcare/client/file_metadata_client.rb | Removes legacy file metadata client. |
| lib/uploadcare/client/file_list_client.rb | Removes legacy file list client. |
| lib/uploadcare/client/file_client.rb | Removes legacy single file client. |
| lib/uploadcare/client/conversion/video_conversion_client.rb | Removes legacy video conversion client. |
| lib/uploadcare/client/conversion/document_conversion_client.rb | Removes legacy document conversion client. |
| lib/uploadcare/client/conversion/base_conversion_client.rb | Removes legacy conversion base client. |
| lib/uploadcare/api/api.rb | Removes legacy Api delegator façade. |
| examples/url_upload.rb | Adds runnable example for URL upload. |
| examples/simple_upload.rb | Adds runnable example for file upload. |
| examples/batch_upload.rb | Adds runnable example for batch file upload. |
| api_examples/upload_api/test_url_upload.rb | Adds Upload API URL upload test script. |
| api_examples/upload_api/put_presigned_url_x.rb | Updates Upload API example to new configuration and usage. |
| api_examples/upload_api/put_multipart_part.rb | Adds example for multipart start + uploading parts. |
| api_examples/upload_api/post_multipart_start.rb | Updates example to explicitly call multipart_start and print parts. |
| api_examples/upload_api/post_multipart_complete.rb | Updates example to explicitly call multipart_complete. |
| api_examples/upload_api/post_group.rb | Updates group creation example to new API shape. |
| api_examples/upload_api/post_from_url.rb | Expands URL upload examples (sync/async/status). |
| api_examples/upload_api/post_base.rb | Updates base upload example to new UploadClient API. |
| api_examples/upload_api/multipart_upload_complete.rb | Adds high-level multipart upload example with progress + threads. |
| api_examples/upload_api/get_info.rb | Updates file info example to new File.info signature. |
| api_examples/upload_api/get_group_info_example.rb | Adds group info example for Upload API. |
| api_examples/upload_api/get_group_info.rb | Updates group info script to new method signature. |
| api_examples/upload_api/get_from_url_status.rb | Updates URL status example to include token polling. |
| api_examples/upload_api/get_file_info_example.rb | Adds file info example for Upload API. |
| api_examples/upload_api/create_group.rb | Adds full workflow example: upload files then create a group. |
| api_examples/rest_api/put_webhooks_id.rb | Updates webhook update example to new method signature. |
| api_examples/rest_api/put_files_uuid_storage.rb | Updates file store example to new resource instance method usage. |
| api_examples/rest_api/put_files_uuid_metadata_key.rb | Updates file metadata update example to new method signature. |
| api_examples/rest_api/put_files_storage.rb | Updates batch store example to new batch API call. |
| api_examples/rest_api/post_webhooks.rb | Updates webhook create example and parameter sourcing. |
| api_examples/rest_api/post_files_remote_copy.rb | Updates remote copy example to new call signature and docs. |
| api_examples/rest_api/post_files_local_copy.rb | Updates local copy example to new call signature. |
| api_examples/rest_api/post_convert_video.rb | Updates video conversion example to new convert signature. |
| api_examples/rest_api/post_convert_document.rb | Updates document conversion example to new convert signature. |
| api_examples/rest_api/post_addons_uc_clamav_virus_scan_execute.rb | Updates addon execute example to new signature. |
| api_examples/rest_api/post_addons_remove_bg_execute.rb | Updates addon execute example to new signature. |
| api_examples/rest_api/post_addons_aws_rekognition_detect_moderation_labels_execute.rb | Updates addon execute example to new naming/signature. |
| api_examples/rest_api/post_addons_aws_rekognition_detect_labels_execute.rb | Updates addon execute example to new naming/signature. |
| api_examples/rest_api/get_webhooks.rb | Updates webhook list example to new configuration. |
| api_examples/rest_api/get_project.rb | Updates project show example to new configuration. |
| api_examples/rest_api/get_groups_uuid.rb | Updates group info example to new signature. |
| api_examples/rest_api/get_groups.rb | Updates groups list example to new list API. |
| api_examples/rest_api/get_files_uuid_metadata_key.rb | Updates metadata index example to new signature. |
| api_examples/rest_api/get_files_uuid_metadata.rb | Updates metadata show example to new signature. |
| api_examples/rest_api/get_files_uuid.rb | Updates file info example to new signature. |
| api_examples/rest_api/get_files.rb | Updates files list example to new list API. |
| api_examples/rest_api/get_convert_video_status_token.rb | Updates video conversion status example to new workflow. |
| api_examples/rest_api/get_convert_document_uuid.rb | Updates document info example to new workflow. |
| api_examples/rest_api/get_convert_document_status_token.rb | Updates doc status example to new workflow. |
| api_examples/rest_api/get_addons_uc_clamav_virus_scan_execute_status.rb | Updates addon status example to new signature. |
| api_examples/rest_api/get_addons_remove_bg_execute_status.rb | Updates addon status example to new signature. |
| api_examples/rest_api/get_addons_aws_rekognition_detect_moderation_labels_execute_status.rb | Updates addon status example to new naming/signature. |
| api_examples/rest_api/get_addons_aws_rekognition_detect_labels_execute_status.rb | Updates addon status example to new naming/signature. |
| api_examples/rest_api/delete_webhooks_unsubscribe.rb | Updates webhook delete example to new signature. |
| api_examples/rest_api/delete_groups_uuid.rb | Updates group delete example to new client usage. |
| api_examples/rest_api/delete_files_uuid_storage.rb | Updates file delete example to new resource instance method usage. |
| api_examples/rest_api/delete_files_uuid_metadata_key.rb | Updates metadata delete example to new signature. |
| api_examples/rest_api/delete_files_storage.rb | Updates batch delete example to new batch API. |
| Rakefile | Adds RuboCop task and runs rubocop in default rake task. |
| Gemfile | Adds stdlib-extracted gems for Ruby 3.4+/4.0+ and dev-tooling deps. |
| .github/workflows/ruby.yml | Updates CI to Ruby 3.3/3.4/4.0 and runs rubocop via bundler. |
| .env.example | Adds example env file for running scripts/examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Akamai signed URL generator. | ||
| class Uploadcare::SignedUrlGenerators::AkamaiGenerator < Uploadcare::SignedUrlGenerators::BaseGenerator | ||
| # UUID validation regex. | ||
| UUID_REGEX = '[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12}' |
There was a problem hiding this comment.
UUID_REGEX is defined as a String, but String#match? requires a Regexp. This will raise TypeError at runtime. Define UUID_REGEX as a Regexp (ideally anchored with \A...\z) and use uuid.match?(UUID_REGEX).
| raise ArgumentError, 'Must contain valid UUID' unless uuid.is_a?(String) | ||
|
|
||
| def delimiter | ||
| '~' | ||
| end | ||
| uuid.match?(UUID_REGEX) |
There was a problem hiding this comment.
UUID_REGEX is defined as a String, but String#match? requires a Regexp. This will raise TypeError at runtime. Define UUID_REGEX as a Regexp (ideally anchored with \A...\z) and use uuid.match?(UUID_REGEX).
| def update(uuid:, key:, value:, request_options: {}) | ||
| encoded_uuid = URI.encode_www_form_component(uuid) | ||
| encoded_key = URI.encode_www_form_component(key) | ||
| put( | ||
| path: "/files/#{encoded_uuid}/metadata/#{encoded_key}/", | ||
| params: value, | ||
| headers: {}, | ||
| request_options: request_options | ||
| ) | ||
| end |
There was a problem hiding this comment.
The metadata update endpoint expects a JSON-encoded string value (historically this was sent as value.to_json). Passing params: value risks sending an unquoted raw string (or otherwise incorrectly encoded body), which can break the API contract. Serialize the value as JSON (and ensure the request uses the correct content-type) when issuing the PUT.
lib/uploadcare/throttle_handler.rb
Outdated
| # This module is included in client classes to provide automatic retry logic | ||
| # when the API returns a throttle error (HTTP 429). It respects the retry-after | ||
| # header and implements exponential backoff. |
There was a problem hiding this comment.
The module documentation claims retry-after handling and exponential backoff, but the implementation currently sleeps exactly e.timeout with no backoff/jitter and no visible retry-after parsing here. Either implement the described backoff/jitter behavior (and/or retry-after support) or update the comments to match the actual logic.
| # handle_throttling do | ||
| # connection.get('/files/') | ||
| # end | ||
| def handle_throttling(max_attempts: nil) |
There was a problem hiding this comment.
The module documentation claims retry-after handling and exponential backoff, but the implementation currently sleeps exactly e.timeout with no backoff/jitter and no visible retry-after parsing here. Either implement the described backoff/jitter behavior (and/or retry-after support) or update the comments to match the actual logic.
lib/uploadcare/throttle_handler.rb
Outdated
| (attempts - 1).times do | ||
| return yield | ||
| rescue(Uploadcare::Exception::ThrottleError) => e | ||
| sleep(e.timeout) | ||
| end |
There was a problem hiding this comment.
The module documentation claims retry-after handling and exponential backoff, but the implementation currently sleeps exactly e.timeout with no backoff/jitter and no visible retry-after parsing here. Either implement the described backoff/jitter behavior (and/or retry-after support) or update the comments to match the actual logic.
| return false unless first.bytesize == second.bytesize | ||
|
|
||
| left = first.unpack('C*') | ||
| res = 0 | ||
| second.each_byte { |byte| res |= byte ^ left.shift } | ||
| res.zero? |
There was a problem hiding this comment.
This rolls a custom "constant-time" compare, but Array#shift is O(n) and can introduce avoidable timing/performance variability for longer strings. Prefer a standard fixed-length secure compare implementation (e.g., OpenSSL.fixed_length_secure_compare if available in your runtime) and avoid shift by indexing bytes.
| return false unless first.bytesize == second.bytesize | |
| left = first.unpack('C*') | |
| res = 0 | |
| second.each_byte { |byte| res |= byte ^ left.shift } | |
| res.zero? | |
| return false if first.nil? || second.nil? | |
| return false unless first.bytesize == second.bytesize | |
| # Prefer OpenSSL's fixed-length constant-time comparison when available | |
| OpenSSL.fixed_length_secure_compare(first, second) | |
| rescue NoMethodError | |
| # Fallback constant-time comparison without using Array#shift | |
| result = 0 | |
| enum_first = first.each_byte | |
| second.each_byte { |byte| result |= byte ^ enum_first.next } | |
| result.zero? |
| paths: paths, | ||
| store: options[:store] ? '1' : '0', | ||
| save_in_group: options[:save_in_group] ? '1' : '0' | ||
| } | ||
|
|
There was a problem hiding this comment.
This always sends store and save_in_group as '0' when the caller omits those options, which can unintentionally override server defaults (and differs from the earlier pattern of omitting nil options). Consider only including these keys when explicitly provided, and converting boolean/string inputs without forcing '0' by default.
| paths: paths, | |
| store: options[:store] ? '1' : '0', | |
| save_in_group: options[:save_in_group] ? '1' : '0' | |
| } | |
| paths: paths | |
| } | |
| if options.key?(:store) | |
| body[:store] = options[:store] ? '1' : '0' | |
| end | |
| if options.key?(:save_in_group) | |
| body[:save_in_group] = options[:save_in_group] ? '1' : '0' | |
| end |
…ubbing - Fix auth/header handling, pagination, upload behavior, and README examples - Add multi-account spec coverage for per-client configuration - Scrub VCR cassettes and harden VCR filters to avoid leaking credentials
CI was failing because RuboCop runs as part of rake. - Refactor RestClient signature body computation to reduce complexity - Extract parallel upload worker to satisfy Metrics/BlockLength - Clean up VCR scrubber regexes and multi-account spec naming
Use per-example generated secret keys to prevent secret scanners from flagging test fixtures.
Replace pubkey:secret placeholders in recorded Authorization headers with a neutral placeholder to avoid triggering secret scanners.
1 participant
Summary
Complete rewrite of the uploadcare-ruby gem for v5.0. This is a ground-up modernization that replaces the v4.x architecture with a cleaner, more maintainable design while preserving familiar configuration syntax.
355 files changed across the entire gem surface.
Architecture
requirestatements (Ruby 3.3+)Uploadcare::File,Uploadcare::FileClient,Uploadcare::Uploaderinstead of deeply nestedUploadcare::Entity::File,Uploadcare::Client::FileClientDependency overhaul
Removed:
dry-configurable,uploadcare-api_struct,mimemagic,parallel,retriesAdded:
zeitwerk,faraday,faraday-multipart,addressable,mime-typesThe gem now has fewer, more focused dependencies.
Key features & improvements
Uploader.uploadauto-selects the right strategy (direct, URL, multipart) based on inputparallelgem)Dry::Configurable(sameconfigureblock syntax)use_subdomainsoptionInvalidRequestError,NotFoundError,UploadError,RetryError,ConfigurationError,AuthError(all inherit fromRequestError)BatchFileResultwith.resultand.problemsaccessorsResource classes
File,Group,FileMetadata,Uploader,Addons,Project,Webhook,DocumentConverter,VideoConverter— all inheriting fromBaseResourceBreaking changes
Entity::*/Client::*→ flat namespaceDry::Configurable→ plain Ruby class (syntax unchanged)check_*_status→*_status(removedcheck_prefix)Testing
Release plan
maingem build && gem installlocallygem push uploadcare-ruby-5.0.0.gemto RubyGemsv5.0.0and changelogSummary by CodeRabbit
New Features
Documentation
Chores