Skip to content

feat(Storage): implement GCS idempotency tokens for all API operations#10

Open
salilg-eng wants to merge 2 commits into
mainfrom
feat/idempotency-token
Open

feat(Storage): implement GCS idempotency tokens for all API operations#10
salilg-eng wants to merge 2 commits into
mainfrom
feat/idempotency-token

Conversation

@salilg-eng

Copy link
Copy Markdown
Owner

This PR adds support for GCS Idempotency Tokens to the Storage client (Fixes #280811217).

The goal here is to send a unique x-goog-gcs-idempotency-token header (UUID) on all JSON API requests. This allows the backend to safely retry operations without the risk of duplicating them.

To get this working correctly across the board—especially for resumable upload chunks which bypass Rest.php—I subclassed RequestWrapper into a new StorageRequestWrapper. This lets us hook into the very bottom of the request flow just for Storage, without polluting the shared google/cloud-core package.

A few technical notes on how it behaves:

Standard Retries: Because we inject the token right before the ExponentialBackoff loop runs, normal retries (like 5xx or 429) will correctly reuse the exact same token.
Mid-stream Downloads: If a download drops mid-stream and we have to fire off a new byte-range request to resume, the retry listener in Rest.php explicitly generates a brand new token for it, as required by the spec.
Header Alignment: To keep things clean, it extracts and reuses the UUID from the gccl-invocation-id metric header when available, rather than generating a second redundant UUID.
Idempotency Config: I moved objects.delete, insert, patch, and update from being conditionally idempotent to fully idempotent in RetryTrait. Since we're sending tokens now, the backend safely handles idempotency for these operations natively.
I've also updated the unit tests in RestTest.php to validate the new wrapper and ensure the token is being preserved or regenerated in the right scenarios. All tests are passing locally!

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces automatic idempotency token generation and injection for Google Cloud Storage requests by implementing a new StorageRequestWrapper and updating the Rest connection class. It also updates retry behaviors and adds corresponding unit and system tests. The review feedback highlights several improvement opportunities, including performing case-insensitive checks when detecting or replacing user-supplied idempotency headers to prevent duplicates, and ensuring that tokens extracted from invocation headers are validated before use.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread Storage/src/Connection/StorageRequestWrapper.php Outdated
Comment thread Storage/src/Connection/StorageRequestWrapper.php Outdated
Comment thread Storage/src/Connection/Rest.php Outdated
@salilg-eng
salilg-eng force-pushed the feat/idempotency-token branch 6 times, most recently from dc0fde8 to 098feee Compare July 6, 2026 08:36
@salilg-eng
salilg-eng force-pushed the feat/idempotency-token branch from 098feee to f0d8907 Compare July 6, 2026 08:48

// Second update uses the exact same UUID, simulating a network retry.
// Even though the metageneration has changed, the backend recognizes
// the idempotency token and returns 200 OK instead of 412 Precondition Failed.

@gurusai-voleti gurusai-voleti Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ensure the idempotency token is resused in retry, don't change anyother header, add the scenario in unit tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants