Skip to content
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.

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

feat: add new Options to allow per method header values #2941

Merged
merged 4 commits into from
Feb 25, 2025

Conversation

BenWhitehead
Copy link
Collaborator

Add new "Option"s for those methods which already have option types to allow providing an ImmutableMap<String, String> to be applied as extra headers to all requests sent as part of that operation.

If an operation has multiple sources of input Options (rewrite) the "first" (i.e. source option) will be the one added to the request.

The following resources do not have "Option"s and therefor do not have extra headers support at this time:

  • Acl
  • DefaultAcl
  • ServiceAccount
  • Notification

Overview of the changes

This blind number of lines is large, but the number of differences is actually fairly low.

  • Add Headers Opt` in UnifiedOpts, this is the primary guts of the new feature and is responsible for holding onto the extra header values and validating the provided headers to ensure no collisions with headers otherwise set by our library/restricted by gcs.
  • The ~800 lines of change in Storage.java are the same "add new option extraHeaders" to each of the 15 option classes along with accompanying javadocs. All javadocs are the exact same, and were copy-pasted for each method.
  • Create new utility method Utils#headerNameToLowerCase to centralize the logic for transforming header names across all locations
  • Add new StorageRpc.Options#EXTRA_HEADERS to support extra headers for JSON calls
    • update JSON request building code in HttpStorageRpc (433 lines), ApiaryUnbufferedReadableByteChannel (13 lines) and JsonResumable* (~45 lines + ~50 lines for existing tests) classes to take EXTRA_HEADERS into consideration
  • test: add new suite of tests ITExtraHeadersOptionTest to validate headers are properly added to each request of various operations
  • test: introduce new interface AssertRequestHeaders which both GrpcRequestAuditing and RequestAuditing now implement -- this interface makes it easy for each transport to share common methods for asserting request headers

Add new "Option"s for those methods which already have option types to allow providing an ImmutableMap<String, String> to be applied as extra headers to all requests sent as part of that operation.

If an operation has multiple sources of input Options (rewrite) the "first" (i.e. source option) will be the one added to the request.

The following resources do not have "Option"s and therefor do not have extra headers support at this time:
* Acl
* DefaultAcl
* ServiceAccount
* Notification
@BenWhitehead BenWhitehead requested a review from a team as a code owner February 19, 2025 19:35
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: storage Issues related to the googleapis/java-storage API. labels Feb 19, 2025
Copy link
Contributor

@ddelgrosso1 ddelgrosso1 left a comment

Choose a reason for hiding this comment

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

I'm not a java expert but LGTM

@BenWhitehead BenWhitehead merged commit 297802d into main Feb 25, 2025
21 checks passed
@BenWhitehead BenWhitehead deleted the extra-headers branch February 25, 2025 18:50
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 googleapis/java-storage API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants