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
46 changes: 46 additions & 0 deletions google-cloud-storage/lib/google/cloud/storage/bucket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,32 @@ def default_kms_key= new_default_kms_key
patch_gapi! :encryption
end

##
# Restart resumable upload
# @param [String, ::File] file Path of the file on the filesystem to
# upload. Can be an File object, or File-like object such as StringIO.
# (If the object does not have path, a `path` argument must be also be
# provided.)
Comment on lines +724 to +725

Choose a reason for hiding this comment

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

medium

The documentation for the file parameter is confusing. It mentions that a path argument must be provided if the file object doesn't have a path, but this method does not accept a path argument. This part of the documentation seems to be a leftover from a copy-paste and should be removed.

# @param [String] upload_id Unique Id of a resumable upload
#
# @return [Google::Apis::StorageV1::Object]
# The newly created storage object.
#
# @example
# require "google/cloud/storage"
#
# storage = Google::Cloud::Storage.new
#
# bucket = storage.bucket "my-bucket"
# bucket.restart_resumable_upload file,upload_id

Choose a reason for hiding this comment

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

medium

For better readability and to adhere to common Ruby style, please add a space after the comma separating the arguments in the example.

        #   bucket.restart_resumable_upload file, upload_id


def restart_resumable_upload file, upload_id
ensure_service!
ensure_io_or_file_exists! file
raise ArgumentError, "Upload Id missing" unless upload_id
service.restart_resumable_upload name, file, upload_id
end

##
# The period of time (in seconds) that files in the bucket must be
# retained, and cannot be deleted, overwritten, or archived.
Expand Down Expand Up @@ -1410,6 +1436,26 @@ def delete if_metageneration_match: nil, if_metageneration_not_match: nil
user_project: user_project
end

##
# Delete resumable upload
# @param [String] upload_id Unique Id of an resumable upload

Choose a reason for hiding this comment

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

medium

There's a small grammatical error in the documentation. It should be "a resumable upload" instead of "an resumable upload".

        # @param [String] upload_id Unique Id of a resumable upload

#
# @return [Boolean] Returns `true` if the resumable upload was deleted.
#
# @example
# require "google/cloud/storage"
#
# storage = Google::Cloud::Storage.new
#
# bucket = storage.bucket "my-bucket"
# bucket.delete_resumable_upload upload_id

def delete_resumable_upload upload_id
ensure_service!
raise ArgumentError, "Upload Id missing" unless upload_id
service.delete_resumable_upload name, upload_id
end

##
# Retrieves a list of files matching the criteria.
#
Expand Down
12 changes: 12 additions & 0 deletions google-cloud-storage/lib/google/cloud/storage/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,18 @@ def delete_file bucket_name,
end
end

def restart_resumable_upload bucket_name, source, upload_id, options: {}
execute do
service.restart_resumable_upload bucket_name, source, upload_id, options: options
end
end

def delete_resumable_upload bucket_name, upload_id, options: {}
execute do
service.delete_resumable_upload bucket_name, upload_id, options: options
end
end

##
# Restore soft deleted bucket
def restore_bucket bucket_name,
Expand Down
63 changes: 63 additions & 0 deletions google-cloud-storage/test/google/cloud/storage/bucket_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,69 @@
end
end

it "restarts a resumable upload with upload_id" do
new_file_name = random_file_path
upload_id = "TEST_ID"

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world"
tmpfile.rewind
mock = Minitest::Mock.new
expected_return_value = create_file_gapi(bucket.name, new_file_name)
mock.expect :restart_resumable_upload, expected_return_value,
[bucket.name, tmpfile, upload_id],
**resumable_upload_args(options: {})
bucket.service.mocked_service = mock
returned_value= bucket.restart_resumable_upload tmpfile, upload_id
assert_equal expected_return_value, returned_value
mock.verify
end
end

it "raises ArgumentError if upload_id is not provided to restart_resumable_upload" do
new_file_name = random_file_path
upload_id = "TEST_ID"

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world"
tmpfile.rewind
mock = Minitest::Mock.new
mock.expect :restart_resumable_upload, create_file_gapi(bucket.name, new_file_name),
[bucket.name, tmpfile, upload_id],
**resumable_upload_args(options: {})
bucket.service.mocked_service = mock
expect do
bucket.restart_resumable_upload tmpfile
end.must_raise ArgumentError
end
end
Comment on lines +1430 to +1446

Choose a reason for hiding this comment

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

medium

This test can be simplified. The mock setup is unnecessary because the ArgumentError is raised before the service method is called. The unused variables new_file_name and upload_id can also be removed. This makes the test simpler and more focused on what it's actually testing: the argument validation.

  it "raises ArgumentError if upload_id is not provided to restart_resumable_upload" do
    Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
      tmpfile.write "Hello world"
      tmpfile.rewind
      expect do
        bucket.restart_resumable_upload tmpfile
      end.must_raise ArgumentError
    end
  end


it "deletes a resumable upload with upload_id" do
upload_id = "TEST_ID"

mock = Minitest::Mock.new
expected_return_value = true
mock.expect :delete_resumable_upload, expected_return_value,
[bucket.name, upload_id],
**resumable_upload_args(options: {})
bucket.service.mocked_service = mock
returned_value = bucket.delete_resumable_upload upload_id
assert_equal expected_return_value, returned_value
mock.verify
end

it "raises ArgumentError if upload_id is not provided to delete_resumable_upload" do
upload_id = "TEST_ID"
mock = Minitest::Mock.new
mock.expect :delete_resumable_upload, true,
[bucket.name, upload_id],
**resumable_upload_args(options: {})
bucket.service.mocked_service = mock
expect do
bucket.delete_resumable_upload
end.must_raise ArgumentError
end
Comment on lines +1462 to +1472

Choose a reason for hiding this comment

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

medium

Similar to the other argument error test, the mock setup here is unnecessary because the ArgumentError is raised before the service method is called. The test can be simplified by removing the mock and the unused upload_id variable.

  it "raises ArgumentError if upload_id is not provided to delete_resumable_upload" do
    expect do
      bucket.delete_resumable_upload
    end.must_raise ArgumentError
  end


def create_file_gapi bucket=nil, name = nil
Google::Apis::StorageV1::Object.from_json random_file_hash(bucket, name).to_json
end
Expand Down
6 changes: 6 additions & 0 deletions google-cloud-storage/test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@ def insert_object_args name: nil,
}
end

def resumable_upload_args options: {}
{
options: options
}
end

def get_object_args generation: nil,
if_generation_match: nil,
if_generation_not_match: nil,
Expand Down