diff --git a/common/api-review/storage.api.md b/common/api-review/storage.api.md index ad83a83eede..cbc4486e0c8 100644 --- a/common/api-review/storage.api.md +++ b/common/api-review/storage.api.md @@ -58,7 +58,7 @@ export class _FirebaseStorageImpl implements FirebaseStorage { constructor( app: FirebaseApp, _authProvider: Provider, _appCheckProvider: Provider, - _pool: ConnectionPool, _url?: string | undefined, _firebaseVersion?: string | undefined); + _url?: string | undefined, _firebaseVersion?: string | undefined); readonly app: FirebaseApp; // (undocumented) readonly _appCheckProvider: Provider; @@ -78,12 +78,13 @@ 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(requestInfo: RequestInfo_2, authToken: string | null, appCheckToken: string | null): Request_2; + _makeRequest(requestInfo: RequestInfo_2, requestFactory: () => Connection, authToken: string | null, appCheckToken: string | null): Request_2; // (undocumented) - makeRequestWithTokens(requestInfo: RequestInfo_2): Promise>; + makeRequestWithTokens(requestInfo: RequestInfo_2, requestFactory: () => Connection): Promise; _makeStorageReference(loc: _Location): _Reference; get maxOperationRetryTime(): number; set maxOperationRetryTime(time: number); @@ -91,10 +92,6 @@ export class _FirebaseStorageImpl implements FirebaseStorage { 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) diff --git a/packages/storage-compat/src/task.ts b/packages/storage-compat/src/task.ts index 13fcc6e8d03..66a4dd9262e 100644 --- a/packages/storage-compat/src/task.ts +++ b/packages/storage-compat/src/task.ts @@ -17,7 +17,7 @@ import { UploadTask, - FirebaseStorageError, + StorageError, UploadTaskSnapshot, TaskEvent, StorageObserver @@ -48,7 +48,7 @@ export class UploadTaskCompat implements types.UploadTask, Compat { then( onFulfilled?: ((a: UploadTaskSnapshotCompat) => unknown) | null, - onRejected?: ((a: FirebaseStorageError) => unknown) | null + onRejected?: ((a: StorageError) => unknown) | null ): Promise { return this._delegate.then(snapshot => { if (onFulfilled) { @@ -65,7 +65,7 @@ export class UploadTaskCompat implements types.UploadTask, Compat { | types.StorageObserver | null | ((a: UploadTaskSnapshotCompat) => unknown), - error?: (error: FirebaseStorageError) => void | null, + error?: (error: StorageError) => void | null, completed?: () => void | null ): Unsubscribe | Subscribe { let wrappedNextOrObserver: @@ -123,7 +123,7 @@ export type NextFn = (value: T) => void; * A function that is called with a `FirebaseStorageError` * if the event stream ends due to an error. */ -export type ErrorFn = (error: FirebaseStorageError) => void; +export type ErrorFn = (error: StorageError) => void; /** * A function that is called if the event stream ends normally. diff --git a/packages/storage-compat/test/unit/reference.test.ts b/packages/storage-compat/test/unit/reference.test.ts index 0d809a3c823..882dbd72328 100644 --- a/packages/storage-compat/test/unit/reference.test.ts +++ b/packages/storage-compat/test/unit/reference.test.ts @@ -162,15 +162,15 @@ describe('Firebase Storage > Reference', () => { describe('Argument verification', () => { describe('list', () => { - it('throws on invalid maxResults', async () => { + it('throws on invalid maxResults', () => { const child = service.refFromURL('gs://test-bucket/hello'); - await expect(child.list({ maxResults: 0 })).to.be.rejectedWith( + expect(() => child.list({ maxResults: 0 })).to.throw( 'storage/invalid-argument' ); - await expect(child.list({ maxResults: -4 })).to.be.rejectedWith( + expect(() => child.list({ maxResults: -4 })).to.throw( 'storage/invalid-argument' ); - await expect(child.list({ maxResults: 1001 })).to.be.rejectedWith( + expect(() => child.list({ maxResults: 1001 })).to.throw( 'storage/invalid-argument' ); }); @@ -195,18 +195,18 @@ describe('Firebase Storage > Reference', () => { it('delete throws', () => { expect(() => root.delete()).to.throw('storage/invalid-root-operation'); }); - it('getMetadata throws', async () => { - await expect(root.getMetadata()).to.be.rejectedWith( + it('getMetadata throws', () => { + expect(() => root.getMetadata()).to.throw( 'storage/invalid-root-operation' ); }); - it('updateMetadata throws', async () => { - await expect(root.updateMetadata({})).to.be.rejectedWith( + it('updateMetadata throws', () => { + expect(() => root.updateMetadata({})).to.throw( 'storage/invalid-root-operation' ); }); it('getDownloadURL throws', async () => { - await expect(root.getDownloadURL()).to.be.rejectedWith( + expect(() => root.getDownloadURL()).to.throw( 'storage/invalid-root-operation' ); }); diff --git a/packages/storage-compat/test/unit/service.test.ts b/packages/storage-compat/test/unit/service.test.ts index 5059052218b..f7632ab6ef0 100644 --- a/packages/storage-compat/test/unit/service.test.ts +++ b/packages/storage-compat/test/unit/service.test.ts @@ -19,11 +19,7 @@ import { expect } from 'chai'; import { stub } from 'sinon'; import * as modularStorage from '@firebase/storage'; import { makeTestCompatStorage, fakeApp, fakeStorage } from '../utils'; -import { - FirebaseStorage, - getStorage, - FirebaseStorageError -} from '@firebase/storage'; +import { FirebaseStorage, getStorage, StorageError } from '@firebase/storage'; import firebase from '@firebase/app-compat'; import { StorageServiceCompat } from '../../src/service'; import { FirebaseApp } from '@firebase/app-types'; @@ -215,10 +211,8 @@ GOOG4-RSA-SHA256` ref.put(new Uint8Array([97])).on( 'state_changed', null, - (err: FirebaseStorageError | Error) => { - expect((err as FirebaseStorageError).code).to.equal( - 'storage/app-deleted' - ); + (err: StorageError | Error) => { + expect((err as StorageError).code).to.equal('storage/app-deleted'); resolve(); }, () => { diff --git a/packages/storage/src/implementation/connectionPool.ts b/packages/storage/src/implementation/connectionPool.ts deleted file mode 100644 index 133f9a72523..00000000000 --- a/packages/storage/src/implementation/connectionPool.ts +++ /dev/null @@ -1,31 +0,0 @@ -/** - * @license - * Copyright 2017 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @fileoverview Replacement for goog.net.XhrIoPool that works with fbs.XhrIo. - */ -import { Connection } from './connection'; -import { newConnection } from '../platform/connection'; - -/** - * Factory-like class for creating XhrIo instances. - */ -export class ConnectionPool { - createConnection(): Connection { - return newConnection(); - } -} diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index 5c992ba6f1c..7eaf3f04439 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -32,7 +32,6 @@ import { RequestHandler, RequestInfo } from './requestinfo'; import { isJustDef } from './type'; import { makeQueryString } from './url'; import { Headers, Connection, ErrorCode } from './connection'; -import { ConnectionPool } from './connectionPool'; export interface Request { getPromise(): Promise; @@ -68,7 +67,7 @@ class NetworkRequest implements Request { private errorCallback_: RequestHandler | null, private timeout_: number, private progressCallback_: ((p1: number, p2: number) => void) | null, - private pool_: ConnectionPool + private connectionFactory_: () => Connection ) { this.promise_ = new Promise((resolve, reject) => { this.resolve_ = resolve as (value?: T | PromiseLike) => void; @@ -89,7 +88,7 @@ class NetworkRequest implements Request { backoffCallback(false, new RequestEndStatus(false, null, true)); return; } - const connection = this.pool_.createConnection(); + const connection = this.connectionFactory_(); this.pendingConnection_ = connection; const progressListener: (progressEvent: ProgressEvent) => void = @@ -272,7 +271,7 @@ export function makeRequest( appId: string | null, authToken: string | null, appCheckToken: string | null, - pool: ConnectionPool, + requestFactory: () => Connection, firebaseVersion?: string ): Request { const queryPart = makeQueryString(requestInfo.urlParams); @@ -293,6 +292,6 @@ export function makeRequest( requestInfo.errorHandler, requestInfo.timeout, requestInfo.progressCallback, - pool + requestFactory ); } diff --git a/packages/storage/src/implementation/requestmaker.ts b/packages/storage/src/implementation/requestmaker.ts deleted file mode 100644 index eede271f062..00000000000 --- a/packages/storage/src/implementation/requestmaker.ts +++ /dev/null @@ -1,28 +0,0 @@ -/** - * @license - * Copyright 2017 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { Request } from './request'; -import { RequestInfo } from './requestinfo'; -import { ConnectionPool } from './connectionPool'; - -type requestMaker = ( - requestInfo: RequestInfo, - appId: string | null, - authToken: string | null, - pool: ConnectionPool -) => Request; - -export { requestMaker }; diff --git a/packages/storage/src/index.node.ts b/packages/storage/src/index.node.ts index 6d8db77c7cf..89af06170d4 100644 --- a/packages/storage/src/index.node.ts +++ b/packages/storage/src/index.node.ts @@ -40,7 +40,6 @@ import { name, version } from '../package.json'; import { FirebaseStorage } from './public-types'; import { STORAGE_TYPE } from './constants'; -import { ConnectionPool } from './implementation/connectionPool'; export * from './api'; export * from './api.node'; @@ -57,7 +56,6 @@ function factory( app, authProvider, appCheckProvider, - new ConnectionPool(), url, SDK_VERSION ); diff --git a/packages/storage/src/index.ts b/packages/storage/src/index.ts index 159c66463c3..1e19f5bf6eb 100644 --- a/packages/storage/src/index.ts +++ b/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 ); diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 093f2425467..087f1d31544 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/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. @@ -124,5 +127,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; } diff --git a/packages/storage/src/platform/connection.ts b/packages/storage/src/platform/connection.ts index bfcc0a1f92a..93863e3a70b 100644 --- a/packages/storage/src/platform/connection.ts +++ b/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); +} diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index e7c70bb227d..5bbadc98141 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -19,6 +19,9 @@ import { ErrorCode, Connection } from '../../implementation/connection'; import { internalError } from '../../implementation/error'; import nodeFetch, { FetchError } from 'node-fetch'; +/** An override for the text-based Connection. Used in tests. */ +let connectionFactoryOverride: (() => Connection) | null = null; + /** * Network layer that works in Node. * @@ -118,5 +121,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; } diff --git a/packages/storage/src/reference.ts b/packages/storage/src/reference.ts index d925ba2091a..a89fd5a66b1 100644 --- a/packages/storage/src/reference.ts +++ b/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 { @@ -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 { +export function getMetadata(ref: Reference): Promise { 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 { * with the new metadata for this object. * See `firebaseStorage.Reference.prototype.getMetadata` */ -export async function updateMetadata( +export function updateMetadata( ref: Reference, metadata: Partial ): Promise { @@ -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 { +export function getDownloadURL(ref: Reference): Promise { 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 { * @param ref - StorageReference for object to delete. * @returns A `Promise` that resolves if the deletion succeeds. */ -export async function deleteObject(ref: Reference): Promise { +export function deleteObject(ref: Reference): Promise { ref._throwIfRoot('deleteObject'); const requestInfo = requestsDeleteObject(ref.storage, ref._location); - return (await ref.storage.makeRequestWithTokens(requestInfo)).getPromise(); + return ref.storage.makeRequestWithTokens(requestInfo, newConnection); } /** diff --git a/packages/storage/src/service.ts b/packages/storage/src/service.ts index b54ac313cc7..c1e2bd080d4 100644 --- a/packages/storage/src/service.ts +++ b/packages/storage/src/service.ts @@ -19,7 +19,6 @@ import { Location } from './implementation/location'; import { FailRequest } from './implementation/failrequest'; import { Request, makeRequest } from './implementation/request'; import { RequestInfo } from './implementation/requestinfo'; -import { ConnectionPool } from './implementation/connectionPool'; import { Reference, _getChild } from './reference'; import { Provider } from '@firebase/component'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; @@ -31,7 +30,7 @@ import { DEFAULT_HOST, DEFAULT_MAX_OPERATION_RETRY_TIME, DEFAULT_MAX_UPLOAD_RETRY_TIME -} from '../src/implementation/constants'; +} from './implementation/constants'; import { invalidArgument, appDeleted, @@ -40,6 +39,7 @@ import { import { validateNumber } from './implementation/type'; import { FirebaseStorage } from './public-types'; import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; +import { Connection } from './implementation/connection'; export function isUrl(path?: string): boolean { return /^[A-Za-z]+:\/\//.test(path as string); @@ -182,7 +182,6 @@ export class FirebaseStorageImpl implements FirebaseStorage { /** * @internal */ - readonly _pool: ConnectionPool, readonly _url?: string, readonly _firebaseVersion?: string ) { @@ -301,6 +300,7 @@ export class FirebaseStorageImpl implements FirebaseStorage { */ _makeRequest( requestInfo: RequestInfo, + requestFactory: () => Connection, authToken: string | null, appCheckToken: string | null ): Request { @@ -310,7 +310,7 @@ export class FirebaseStorageImpl implements FirebaseStorage { this._appId, authToken, appCheckToken, - this._pool, + requestFactory, this._firebaseVersion ); this._requests.add(request); @@ -326,13 +326,19 @@ export class FirebaseStorageImpl implements FirebaseStorage { } async makeRequestWithTokens( - requestInfo: RequestInfo - ): Promise> { + requestInfo: RequestInfo, + requestFactory: () => Connection + ): Promise { const [authToken, appCheckToken] = await Promise.all([ this._getAuthToken(), this._getAppCheckToken() ]); - return this._makeRequest(requestInfo, authToken, appCheckToken); + return this._makeRequest( + requestInfo, + requestFactory, + authToken, + appCheckToken + ).getPromise(); } } diff --git a/packages/storage/src/task.ts b/packages/storage/src/task.ts index 6ba1d861b86..3cf0fb9f0e8 100644 --- a/packages/storage/src/task.ts +++ b/packages/storage/src/task.ts @@ -52,6 +52,7 @@ import { multipartUpload } from './implementation/requests'; import { Reference } from './reference'; +import { newConnection } from './platform/connection'; /** * Represents a blob being uploaded. Can be used to pause/resume/cancel the @@ -207,6 +208,7 @@ export class UploadTask { ); const createRequest = this._ref.storage._makeRequest( requestInfo, + newConnection, authToken, appCheckToken ); @@ -232,6 +234,7 @@ export class UploadTask { ); const statusRequest = this._ref.storage._makeRequest( requestInfo, + newConnection, authToken, appCheckToken ); @@ -278,6 +281,7 @@ export class UploadTask { } const uploadRequest = this._ref.storage._makeRequest( requestInfo, + newConnection, authToken, appCheckToken ); @@ -314,6 +318,7 @@ export class UploadTask { ); const metadataRequest = this._ref.storage._makeRequest( requestInfo, + newConnection, authToken, appCheckToken ); @@ -337,6 +342,7 @@ export class UploadTask { ); const multipartRequest = this._ref.storage._makeRequest( requestInfo, + newConnection, authToken, appCheckToken ); diff --git a/packages/storage/test/unit/connection.ts b/packages/storage/test/unit/connection.ts index 24e6cd76044..11c27d29a6a 100644 --- a/packages/storage/test/unit/connection.ts +++ b/packages/storage/test/unit/connection.ts @@ -134,3 +134,7 @@ export class TestingConnection implements Connection { // TODO(andysoto): impl } } + +export function newTestConnection(sendHook?: SendHook | null): Connection { + return new TestingConnection(sendHook ?? null); +} diff --git a/packages/storage/test/unit/reference.test.ts b/packages/storage/test/unit/reference.test.ts index 66f53017f87..fe9299ad2b0 100644 --- a/packages/storage/test/unit/reference.test.ts +++ b/packages/storage/test/unit/reference.test.ts @@ -32,35 +32,29 @@ import { } from '../../src/reference'; import { FirebaseStorageImpl, ref } from '../../src/service'; import * as testShared from './testshared'; -import { SendHook, TestingConnection } from './connection'; +import { newTestConnection, TestingConnection } from './connection'; import { DEFAULT_HOST } from '../../src/implementation/constants'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { Provider } from '@firebase/component'; import { AppCheckInternalComponentName } from '@firebase/app-check-interop-types'; import { fakeServerHandler, storageServiceWithHandler } from './testshared'; import { decodeUint8Array } from '../../src/platform/base64'; +import { injectTestConnection } from '../../src/platform/connection'; /* eslint-disable @typescript-eslint/no-floating-promises */ function makeFakeService( app: FirebaseApp, authProvider: Provider, - appCheckProvider: Provider, - sendHook: SendHook + appCheckProvider: Provider ): FirebaseStorageImpl { - return new FirebaseStorageImpl( - app, - authProvider, - appCheckProvider, - testShared.makePool(sendHook) - ); + return new FirebaseStorageImpl(app, authProvider, appCheckProvider); } function makeStorage(url: string): Reference { const service = new FirebaseStorageImpl( {} as FirebaseApp, testShared.emptyAuthProvider, - testShared.fakeAppCheckTokenProvider, - testShared.makePool(null) + testShared.fakeAppCheckTokenProvider ); return new Reference(service, url); } @@ -85,14 +79,15 @@ function withFakeSend( text.then(text => { testFn(text, headers); connection.abort(); + injectTestConnection(null); resolveFn(); }); } + injectTestConnection(() => newTestConnection(newSend)); const service = makeFakeService( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - newSend + testShared.fakeAppCheckTokenProvider ); return ref(service, 'gs://test-bucket'); } @@ -231,14 +226,15 @@ describe('Firebase Storage > Reference', () => { ): void { expect(headers).to.not.be.undefined; expect(headers!['Authorization']).to.be.undefined; + injectTestConnection(null); done(); } + injectTestConnection(() => newTestConnection(newSend)); const service = makeFakeService( testShared.fakeApp, testShared.emptyAuthProvider, - testShared.fakeAppCheckTokenProvider, - newSend + testShared.fakeAppCheckTokenProvider ); const reference = ref(service, 'gs://test-bucket'); getMetadata(ref(reference, 'foo')); @@ -257,14 +253,15 @@ describe('Firebase Storage > Reference', () => { expect(headers!['Authorization']).to.equal( 'Firebase ' + testShared.authToken ); + injectTestConnection(null); done(); } + injectTestConnection(() => newTestConnection(newSend)); const service = makeFakeService( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - newSend + testShared.fakeAppCheckTokenProvider ); const reference = ref(service, 'gs://test-bucket'); getMetadata(ref(reference, 'foo')); @@ -325,13 +322,13 @@ describe('Firebase Storage > Reference', () => { describe('Argument verification', () => { describe('list', () => { it('throws on invalid maxResults', async () => { - await expect(list(child, { maxResults: 0 })).to.be.rejectedWith( + expect(() => list(child, { maxResults: 0 })).to.throw( 'storage/invalid-argument' ); - await expect(list(child, { maxResults: -4 })).to.be.rejectedWith( + expect(() => list(child, { maxResults: -4 })).to.throw( 'storage/invalid-argument' ); - await expect(list(child, { maxResults: 1001 })).to.be.rejectedWith( + expect(() => list(child, { maxResults: 1001 })).to.throw( 'storage/invalid-argument' ); }); @@ -355,22 +352,22 @@ describe('Firebase Storage > Reference', () => { ); }); it('deleteObject throws', async () => { - await expect(deleteObject(root)).to.be.rejectedWith( + expect(() => deleteObject(root)).to.throw( 'storage/invalid-root-operation' ); }); it('getMetadata throws', async () => { - await expect(getMetadata(root)).to.be.rejectedWith( + expect(() => getMetadata(root)).to.throw( 'storage/invalid-root-operation' ); }); it('updateMetadata throws', async () => { - await expect(updateMetadata(root, {} as Metadata)).to.be.rejectedWith( + expect(() => updateMetadata(root, {} as Metadata)).to.throw( 'storage/invalid-root-operation' ); }); it('getDownloadURL throws', async () => { - await expect(getDownloadURL(root)).to.be.rejectedWith( + expect(() => getDownloadURL(root)).to.throw( 'storage/invalid-root-operation' ); }); diff --git a/packages/storage/test/unit/request.test.ts b/packages/storage/test/unit/request.test.ts index 26304b271f8..633e828cc49 100644 --- a/packages/storage/test/unit/request.test.ts +++ b/packages/storage/test/unit/request.test.ts @@ -19,8 +19,7 @@ import * as sinon from 'sinon'; import { makeRequest } from '../../src/implementation/request'; import { RequestInfo } from '../../src/implementation/requestinfo'; import { Connection } from '../../src/implementation/connection'; -import { makePool } from './testshared'; -import { TestingConnection } from './connection'; +import { TestingConnection, newTestConnection } from './connection'; const TEST_VERSION = '1.2.3'; @@ -64,7 +63,7 @@ describe('Firebase Storage > Request', () => { null, null, null, - makePool(spiedSend), + () => newTestConnection(spiedSend), TEST_VERSION ) .getPromise() @@ -108,7 +107,9 @@ describe('Firebase Storage > Request', () => { requestInfo.urlParams[p1] = v1; requestInfo.urlParams[p2] = v2; requestInfo.body = 'thisistherequestbody'; - return makeRequest(requestInfo, null, null, null, makePool(spiedSend)) + return makeRequest(requestInfo, null, null, null, () => + newTestConnection(spiedSend) + ) .getPromise() .then( () => { @@ -151,7 +152,9 @@ describe('Firebase Storage > Request', () => { timeout ); - return makeRequest(requestInfo, null, null, null, makePool(newSend)) + return makeRequest(requestInfo, null, null, null, () => + newTestConnection(newSend) + ) .getPromise() .then( () => { @@ -173,7 +176,13 @@ describe('Firebase Storage > Request', () => { handler, timeout ); - const request = makeRequest(requestInfo, null, null, null, makePool(null)); + const request = makeRequest( + requestInfo, + null, + null, + null, + newTestConnection + ); const promise = request.getPromise().then( () => { assert.fail('Succeeded when handler gave error'); @@ -205,7 +214,7 @@ describe('Firebase Storage > Request', () => { /* appId= */ null, authToken, null, - makePool(spiedSend), + () => newTestConnection(spiedSend), TEST_VERSION ); return request.getPromise().then( @@ -246,7 +255,7 @@ describe('Firebase Storage > Request', () => { appId, null, null, - makePool(spiedSend), + () => newTestConnection(spiedSend), TEST_VERSION ); return request.getPromise().then( @@ -287,7 +296,7 @@ describe('Firebase Storage > Request', () => { null, null, appCheckToken, - makePool(spiedSend), + () => newTestConnection(spiedSend), TEST_VERSION ); return request.getPromise().then( diff --git a/packages/storage/test/unit/requests.test.ts b/packages/storage/test/unit/requests.test.ts index 1996a600e85..25a02ce0337 100644 --- a/packages/storage/test/unit/requests.test.ts +++ b/packages/storage/test/unit/requests.test.ts @@ -37,7 +37,6 @@ import { import { makeUrl } from '../../src/implementation/url'; import { unknown, StorageErrorCode } from '../../src/implementation/error'; import { RequestInfo } from '../../src/implementation/requestinfo'; -import { ConnectionPool } from '../../src/implementation/connectionPool'; import { Metadata } from '../../src/metadata'; import { FirebaseStorageImpl } from '../../src/service'; import { @@ -82,8 +81,7 @@ describe('Firebase Storage > Requests', () => { const storageService = new FirebaseStorageImpl( mockApp, fakeAuthProvider, - fakeAppCheckTokenProvider, - new ConnectionPool() + fakeAppCheckTokenProvider ); const contentTypeInMetadata = 'application/jason'; diff --git a/packages/storage/test/unit/service.test.ts b/packages/storage/test/unit/service.test.ts index 69b25eef685..be42bb8dd6e 100644 --- a/packages/storage/test/unit/service.test.ts +++ b/packages/storage/test/unit/service.test.ts @@ -17,7 +17,6 @@ import { expect } from 'chai'; import { TaskEvent } from '../../src/implementation/taskenums'; import { Headers } from '../../src/implementation/connection'; -import { ConnectionPool } from '../../src/implementation/connectionPool'; import { FirebaseStorageImpl, ref, @@ -33,12 +32,12 @@ import { getDownloadURL } from '../../src/reference'; import { Location } from '../../src/implementation/location'; -import { TestingConnection } from './connection'; +import { newTestConnection, TestingConnection } from './connection'; +import { injectTestConnection } from '../../src/platform/connection'; const fakeAppGs = testShared.makeFakeApp('gs://mybucket'); const fakeAppGsEndingSlash = testShared.makeFakeApp('gs://mybucket/'); const fakeAppInvalidGs = testShared.makeFakeApp('gs://mybucket/hello'); -const connectionPool = new ConnectionPool(); const testLocation = new Location('bucket', 'object'); function makeGsUrl(child: string = ''): string { @@ -46,12 +45,14 @@ function makeGsUrl(child: string = ''): string { } describe('Firebase Storage > Service', () => { + before(() => injectTestConnection(newTestConnection)); + after(() => injectTestConnection(null)); + describe('simple constructor', () => { const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); it('Root refs point to the right place', () => { const reference = ref(service); @@ -68,7 +69,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, 'gs://foo-bar.appspot.com' ); const reference = ref(service); @@ -79,7 +79,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, `http://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o` ); const reference = ref(service); @@ -90,7 +89,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, `https://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o` ); const reference = ref(service); @@ -102,7 +100,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, 'foo-bar.appspot.com' ); const reference = ref(service); @@ -113,7 +110,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, 'foo-bar.appspot.com' ); const reference = ref(service, 'path/to/child'); @@ -127,7 +123,6 @@ describe('Firebase Storage > Service', () => { testShared.fakeApp, testShared.fakeAuthProvider, testShared.fakeAppCheckTokenProvider, - connectionPool, 'gs://bucket/object/' ); }, 'storage/invalid-default-bucket'); @@ -139,8 +134,7 @@ describe('Firebase Storage > Service', () => { const service = new FirebaseStorageImpl( fakeAppGs, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); expect(ref(service)?.toString()).to.equal('gs://mybucket/'); }); @@ -148,8 +142,7 @@ describe('Firebase Storage > Service', () => { const service = new FirebaseStorageImpl( fakeAppGsEndingSlash, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); expect(ref(service)?.toString()).to.equal('gs://mybucket/'); }); @@ -158,8 +151,7 @@ describe('Firebase Storage > Service', () => { new FirebaseStorageImpl( fakeAppInvalidGs, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); }, 'storage/invalid-default-bucket'); }); @@ -168,8 +160,7 @@ describe('Firebase Storage > Service', () => { const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); it('Works with gs:// URLs', () => { const reference = ref(service, 'gs://mybucket/child/path/image.png'); @@ -237,24 +228,20 @@ GOOG4-RSA-SHA256` }); describe('connectStorageEmulator(service, host, port, options)', () => { it('sets emulator host correctly', done => { - function newSend( - connection: TestingConnection, - url: string, - method: string, - body?: ArrayBufferView | Blob | string | null, - headers?: Headers - ): void { + function newSend(connection: TestingConnection, url: string): void { // Expect emulator host to be in url of storage operations requests, // in this case getDownloadURL. expect(url).to.match(/^http:\/\/test\.host\.org:1234.+/); connection.abort(); + injectTestConnection(null); done(); } + + injectTestConnection(() => newTestConnection(newSend)); const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - testShared.makePool(newSend) + testShared.fakeAppCheckTokenProvider ); connectStorageEmulator(service, 'test.host.org', 1234); expect(service.host).to.equal('test.host.org:1234'); @@ -275,13 +262,14 @@ GOOG4-RSA-SHA256` expect(url).to.match(/^http:\/\/test\.host\.org:1234.+/); expect(headers?.['Authorization']).to.eql(`Firebase ${mockUserToken}`); connection.abort(); + injectTestConnection(null); done(); } + injectTestConnection(() => newTestConnection(newSend)); const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - testShared.makePool(newSend) + testShared.fakeAppCheckTokenProvider ); connectStorageEmulator(service, 'test.host.org', 1234, { mockUserToken }); expect(service.host).to.equal('test.host.org:1234'); @@ -303,14 +291,14 @@ GOOG4-RSA-SHA256` expect(url).to.match(/^http:\/\/test\.host\.org:1234.+/); expect(headers?.['Authorization']).to.eql(`Firebase ${token}`); connection.abort(); + injectTestConnection(null); done(); } - + injectTestConnection(() => newTestConnection(newSend)); const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - testShared.makePool(newSend) + testShared.fakeAppCheckTokenProvider ); connectStorageEmulator(service, 'test.host.org', 1234, { mockUserToken: { sub: 'alice' } @@ -327,8 +315,7 @@ GOOG4-RSA-SHA256` const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); it('Works with non URL paths', () => { const newRef = ref(service, 'child/path/image.png'); @@ -343,8 +330,7 @@ GOOG4-RSA-SHA256` const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); const reference = new Reference(service, testLocation); it('Throws calling ref(reference, path) with a gs:// URL', () => { @@ -381,8 +367,7 @@ GOOG4-RSA-SHA256` const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); it('In-flight requests are canceled when the service is deleted', async () => { const reference = ref(service, 'gs://mybucket/image.jpg'); @@ -424,8 +409,7 @@ GOOG4-RSA-SHA256` const service = new FirebaseStorageImpl( testShared.fakeApp, testShared.fakeAuthProvider, - testShared.fakeAppCheckTokenProvider, - connectionPool + testShared.fakeAppCheckTokenProvider ); describe('setMaxUploadRetryTime', () => { it('Throws on negative arg', () => { diff --git a/packages/storage/test/unit/task.test.ts b/packages/storage/test/unit/task.test.ts index 93a3d4b9620..cdd57152cb0 100644 --- a/packages/storage/test/unit/task.test.ts +++ b/packages/storage/test/unit/task.test.ts @@ -22,12 +22,15 @@ import { TaskEvent, TaskState } from '../../src/implementation/taskenums'; import { Reference } from '../../src/reference'; import { UploadTask } from '../../src/task'; import { fakeServerHandler, storageServiceWithHandler } from './testshared'; +import { injectTestConnection } from '../../src/platform/connection'; const testLocation = new Location('bucket', 'object'); const smallBlob = new FbsBlob(new Uint8Array([97])); const bigBlob = new FbsBlob(new ArrayBuffer(1024 * 1024)); describe('Firebase Storage > Upload Task', () => { + after(() => injectTestConnection(null)); + it('Works for a small upload w/ an observer', done => { const storageService = storageServiceWithHandler(fakeServerHandler()); const task = new UploadTask( diff --git a/packages/storage/test/unit/testshared.ts b/packages/storage/test/unit/testshared.ts index 835090d79af..2a35386b653 100644 --- a/packages/storage/test/unit/testshared.ts +++ b/packages/storage/test/unit/testshared.ts @@ -23,8 +23,7 @@ import { FirebaseApp } from '@firebase/app-types'; import { CONFIG_STORAGE_BUCKET_KEY } from '../../src/implementation/constants'; import { StorageError } from '../../src/implementation/error'; import { Headers, Connection } from '../../src/implementation/connection'; -import { ConnectionPool } from '../../src/implementation/connectionPool'; -import { SendHook, TestingConnection } from './connection'; +import { newTestConnection, TestingConnection } from './connection'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { Provider, @@ -35,6 +34,7 @@ import { import { AppCheckInternalComponentName } from '@firebase/app-check-interop-types'; import { FirebaseStorageImpl } from '../../src/service'; import { Metadata } from '../../src/metadata'; +import { injectTestConnection } from '../../src/platform/connection'; export const authToken = 'totally-legit-auth-token'; export const appCheckToken = 'totally-shady-token'; @@ -106,15 +106,6 @@ export function makeFakeAppCheckProvider(tokenResult: { return provider as Provider; } -export function makePool(sendHook: SendHook | null): ConnectionPool { - const pool: any = { - createConnection() { - return new TestingConnection(sendHook); - } - }; - return pool as ConnectionPool; -} - /** * Returns something that looks like an fbs.XhrIo with the given headers * and status. @@ -224,11 +215,11 @@ export function storageServiceWithHandler( ); } + injectTestConnection(() => newTestConnection(newSend)); return new FirebaseStorageImpl( {} as FirebaseApp, emptyAuthProvider, - fakeAppCheckTokenProvider, - makePool(newSend) + fakeAppCheckTokenProvider ); }