Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

fix!: Fix PATCH being unsupported #1465

Merged
merged 2 commits into from Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -33,6 +33,7 @@
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;
Expand Down Expand Up @@ -112,15 +113,50 @@ HttpRequest createHttpRequest() throws IOException {
}
}

HttpRequest httpRequest =
requestFactory.buildRequest(getApiMethodDescriptor().getHttpMethod(), url, jsonHttpContent);
HttpRequest httpRequest = buildRequest(requestFactory, url, jsonHttpContent);

for (HttpJsonHeaderEnhancer enhancer : getHeaderEnhancers()) {
enhancer.enhance(httpRequest.getHeaders());
}
httpRequest.setParser(new JsonObjectParser(getJsonFactory()));
return httpRequest;
}

private HttpRequest buildRequest(
HttpRequestFactory requestFactory, GenericUrl url, HttpContent jsonHttpContent)
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.
//
// Long story short, the problems is as follows: gax-httpjson depends on NetHttpTransport class
// from google-http-client, which depends on JDK standard java.net.HttpUrlConnection, which does
// not support PATCH http method.
//
// It is a won't fix for JDK8: https://bugs.openjdk.java.net/browse/JDK-8207840.
// A corresponding google-http-client issue:
// https://github.com/googleapis/google-http-java-client/issues/167
//
// In JDK11 there is java.net.http.HttpRequest with PATCH method support but, gax-httpjson must
// remain compatible with Java 8.
//
// Using "X-HTTP-Method-Override" header is probably the cleanest way to fix it. Other options
// would be: hideous reflection hacks (not a safe option in a generic library, which
// gax-httpjson is), writing own implementation of HttpUrlConnection (fragile and a lot of
// work), depending on v2.ApacheHttpTransport (it has many extra dependencies, does not support
// mtls etc).
String actualHttpMethod = getApiMethodDescriptor().getHttpMethod();
String originalHttpMethod = actualHttpMethod;
if (HttpMethods.PATCH.equals(actualHttpMethod)) {
actualHttpMethod = HttpMethods.POST;
}
HttpRequest httpRequest = requestFactory.buildRequest(actualHttpMethod, url, jsonHttpContent);
if (originalHttpMethod != null && !originalHttpMethod.equals(actualHttpMethod)) {
HttpHeadersUtils.setHeader(
httpRequest.getHeaders(), "X-HTTP-Method-Override", originalHttpMethod);
}
return httpRequest;
}

// This will be frequently executed, so avoiding using regexps if not necessary.
private String normalizeEndpoint(String endpoint) {
String normalized = endpoint;
Expand Down
Expand Up @@ -56,10 +56,8 @@ public class HttpRequestRunnableTest {
private static HttpJsonCallOptions fakeCallOptions;
private static CatMessage catMessage;
private static final String ENDPOINT = "https://www.googleapis.com/animals/v1/projects/";
private static HttpRequestRunnable httpRequestRunnable;
private static HttpRequestFormatter<CatMessage> catFormatter;
private static HttpResponseParser<EmptyMessage> catParser;
private static ApiMethodDescriptor<CatMessage, EmptyMessage> methodDescriptor;
private static PathTemplate nameTemplate = PathTemplate.create("name/{name}");
private static Set<String> queryParams =
Sets.newTreeSet(Lists.newArrayList("food", "size", "gibberish"));
Expand Down Expand Up @@ -139,29 +137,29 @@ public String serialize(EmptyMessage response) {
return null;
}
};
}

methodDescriptor =
@Test
public void testRequestUrl() throws IOException {
ApiMethodDescriptor<CatMessage, EmptyMessage> methodDescriptor =
ApiMethodDescriptor.<CatMessage, EmptyMessage>newBuilder()
.setFullMethodName("house.cat.get")
.setHttpMethod(null)
.setRequestFormatter(catFormatter)
.setResponseParser(catParser)
.build();

httpRequestRunnable =
HttpRequestRunnable<CatMessage, EmptyMessage> httpRequestRunnable =
HttpRequestRunnable.<CatMessage, EmptyMessage>newBuilder()
.setHttpJsonCallOptions(fakeCallOptions)
.setEndpoint(ENDPOINT)
.setRequest(catMessage)
.setApiMethodDescriptor(methodDescriptor)
.setHttpTransport(new MockHttpTransport())
.setJsonFactory(new GsonFactory())
.setResponseFuture(SettableApiFuture.<EmptyMessage>create())
.setResponseFuture(SettableApiFuture.create())
.build();
}

