Remove ConnectionPool

common/api-review/storage.api.md

@@ -58,7 +58,7 @@ export class _FirebaseStorageImpl implements FirebaseStorage {
     constructor(
     app: FirebaseApp, _authProvider: Provider<FirebaseAuthInternalName>,
     _appCheckProvider: Provider<AppCheckInternalComponentName>,
-    _pool: ConnectionPool, _url?: string | undefined, _firebaseVersion?: string | undefined);
+    _url?: string | undefined, _firebaseVersion?: string | undefined);
     readonly app: FirebaseApp;
     // (undocumented)
     readonly _appCheckProvider: Provider<AppCheckInternalComponentName>;
@@ -78,23 +78,20 @@ export class _FirebaseStorageImpl implements FirebaseStorage {
     get host(): string;
     set host(host: string);
     // Warning: (ae-forgotten-export) The symbol "RequestInfo" needs to be exported by the entry point index.d.ts
+    // Warning: (ae-forgotten-export) The symbol "Connection" needs to be exported by the entry point index.d.ts
     // Warning: (ae-forgotten-export) The symbol "Request" needs to be exported by the entry point index.d.ts
     //
     // (undocumented)
-    _makeRequest<T>(requestInfo: RequestInfo_2<T>, authToken: string | null, appCheckToken: string | null): Request_2<T>;
+    _makeRequest<T>(requestInfo: RequestInfo_2<T>, requestFactory: () => Connection, authToken: string | null, appCheckToken: string | null): Request_2<T>;
     // (undocumented)
-    makeRequestWithTokens<T>(requestInfo: RequestInfo_2<T>): Promise<Request_2<T>>;
+    makeRequestWithTokens<T>(requestInfo: RequestInfo_2<T>, requestFactory: () => Connection): Promise<T>;
     _makeStorageReference(loc: _Location): _Reference;
     get maxOperationRetryTime(): number;
     set maxOperationRetryTime(time: number);
     get maxUploadRetryTime(): number;
     set maxUploadRetryTime(time: number);
     // (undocumented)
     _overrideAuthToken?: string;
-    // Warning: (ae-forgotten-export) The symbol "ConnectionPool" needs to be exported by the entry point index.d.ts
-    //
-    // (undocumented)
-    readonly _pool: ConnectionPool;
     // (undocumented)
     _protocol: string;
     // (undocumented)

packages/storage/src/implementation/request.ts

@@ -32,7 +32,6 @@ import { RequestInfo } from './requestinfo';
 import { isJustDef } from './type';
 import { makeQueryString } from './url';
 import { Headers, Connection, ErrorCode } from './connection';
-import { ConnectionPool } from './connectionPool';
 
 export interface Request<T> {
   getPromise(): Promise<T>;
@@ -67,7 +66,6 @@ class NetworkRequest<T> implements Request<T> {
     | null;
   private progressCallback_: ((p1: number, p2: number) => void) | null;
   private timeout_: number;
-  private pool_: ConnectionPool;
   promise_: Promise<T>;
 
   constructor(
@@ -78,12 +76,10 @@ class NetworkRequest<T> implements Request<T> {
     successCodes: number[],
     additionalRetryCodes: number[],
     callback: (p1: Connection, p2: string) => T,
-    errorCallback:
-      | ((p1: Connection, p2: StorageError) => StorageError)
-      | null,
+    errorCallback: ((p1: Connection, p2: StorageError) => StorageError) | null,
     timeout: number,
     progressCallback: ((p1: number, p2: number) => void) | null,
-    pool: ConnectionPool
+    private connectionFactory_: () => Connection
   ) {
     this.url_ = url;
     this.method_ = method;
@@ -95,7 +91,6 @@ class NetworkRequest<T> implements Request<T> {
     this.errorCallback_ = errorCallback;
     this.progressCallback_ = progressCallback;
     this.timeout_ = timeout;
-    this.pool_ = pool;
     this.promise_ = new Promise((resolve, reject) => {
       this.resolve_ = resolve as (value?: T | PromiseLike<T>) => void;
       this.reject_ = reject;
@@ -117,7 +112,7 @@ class NetworkRequest<T> implements Request<T> {
         backoffCallback(false, new RequestEndStatus(false, null, true));
         return;
       }
-      const connection = self.pool_.createConnection();
+      const connection = self.connectionFactory_();
       self.pendingConnection_ = connection;
 
       function progressListener(progressEvent: ProgressEvent): void {
@@ -296,7 +291,7 @@ export function makeRequest<T>(
   appId: string | null,
   authToken: string | null,
   appCheckToken: string | null,
-  pool: ConnectionPool,
+  requestFactory: () => Connection,
   firebaseVersion?: string
 ): Request<T> {
   const queryPart = makeQueryString(requestInfo.urlParams);
@@ -317,6 +312,6 @@ export function makeRequest<T>(
     requestInfo.errorHandler,
     requestInfo.timeout,
     requestInfo.progressCallback,
-    pool
+    requestFactory
   );
 }

packages/storage/src/index.ts

@@ -27,7 +27,6 @@ import {
   SDK_VERSION
 } from '@firebase/app';
 
-import { ConnectionPool } from '../src/implementation/connectionPool';
 import { FirebaseStorageImpl } from '../src/service';
 import {
   Component,
@@ -56,7 +55,6 @@ function factory(
     app,
     authProvider,
     appCheckProvider,
-    new ConnectionPool(),
     url,
     SDK_VERSION
   );

packages/storage/src/platform/browser/connection.ts

@@ -22,6 +22,9 @@ import {
 } from '../../implementation/connection';
 import { internalError } from '../../implementation/error';
 
+/** An override for the text-based Connection. Used in tests. */
+let connectionFactoryOverride: (() => Connection) | null = null;
+
 /**
  * Network layer for browsers. We use this instead of goog.net.XhrIo because
  * goog.net.XhrIo is hyuuuuge and doesn't work in React Native on Android.
@@ -148,5 +151,11 @@ export class XhrConnection implements Connection {
 }
 
 export function newConnection(): Connection {
-  return new XhrConnection();
+  return connectionFactoryOverride
+    ? connectionFactoryOverride()
+    : new XhrConnection();
+}
+
+export function injectTestConnection(factory: (() => Connection) | null): void {
+  connectionFactoryOverride = factory;
 }

packages/storage/src/platform/connection.ts

@@ -15,9 +15,17 @@
  * limitations under the License.
  */
 import { Connection } from '../implementation/connection';
-import { newConnection as nodeNewConnection } from './node/connection';
+import {
+  newConnection as nodeNewConnection,
+  injectTestConnection as nodeInjectTestConnection
+} from './node/connection';
 
 export function newConnection(): Connection {
   // This file is only used under ts-node.
   return nodeNewConnection();
 }
+
+export function injectTestConnection(factory: (() => Connection) | null): void {
+  // This file is only used under ts-node.
+  nodeInjectTestConnection(factory);
+}

packages/storage/src/platform/node/connection.ts

@@ -22,6 +22,9 @@ import nodeFetch from 'node-fetch';
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const fetch: typeof window.fetch = nodeFetch as any;
 
+/** An override for the text-based Connection. Used in tests. */
+let connectionFactoryOverride: (() => Connection) | null = null;
+
 /**
  * Network layer that works in Node.
  *
@@ -114,5 +117,11 @@ export class FetchConnection implements Connection {
 }
 
 export function newConnection(): Connection {
-  return new FetchConnection();
+  return connectionFactoryOverride
+    ? connectionFactoryOverride()
+    : new FetchConnection();
+}
+
+export function injectTestConnection(factory: (() => Connection) | null): void {
+  connectionFactoryOverride = factory;
 }

packages/storage/src/reference.ts

@@ -39,6 +39,7 @@ import { ListResult } from './list';
 import { UploadTask } from './task';
 import { invalidRootOperation, noDownloadURL } from './implementation/error';
 import { validateNumber } from './implementation/type';
+import { newConnection } from './platform/connection';
 
 /**
  * Provides methods to interact with a bucket in the Firebase Storage service.
@@ -165,8 +166,7 @@ export function uploadBytes(
     metadata
   );
   return ref.storage
-    .makeRequestWithTokens(requestInfo)
-    .then(request => request.getPromise())
+    .makeRequestWithTokens(requestInfo, newConnection)
     .then(finalMetadata => {
       return {
         metadata: finalMetadata,
@@ -290,7 +290,7 @@ async function listAllHelper(
  *      contains references to objects in this folder. `nextPageToken`
  *      can be used to get the rest of the results.
  */
-export async function list(
+export function list(
   ref: Reference,
   options?: ListOptions | null
 ): Promise<ListResult> {
@@ -312,7 +312,7 @@ export async function list(
     op.pageToken,
     op.maxResults
   );
-  return (await ref.storage.makeRequestWithTokens(requestInfo)).getPromise();
+  return ref.storage.makeRequestWithTokens(requestInfo, newConnection);
 }
 
 /**
@@ -322,14 +322,14 @@ export async function list(
  * @public
  * @param ref - StorageReference to get metadata from.
  */
-export async function getMetadata(ref: Reference): Promise<Metadata> {
+export function getMetadata(ref: Reference): Promise<Metadata> {
   ref._throwIfRoot('getMetadata');
   const requestInfo = requestsGetMetadata(
     ref.storage,
     ref._location,
     getMappings()
   );
-  return (await ref.storage.makeRequestWithTokens(requestInfo)).getPromise();
+  return ref.storage.makeRequestWithTokens(requestInfo, newConnection);
 }
 
 /**
@@ -343,7 +343,7 @@ export async function getMetadata(ref: Reference): Promise<Metadata> {
  *     with the new metadata for this object.
  *     See `firebaseStorage.Reference.prototype.getMetadata`
  */
-export async function updateMetadata(
+export function updateMetadata(
   ref: Reference,
   metadata: Partial<Metadata>
 ): Promise<Metadata> {
@@ -354,7 +354,7 @@ export async function updateMetadata(
     metadata,
     getMappings()
   );
-  return (await ref.storage.makeRequestWithTokens(requestInfo)).getPromise();
+  return ref.storage.makeRequestWithTokens(requestInfo, newConnection);
 }
 
 /**
@@ -363,15 +363,15 @@ export async function updateMetadata(
  * @returns A `Promise` that resolves with the download
  *     URL for this object.
  */
-export async function getDownloadURL(ref: Reference): Promise<string> {
+export function getDownloadURL(ref: Reference): Promise<string> {
   ref._throwIfRoot('getDownloadURL');
   const requestInfo = requestsGetDownloadUrl(
     ref.storage,
     ref._location,
     getMappings()
   );
-  return (await ref.storage.makeRequestWithTokens(requestInfo))
-    .getPromise()
+  return ref.storage
+    .makeRequestWithTokens(requestInfo, newConnection)
     .then(url => {
       if (url === null) {
         throw noDownloadURL();
@@ -386,10 +386,10 @@ export async function getDownloadURL(ref: Reference): Promise<string> {
  * @param ref - StorageReference for object to delete.
  * @returns A `Promise` that resolves if the deletion succeeds.
  */
-export async function deleteObject(ref: Reference): Promise<void> {
+export function deleteObject(ref: Reference): Promise<void> {
   ref._throwIfRoot('deleteObject');
   const requestInfo = requestsDeleteObject(ref.storage, ref._location);
-  return (await ref.storage.makeRequestWithTokens(requestInfo)).getPromise();
+  return ref.storage.makeRequestWithTokens(requestInfo, newConnection);
 }
 
 /**