Address PR comments

Author: hsubox76

packages/storage/src/implementation/metadata.ts

@@ -187,7 +187,7 @@ export function downloadUrlFromResourceString(
 }
 
 export function toResourceString(
-  metadata: Record<string, unknown>,
+  metadata: Partial<Metadata>,
   mappings: Mappings
 ): string {
   const resource: {

packages/storage/src/implementation/requests.ts

@@ -215,7 +215,7 @@ export function getDownloadUrl(
 export function updateMetadata(
   service: StorageService,
   location: Location,
-  metadata: Record<string, unknown>,
+  metadata: Partial<Metadata>,
   mappings: Mappings
 ): RequestInfo<Metadata> {
   const urlPart = location.fullServerUrl();

packages/storage/src/reference.ts

@@ -138,7 +138,25 @@ export function uploadBytes(
   metadata?: Metadata
 ): Promise<UploadResult> {
   ref._throwIfRoot('uploadBytes');
-  return _nonResumableUpload(ref, data, metadata);
+  return ref.storage
+    .getAuthToken()
+    .then(authToken => {
+      const requestInfo = multipartUpload(
+        ref.storage,
+        ref._location,
+        getMappings(),
+        new FbsBlob(data, true),
+        metadata
+      );
+      const multipartRequest = ref.storage.makeRequest(requestInfo, authToken);
+      return multipartRequest.getPromise();
+    })
+    .then(finalMetadata => {
+      return {
+        metadata: finalMetadata,
+        ref
+      };
+    });
 }
 
 /**
@@ -148,7 +166,7 @@ export function uploadBytes(
  * @param ref - StorageReference where data should be uploaded.
  * @param data - The data to upload.
  * @param metadata - Metadata for the newly uploaded data.
- * @returns An UploadTaskSnapshot
+ * @returns An UploadTask
  */
 export function uploadBytesResumable(
   ref: StorageReference,
@@ -181,31 +199,7 @@ export function uploadString(
   if (metadataClone['contentType'] == null && data.contentType != null) {
     metadataClone['contentType'] = data.contentType!;
   }
-  return _nonResumableUpload(ref, data.data, metadataClone);
-}
-
-/**
- * Shared code for nonresumable upload, used by uploadString and uploadBytes.
- */
-export async function _nonResumableUpload(
-  ref: StorageReference,
-  data: Uint8Array | Blob | ArrayBuffer,
-  metadata?: Metadata
-): Promise<UploadResult> {
-  const authToken = await ref.storage.getAuthToken();
-  const requestInfo = multipartUpload(
-    ref.storage,
-    ref._location,
-    getMappings(),
-    new FbsBlob(data, true),
-    metadata
-  );
-  const multipartRequest = ref.storage.makeRequest(requestInfo, authToken);
-  const finalMetadata = await multipartRequest.getPromise();
-  return {
-    metadata: finalMetadata,
-    ref
-  };
+  return uploadBytes(ref, data.data, metadataClone);
 }
 
 /**
@@ -338,7 +332,7 @@ export async function getMetadata(ref: StorageReference): Promise<Metadata> {
  */
 export async function updateMetadata(
   ref: StorageReference,
-  metadata: Record<string, unknown>
+  metadata: Partial<Metadata>
 ): Promise<Metadata> {
   ref._throwIfRoot('updateMetadata');
   const authToken = await ref.storage.getAuthToken();

packages/storage/test/integration/integration.exp.test.ts

@@ -146,7 +146,7 @@ describe('FirebaseStorage Exp', () => {
     await uploadString(referenceA, '');
     await uploadString(referenceB, '');
     await uploadString(referenceCD, '');
-    const listResult = await listAll(await ref(storage, 'public/exp-list'));
+    const listResult = await listAll(ref(storage, 'public/exp-list'));
     expect(listResult.items.map(v => v.name)).to.have.members(['a', 'b']);
     expect(listResult.prefixes.map(v => v.name)).to.have.members(['c']);
   });

packages/storage/test/unit/reference.exp.test.ts

@@ -28,8 +28,7 @@ import {
   getMetadata,
   updateMetadata,
   getDownloadURL,
-  uploadBytes,
-  _nonResumableUpload
+  uploadBytes
 } from '../../src/reference';
 import { StorageService, ref } from '../../src/service';
 import * as testShared from './testshared';
@@ -296,16 +295,13 @@ describe('Firebase Storage > Reference', () => {
         contentType: 'lol/wut'
       } as Metadata);
     });
-  });
-
-  describe('nonResumableUpload', () => {
     it('uploads without error', async () => {
       const storageService = storageServiceWithHandler(fakeServerHandler({}));
       const root = ref(storageService, 'gs://test-bucket/');
       const childRef = ref(root, 'child');
       const blob = new Blob(['a']);
-      const snapshot = await _nonResumableUpload(childRef, blob);
-      expect(snapshot.ref).to.equal(childRef);
+      const result = await uploadBytes(childRef, blob);
+      expect(result.ref).to.equal(childRef);
     });
   });
 

packages/storage/test/unit/testshared.ts

@@ -33,6 +33,7 @@ import {
   ComponentType
 } from '@firebase/component';
 import { StorageService } from '../../src/service';
+import { Metadata } from '../../src/metadata';
 
 export const authToken = 'totally-legit-auth-token';
 export const bucket = 'mybucket';
@@ -164,7 +165,7 @@ export function assertObjectIncludes(
   }
 }
 
-const defaultFakeMetadata = { 'downloadTokens': 'a,b' };
+const defaultFakeMetadata: Partial<Metadata> = { 'downloadTokens': ['a', 'b'] };
 
 interface Response {
   status: number;
@@ -200,7 +201,7 @@ export function storageServiceWithHandler(
 }
 
 export function fakeServerHandler(
-  fakeMetadata: Record<string, unknown> = defaultFakeMetadata
+  fakeMetadata: Partial<Metadata> = defaultFakeMetadata
 ): RequestHandler {
   const stats: {
     [num: number]: {