Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions google-apis-core/lib/google/apis/core/storage_upload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def initiate_resumable_upload(client)
# Reinitiating resumable upload
def reinitiate_resumable_upload(client)
logger.debug { sprintf('Restarting resumable upload command to %s', url) }
check_resumable_upload client
return false unless check_resumable_upload client
upload_io.pos = @offset
end

Expand Down Expand Up @@ -224,6 +224,10 @@ def check_resumable_upload(client)
# Initiating call
response = client.put(@upload_url, "", request_header)
handle_resumable_upload_http_response_codes(response)
if response.status.to_i == 404
logger.debug { "Failed to fetch upload session. Response: #{response.status.to_i} - #{response.body}" }
false
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return true here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to check the upload ID's status failed with a 404 error. That status code means the upload ID is not recognized by the server, so the process is stopping and returning false.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it thanks

end

# Cancel resumable upload
Expand All @@ -235,13 +239,17 @@ def cancel_resumable_upload(client)
response = client.delete(@upload_url, nil, request_header)
handle_resumable_upload_http_response_codes(response)

if !@upload_incomplete && (400..499).include?(response.status.to_i)
is_successful_cancellation =
!@upload_incomplete &&
(400..499).cover?(response.status.to_i) &&
response.status.to_i != 404
if is_successful_cancellation
@close_io_on_finish = true
true # method returns true if upload is successfully cancelled
return true # method returns true if upload is successfully cancelled
else
logger.debug { sprintf("Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}") }
logger.debug { "Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}" }
return false
end

end

def handle_resumable_upload_http_response_codes(response)
Expand Down
36 changes: 36 additions & 0 deletions google-apis-core/spec/google/apis/core/service_spec.rb

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add tests for success cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

success test cases were added and have been merged already here

Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,24 @@
expect(a_request(:put, upload_url)).to have_been_made
end
end
context 'Should return false if wrong upload id is provided' do
let(:upload_id) { 'wrong_id' }
let(:command) do
service.send(
:restart_resumable_upload,
bucket_name, file, upload_id,
options: { upload_chunk_size: 11}
)
end
before(:example) do
stub_request(:put, upload_url)
.to_return(status: 404)
end
it 'should return false' do
expect(command).to be(false)
end
end

end

context 'delete resumable upload with upload_id' do
Expand All @@ -312,6 +330,24 @@
expect(a_request(:delete, upload_url)).to have_been_made
expect(command).to be_truthy
end

context 'Should return false if wrong upload id is provided' do
let(:upload_id) { 'wrong_id' }
let(:command) do
service.send(
:delete_resumable_upload,
bucket_name, upload_id,
options: { upload_chunk_size: 11}
)
end
before(:example) do
stub_request(:delete, upload_url)
.to_return(status: 404)
end
it 'should return false' do
expect(command).to be(false)
end
end
end

context 'with batch' do
Expand Down
39 changes: 39 additions & 0 deletions google-apis-core/spec/google/apis/core/storage_upload_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,26 @@
expect(a_request(:put, upload_url)
.with(body: 'Hello world')).to have_been_made
end

end

context('restart resumable upload with wrong upload_id') do
let(:file) { StringIO.new('Hello world' * 3) }
let(:upload_id) { 'wrong_upload_id' }
let(:upload_url) { "https://www.googleapis.com/zoo/animals?uploadType=resumable&upload_id=#{upload_id}" }

before(:example) do
stub_request(:put, upload_url)
.to_return(status: 404)
end

it 'should return false if wrong upload id is provided' do
stub_request(:put, upload_url)
.to_return(status: 404)
command.options.upload_chunk_size = 11
command.upload_id = upload_id
expect(command.execute(client)).to be_falsy
end
end

context('should not restart resumable upload if upload is completed') do
Expand Down Expand Up @@ -235,6 +255,25 @@
expect(a_request(:put, upload_url)
.with(body: 'Hello world')).to have_not_been_made
end

end

context('delete resumable upload with upload_id') do
let(:file) { StringIO.new('Hello world' * 3) }
let(:upload_id) { 'wrong_upload_id' }
let(:upload_url) { "https://www.googleapis.com/zoo/animals?uploadType=resumable&upload_id=#{upload_id}" }

before(:example) do
stub_request(:delete, upload_url)
.to_return(status: 404)
end

it 'should return false if wrong upload id is provided' do
command.options.upload_chunk_size = 11
command.upload_id = upload_id
command.delete_upload = true
expect(command.execute(client)).to be_falsy
end
end

context('with chunking disabled') do
Expand Down