fix: Handling JSON serialization/response interception at ErrorHandlingHttpClient

pom.xml

@@ -198,7 +198,7 @@
                     <plugin>
                         <groupId>org.jacoco</groupId>
                         <artifactId>jacoco-maven-plugin</artifactId>
-                        <version>0.7.9</version>
+                        <version>0.8.5</version>
                         <executions>
                             <execution>
                                 <id>pre-unit-test</id>
@@ -289,7 +289,7 @@
             <!-- Test Phase -->
             <plugin>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <version>2.19.1</version>
+                <version>2.22.0</version>
                 <configuration>
                     <skipTests>${skipUTs}</skipTests>
                 </configuration>

src/main/java/com/google/firebase/auth/FirebaseUserManager.java

@@ -80,7 +80,7 @@ class FirebaseUserManager {
   private final JsonFactory jsonFactory;
   private final AuthHttpClient httpClient;
 
-  FirebaseUserManager(Builder builder) {
+  private FirebaseUserManager(Builder builder) {
     FirebaseApp app = checkNotNull(builder.app, "FirebaseApp must not be null");
     String projectId = ImplFirebaseTrampolines.getProjectId(app);
     checkArgument(!Strings.isNullOrEmpty(projectId),

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

@@ -17,10 +17,8 @@
 package com.google.firebase.auth.internal;
 
 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.collect.ImmutableSortedSet;
 import com.google.firebase.IncomingHttpResponse;
@@ -41,13 +39,9 @@ public final class AuthHttpClient {
 
   private static final String CLIENT_VERSION = "Java/Admin/" + SdkUtils.getVersion();
 
-  private final JsonFactory jsonFactory;
   private final ErrorHandlingHttpClient<FirebaseAuthException> httpClient;
 
-  private HttpResponseInterceptor interceptor;
-
   public AuthHttpClient(JsonFactory jsonFactory, HttpRequestFactory requestFactory) {
-    this.jsonFactory = jsonFactory;
     AuthErrorHandler authErrorHandler = new AuthErrorHandler(jsonFactory);
     this.httpClient = new ErrorHandlingHttpClient<>(requestFactory, jsonFactory, authErrorHandler);
   }
@@ -68,25 +62,21 @@ public static Set<String> generateMask(Map<String, Object> properties) {
   }
 
   public void setInterceptor(HttpResponseInterceptor interceptor) {
-    this.interceptor = interceptor;
+    this.httpClient.setInterceptor(interceptor);
   }
 
   public IncomingHttpResponse sendRequest(
       String method, GenericUrl url, @Nullable Object content) throws FirebaseAuthException {
-    HttpContent httpContent = content != null ? new JsonHttpContent(jsonFactory, content) : null;
-    HttpRequestInfo request = HttpRequestInfo.buildRequest(method, url, httpContent)
-        .addHeader(CLIENT_VERSION_HEADER, CLIENT_VERSION)
-        .setResponseInterceptor(interceptor);
-    if (method.equals("PATCH")) {
-      request.addHeader("X-HTTP-Method-Override", "PATCH");
-    }
-
+    HttpRequestInfo request = HttpRequestInfo.buildJsonRequest(method, url, content)
+        .addHeader(CLIENT_VERSION_HEADER, CLIENT_VERSION);
     return httpClient.send(request);
   }
 
   public <T> T sendRequest(
-      String method, GenericUrl url,
-      @Nullable Object content, Class<T> clazz) throws FirebaseAuthException {
+      String method,
+      GenericUrl url,
+      @Nullable Object content,
+      Class<T> clazz) throws FirebaseAuthException {
 
     IncomingHttpResponse response = this.sendRequest(method, url, content);
     return this.parse(response, clazz);

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

@@ -8,7 +8,6 @@
 import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponse;
 import com.google.api.client.http.HttpResponseInterceptor;
-import com.google.api.client.http.json.JsonHttpContent;
 import com.google.api.client.json.GenericJson;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.util.StringUtils;
@@ -75,35 +74,30 @@ static class IAMCryptoSigner implements CryptoSigner {
         "https://iam.googleapis.com/v1/projects/-/serviceAccounts/%s:signBlob";
 
     private final String serviceAccount;
-    private final JsonFactory jsonFactory;
     private final ErrorHandlingHttpClient<FirebaseAuthException> httpClient;
-    private HttpResponseInterceptor interceptor;
 
     IAMCryptoSigner(
         @NonNull HttpRequestFactory requestFactory,
         @NonNull JsonFactory jsonFactory,
         @NonNull String serviceAccount) {
       checkArgument(!Strings.isNullOrEmpty(serviceAccount));
       this.serviceAccount = serviceAccount;
-      this.jsonFactory = checkNotNull(jsonFactory);
       this.httpClient = new ErrorHandlingHttpClient<>(
           requestFactory,
           jsonFactory,
           new IAMErrorHandler(jsonFactory));
     }
 
     void setInterceptor(HttpResponseInterceptor interceptor) {
-      this.interceptor = interceptor;
+      httpClient.setInterceptor(interceptor);
     }
 
     @Override
     public byte[] sign(byte[] payload) throws FirebaseAuthException {
       String encodedPayload = BaseEncoding.base64().encode(payload);
       Map<String, String> content = ImmutableMap.of("bytesToSign", encodedPayload);
       String encodedUrl = String.format(IAM_SIGN_BLOB_URL, serviceAccount);
-      HttpRequestInfo requestInfo = HttpRequestInfo
-          .buildPostRequest(encodedUrl, new JsonHttpContent(jsonFactory, content))
-          .setResponseInterceptor(interceptor);
+      HttpRequestInfo requestInfo = HttpRequestInfo.buildJsonPostRequest(encodedUrl, content);
       GenericJson parsed = httpClient.sendAndParse(requestInfo, GenericJson.class);
       return BaseEncoding.base64().decode((String) parsed.get("signature"));
     }

src/main/java/com/google/firebase/iid/FirebaseInstanceId.java

@@ -65,8 +65,6 @@ public class FirebaseInstanceId {
   private final String projectId;
   private final ErrorHandlingHttpClient<FirebaseInstanceIdException> httpClient;
 
-  private HttpResponseInterceptor interceptor;
-
   private FirebaseInstanceId(FirebaseApp app) {
     this(app, null);
   }
@@ -115,7 +113,7 @@ public static synchronized FirebaseInstanceId getInstance(FirebaseApp app) {
 
   @VisibleForTesting
   void setInterceptor(HttpResponseInterceptor interceptor) {
-    this.interceptor = interceptor;
+    httpClient.setInterceptor(interceptor);
   }
 
   /**
@@ -154,8 +152,7 @@ private CallableOperation<Void, FirebaseInstanceIdException> deleteInstanceIdOp(
       protected Void execute() throws FirebaseInstanceIdException {
         String url = String.format(
             "%s/project/%s/instanceId/%s", IID_SERVICE_URL, projectId, instanceId);
-        HttpRequestInfo request = HttpRequestInfo.buildDeleteRequest(url)
-            .setResponseInterceptor(interceptor);
+        HttpRequestInfo request = HttpRequestInfo.buildDeleteRequest(url);
         httpClient.send(request);
         return null;
       }

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

@@ -22,6 +22,7 @@
 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.HttpResponseInterceptor;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.json.JsonParser;
@@ -41,6 +42,9 @@ public final class ErrorHandlingHttpClient<T extends FirebaseException> {
   private final HttpRequestFactory requestFactory;
   private final JsonFactory jsonFactory;
   private final HttpErrorHandler<T> errorHandler;
+  private final JsonObjectParser jsonParser;
+
+  private HttpResponseInterceptor interceptor;
 
   public ErrorHandlingHttpClient(
       HttpRequestFactory requestFactory,
@@ -49,6 +53,12 @@ public ErrorHandlingHttpClient(
     this.requestFactory = checkNotNull(requestFactory, "requestFactory must not be null");
     this.jsonFactory = checkNotNull(jsonFactory, "jsonFactory must not be null");
     this.errorHandler = checkNotNull(errorHandler, "errorHandler must not be null");
+    this.jsonParser = new JsonObjectParser(jsonFactory);
+  }
+
+  public ErrorHandlingHttpClient<T> setInterceptor(HttpResponseInterceptor interceptor) {
+    this.interceptor = interceptor;
+    return this;
   }
 
   /**
@@ -127,8 +137,9 @@ public void parse(IncomingHttpResponse response, Object destination) throws T {
 
   private HttpRequest createHttpRequest(HttpRequestInfo requestInfo) throws T {
     try {
-      return requestInfo.newHttpRequest(requestFactory)
-          .setParser(new JsonObjectParser(jsonFactory));
+      return requestInfo.newHttpRequest(requestFactory, jsonFactory)
+          .setParser(jsonParser)
+          .setResponseInterceptor(interceptor);
     } catch (IOException e) {
       // Handle request initialization errors (credential loading and other config errors)
       throw errorHandler.handleIOException(e);

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

@@ -24,7 +24,8 @@
 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.HttpResponseInterceptor;
+import com.google.api.client.http.json.JsonHttpContent;
+import com.google.api.client.json.JsonFactory;
 import com.google.common.base.Strings;
 import java.io.IOException;
 import java.util.HashMap;
@@ -39,14 +40,15 @@ public final class HttpRequestInfo {
   private final String method;
   private final GenericUrl url;
   private final HttpContent content;
+  private final Object jsonContent;
   private final Map<String, String> headers = new HashMap<>();
-  private HttpResponseInterceptor interceptor;
 
-  private HttpRequestInfo(String method, GenericUrl url, HttpContent content) {
+  private HttpRequestInfo(String method, GenericUrl url, HttpContent content, Object jsonContent) {
     checkArgument(!Strings.isNullOrEmpty(method), "method must not be null");
     this.method = method;
     this.url = checkNotNull(url, "url must not be null");
     this.content = content;
+    this.jsonContent = jsonContent;
   }
 
   public HttpRequestInfo addHeader(String name, String value) {
@@ -59,11 +61,6 @@ public HttpRequestInfo addAllHeaders(Map<String, String> headers) {
     return this;
   }
 
-  public HttpRequestInfo setResponseInterceptor(HttpResponseInterceptor interceptor) {
-    this.interceptor = interceptor;
-    return this;
-  }
-
   public static HttpRequestInfo buildGetRequest(String url) {
     return buildRequest(HttpMethods.GET, url, null);
   }
@@ -72,37 +69,60 @@ public static HttpRequestInfo buildDeleteRequest(String url) {
     return buildRequest(HttpMethods.DELETE, url, null);
   }
 
-  public static HttpRequestInfo buildPostRequest(String url, HttpContent content) {
-    return buildRequest(HttpMethods.POST, url, 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, @Nullable HttpContent content) {
-    return new HttpRequestInfo(method, url, content);
+    return new HttpRequestInfo(method, url, content, null);
+  }
+
+  public static HttpRequestInfo buildJsonPostRequest(String url, @Nullable Object content) {
+    return buildJsonRequest(HttpMethods.POST, url, content);
+  }
+
+  public static HttpRequestInfo buildJsonRequest(
+      String method, String url, @Nullable Object content) {
+    return buildJsonRequest(method, new GenericUrl(url), content);
   }
 
-  HttpRequest newHttpRequest(HttpRequestFactory factory) throws IOException {
+  public static HttpRequestInfo buildJsonRequest(
+      String method, GenericUrl url, @Nullable Object content) {
+    return new HttpRequestInfo(method, url, null, content);
+  }
+
+  HttpRequest newHttpRequest(
+      HttpRequestFactory factory, JsonFactory jsonFactory) throws IOException {
     HttpRequest request;
+    HttpContent httpContent = getContent(jsonFactory);
     if (factory.getTransport().supportsMethod(method)) {
-      request = factory.buildRequest(method, url, content);
+      request = factory.buildRequest(method, url, httpContent);
     } 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 = factory.buildPostRequest(url, httpContent);
       request.getHeaders().set("X-HTTP-Method-Override", method);
     }
 
     for (Map.Entry<String, String> entry : headers.entrySet()) {
       request.getHeaders().set(entry.getKey(), entry.getValue());
     }
-    request.setResponseInterceptor(interceptor);
 
     return request;
   }
+
+  private HttpContent getContent(JsonFactory jsonFactory) {
+    if (content != null) {
+      return content;
+    }
+
+    if (jsonContent != null) {
+      return new JsonHttpContent(jsonFactory, jsonContent);
+    }
+
+    return null;
+  }
 }

src/main/java/com/google/firebase/messaging/FirebaseMessagingClientImpl.java

@@ -84,8 +84,8 @@ private FirebaseMessagingClientImpl(Builder builder) {
     this.jsonFactory = checkNotNull(builder.jsonFactory);
     this.responseInterceptor = builder.responseInterceptor;
     this.errorHandler = new MessagingErrorHandler(this.jsonFactory);
-    this.httpClient = new ErrorHandlingHttpClient<>(
-        this.requestFactory, this.jsonFactory, this.errorHandler);
+    this.httpClient = new ErrorHandlingHttpClient<>(requestFactory, jsonFactory, errorHandler)
+      .setInterceptor(responseInterceptor);
     this.batchClient = new MessagingBatchClient(requestFactory.getTransport(), jsonFactory);
   }
 
@@ -121,10 +121,9 @@ public BatchResponse sendAll(
   private String sendSingleRequest(
       Message message, boolean dryRun) throws FirebaseMessagingException {
     HttpRequestInfo request =
-        HttpRequestInfo.buildPostRequest(
-            fcmSendUrl, new JsonHttpContent(jsonFactory, message.wrapForTransport(dryRun)))
-            .addAllHeaders(COMMON_HEADERS)
-            .setResponseInterceptor(responseInterceptor);
+        HttpRequestInfo.buildJsonPostRequest(
+            fcmSendUrl, message.wrapForTransport(dryRun))
+            .addAllHeaders(COMMON_HEADERS);
     MessagingServiceResponse parsed = httpClient.sendAndParse(
         request, MessagingServiceResponse.class);
     return parsed.getMessageId();
@@ -172,6 +171,8 @@ private HttpRequestInitializer getBatchRequestInitializer() {
     return new HttpRequestInitializer() {
       @Override
       public void initialize(HttpRequest request) throws IOException {
+        // Batch requests are not executed on the ErrorHandlingHttpClient. Therefore, they
+        // require some special handling at initialization.
         HttpRequestInitializer initializer = requestFactory.getInitializer();
         if (initializer != null) {
           initializer.initialize(request);