Migrate storage to component framework (#2326)

Feiyang1 · Feiyang1 · commit fb55c6c4e3b8 · 2019-11-14T14:55:09.000-08:00
diff --git a/packages/storage/index.ts b/packages/storage/index.ts
@@ -16,31 +16,36 @@
  */
 
 import firebase from '@firebase/app';
-import { FirebaseApp } from '@firebase/app-types';
-import {
-  FirebaseServiceFactory,
-  _FirebaseNamespace
-} from '@firebase/app-types/private';
+import { _FirebaseNamespace } from '@firebase/app-types/private';
 import { StringFormat } from './src/implementation/string';
 import { TaskEvent, TaskState } from './src/implementation/taskenums';
 
 import { XhrIoPool } from './src/implementation/xhriopool';
 import { Reference } from './src/reference';
 import { Service } from './src/service';
 import * as types from '@firebase/storage-types';
+import {
+  Component,
+  ComponentType,
+  ComponentContainer
+} from '@firebase/component';
 
 /**
  * Type constant for Firebase Storage.
  */
 const STORAGE_TYPE = 'storage';
 
 function factory(
-  app: FirebaseApp,
-  unused: unknown,
+  container: ComponentContainer,
   url?: string
 ): types.FirebaseStorage {
+  // Dependencies
+  const app = container.getProvider('app').getImmediate();
+  const authProvider = container.getProvider('auth-internal');
+
   return (new Service(
     app,
+    authProvider,
     new XhrIoPool(),
     url
   ) as unknown) as types.FirebaseStorage;
@@ -55,13 +60,10 @@ export function registerStorage(instance: _FirebaseNamespace): void {
     Storage: Service,
     Reference
   };
-  instance.INTERNAL.registerService(
-    STORAGE_TYPE,
-    factory as FirebaseServiceFactory,
-    namespaceExports,
-    undefined,
-    // Allow multiple storage instances per app.
-    true
+  instance.INTERNAL.registerComponent(
+    new Component(STORAGE_TYPE, factory, ComponentType.PUBLIC)
+      .setServiceProps(namespaceExports)
+      .setMultipleInstances(true)
   );
 }
 
diff --git a/packages/storage/package.json b/packages/storage/package.json
@@ -22,6 +22,7 @@
   "dependencies": {
     "@firebase/storage-types": "0.3.5",
     "@firebase/util": "0.2.31",
+    "@firebase/component": "0.1.0",
     "tslib": "1.10.0"
   },
   "peerDependencies": {
diff --git a/packages/storage/src/implementation/authwrapper.ts b/packages/storage/src/implementation/authwrapper.ts
@@ -31,6 +31,8 @@ import {
   _FirebaseApp,
   FirebaseAuthTokenData
 } from '@firebase/app-types/private';
+import { Provider } from '@firebase/component';
+import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
 
 /**
  * @param app If null, getAuthToken always resolves with null.
@@ -40,6 +42,7 @@ import {
  */
 export class AuthWrapper {
   private app_: FirebaseApp | null;
+  private authProvider_: Provider<FirebaseAuthInternal>;
   private bucket_: string | null = null;
 
   private storageRefMaker_: (p1: AuthWrapper, p2: Location) => Reference;
@@ -53,6 +56,7 @@ export class AuthWrapper {
 
   constructor(
     app: FirebaseApp | null,
+    authProvider: Provider<FirebaseAuthInternal>,
     maker: (p1: AuthWrapper, p2: Location) => Reference,
     requestMaker: requestMaker,
     service: Service,
@@ -65,6 +69,7 @@ export class AuthWrapper {
         this.bucket_ = AuthWrapper.extractBucket_(options);
       }
     }
+    this.authProvider_ = authProvider;
     this.storageRefMaker_ = maker;
     this.requestMaker_ = requestMaker;
     this.pool_ = pool;
@@ -84,14 +89,9 @@ export class AuthWrapper {
   }
 
   getAuthToken(): Promise<string | null> {
-    // TODO(andysoto): remove ifDef checks after firebase-app implements stubs
-    // (b/28673818).
-    if (
-      this.app_ !== null &&
-      type.isDef((this.app_ as _FirebaseApp).INTERNAL) &&
-      type.isDef((this.app_ as _FirebaseApp).INTERNAL.getToken)
-    ) {
-      return (this.app_ as _FirebaseApp).INTERNAL.getToken().then(
+    const auth = this.authProvider_.getImmediate(undefined, { optional: true });
+    if (auth) {
+      return auth.getToken().then(
         (response: FirebaseAuthTokenData | null): string | null => {
           if (response !== null) {
             return response.accessToken;
diff --git a/packages/storage/src/service.ts b/packages/storage/src/service.ts
@@ -22,6 +22,8 @@ import { Location } from './implementation/location';
 import * as RequestExports from './implementation/request';
 import { XhrIoPool } from './implementation/xhriopool';
 import { Reference } from './reference';
+import { Provider } from '@firebase/component';
+import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
 
 /**
  * A service that provides firebaseStorage.Reference instances.
@@ -35,12 +37,18 @@ export class Service {
   private bucket_: Location | null = null;
   private internals_: ServiceInternals;
 
-  constructor(app: FirebaseApp, pool: XhrIoPool, url?: string) {
+  constructor(
+    app: FirebaseApp,
+    authProvider: Provider<FirebaseAuthInternal>,
+    pool: XhrIoPool,
+    url?: string
+  ) {
     function maker(authWrapper: AuthWrapper, loc: Location): Reference {
       return new Reference(authWrapper, loc);
     }
     this.authWrapper_ = new AuthWrapper(
       app,
+      authProvider,
       maker,
       RequestExports.makeRequest,
       this,
diff --git a/packages/storage/test/reference.test.ts b/packages/storage/test/reference.test.ts
@@ -26,9 +26,16 @@ import { Service } from '../src/service';
 import * as testShared from './testshared';
 import { SendHook, TestingXhrIo } from './xhrio';
 import { DEFAULT_HOST } from '../src/implementation/constants';
+import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
+import { Provider } from '@firebase/component';
+
 /* eslint-disable @typescript-eslint/no-floating-promises */
-function makeFakeService(app: FirebaseApp, sendHook: SendHook): Service {
-  return new Service(app, testShared.makePool(sendHook));
+function makeFakeService(
+  app: FirebaseApp,
+  authProvider: Provider<FirebaseAuthInternal>,
+  sendHook: SendHook
+): Service {
+  return new Service(app, authProvider, testShared.makePool(sendHook));
 }
 
 function makeStorage(url: string): Reference {
@@ -38,6 +45,7 @@ function makeStorage(url: string): Reference {
 
   const authWrapper = new AuthWrapper(
     null,
+    testShared.emptyAuthProvider,
     maker,
     makeRequest,
     ({} as any) as Service,
@@ -190,7 +198,11 @@ describe('Firebase Storage > Reference', () => {
       done();
     }
 
-    const service = makeFakeService(testShared.fakeAppNoAuth, newSend);
+    const service = makeFakeService(
+      testShared.fakeApp,
+      testShared.emptyAuthProvider,
+      newSend
+    );
     const ref = service.refFromURL('gs://test-bucket');
     ref.child('foo').getMetadata();
   });
@@ -212,7 +224,11 @@ describe('Firebase Storage > Reference', () => {
       done();
     }
 
-    const service = makeFakeService(testShared.fakeApp, newSend);
+    const service = makeFakeService(
+      testShared.fakeApp,
+      testShared.fakeAuthProvider,
+      newSend
+    );
     const ref = service.refFromURL('gs://test-bucket');
     ref.child('foo').getMetadata();
   });
diff --git a/packages/storage/test/requests.test.ts b/packages/storage/test/requests.test.ts
@@ -28,7 +28,11 @@ import { XhrIoPool } from '../src/implementation/xhriopool';
 import { Metadata } from '../src/metadata';
 import { Reference } from '../src/reference';
 import { Service } from '../src/service';
-import { assertObjectIncludes, fakeXhrIo } from './testshared';
+import {
+  assertObjectIncludes,
+  fakeXhrIo,
+  fakeAuthProvider
+} from './testshared';
 import {
   DEFAULT_HOST,
   CONFIG_STORAGE_BUCKET_KEY
@@ -61,6 +65,7 @@ describe('Firebase Storage > Requests', () => {
 
   const authWrapper = new AuthWrapper(
     mockApp,
+    fakeAuthProvider,
     (authWrapper, loc) => {
       return new Reference(authWrapper, loc);
     },
diff --git a/packages/storage/test/service.test.ts b/packages/storage/test/service.test.ts
@@ -22,9 +22,9 @@ import * as testShared from './testshared';
 import { DEFAULT_HOST } from '../src/implementation/constants';
 import { FirebaseStorageError } from '../src/implementation/error';
 
-const fakeAppGs = testShared.makeFakeApp(null, 'gs://mybucket');
-const fakeAppGsEndingSlash = testShared.makeFakeApp(null, 'gs://mybucket/');
-const fakeAppInvalidGs = testShared.makeFakeApp(null, 'gs://mybucket/hello');
+const fakeAppGs = testShared.makeFakeApp('gs://mybucket');
+const fakeAppGsEndingSlash = testShared.makeFakeApp('gs://mybucket/');
+const fakeAppInvalidGs = testShared.makeFakeApp('gs://mybucket/hello');
 const xhrIoPool = new XhrIoPool();
 
 function makeGsUrl(child: string = ''): string {
@@ -33,7 +33,11 @@ function makeGsUrl(child: string = ''): string {
 
 describe('Firebase Storage > Service', () => {
   describe('simple constructor', () => {
-    const service = new Service(testShared.fakeApp, xhrIoPool);
+    const service = new Service(
+      testShared.fakeApp,
+      testShared.fakeAuthProvider,
+      xhrIoPool
+    );
     it('Root refs point to the right place', () => {
       const ref = service.ref();
       assert.equal(ref.toString(), makeGsUrl());
@@ -65,6 +69,7 @@ describe('Firebase Storage > Service', () => {
     it('gs:// custom bucket constructor refs point to the right place', () => {
       const service = new Service(
         testShared.fakeApp,
+        testShared.fakeAuthProvider,
         xhrIoPool,
         'gs://foo-bar.appspot.com'
       );
@@ -74,6 +79,7 @@ describe('Firebase Storage > Service', () => {
     it('http:// custom bucket constructor refs point to the right place', () => {
       const service = new Service(
         testShared.fakeApp,
+        testShared.fakeAuthProvider,
         xhrIoPool,
         `http://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o`
       );
@@ -83,6 +89,7 @@ describe('Firebase Storage > Service', () => {
     it('https:// custom bucket constructor refs point to the right place', () => {
       const service = new Service(
         testShared.fakeApp,
+        testShared.fakeAuthProvider,
         xhrIoPool,
         `https://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o`
       );
@@ -93,6 +100,7 @@ describe('Firebase Storage > Service', () => {
     it('Bare bucket name constructor refs point to the right place', () => {
       const service = new Service(
         testShared.fakeApp,
+        testShared.fakeAuthProvider,
         xhrIoPool,
         'foo-bar.appspot.com'
       );
@@ -102,6 +110,7 @@ describe('Firebase Storage > Service', () => {
     it('Child refs point to the right place', () => {
       const service = new Service(
         testShared.fakeApp,
+        testShared.fakeAuthProvider,
         xhrIoPool,
         'foo-bar.appspot.com'
       );
@@ -110,28 +119,45 @@ describe('Firebase Storage > Service', () => {
     });
     it('Throws trying to construct with a gs:// URL containing an object path', () => {
       const error = testShared.assertThrows(() => {
-        new Service(testShared.fakeApp, xhrIoPool, 'gs://bucket/object/');
+        new Service(
+          testShared.fakeApp,
+          testShared.fakeAuthProvider,
+          xhrIoPool,
+          'gs://bucket/object/'
+        );
       }, 'storage/invalid-default-bucket');
       assert.match(error.message, /Invalid default bucket/);
     });
   });
   describe('default bucket config', () => {
     it('gs:// works without ending slash', () => {
-      const service = new Service(fakeAppGs, xhrIoPool);
+      const service = new Service(
+        fakeAppGs,
+        testShared.fakeAuthProvider,
+        xhrIoPool
+      );
       assert.equal(service.ref().toString(), 'gs://mybucket/');
     });
     it('gs:// works with ending slash', () => {
-      const service = new Service(fakeAppGsEndingSlash, xhrIoPool);
+      const service = new Service(
+        fakeAppGsEndingSlash,
+        testShared.fakeAuthProvider,
+        xhrIoPool
+      );
       assert.equal(service.ref().toString(), 'gs://mybucket/');
     });
     it('Throws when config bucket is gs:// with an object path', () => {
       testShared.assertThrows(() => {
-        new Service(fakeAppInvalidGs, xhrIoPool);
+        new Service(fakeAppInvalidGs, testShared.fakeAuthProvider, xhrIoPool);
       }, 'storage/invalid-default-bucket');
     });
   });
   describe('refFromURL', () => {
-    const service = new Service(testShared.fakeApp, xhrIoPool);
+    const service = new Service(
+      testShared.fakeApp,
+      testShared.fakeAuthProvider,
+      xhrIoPool
+    );
     it('Throws on non-URL arg', () => {
       const error = testShared.assertThrows(() => {
         service.refFromURL('path/to/child');
@@ -158,7 +184,11 @@ describe('Firebase Storage > Service', () => {
     });
   });
   describe('Argument verification', () => {
-    const service = new Service(testShared.fakeApp, xhrIoPool);
+    const service = new Service(
+      testShared.fakeApp,
+      testShared.fakeAuthProvider,
+      xhrIoPool
+    );
     describe('ref', () => {
       it('Throws with two args', () => {
         testShared.assertThrows(
@@ -272,7 +302,11 @@ describe('Firebase Storage > Service', () => {
   });
 
   describe('Deletion', () => {
-    const service = new Service(testShared.fakeApp, xhrIoPool);
+    const service = new Service(
+      testShared.fakeApp,
+      testShared.fakeAuthProvider,
+      xhrIoPool
+    );
     it('In-flight requests are canceled when the service is deleted', () => {
       const ref = service.refFromURL('gs://mybucket/image.jpg');
       const toReturn = ref.getMetadata().then(
diff --git a/packages/storage/test/task.test.ts b/packages/storage/test/task.test.ts
@@ -26,7 +26,12 @@ import { Headers } from '../src/implementation/xhrio';
 import { Reference } from '../src/reference';
 import { Service } from '../src/service';
 import { UploadTask } from '../src/task';
-import { assertThrows, bind as fbsBind, makePool } from './testshared';
+import {
+  assertThrows,
+  bind as fbsBind,
+  makePool,
+  emptyAuthProvider
+} from './testshared';
 import { StringHeaders, TestingXhrIo } from './xhrio';
 
 const testLocation = new Location('bucket', 'object');
@@ -63,6 +68,7 @@ function authWrapperWithHandler(handler: RequestHandler): AuthWrapper {
 
   return new AuthWrapper(
     null,
+    emptyAuthProvider,
     () => {
       return {} as Reference;
     },
diff --git a/packages/storage/test/testshared.ts b/packages/storage/test/testshared.ts
