feat(storage): setting idempotency token header by shubhangi-google · Pull Request #27338 · googleapis/google-api-ruby-client

shubhangi-google · 2026-07-17T08:28:53Z

This pull request introduces support for sending an idempotency token header (X-Goog-Gcs-Idempotency-Token) in API commands

cpriti-os

AI review: there are two issues.

Global Default Scope Leak (Critical Issue):
By setting RequestOptions.default.add_idempotency_token_header = true in google/apis/options.rb, this PR enables the GCS-specific X-Goog-Gcs-Idempotency-Token header globally for all 400+ Google API clients using google-apis-core (e.g., YouTube, Drive, Compute Engine, Calendar, Bigtable). This violates service abstraction boundaries and adds a 36-byte UUID overhead and confusing headers to every non-GCS request.
Fix: Default add_idempotency_token_header to false in google-apis-core and explicitly enable it in the google-apis-storage_v1 client generation or configuration.
Redundant Memoization:
In set_idempotency_token_header, the @idempotency_token ||= SecureRandom.uuid memoization is functionally dead code because of the early return on the line right before it:

return if header.any? { |k, _| k.to_s.downcase == 'x-goog-gcs-idempotency-token' }
@idempotency_token ||= SecureRandom.uuid

Because the header Hash is mutated in place and retained across retries of the ApiCommand instance, the first network attempt injects the header into header. On subsequent retries, the loop evaluates return if header.any? { ... }, successfully finding the original key, and returning early. Therefore, @idempotency_token is never evaluated or used again across retries.
Fix: You can simply do header['X-Goog-Gcs-Idempotency-Token'] = SecureRandom.uuid, as caching the UUID as an instance variable is unnecessary.

bajajneha27 · 2026-09-21T09:55:28Z

+
        def invocation_id_header
-          "gccl-invocation-id/#{SecureRandom.uuid}"
+          @invocation_id ||= SecureRandom.uuid


Why are we making this an instance variable?

without an instance variable, every retry would generate a brand new UUID. We use an instance variable to memoize the UUID.

As discussed offline, please do the required testing.

hi @bajajneha27 we already have test to check the Invocation ID across retries
https://github.com/googleapis/google-api-ruby-client/blob/main/google-apis-core/spec/google/apis/core/api_command_spec.rb#L249
we did the testing ,the functionality is working fine with instance variable

bajajneha27 · 2026-09-21T10:06:37Z


+        def set_idempotency_token_header
+          return if options&.header&.any? { |k, _| k.to_s.downcase == 'x-goog-gcs-idempotency-token' }
+          return if header.any? { |k, _| k.to_s.downcase == 'x-goog-gcs-idempotency-token' }


Can you please help me understand how header is populated?

When a request is made for the first time, this hash doesn't have the idempotency token, so the code populates it with a SecureRandom.uuid.
However, if the API call fails (e.g., due to a 503 Service Unavailable) and the library automatically retries the request, the prepare! method is called again. During this retry, the internal header hash still contains the UUID that was generated on the first attempt. Above line detects this and returns early, ensuring the exact same UUID is sent again on the retry.

My question is how we're populating header hash ?

The header hash is an instance attribute (accessor for @Header) of the command object, which is inherited from the base HttpCommand class. It gets initialized (typically as an empty hash) when the command object is first instantiated.

Throughout the lifecycle of the request, the prepare! method and its helpers (like set_api_client_header, set_user_project_header, and set_idempotency_token_header) mutate this exact same instance variable. Because the library reuses the same command object instance during a retry, the header hash retains the values (like the UUID) that were populated into it during the initial request's prepare! call.

bajajneha27 · 2026-09-21T10:07:22Z

    RequestOptions.default.quota_project = nil
    RequestOptions.default.add_invocation_id_header = false
    RequestOptions.default.upload_chunk_size = 100 * 1024 * 1024 # 100 MB
+    RequestOptions.default.add_idempotency_token_header = false


Is adding idempotency token going to be optional?

I thought it was going to be added by default.

as per this comment #27338 (review)
It has to be optional and default to false in core library, It is being enabled by default inside the Storage API client specifically.

My question is, do we need this option at all?
Isn't the requirement to always keep the header?

x-goog-gcs-idempotency-token is being used for storage operations specifically (as the header name contained x-goog-gcs) we don't need to keep it optional and we make it default as it will be ignored by other libraries (e.g., YouTube, Drive, Compute Engine, Calendar) which are using core library

shubhangi-google · 2026-09-25T10:19:09Z

AI review: there are two issues.

