diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestRunnable.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestRunnable.java index bbeb2362a..5f61588ef 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestRunnable.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestRunnable.java @@ -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; @@ -112,8 +113,8 @@ 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()); } @@ -121,6 +122,41 @@ HttpRequest createHttpRequest() throws IOException { 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; diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java index 0df3815ea..e0a1cb5e0 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java @@ -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 catFormatter; private static HttpResponseParser catParser; - private static ApiMethodDescriptor methodDescriptor; private static PathTemplate nameTemplate = PathTemplate.create("name/{name}"); private static Set queryParams = Sets.newTreeSet(Lists.newArrayList("food", "size", "gibberish")); @@ -139,8 +137,11 @@ public String serialize(EmptyMessage response) { return null; } }; + } - methodDescriptor = + @Test + public void testRequestUrl() throws IOException { + ApiMethodDescriptor methodDescriptor = ApiMethodDescriptor.newBuilder() .setFullMethodName("house.cat.get") .setHttpMethod(null) @@ -148,7 +149,7 @@ public String serialize(EmptyMessage response) { .setResponseParser(catParser) .build(); - httpRequestRunnable = + HttpRequestRunnable httpRequestRunnable = HttpRequestRunnable.newBuilder() .setHttpJsonCallOptions(fakeCallOptions) .setEndpoint(ENDPOINT) @@ -156,12 +157,9 @@ public String serialize(EmptyMessage response) { .setApiMethodDescriptor(methodDescriptor) .setHttpTransport(new MockHttpTransport()) .setJsonFactory(new GsonFactory()) - .setResponseFuture(SettableApiFuture.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"; @@ -170,7 +168,44 @@ public void testRequestUrl() throws IOException { @Test public void testRequestUrlUnnormalized() throws IOException { - httpRequestRunnable = + ApiMethodDescriptor methodDescriptor = + ApiMethodDescriptor.newBuilder() + .setFullMethodName("house.cat.get") + .setHttpMethod("PUT") + .setRequestFormatter(catFormatter) + .setResponseParser(catParser) + .build(); + + HttpRequestRunnable httpRequestRunnable = + HttpRequestRunnable.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 methodDescriptor = + ApiMethodDescriptor.newBuilder() + .setFullMethodName("house.cat.get") + .setHttpMethod("PATCH") + .setRequestFormatter(catFormatter) + .setResponseParser(catParser) + .build(); + + HttpRequestRunnable httpRequestRunnable = HttpRequestRunnable.newBuilder() .setHttpJsonCallOptions(fakeCallOptions) .setEndpoint("www.googleapis.com/animals/v1/projects") @@ -178,13 +213,15 @@ public void testRequestUrlUnnormalized() throws IOException { .setApiMethodDescriptor(methodDescriptor) .setHttpTransport(new MockHttpTransport()) .setJsonFactory(new GsonFactory()) - .setResponseFuture(SettableApiFuture.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 diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/MockHttpService.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/MockHttpService.java index 98584e6d6..68f6fa6b8 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/MockHttpService.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/MockHttpService.java @@ -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; @@ -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();