-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(gax): allow non-JSON HttpContent and absolute target URLs in HttpRequest Formatter and Runnable #14085
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(gax): allow non-JSON HttpContent and absolute target URLs in HttpRequest Formatter and Runnable #14085
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -29,26 +29,20 @@ | |||||
| */ | ||||||
| package com.google.api.gax.httpjson; | ||||||
|
|
||||||
| import com.google.api.client.http.EmptyContent; | ||||||
| import com.google.api.client.http.GenericUrl; | ||||||
| import com.google.api.client.http.HttpContent; | ||||||
| import com.google.api.client.http.HttpMediaType; | ||||||
| import com.google.api.client.http.HttpMethods; | ||||||
| import com.google.api.client.http.HttpRequest; | ||||||
| import com.google.api.client.http.HttpRequestFactory; | ||||||
| import com.google.api.client.http.HttpResponse; | ||||||
| import com.google.api.client.http.HttpResponseException; | ||||||
| import com.google.api.client.http.HttpTransport; | ||||||
| import com.google.api.client.http.json.JsonHttpContent; | ||||||
| import com.google.api.client.json.JsonFactory; | ||||||
| import com.google.api.client.json.JsonObjectParser; | ||||||
| import com.google.api.client.json.gson.GsonFactory; | ||||||
| import com.google.api.client.util.GenericData; | ||||||
| import com.google.api.gax.tracing.ApiTracer; | ||||||
| import com.google.auth.Credentials; | ||||||
| import com.google.auth.http.HttpCredentialsAdapter; | ||||||
| import com.google.auto.value.AutoValue; | ||||||
| import com.google.common.base.Strings; | ||||||
| import java.io.ByteArrayInputStream; | ||||||
| import java.io.IOException; | ||||||
| import java.io.InputStream; | ||||||
|
|
@@ -154,8 +148,6 @@ public void run() { | |||||
| } | ||||||
|
|
||||||
| HttpRequest createHttpRequest() throws IOException { | ||||||
| GenericData tokenRequest = new GenericData(); | ||||||
|
|
||||||
| HttpRequestFormatter<RequestT> requestFormatter = methodDescriptor.getRequestFormatter(); | ||||||
|
|
||||||
| HttpRequestFactory requestFactory; | ||||||
|
|
@@ -166,24 +158,18 @@ HttpRequest createHttpRequest() throws IOException { | |||||
| requestFactory = httpTransport.createRequestFactory(); | ||||||
| } | ||||||
|
|
||||||
| JsonFactory jsonFactory = GsonFactory.getDefaultInstance(); | ||||||
| // Create HTTP request body. | ||||||
| String requestBody = requestFormatter.getRequestBody(request); | ||||||
| HttpContent jsonHttpContent; | ||||||
| if (!Strings.isNullOrEmpty(requestBody)) { | ||||||
| jsonFactory.createJsonParser(requestBody).parse(tokenRequest); | ||||||
| jsonHttpContent = | ||||||
| new JsonHttpContent(jsonFactory, tokenRequest) | ||||||
| .setMediaType((new HttpMediaType("application/json; charset=utf-8"))); | ||||||
| } else { | ||||||
| // Force underlying HTTP lib to set Content-Length header to avoid 411s. | ||||||
| // See EmptyContent.java. | ||||||
| jsonHttpContent = new EmptyContent(); | ||||||
| } | ||||||
| HttpContent httpContent = requestFormatter.getHttpContent(request); | ||||||
|
|
||||||
| // Populate URL path and query parameters. | ||||||
| String normalizedEndpoint = normalizeEndpoint(endpoint); | ||||||
| GenericUrl url = new GenericUrl(normalizedEndpoint + requestFormatter.getPath(request)); | ||||||
| String path = requestFormatter.getPath(request); | ||||||
| GenericUrl url; | ||||||
| if (path.startsWith("http://") || path.startsWith("https://")) { | ||||||
| url = new GenericUrl(path); | ||||||
| } else { | ||||||
| String normalizedEndpoint = normalizeEndpoint(endpoint); | ||||||
| url = new GenericUrl(normalizedEndpoint + path); | ||||||
| } | ||||||
| Map<String, List<String>> queryParams = requestFormatter.getQueryParamNames(request); | ||||||
| for (Entry<String, List<String>> queryParam : queryParams.entrySet()) { | ||||||
| if (queryParam.getValue() != null) { | ||||||
|
|
@@ -196,20 +182,20 @@ HttpRequest createHttpRequest() throws IOException { | |||||
| tracer.requestUrlResolved(url.build()); | ||||||
| } | ||||||
|
|
||||||
| HttpRequest httpRequest = buildRequest(requestFactory, url, jsonHttpContent); | ||||||
| HttpRequest httpRequest = buildRequest(requestFactory, url, httpContent); | ||||||
|
|
||||||
| for (Map.Entry<String, Object> entry : headers.getHeaders().entrySet()) { | ||||||
| HttpHeadersUtils.setHeader( | ||||||
| httpRequest.getHeaders(), entry.getKey(), (String) entry.getValue()); | ||||||
| } | ||||||
|
|
||||||
| httpRequest.setParser(new JsonObjectParser(jsonFactory)); | ||||||
| httpRequest.setParser(new JsonObjectParser(GsonFactory.getDefaultInstance())); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid allocating a new
Suggested change
References
|
||||||
|
|
||||||
| return httpRequest; | ||||||
| } | ||||||
|
|
||||||
| private HttpRequest buildRequest( | ||||||
| HttpRequestFactory requestFactory, GenericUrl url, HttpContent jsonHttpContent) | ||||||
| HttpRequestFactory requestFactory, GenericUrl url, HttpContent httpContent) | ||||||
| throws IOException { | ||||||
| // A workaround to support PATCH request. This assumes support of "X-HTTP-Method-Override" | ||||||
| // header on the server side, which GCP services usually do. | ||||||
|
|
@@ -235,7 +221,7 @@ private HttpRequest buildRequest( | |||||
| if (HttpMethods.PATCH.equals(actualHttpMethod)) { | ||||||
| actualHttpMethod = HttpMethods.POST; | ||||||
| } | ||||||
| HttpRequest httpRequest = requestFactory.buildRequest(actualHttpMethod, url, jsonHttpContent); | ||||||
| HttpRequest httpRequest = requestFactory.buildRequest(actualHttpMethod, url, httpContent); | ||||||
| if (originalHttpMethod != null && !originalHttpMethod.equals(actualHttpMethod)) { | ||||||
| HttpHeadersUtils.setHeader( | ||||||
| httpRequest.getHeaders(), "X-HTTP-Method-Override", originalHttpMethod); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the absolute URL detection robust against uppercase schemes (e.g.,
HTTP://orHTTPS://) without allocating new string objects, consider usingregionMatcheswith case-insensitivity enabled.