@Test
public void testRequestUrl() throws IOException {
HttpRequest httpRequest = httpRequestRunnable.createHttpRequest();
Truth.assertThat(httpRequest.getContent()).isInstanceOf(EmptyContent.class);
String expectedUrl = ENDPOINT + "name/feline" + "?food=bird&food=mouse&size=small";
Expand All @@ -170,21 +168,60 @@ public void testRequestUrl() throws IOException {

@Test
public void testRequestUrlUnnormalized() throws IOException {
httpRequestRunnable =
ApiMethodDescriptor<CatMessage, EmptyMessage> methodDescriptor =
ApiMethodDescriptor.<CatMessage, EmptyMessage>newBuilder()
.setFullMethodName("house.cat.get")
.setHttpMethod("PUT")
.setRequestFormatter(catFormatter)
.setResponseParser(catParser)
.build();

HttpRequestRunnable<CatMessage, EmptyMessage> httpRequestRunnable =
HttpRequestRunnable.<CatMessage, EmptyMessage>newBuilder()
.setHttpJsonCallOptions(fakeCallOptions)
.setEndpoint("www.googleapis.com/animals/v1/projects")
.setRequest(catMessage)
.setApiMethodDescriptor(methodDescriptor)
.setHttpTransport(new MockHttpTransport())
.setJsonFactory(new GsonFactory())
.setResponseFuture(SettableApiFuture.create())
.build();
HttpRequest httpRequest = httpRequestRunnable.createHttpRequest();
Truth.assertThat(httpRequest.getContent()).isInstanceOf(EmptyContent.class);
String expectedUrl =
"https://www.googleapis.com/animals/v1/projects/name/feline?food=bird&food=mouse&size=small";
Truth.assertThat(httpRequest.getUrl().toString()).isEqualTo(expectedUrl);
Truth.assertThat(httpRequest.getRequestMethod()).isEqualTo("PUT");
Truth.assertThat(httpRequest.getHeaders().get("X-HTTP-Method-Override")).isNull();
}

@Test
public void testRequestUrlUnnormalizedPatch() throws IOException {
ApiMethodDescriptor<CatMessage, EmptyMessage> methodDescriptor =
ApiMethodDescriptor.<CatMessage, EmptyMessage>newBuilder()
.setFullMethodName("house.cat.get")
.setHttpMethod("PATCH")
.setRequestFormatter(catFormatter)
.setResponseParser(catParser)
.build();

HttpRequestRunnable<CatMessage, EmptyMessage> httpRequestRunnable =
HttpRequestRunnable.<CatMessage, EmptyMessage>newBuilder()
.setHttpJsonCallOptions(fakeCallOptions)
.setEndpoint("www.googleapis.com/animals/v1/projects")
.setRequest(catMessage)
.setApiMethodDescriptor(methodDescriptor)
.setHttpTransport(new MockHttpTransport())
.setJsonFactory(new GsonFactory())
.setResponseFuture(SettableApiFuture.<EmptyMessage>create())
.setResponseFuture(SettableApiFuture.create())
.build();
HttpRequest httpRequest = httpRequestRunnable.createHttpRequest();
Truth.assertThat(httpRequest.getContent()).isInstanceOf(EmptyContent.class);
String expectedUrl =
"https://www.googleapis.com/animals/v1/projects/name/feline?food=bird&food=mouse&size=small";
Truth.assertThat(httpRequest.getUrl().toString()).isEqualTo(expectedUrl);
Truth.assertThat(httpRequest.getRequestMethod()).isEqualTo("POST");
Truth.assertThat(httpRequest.getHeaders().get("X-HTTP-Method-Override")).isEqualTo("PATCH");
}

// TODO(andrealin): test request body
Expand Down
Expand Up @@ -29,6 +29,7 @@
*/
package com.google.api.gax.httpjson.testing;

import com.google.api.client.http.HttpMethods;
import com.google.api.client.http.LowLevelHttpRequest;
import com.google.api.client.http.LowLevelHttpResponse;
import com.google.api.client.testing.http.MockHttpTransport;
Expand Down Expand Up @@ -100,8 +101,14 @@ public MockLowLevelHttpResponse getHttpResponse(String httpMethod, String fullTa
String relativePath = getRelativePath(fullTargetUrl);

for (ApiMethodDescriptor methodDescriptor : serviceMethodDescriptors) {
if (!httpMethod.equals(methodDescriptor.getHttpMethod())) {
continue;
// Check the comment in com.google.api.gax.httpjson.HttpRequestRunnable.buildRequest()
// method for details why it is needed.
String descriptorHttpMethod = methodDescriptor.getHttpMethod();
if (!httpMethod.equals(descriptorHttpMethod)) {
if (!(HttpMethods.PATCH.equals(descriptorHttpMethod)
&& HttpMethods.POST.equals(httpMethod))) {
continue;
}
}

PathTemplate pathTemplate = methodDescriptor.getRequestFormatter().getPathTemplate();
Expand Down