Global Default Scope Leak (Critical Issue):
By setting RequestOptions.default.add_idempotency_token_header = true in google/apis/options.rb, this PR enables the GCS-specific X-Goog-Gcs-Idempotency-Token header globally for all 400+ Google API clients using google-apis-core (e.g., YouTube, Drive, Compute Engine, Calendar, Bigtable). This violates service abstraction boundaries and adds a 36-byte UUID overhead and confusing headers to every non-GCS request.
Fix: Default add_idempotency_token_header to false in google-apis-core and explicitly enable it in the google-apis-storage_v1 client generation or configuration.

Redundant Memoization:
In set_idempotency_token_header, the @idempotency_token ||= SecureRandom.uuid memoization is functionally dead code because of the early return on the line right before it:
return if header.any? { |k, _| k.to_s.downcase == 'x-goog-gcs-idempotency-token' }
@idempotency_token ||= SecureRandom.uuid
Because the header Hash is mutated in place and retained across retries of the ApiCommand instance, the first network attempt injects the header into header. On subsequent retries, the loop evaluates return if header.any? { ... }, successfully finding the original key, and returning early. Therefore, @idempotency_token is never evaluated or used again across retries. Fix: You can simply do header['X-Goog-Gcs-Idempotency-Token'] = SecureRandom.uuid, as caching the UUID as an instance variable is unnecessary.

hi @cpriti-os x-goog-gcs-idempotency-token is being used for storage operations specifically (as the header name contained x-goog-gcs) we don't need to keep it optional and we make it default as it will be ignored by other libraries (e.g., YouTube, Drive, Compute Engine, Calendar) which are using core library

cpriti-os

Requesting changes for a few issues around gating the GCS header, multi-chunk resumable uploads, and test cleanup.

cpriti-os · 2026-10-07T09:23:37Z

          header['X-Goog-User-Project'] = quota_project_id if quota_project_id
        end

+        def set_idempotency_token_header


Please gate this behind return unless options&.add_invocation_id_header (just like invocation_id_header on line 166). google-cloud-storage already sets add_invocation_id_header = true, so it will be enabled automatically for Storage without sending a GCS-specific header or calling SecureRandom.uuid on all 400+ non-GCS APIs (and you can revert the test change in service_spec.rb). Also, you can reuse @invocation_id here instead of calling SecureRandom.uuid a second time, and use k.to_s.casecmp?('x-goog-gcs-idempotency-token').

cpriti-os · 2026-10-07T09:23:37Z

        def prepare!
          set_api_client_header
          set_user_project_header
+          set_idempotency_token_header


Edge case with StorageUploadCommand (storage_upload.rb): prepare! is only called once at the start of execute, which means initiate_resumable_upload, check_resumable_upload, and every chunk's send_upload_command in the while @upload_incomplete loop will send the same X-Goog-Gcs-Idempotency-Token. Per the GCS idempotency spec, each distinct RPC (initiate, status check, each chunk PUT) needs its own token (reused only across retries of that specific RPC). Correct me if this isn't the case

thanks for pointing this out I have made changes in code to ensure distinct RPC step generates a fresh token, while still reusing that specific token if that individual chunk happens to retry.

cpriti-os · 2026-10-07T09:23:37Z

        .with { |req| req.headers['X-Goog-Api-Client'] == invocation_id_header }).to have_been_made.times(2)
    end
+
+    it 'should keep the same gccl-invocation-id across retries' do


This test ('should keep the same gccl-invocation-id across retries') duplicates 'should keep same invocation_id header across retries' right above it on line 280. We can remove this duplicate.

cpriti-os · 2026-10-07T09:23:37Z

    expect(defaults.quota_project).to be_nil
    expect(defaults.upload_chunk_size).to eq(100 * 1024 * 1024)
- end
+  end


Nit: revert unrelated whitespace change.

cpriti-os

Thanks for the updates! Found a retry issue in StorageUploadCommand where the token gets regenerated on retry attempts, plus a couple of spec fixes.

cpriti-os · 2026-10-14T09:59:45Z

        # @return [HTTP::Message]
        # @raise [Google::Apis::ServerError] Unable to send the request
        def send_upload_command(client)
+          refresh_idempotency_token


Calling refresh_idempotency_token inside initiate_resumable_upload, send_upload_command, and check_resumable_upload means that when do_retry retries one of these methods after a transient error (e.g. 503), refresh_idempotency_token will run again on every retry attempt and overwrite the header with a new UUID.

Instead, please move refresh_idempotency_token out of these methods and call it right before each do_retry call in execute(client) (lines 116–126, including before do_retry :cancel_resumable_upload, client). That way each distinct RPC step gets a fresh token, while retries of the same step inside do_retry keep the same token.

