Using the new HttpClient API in instanceId module (#321)

* Using the new HttpClient API in instanceId module

* Fixing some lint errors

Author: hiranya911

src/instance-id/instance-id-request.ts

@@ -15,9 +15,9 @@
  */
 
 import {FirebaseApp} from '../firebase-app';
-import {FirebaseError, FirebaseInstanceIdError, InstanceIdClientErrorCode} from '../utils/error';
+import {FirebaseInstanceIdError, InstanceIdClientErrorCode} from '../utils/error';
 import {
-  HttpMethod, SignedApiRequestHandler, ApiSettings,
+  ApiSettings, AuthorizedHttpClient, HttpRequestConfig, HttpError,
 } from '../utils/api-request';
 
 import * as validator from '../utils/validator';
@@ -48,11 +48,11 @@ const ERROR_CODES = {
  */
 export class FirebaseInstanceIdRequestHandler {
 
-  private host: string = FIREBASE_IID_HOST;
-  private port: number = FIREBASE_IID_PORT;
-  private timeout: number = FIREBASE_IID_TIMEOUT;
-  private signedApiRequestHandler: SignedApiRequestHandler;
-  private path: string;
+  private readonly host: string = FIREBASE_IID_HOST;
+  private readonly port: number = FIREBASE_IID_PORT;
+  private readonly timeout: number = FIREBASE_IID_TIMEOUT;
+  private readonly httpClient: AuthorizedHttpClient;
+  private readonly path: string;
 
   /**
    * @param {FirebaseApp} app The app used to fetch access tokens to sign API requests.
@@ -61,7 +61,7 @@ export class FirebaseInstanceIdRequestHandler {
    * @constructor
    */
   constructor(app: FirebaseApp, projectId: string) {
-    this.signedApiRequestHandler = new SignedApiRequestHandler(app);
+    this.httpClient = new AuthorizedHttpClient(app);
     this.path = FIREBASE_IID_PATH + `project/${projectId}/instanceId/`;
   }
 
@@ -83,28 +83,31 @@ export class FirebaseInstanceIdRequestHandler {
    */
   private invokeRequestHandler(apiSettings: ApiSettings): Promise<object> {
     const path: string = this.path + apiSettings.getEndpoint();
-    const httpMethod: HttpMethod = apiSettings.getHttpMethod();
     return Promise.resolve()
       .then(() => {
-        return this.signedApiRequestHandler.sendRequest(
-            this.host, this.port, path, httpMethod, undefined, undefined, this.timeout);
+        const req: HttpRequestConfig = {
+          url: `https://${this.host}${path}`,
+          method: apiSettings.getHttpMethod(),
+          timeout: this.timeout,
+        };
+        return this.httpClient.send(req);
       })
       .then((response) => {
-        return response;
+        return response.data;
       })
-      .catch((response) => {
-        const error = (typeof response === 'object' && 'error' in response) ?
-          response.error : response;
-        if (error instanceof FirebaseError) {
-          // In case of timeouts and other network errors, the API request handler returns a
-          // FirebaseError wrapped in the response. Simply throw it here.
-          throw error;
+      .catch((err) => {
+        if (err instanceof HttpError) {
+          const response = err.response;
+          const errorMessage: string = (response.isJson() && 'error' in response.data) ?
+            response.data.error : response.text;
+          const template: string = ERROR_CODES[response.status];
+          const message: string = template ?
+            `Instance ID "${apiSettings.getEndpoint()}": ${template}` : errorMessage;
+          throw new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR, message);
         }
-
-        const template: string = ERROR_CODES[response.statusCode];
-        const message: string = template ?
-          `Instance ID "${apiSettings.getEndpoint()}": ${template}` : JSON.stringify(error);
-        throw new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR, message);
+        // In case of timeouts and other network errors, the HttpClient returns a
+        // FirebaseError wrapped in the response. Simply throw it here.
+        throw err;
       });
   }
 }

test/unit/instance-id/instance-id-request.spec.ts

@@ -27,7 +27,7 @@ import * as utils from '../utils';
 import * as mocks from '../../resources/mocks';
 
 import {FirebaseApp} from '../../../src/firebase-app';
-import {HttpRequestHandler} from '../../../src/utils/api-request';
+import {HttpClient} from '../../../src/utils/api-request';
 import {FirebaseInstanceIdRequestHandler} from '../../../src/instance-id/instance-id-request';
 
 chai.should();
@@ -56,6 +56,7 @@ describe('FirebaseInstanceIdRequestHandler', () => {
     expectedHeaders = {
       Authorization: 'Bearer ' + mockAccessToken,
     };
+    return mockApp.INTERNAL.getToken();
   });
 
   afterEach(() => {
@@ -75,31 +76,31 @@ describe('FirebaseInstanceIdRequestHandler', () => {
   describe('deleteInstanceId', () => {
     const httpMethod = 'DELETE';
     const host = 'console.firebase.google.com';
-    const port = 443;
     const path = `/v1/project/${projectId}/instanceId/test-iid`;
     const timeout = 10000;
 
     it('should be fulfilled given a valid instance ID', () => {
       const expectedResult = {};
-
-      const stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest')
-        .returns(Promise.resolve(expectedResult));
+      const stub = sinon.stub(HttpClient.prototype, 'send')
+        .resolves(utils.responseFrom(expectedResult));
       stubs.push(stub);
 
       const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId);
       return requestHandler.deleteInstanceId('test-iid')
         .then((result) => {
           expect(result).to.deep.equal(expectedResult);
-          expect(stub).to.have.been.calledOnce.and.calledWith(
-              host, port, path, httpMethod, undefined, expectedHeaders, timeout);
+          expect(stub).to.have.been.calledOnce.and.calledWith({
+            method: httpMethod,
+            url: `https://${host}${path}`,
+            headers: expectedHeaders,
+            timeout,
+          });
         });
     });
 
     it('should throw for HTTP 404 errors', () => {
-        const expectedResult = {statusCode: 404};
-
-        const stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest')
-          .rejects(expectedResult);
+        const stub = sinon.stub(HttpClient.prototype, 'send')
+          .rejects(utils.errorFrom({}, 404));
         stubs.push(stub);
 
         const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId);
@@ -114,10 +115,8 @@ describe('FirebaseInstanceIdRequestHandler', () => {
     });
 
     it('should throw for HTTP 409 errors', () => {
-        const expectedResult = {statusCode: 409};
-
-        const stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest')
-          .rejects(expectedResult);
+        const stub = sinon.stub(HttpClient.prototype, 'send')
+          .rejects(utils.errorFrom({}, 409));
         stubs.push(stub);
 
         const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId);
@@ -132,10 +131,9 @@ describe('FirebaseInstanceIdRequestHandler', () => {
     });
 
     it('should throw for unexpected HTTP errors', () => {
-        const expectedResult = {statusCode: 511};
-
-        const stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest')
-          .rejects(expectedResult);
+        const expectedResult = {error: 'test error'};
+        const stub = sinon.stub(HttpClient.prototype, 'send')
+          .rejects(utils.errorFrom(expectedResult, 511));
         stubs.push(stub);
 
         const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId);
@@ -145,7 +143,7 @@ describe('FirebaseInstanceIdRequestHandler', () => {
           })
           .catch((error) => {
             expect(error.code).to.equal('instance-id/api-error');
-            expect(error.message).to.equal(JSON.stringify(expectedResult));
+            expect(error.message).to.equal('test error');
           });
     });
   });