fix: Handling http method override in ErrorHandlingHttpClient

src/main/java/com/google/firebase/auth/internal/AuthHttpClient.java

@@ -16,16 +16,12 @@
 
 package com.google.firebase.auth.internal;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.api.client.http.GenericUrl;
 import com.google.api.client.http.HttpContent;
 import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponseInterceptor;
 import com.google.api.client.http.json.JsonHttpContent;
 import com.google.api.client.json.JsonFactory;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.firebase.IncomingHttpResponse;
 import com.google.firebase.auth.FirebaseAuthException;
@@ -77,11 +73,8 @@ public void setInterceptor(HttpResponseInterceptor interceptor) {
 
   public IncomingHttpResponse sendRequest(
       String method, GenericUrl url, @Nullable Object content) throws FirebaseAuthException {
-    checkArgument(!Strings.isNullOrEmpty(method), "method must not be null or empty");
-    checkNotNull(url, "url must not be null");
-    String httpMethod = method.equals("PATCH") ? "POST" : method;
     HttpContent httpContent = content != null ? new JsonHttpContent(jsonFactory, content) : null;
-    HttpRequestInfo request = HttpRequestInfo.buildRequest(httpMethod, url, httpContent)
+    HttpRequestInfo request = HttpRequestInfo.buildRequest(method, url, httpContent)
         .addHeader(CLIENT_VERSION_HEADER, CLIENT_VERSION)
         .setResponseInterceptor(interceptor);
     if (method.equals("PATCH")) {

src/main/java/com/google/firebase/internal/HttpRequestInfo.java

@@ -44,9 +44,8 @@ public final class HttpRequestInfo {
 
   private HttpRequestInfo(String method, GenericUrl url, HttpContent content) {
     checkArgument(!Strings.isNullOrEmpty(method), "method must not be null");
-    checkNotNull(url, "url must not be null");
     this.method = method;
-    this.url = url;
+    this.url = checkNotNull(url, "url must not be null");
     this.content = content;
   }
 
@@ -77,16 +76,28 @@ public static HttpRequestInfo buildPostRequest(String url, HttpContent content)
     return buildRequest(HttpMethods.POST, url, content);
   }
 
-  public static HttpRequestInfo buildRequest(String method, String url, HttpContent content) {
+  public static HttpRequestInfo buildRequest(
+      String method, String url, @Nullable HttpContent content) {
     return buildRequest(method, new GenericUrl(url), content);
   }
 
-  public static HttpRequestInfo buildRequest(String method, GenericUrl url, HttpContent content) {
+  public static HttpRequestInfo buildRequest(
+      String method, GenericUrl url, @Nullable HttpContent content) {
     return new HttpRequestInfo(method, url, content);
   }
 
   HttpRequest newHttpRequest(HttpRequestFactory factory) throws IOException {
-    HttpRequest request = factory.buildRequest(method, url, content);
+    HttpRequest request;
+    if (factory.getTransport().supportsMethod(method)) {
+      request = factory.buildRequest(method, url, content);
+    } else {
+      // Some HttpTransport implementations (notably NetHttpTransport) don't support new methods
+      // like PATCH. We try to emulate such requests over POST by setting the method override
+      // header, which is recognized by most Google backend APIs.
+      request = factory.buildPostRequest(url, content);
+      request.getHeaders().set("X-HTTP-Method-Override", method);
+    }
+
     for (Map.Entry<String, String> entry : headers.entrySet()) {
       request.getHeaders().set(entry.getKey(), entry.getValue());
     }

src/main/java/com/google/firebase/projectmanagement/HttpHelper.java

@@ -16,11 +16,11 @@
 
 package com.google.firebase.projectmanagement;
 
+import com.google.api.client.http.HttpMethods;
 import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponseInterceptor;
 import com.google.api.client.http.json.JsonHttpContent;
 import com.google.api.client.json.JsonFactory;
-import com.google.common.annotations.VisibleForTesting;
 import com.google.firebase.FirebaseException;
 import com.google.firebase.IncomingHttpResponse;
 import com.google.firebase.internal.AbstractPlatformErrorHandler;
@@ -30,8 +30,6 @@
 
 final class HttpHelper {
 
-  @VisibleForTesting static final String PATCH_OVERRIDE_KEY = "X-HTTP-Method-Override";
-  @VisibleForTesting static final String PATCH_OVERRIDE_VALUE = "PATCH";
   private static final String CLIENT_VERSION_HEADER = "X-Client-Version";
 
   private final String clientVersion = "Java/Admin/" + SdkUtils.getVersion();
@@ -81,9 +79,8 @@ <T> void makePatchRequest(
       T parsedResponseInstance,
       String requestIdentifier,
       String requestIdentifierDescription) throws FirebaseProjectManagementException {
-    HttpRequestInfo baseRequest = HttpRequestInfo.buildPostRequest(
-        url, new JsonHttpContent(jsonFactory, payload));
-    baseRequest.addHeader(PATCH_OVERRIDE_KEY, PATCH_OVERRIDE_VALUE);
+    HttpRequestInfo baseRequest = HttpRequestInfo.buildRequest(
+        HttpMethods.PATCH, url, new JsonHttpContent(jsonFactory, payload));
     makeRequest(
         baseRequest, parsedResponseInstance, requestIdentifier, requestIdentifierDescription);
   }

src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java

@@ -2765,13 +2765,7 @@ private static void checkRequestHeaders(TestResponseInterceptor interceptor) {
 
   private static void checkUrl(TestResponseInterceptor interceptor, String method, String url) {
     HttpRequest request = interceptor.getResponse().getRequest();
-    if (method.equals("PATCH")) {
-      assertEquals("PATCH",
-          request.getHeaders().getFirstHeaderStringValue("X-HTTP-Method-Override"));
-      assertEquals("POST", request.getRequestMethod());
-    } else {
-      assertEquals(method, request.getRequestMethod());
-    }
+    assertEquals(method, request.getRequestMethod());
     assertEquals(url, request.getUrl().toString().split("\\?")[0]);
   }
 

src/test/java/com/google/firebase/auth/multitenancy/FirebaseTenantClientTest.java

@@ -308,13 +308,7 @@ private static void checkRequestHeaders(TestResponseInterceptor interceptor) {
 
   private static void checkUrl(TestResponseInterceptor interceptor, String method, String url) {
     HttpRequest request = interceptor.getResponse().getRequest();
-    if (method.equals("PATCH")) {
-      assertEquals("PATCH",
-          request.getHeaders().getFirstHeaderStringValue("X-HTTP-Method-Override"));
-      assertEquals("POST", request.getRequestMethod());
-    } else {
-      assertEquals(method, request.getRequestMethod());
-    }
+    assertEquals(method, request.getRequestMethod());
     assertEquals(url, request.getUrl().toString().split("\\?")[0]);
   }
 

src/test/java/com/google/firebase/internal/ErrorHandlingHttpClientTest.java

@@ -38,6 +38,7 @@
 import com.google.auth.oauth2.AccessToken;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.firebase.ErrorCode;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.FirebaseException;
@@ -132,6 +133,32 @@ public void testSuccessfulRequestWithHeadersAndBody() throws FirebaseException,
     assertEquals("{\"key\":\"value\"}", out.toString());
   }
 
+  @Test
+  public void testUnsupportedMethod() throws FirebaseException, IOException {
+    MockHttpTransport transport = new MockHttpTransport.Builder()
+        .setLowLevelHttpResponse(new MockLowLevelHttpResponse().setContent("{}"))
+        .setSupportedMethods(ImmutableSet.of(HttpMethods.GET, HttpMethods.POST))
+        .build();
+    ErrorHandlingHttpClient<FirebaseException> client = new ErrorHandlingHttpClient<>(
+        transport.createRequestFactory(), DEFAULT_JSON_FACTORY, new TestHttpErrorHandler());
+    HttpRequestInfo patchRequest = HttpRequestInfo.buildRequest(
+        HttpMethods.PATCH,
+        "https://firebase.google.com",
+        new JsonHttpContent(DEFAULT_JSON_FACTORY, ImmutableMap.of("key", "value")));
+    TestResponseInterceptor interceptor = new TestResponseInterceptor();
+    patchRequest.setResponseInterceptor(interceptor);
+
+    client.sendAndParse(patchRequest, GenericData.class);
+
+    HttpRequest last = interceptor.getLastRequest();
+    assertEquals(HttpMethods.POST, last.getRequestMethod());
+    assertEquals(HttpMethods.PATCH, last.getHeaders().get("X-HTTP-Method-Override"));
+
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    last.getContent().writeTo(out);
+    assertEquals("{\"key\":\"value\"}", out.toString());
+  }
+
   @Test
   public void testNetworkError() {
     final IOException exception = new IOException("Test");

src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java

@@ -18,8 +18,6 @@
 
 import static com.google.firebase.projectmanagement.FirebaseProjectManagementServiceImpl.FIREBASE_PROJECT_MANAGEMENT_URL;
 import static com.google.firebase.projectmanagement.FirebaseProjectManagementServiceImpl.MAXIMUM_LIST_APPS_PAGE_SIZE;
-import static com.google.firebase.projectmanagement.HttpHelper.PATCH_OVERRIDE_KEY;
-import static com.google.firebase.projectmanagement.HttpHelper.PATCH_OVERRIDE_VALUE;
 import static com.google.firebase.projectmanagement.ShaCertificateType.SHA_1;
 import static com.google.firebase.projectmanagement.ShaCertificateType.SHA_256;
 import static org.junit.Assert.assertEquals;
@@ -530,11 +528,10 @@ public void setIosDisplayName() throws Exception {
         "%s/v1beta1/projects/-/iosApps/%s?update_mask=display_name",
         FIREBASE_PROJECT_MANAGEMENT_URL,
         IOS_APP_ID);
-    checkRequestHeader(expectedUrl, HttpMethod.POST);
+    checkRequestHeader(expectedUrl, HttpMethod.PATCH);
     ImmutableMap<String, String> payload =
         ImmutableMap.<String, String>builder().put("display_name", DISPLAY_NAME).build();
     checkRequestPayload(payload);
-    checkPatchRequest();
   }
 
   @Test
@@ -548,11 +545,10 @@ public void setIosDisplayNameAsync() throws Exception {
         "%s/v1beta1/projects/-/iosApps/%s?update_mask=display_name",
         FIREBASE_PROJECT_MANAGEMENT_URL,
         IOS_APP_ID);
-    checkRequestHeader(expectedUrl, HttpMethod.POST);
+    checkRequestHeader(expectedUrl, HttpMethod.PATCH);
     ImmutableMap<String, String> payload =
         ImmutableMap.<String, String>builder().put("display_name", DISPLAY_NAME).build();
     checkRequestPayload(payload);
-    checkPatchRequest();
   }
 
   @Test
@@ -872,11 +868,10 @@ public void setAndroidDisplayName() throws Exception {
         "%s/v1beta1/projects/-/androidApps/%s?update_mask=display_name",
         FIREBASE_PROJECT_MANAGEMENT_URL,
         ANDROID_APP_ID);
-    checkRequestHeader(expectedUrl, HttpMethod.POST);
+    checkRequestHeader(expectedUrl, HttpMethod.PATCH);
     ImmutableMap<String, String> payload =
         ImmutableMap.<String, String>builder().put("display_name", DISPLAY_NAME).build();
     checkRequestPayload(payload);
-    checkPatchRequest();
   }
 
   @Test
@@ -890,11 +885,10 @@ public void setAndroidDisplayNameAsync() throws Exception {
         "%s/v1beta1/projects/-/androidApps/%s?update_mask=display_name",
         FIREBASE_PROJECT_MANAGEMENT_URL,
         ANDROID_APP_ID);
-    checkRequestHeader(expectedUrl, HttpMethod.POST);
+    checkRequestHeader(expectedUrl, HttpMethod.PATCH);
     ImmutableMap<String, String> payload =
         ImmutableMap.<String, String>builder().put("display_name", DISPLAY_NAME).build();
     checkRequestPayload(payload);
-    checkPatchRequest();
   }
 
   @Test
@@ -1147,21 +1141,11 @@ private void checkRequestPayload(int index, Map<String, String> expected) throws
     assertEquals(expected, parsed);
   }
 
-  private void checkPatchRequest() {
-    assertEquals(
-        "The number of HttpResponses is not equal to 1.", 1, interceptor.getNumberOfResponses());
-    checkPatchRequest(0);
-  }
-
-  private void checkPatchRequest(int index) {
-    assertTrue(interceptor.getResponse(index).getRequest().getHeaders()
-        .getHeaderStringValues(PATCH_OVERRIDE_KEY).contains(PATCH_OVERRIDE_VALUE));
-  }
-
   private enum HttpMethod {
     GET,
     POST,
-    DELETE
+    DELETE,
+    PATCH,
   }
 
   /**