cpriti-os · 2026-10-14T09:59:45Z

        .with { |req| req.headers['X-Goog-Api-Client'] == invocation_id_header }).to have_been_made.times(2)
    end
+
+    it 'should keep same idempotency_token across retries' do


command.options.add_invocation_id_header = true is missing here, so idempotency_token_header is nil and this test is asserting nil == nil. Please set command.options.add_invocation_id_header = true and add expect(idempotency_token_header).not_to be_nil.

cpriti-os · 2026-10-14T09:59:45Z

+      expect(cmd1.header['X-Goog-Gcs-Idempotency-Token']).not_to eq(cmd2.header['X-Goog-Gcs-Idempotency-Token'])
+    end
+
+    it "should respect custom X-Goog-Gcs-Idempotency-Token in options.header regardless of casing" do


Please also set command.options.add_invocation_id_header = true in this test—otherwise set_idempotency_token_header returns on the first guard (return unless options&.add_invocation_id_header) without testing the custom header check.

cpriti-os · 2026-10-14T09:59:45Z

+
+      expect(command).to have_received(:refresh_idempotency_token).twice
+    end
+


Could we also add a test here where send_upload_command fails once with a 503 and succeeds on retry, verifying that (1) the initial POST and the chunk PUT have different X-Goog-Gcs-Idempotency-Token values, and (2) both attempts of the retried chunk PUT carry the same X-Goog-Gcs-Idempotency-Token?

cpriti-os · 2026-10-14T09:59:45Z

      Google::Apis::RequestOptions.default.authorization = 'a token'
      service.batch_upload do |service|
        command = service.send(:make_upload_command, :post, 'zoo/animals', {})
+        command.options.add_invocation_id_header = true


Nit: since add_invocation_id_header defaults to false, we can revert the changes to service_spec.rb so the existing batch upload spec stays as-is.

cpriti-os

LGTM!

shubhangi-google added 15 commits April 16, 2025 12:35

setting idempotency header

c553b86

setting new token value

2f5a45a

fixing spec

a7a03f4

Merge branch 'main' into add_idempotency_token

1940188

checking token in retry case

b53b80d

updating token value

cd1a711

updating test case

e8f611a

removing not needed code

f032d76

Merge branch 'main' into add_idempotency_token

c59ec25

removing idempotency token header from options

a6db642

wip

f34f24c

adding lazzy load

af8dd7d

additional changes

fa82104

Merge branch 'main' into add_idempotency_token_modified

67e3088

Merge branch 'main' into add_idempotency_token_modified

3c6fa07

shubhangi-google mentioned this pull request Jul 17, 2026

feat(storage): Support for idempotency_token_header googleapis/google-cloud-ruby#34956

Closed

shubhangi-google marked this pull request as ready for review July 17, 2026 10:58

shubhangi-google requested a review from a team as a code owner July 17, 2026 10:58

cpriti-os suggested changes Sep 11, 2026

View reviewed changes

shubhangi-google added 3 commits September 15, 2026 12:39

Merge branch 'main' into add_idempotency_token_modified

dae7d41

removing redundant Memoization

ece2fd9

updating test

25612b9

shubhangi-google marked this pull request as draft September 16, 2026 06:01

fixing tests

6a92844

shubhangi-google marked this pull request as ready for review September 18, 2026 06:54

shubhangi-google requested a review from cpriti-os September 18, 2026 07:07

bajajneha27 suggested changes Sep 21, 2026

View reviewed changes

making idempotency header as default

60e489d

shubhangi-google requested a review from bajajneha27 September 30, 2026 09:25

cpriti-os suggested changes Oct 7, 2026

View reviewed changes

refactor resumable upload functiality

8092355

shubhangi-google requested a review from cpriti-os October 9, 2026 12:14

amanda-tarafa added the api: storage Issues related to the Cloud Storage API. label Oct 10, 2026

cpriti-os suggested changes Oct 14, 2026

View reviewed changes

shubhangi-google added 2 commits October 15, 2026 11:44

refactoring retry logic

7c267c6

undo spec

ed95b6f

shubhangi-google requested a review from cpriti-os October 15, 2026 12:18

cpriti-os approved these changes Oct 15, 2026

View reviewed changes


		expect(command).to have_received(:refresh_idempotency_token).twice
		end

Conversation

shubhangi-google commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpriti-os left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shubhangi-google commented Sep 25, 2026

Uh oh!

cpriti-os left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpriti-os Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpriti-os left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhangi-google commented Jul 17, 2026 •

edited

Loading

cpriti-os left a comment •

edited

Loading

cpriti-os Oct 7, 2026 •

edited

Loading