Skip to content

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

Open
salilg-eng wants to merge 4 commits into
googleapis:mainfrom
salilg-eng:feat/idempotency-token
Open

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

Conversation

@salilg-eng

Copy link
Copy Markdown
Contributor

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!

@salilg-eng
salilg-eng requested review from a team as code owners August 13, 2026 03:33
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Aug 13, 2026
@google-cla

google-cla Bot commented Aug 13, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Comment thread Storage/src/Connection/StorageRequestWrapper.php Outdated
Comment thread Storage/tests/Unit/Connection/RestTest.php Outdated
Comment thread Storage/src/Connection/StorageRequestWrapper.php Outdated
Comment thread Storage/tests/System/ManageObjectsTest.php Outdated
@salilg-eng
salilg-eng force-pushed the feat/idempotency-token branch from 66b4ed7 to 02d4e0f Compare August 13, 2026 07:38
@cy-yun

cy-yun commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

please resolve the failing cla check

@cy-yun cy-yun left a comment

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.

Overall, great approach using StorageRequestWrapper to seamlessly add the token at the bottom of the request flow! The token generation and retry header fallback look very solid.

However, it looks like you forgot to commit the changes to RetryTrait.php that you mentioned in the PR description ("I moved objects.delete, insert, patch, and update from being conditionally idempotent to fully idempotent...").

You can use the suggestions below to apply the missing changes:

File: Storage/src/Connection/RetryTrait.php
Line: ~71-73 (End of $idempotentOps array)

        'serviceaccount.get',
        'signBlob.execute',
        'objects.delete',
        'objects.insert',
        'objects.patch',
        'objects.update'
    ];

File: Storage/src/Connection/RetryTrait.php
Line: ~80-93 ($condIdempotentOps array)

    private static $condIdempotentOps = [
        'buckets.patch' => ['ifMetagenerationMatch', 'etag'],
        // Currently etag is not supported, so this preCondition never available
        'buckets.setIamPolicy' => ['etag'],
        'buckets.update' => ['ifMetagenerationMatch', 'etag'],
        'hmacKey.update' => ['etag'],
        'objects.compose' => ['ifGenerationMatch'],
        'objects.copy' => ['ifGenerationMatch'],
        'objects.rewrite' => ['ifGenerationMatch']
    ];

@salilg-eng

Copy link
Copy Markdown
Contributor Author

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.

@salilg-eng
salilg-eng force-pushed the feat/idempotency-token branch 2 times, most recently from 019088e to 89047db Compare August 19, 2026 03:32
@salilg-eng
salilg-eng requested a review from cy-yun August 19, 2026 03:50
@salilg-eng
salilg-eng force-pushed the feat/idempotency-token branch from 22e86e7 to 8e6664d Compare August 24, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants