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.

Sign up for GitHub

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

Jump to bottom

feat: Add ApiVersion Support #2462

Merged
merged 6 commits into from
May 9, 2024
Merged

feat: Add ApiVersion Support #2462

merged 6 commits into from
May 9, 2024

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented May 6, 2024 •
edited
Loading

Changes:

  • Pass X-Google-Api-Version as a header as part of requests if apiVersion exists in the Discovery Doc

Corresponding generator change googleapis/google-api-java-client-services#20930

This PR will go in first then merge in the generator change once this is released and in maven central.

Logs:

2024-05-09 14:37:57,833 CONFIG   com.google.api.client.http.HttpTransport - -------------- REQUEST  --------------
GET {URL}
Accept-Encoding: gzip
Authorization: Bearer {BEARER_TOKEN}
User-Agent: DeltaV Example Java App Google-API-Java-Client/2.4.1-SNAPSHOT Google-HTTP-Java-Client/1.44.1 (gzip)
x-goog-user-project: {PROJECT_ID}
x-goog-api-client: gl-java/17.0.10 gdcl/2.4.1 linux/6.6.15
x-google-api-version: v1_20230601

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label May 6, 2024
@lqiu96 lqiu96 mentioned this pull request May 6, 2024
Merged
lqiu96
lqiu96 commented May 8, 2024
View reviewed changes
...ent/src/main/java/com/google/api/client/googleapis/services/AbstractGoogleClientRequest.java
@@ -62,7 +63,8 @@ public abstract class AbstractGoogleClientRequest<T> extends GenericData {
*/
public static final String USER_AGENT_SUFFIX = "Google-API-Java-Client";

private static final String API_VERSION_HEADER = "X-Goog-Api-Client";
private static final String API_CLIENT_HEADER = "X-Goog-Api-Client";
protected static final String API_VERSION_HEADER = "X-Google-Api-Version";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

protected access as this will be used in the generated client classes.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add that (the generated code uses this field to set the header) in the source code comment?

Also if there's any public document available for API versions in https://google.aip.dev, reference it from here.

@lqiu96 lqiu96 requested a review from suztomo May 9, 2024 15:02
@lqiu96 lqiu96 marked this pull request as ready for review May 9, 2024 15:02
@lqiu96 lqiu96 requested a review from a team as a code owner May 9, 2024 15:02
@lqiu96 lqiu96 requested a review from diegomarquezp May 9, 2024 15:02
lqiu96
lqiu96 commented May 9, 2024
View reviewed changes
...src/main/java/com/google/api/client/googleapis/testing/services/MockGoogleClientRequest.java
Comment on lines +53 to +75
/**
* @param client Google client
* @param method HTTP Method
* @param uriTemplate URI template for the path relative to the base URL. If it starts with a "/"
* the base path from the base URL will be stripped out. The URI template can also be a full
* URL. URI template expansion is done using {@link UriTemplate#expand(String, String, Object,
* boolean)}
* @param content HTTP content or {@code null} for none
* @param responseClass response class to parse into
* @param apiVersion ApiVersion to be passed to the header
*/
public MockGoogleClientRequest(
AbstractGoogleClient client,
String method,
String uriTemplate,
HttpContent content,
Class<T> responseClass,
String apiVersion) {
super(client, method, uriTemplate, content, responseClass);
// Matches generator code: Null or Empty String is not set to the header
if (!Strings.isNullOrEmpty(apiVersion)) {
getRequestHeaders().set(API_VERSION_HEADER, apiVersion);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a constructor with new ApiVersion param. This allows us to mimic the behavior where the generated clients pass the apiVersion to the AbstractGoogleClientRequest class.

suztomo
suztomo approved these changes May 9, 2024
View reviewed changes
...ent/src/main/java/com/google/api/client/googleapis/services/AbstractGoogleClientRequest.java
@@ -62,7 +63,8 @@ public abstract class AbstractGoogleClientRequest<T> extends GenericData {
*/
public static final String USER_AGENT_SUFFIX = "Google-API-Java-Client";

private static final String API_VERSION_HEADER = "X-Goog-Api-Client";
private static final String API_CLIENT_HEADER = "X-Goog-Api-Client";
protected static final String API_VERSION_HEADER = "X-Google-Api-Version";
Copy link
Member

Choose a reason for hiding this comment

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

Can you add that (the generated code uses this field to set the header) in the source code comment?

Also if there's any public document available for API versions in https://google.aip.dev, reference it from here.

@lqiu96
Copy link
Contributor Author

lqiu96 commented May 9, 2024

Can you add that (the generated code uses this field to set the header) in the source code comment? Also if there's any public document available for API versions in https://google.aip.dev/, reference it from here.

Will do, thanks!

@lqiu96 lqiu96 merged commit 9f3f64c into main May 9, 2024
16 checks passed
@lqiu96 lqiu96 deleted the add-apiVersion-support branch May 9, 2024 20:21
@release-please release-please bot mentioned this pull request May 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegomarquezp diegomarquezp diegomarquezp approved these changes

@suztomo suztomo suztomo approved these changes

Assignees
No one assigned
Labels
size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lqiu96 @suztomo @diegomarquezp