Skip to content

feat(Storage): update StorageObject::exists to use HEAD request instead of GET#9196

Open
salilg-eng wants to merge 3 commits into
googleapis:mainfrom
salilg-eng:feat/storage-object-exists
Open

feat(Storage): update StorageObject::exists to use HEAD request instead of GET#9196
salilg-eng wants to merge 3 commits into
googleapis:mainfrom
salilg-eng:feat/storage-object-exists

Conversation

@salilg-eng
Copy link
Copy Markdown
Contributor

Fixes / Addresses

Addresses b/437200793

Behavior Change & Motivation

Previously, StorageObject::exists() invoked getObject() (making an HTTP GET request) to verify if an object existed. Following a server-side enhancement to Google Cloud Storage Audit Logs, failed GET requests against non-existent objects started emitting severity ERROR log messages rather than INFO.

Because checking whether an object exists is intended for workflows where an object may not be present, it should not generate false-alarm error severity records in Cloud Audit Logs or trigger unnecessary payload data downloads.

Technical Implementation

  • Introduced headObject() definition in ConnectionInterface and implemented it in Rest.
  • Updated StorageObject::exists() to use headObject() (HTTP HEAD) instead of getObject() (HTTP GET).
  • Updated corresponding snippet and unit test expectations.

@salilg-eng salilg-eng requested review from a team as code owners May 19, 2026 08:01
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 19, 2026
Comment thread Storage/src/Connection/Rest.php Outdated
…ad of GET

Optimizes exists() check on GCS objects using lighter HEAD requests and resolves GCS retry conformance test failures.
Adds : array return type hint to ConnectionInterface and Rest connection client implementations, and fixes GCS retry conformance test TypeError crash by correctly returning GCS response headers array.
@salilg-eng salilg-eng force-pushed the feat/storage-object-exists branch from 8627de5 to 488e1da Compare May 29, 2026 05:42
@salilg-eng salilg-eng requested a review from Hectorhammett May 29, 2026 09:36
@Hectorhammett
Copy link
Copy Markdown
Collaborator

@salilg-eng Thanks so much for updating the PR!

One more thing, there is one test that is failing that should not be failing (PHPSTAN static analysis), can you please update your branch with master? Either a rebase or a merge should be fine!

@salilg-eng
Copy link
Copy Markdown
Contributor Author

Hello @Hectorhammett

The single PHPStan Static Analysis failure is a preexisting upstream repository issue and is completely unrelated to the changes in this PR.

This PR strictly modifies 4 files under the Storage package, which is 100% static-analysis green. The failure occurs entirely under the Spanner component due to the release of PHPStan 2.2.0 yesterday (May 28).

Since the CI workflow uses composer update -d dev on every run, the build automatically pulled the new 2.2.0 release which has stricter sealed array validation rules, flagging preexisting array shapes inside Spanner/src/ValueMapper.php and Spanner/src/Operation.php. Upstream main commits analyzed before yesterday passed solely because they ran under the older 2.1.56 release.

Ready for a final look and merge!

@salilg-eng
Copy link
Copy Markdown
Contributor Author

Hi team,

I did some Git and GitHub API forensics on the two failing Spanner files to understand exactly how they were merged in the first place without these errors blocking them, and why they are blocking my PR now. It turns out they represent two different situations:

  1. ValueMapper.php (PR fix(Spanner): null array offset in value mapper #8949 - Merged on Feb 27, 2026)

    • The checks DID fail on their PR when they merged it!
    • Check Run API URL: https://api.github.com/repos/googleapis/google-cloud-php/check-runs/78433114109
    • Result: conclusion: "failure" (Failing on PHPStan Static Analysis).
    • How it was merged: At the time of their merge, the CS Test Suite / PHPStan check was not marked as a mandatory/required status check on the main branch, allowing the Spanner team to merge the PR despite the failing status.
  2. Operation.php (PR chore(Spanner): V2 cleanup #8665 - Merged on Oct 16, 2025)

    • The checks DID pass on their PR back in October!
    • Why it fails on my PR today: Our CI workflow runs a fresh composer update -d dev on every build run rather than pinning versions via a composer.lock file. Just yesterday (May 28, 2026), PHPStan released versions v2.2.0 and v2.2.1, introducing significantly stricter rules for nested array shapes.
    • A fresh run on my branch today automatically downloads these brand-new, stricter PHPStan versions, which now catches this 7-month-old Spanner return type mismatch and flags it as a failure.

💡 Gaps in Coverage & Prevention Recommendations:

To prevent these pre-existing failures in other packages from constantly blocking active pull requests, the repo owners could consider two actions:

  • Pin Dev/Test Dependencies: Pin dev dependencies (like phpstan/phpstan and code style tools) using a locked file in our Actions workflows, rather than executing a blind composer update on every run. This ensures local and CI check consistency.
  • Resolve the Active Spanner Failures: I would be more than happy to push the Spanner fixes directly to my branch so everything turns completely green, or the repo owners can apply them directly on main so the main build remains healthy! Let me know which you prefer.

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