Skip to content

Commit cd3ea32

Browse files
committed
Fix RuboCop offenses in VCR scrubber and parallel upload
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
1 parent 3abc2e9 commit cd3ea32

File tree

4 files changed

+73
-70
lines changed

4 files changed

+73
-70
lines changed

lib/uploadcare/clients/rest_client.rb

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,25 +144,21 @@ def build_request_uri(path, params, method)
144144
end
145145

146146
def prepare_headers(req, method, uri, params, headers)
147-
# For authentication, we need to know the body content for signature generation
148-
body_content = if method == HTTP_GET
149-
''
150-
else
151-
if params.nil? || (params.respond_to?(:empty?) && params.empty?)
152-
''
153-
elsif params.is_a?(String)
154-
params
155-
else
156-
params.to_json
157-
end
158-
end
159-
147+
body_content = body_content_for_signature(method, params)
160148
content_type = headers['Content-Type'] || authenticator.default_headers['Content-Type']
161149
auth_headers = authenticator.headers(method, uri, body_content, content_type)
162150
req.headers.merge!(auth_headers)
163151
req.headers.merge!(headers)
164152
end
165153

154+
def body_content_for_signature(method, params)
155+
return '' if method == HTTP_GET
156+
return '' if params.nil? || (params.respond_to?(:empty?) && params.empty?)
157+
return params if params.is_a?(String)
158+
159+
params.to_json
160+
end
161+
166162
def prepare_body_or_params(req, method, params)
167163
if method == HTTP_GET
168164
req.params.update(params) unless params.nil? || params.empty?

lib/uploadcare/clients/upload_client.rb

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ def upload_parts_sequential(file, presigned_urls, part_size, &block)
621621
# @api private
622622
def upload_parts_parallel(file, presigned_urls, part_size, threads, &block)
623623
total_size = file.respond_to?(:size) ? file.size : ::File.size(file.path)
624-
uploaded = 0
624+
uploaded = { value: 0 }
625625
mutex = Mutex.new
626626
queue = Queue.new
627627

@@ -630,47 +630,52 @@ def upload_parts_parallel(file, presigned_urls, part_size, threads, &block)
630630

631631
errors = []
632632
file_path = file.path
633+
total_parts = presigned_urls.length
633634

634635
workers = threads.times.map do
635636
Thread.new do
636-
worker_file = ::File.open(file_path, 'rb')
637-
begin
638-
loop do
639-
job = begin
640-
queue.pop
641-
rescue ThreadError
642-
break
643-
end
644-
break if job.nil?
645-
646-
presigned_url, index = job
647-
offset = index * part_size
648-
break if offset >= total_size
649-
650-
worker_file.seek(offset)
651-
part_data = worker_file.read(part_size)
652-
break if part_data.nil? || part_data.empty?
653-
654-
Uploadcare::Result.unwrap(multipart_upload_part(presigned_url: presigned_url, part_data: part_data))
655-
656-
mutex.synchronize do
657-
uploaded += part_data.bytesize
658-
block&.call({ uploaded: uploaded, total: total_size, part: index + 1,
659-
total_parts: presigned_urls.length })
660-
end
661-
end
662-
rescue StandardError => e
663-
mutex.synchronize { errors << e }
664-
ensure
665-
worker_file.close
666-
end
637+
run_parallel_worker(queue, file_path, part_size, total_size, total_parts, mutex, uploaded, errors, &block)
667638
end
668639
end
669640

670641
workers.each(&:join)
671642
raise errors.first if errors.any?
672643
end
673644

645+
def run_parallel_worker(queue, file_path, part_size, total_size, total_parts, mutex, uploaded, errors, &block)
646+
worker_file = ::File.open(file_path, 'rb')
647+
begin
648+
loop do
649+
job = begin
650+
queue.pop
651+
rescue ThreadError
652+
break
653+
end
654+
break if job.nil?
655+
656+
presigned_url, index = job
657+
offset = index * part_size
658+
break if offset >= total_size
659+
660+
worker_file.seek(offset)
661+
part_data = worker_file.read(part_size)
662+
break if part_data.nil? || part_data.empty?
663+
664+
Uploadcare::Result.unwrap(multipart_upload_part(presigned_url: presigned_url, part_data: part_data))
665+
666+
mutex.synchronize do
667+
uploaded[:value] += part_data.bytesize
668+
block&.call({ uploaded: uploaded[:value], total: total_size, part: index + 1,
669+
total_parts: total_parts })
670+
end
671+
end
672+
rescue StandardError => e
673+
mutex.synchronize { errors << e }
674+
ensure
675+
worker_file.close
676+
end
677+
end
678+
674679
def success_response?(response)
675680
response.status >= 200 && response.status < 300
676681
end

spec/support/vcr.rb

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,25 @@
2020
end
2121

2222
if (auth = i.request.headers['Authorization']&.first)
23-
auth = auth.gsub(/\AUploadcare\\.Simple\\s+[^:\\s]+:[^\\s]+\\z/, 'Uploadcare.Simple <uploadcare_public_key>:<uploadcare_secret_key>')
24-
auth = auth.gsub(/\AUploadcare\\s+[^:\\s]+:[^\\s]+\\z/, 'Uploadcare <uploadcare_public_key>:<uploadcare_rest_signature>')
23+
auth = auth.gsub(/\AUploadcare\.Simple\s+[^:\s]+:[^\s]+\z/,
24+
'Uploadcare.Simple <uploadcare_public_key>:<uploadcare_secret_key>')
25+
auth = auth.gsub(/\AUploadcare\s+[^:\s]+:[^\s]+\z/,
26+
'Uploadcare <uploadcare_public_key>:<uploadcare_rest_signature>')
2527
i.request.headers['Authorization'] = [auth]
2628
end
2729

28-
if i.request.uri
29-
i.request.uri = i.request.uri.gsub(/([?&]token=)[^&]+/, '\\1<uploadcare_upload_token>')
30-
end
30+
i.request.uri = i.request.uri.gsub(/([?&]token=)[^&]+/, '\1<uploadcare_upload_token>') if i.request.uri
3131

3232
if i.request.body
33-
i.request.body = i.request.body.gsub(/(\"token\"\\s*:\\s*\")[^\"]+(\"\\s*(?:\\x7D|\\x5D))/, '\\1<uploadcare_upload_token>\\2')
34-
i.request.body = i.request.body.gsub(/(name=\"signature\"\\r\\n\\r\\n)[^\\r\\n]+/, '\\1<uploadcare_signature>')
35-
i.request.body = i.request.body.gsub(/(name=\"expire\"\\r\\n\\r\\n)[^\\r\\n]+/, '\\1<uploadcare_expire>')
33+
i.request.body = i.request.body.gsub(/("token"\s*:\s*")[^"]+("\s*(?:\x7D|\x5D))/,
34+
'\1<uploadcare_upload_token>\2')
35+
i.request.body = i.request.body.gsub(/(name="signature"\r\n\r\n)[^\r\n]+/, '\1<uploadcare_signature>')
36+
i.request.body = i.request.body.gsub(/(name="expire"\r\n\r\n)[^\r\n]+/, '\1<uploadcare_expire>')
3637
end
3738

3839
if i.response.body.is_a?(String)
39-
i.response.body = i.response.body.gsub(/(\"token\"\\s*:\\s*\")[^\"]+(\"\\s*(?:\\x7D|\\x5D))/, '\\1<uploadcare_upload_token>\\2')
40+
i.response.body = i.response.body.gsub(/("token"\s*:\s*")[^"]+("\s*(?:\x7D|\x5D))/,
41+
'\1<uploadcare_upload_token>\2')
4042
end
4143
end
4244
config.configure_rspec_metadata!

spec/uploadcare/multi_account_spec.rb

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
require 'spec_helper'
44

55
RSpec.describe 'Multi-account configuration' do
6-
let(:config1) do
6+
let(:account_one_config) do
77
Uploadcare::Configuration.new(
88
public_key: 'public_key1',
99
secret_key: 'secret_key1',
@@ -12,7 +12,7 @@
1212
)
1313
end
1414

15-
let(:config2) do
15+
let(:account_two_config) do
1616
Uploadcare::Configuration.new(
1717
public_key: 'public_key2',
1818
secret_key: 'secret_key2',
@@ -22,38 +22,38 @@
2222
end
2323

2424
it 'generates different CDN bases per Configuration instance' do
25-
base1 = config1.cdn_base.call
26-
base2 = config2.cdn_base.call
25+
base1 = account_one_config.cdn_base.call
26+
base2 = account_two_config.cdn_base.call
2727

2828
expect(base1).not_to eq(base2)
2929
expect(base1).to match(%r{\Ahttps://[0-9a-z]{10}\.ucarecd\.net/\z})
3030
expect(base2).to match(%r{\Ahttps://[0-9a-z]{10}\.ucarecd\.net/\z})
3131
end
3232

3333
it 'uses the instance config when building resource CDN URLs' do
34-
file1 = Uploadcare::File.new({ uuid: 'uuid-1' }, config1)
35-
file2 = Uploadcare::File.new({ uuid: 'uuid-2' }, config2)
34+
file1 = Uploadcare::File.new({ uuid: 'uuid-1' }, account_one_config)
35+
file2 = Uploadcare::File.new({ uuid: 'uuid-2' }, account_two_config)
3636

37-
expect(file1.cdn_url).to start_with(config1.cdn_base.call)
38-
expect(file2.cdn_url).to start_with(config2.cdn_base.call)
39-
expect(file1.cdn_url).not_to start_with(config2.cdn_base.call)
40-
expect(file2.cdn_url).not_to start_with(config1.cdn_base.call)
37+
expect(file1.cdn_url).to start_with(account_one_config.cdn_base.call)
38+
expect(file2.cdn_url).to start_with(account_two_config.cdn_base.call)
39+
expect(file1.cdn_url).not_to start_with(account_two_config.cdn_base.call)
40+
expect(file2.cdn_url).not_to start_with(account_one_config.cdn_base.call)
4141
end
4242

4343
it 'generates secure auth headers using the client config' do
44-
config1.auth_type = 'Uploadcare'
45-
config2.auth_type = 'Uploadcare'
44+
account_one_config.auth_type = 'Uploadcare'
45+
account_two_config.auth_type = 'Uploadcare'
4646

47-
auth1 = Uploadcare::Authenticator.new(config: config1)
48-
auth2 = Uploadcare::Authenticator.new(config: config2)
47+
auth1 = Uploadcare::Authenticator.new(config: account_one_config)
48+
auth2 = Uploadcare::Authenticator.new(config: account_two_config)
4949

5050
allow(Time).to receive(:now).and_return(Time.at(0))
5151

5252
h1 = auth1.headers('GET', '/files/', '')
5353
h2 = auth2.headers('GET', '/files/', '')
5454

55-
expect(h1['Authorization']).to start_with("Uploadcare #{config1.public_key}:")
56-
expect(h2['Authorization']).to start_with("Uploadcare #{config2.public_key}:")
55+
expect(h1['Authorization']).to start_with("Uploadcare #{account_one_config.public_key}:")
56+
expect(h2['Authorization']).to start_with("Uploadcare #{account_two_config.public_key}:")
5757
expect(h1['Authorization']).not_to eq(h2['Authorization'])
5858
end
5959
end

0 commit comments

Comments
 (0)