Skip to content

storage(feat): Add accept encoding gzip for media operation only #26530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aandreassa merged 8 commits into from
Jun 3, 2026
Merged

storage(feat): Add accept encoding gzip for media operation only #26530

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f82a200
adding header for storage_upload and storage_download
shubhangi-google May 8, 2026
9344f30
udo unwanted change
shubhangi-google May 8, 2026
33c3e42
undo
shubhangi-google May 8, 2026
8b6b896
adding header as optional header
shubhangi-google May 19, 2026
751d8c3
adding
shubhangi-google May 19, 2026
182f1ef
adding
shubhangi-google May 19, 2026
1d3bee1
refactor
shubhangi-google May 19, 2026
a363dab
adding test
shubhangi-google Jun 2, 2026
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
8 changes: 8 additions & 0 deletions google-apis-core/lib/google/apis/core/base_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ def make_storage_upload_command(method, path, options)
template = Addressable::Template.new(root_url + upload_path + path)
command = StorageUploadCommand.new(method, template, client_version: client_version)
command.options = request_options.merge(options)
command.options.header = command.options.header&.dup || {}
unless command.options.header.any? { |k, _| k.to_s.casecmp('accept-encoding') == 0 }

@bajajneha27 bajajneha27 May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we doing this?

@shubhangi-google shubhangi-google May 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we are checking if the accept-encoding header already exists if not then we will add it
(casecmp is being used to check the header with respect to case senstivity)

@bajajneha27 bajajneha27 May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

header is a hashmap, right? Can't we directly check header['accept-encoding] ?

@shubhangi-google shubhangi-google May 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, header is a hash, but it's a standard Ruby Hash where keys are case-sensitive. Because HTTP header names are case-insensitive, a user might have already provided it as 'Accept-Encoding' or 'ACCEPT-ENCODING'.

If we used header['accept-encoding'] directly, it would return nil if the casing didn't match exactly. Using any? with casecmp ensures we detect the header regardless of how the user cased it, so we don't add a duplicate.

command.options.header['Accept-Encoding'] = 'gzip'
end
apply_command_defaults(command)
command
end
Expand Down Expand Up @@ -459,6 +463,10 @@ def make_storage_download_command(method, path, options)
template = Addressable::Template.new(root_url + base_path + path)
command = StorageDownloadCommand.new(method, template, client_version: client_version)
command.options = request_options.merge(options)
command.options.header = command.options.header&.dup || {}
unless command.options.header.any? { |k, _| k.to_s.casecmp('accept-encoding') == 0 }
command.options.header['Accept-Encoding'] = 'gzip'
end
command.query['alt'] = 'media'
apply_command_defaults(command)
command
Expand Down
31 changes: 31 additions & 0 deletions google-apis-core/spec/google/apis/core/service_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@
end.to raise_error(Google::Apis::UniverseDomainError)
end

it 'should not include Accept-Encoding header' do
expect(command.options.header&.[]('Accept-Encoding')).to be_nil
end

include_examples 'with options'
end

Expand Down Expand Up @@ -173,6 +177,10 @@
expect(command.query).to include('alt' => 'media')
end

it 'should not include Accept-Encoding header' do
expect(command.options.header&.[]('Accept-Encoding')).to be_nil
end

include_examples 'with options'
end

Expand All @@ -192,6 +200,10 @@
expect(command.query).to include('alt' => 'media')
end

it 'should include Accept-Encoding header' do
expect(command.options.header['Accept-Encoding']).to eq('gzip')

@aandreassa aandreassa Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a test where you verify different capitalization (e.g. ACCEPT-ENCODING)

@shubhangi-google shubhangi-google Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added

end

include_examples 'with options'
end

Expand All @@ -207,6 +219,10 @@
expect(url).to eql 'https://www.googleapis.com/upload/zoo/animals'
end

it 'should not include Accept-Encoding header' do
expect(command.options.header&.[]('Accept-Encoding')).to be_nil
end

include_examples 'with options'
end

Expand All @@ -222,6 +238,10 @@
expect(url).to eql 'https://www.googleapis.com/upload/zoo/animals'
end

it 'should include Accept-Encoding header' do
expect(command.options.header['Accept-Encoding']).to eq('gzip')
end

include_examples 'with options'
end

Expand Down Expand Up @@ -263,6 +283,12 @@
command
expect(a_request(:put, upload_url)).to have_been_made.twice
end

it 'should include an accept-encoding header' do
command
expect(a_request(:put, upload_url).with { |req| req.headers['Accept-Encoding'] == 'gzip' }).to have_been_made.twice
end

end
context 'not restart resumable upload if upload is completed' do
before(:example) do
Expand All @@ -286,6 +312,11 @@
command
expect(a_request(:put, upload_url)).to have_been_made
end

it 'should include an accept-encoding header' do
command
expect(a_request(:put, upload_url).with { |req| req.headers['Accept-Encoding'] == 'gzip' }).to have_been_made
end
end
end

Expand Down
6 changes: 6 additions & 0 deletions google-apis-core/spec/google/apis/core/storage_download_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
).to have_been_made
end

it 'should include an Accept-Encoding header for media downloads' do
command.execute(client)
expect(a_request(:get, 'https://www.googleapis.com/zoo/animals')
.with { |req| req.headers['Accept-Encoding'].include?('gzip') }).to have_been_made
end

it 'should receive content' do
expect(received).to eql 'Hello world'
end
Expand Down
6 changes: 6 additions & 0 deletions google-apis-core/spec/google/apis/core/storage_upload_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@
expect(a_request(:put, 'https://www.googleapis.com/zoo/animals')
.with(body: 'Hello world')).to have_been_made
end

it 'should include an Accept-Encoding header in the outgoing request' do
command.execute(client)
expect(a_request(:put, 'https://www.googleapis.com/zoo/animals')
.with { |req| req.headers['Accept-Encoding'].include?('gzip') }).to have_been_made
end
end

context('with StringIO input') do
Expand Down
Loading