From 62eadc3cdce0e2fcb74b601186d27611e3737848 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 27 Oct 2021 14:48:48 -0600 Subject: [PATCH 01/23] Add getBytes() --- packages/storage/.run/All Tests.run.xml | 4 +- packages/storage/src/api.browser.ts | 15 +- packages/storage/src/api.node.ts | 14 +- packages/storage/src/api.ts | 25 ++- .../storage/src/implementation/connection.ts | 17 +- .../storage/src/implementation/request.ts | 66 +++---- .../storage/src/implementation/requestinfo.ts | 27 ++- .../storage/src/implementation/requests.ts | 138 ++++++++++++--- .../src/platform/browser/connection.ts | 136 +++++++++++++-- packages/storage/src/platform/connection.ts | 30 +++- .../storage/src/platform/node/connection.ts | 161 +++++++++++++----- packages/storage/src/reference.ts | 112 +++++++++++- packages/storage/src/service.ts | 16 +- packages/storage/src/task.ts | 12 +- packages/storage/test/browser/blob.test.ts | 51 +++++- .../storage/test/browser/connection.test.ts | 4 +- .../test/integration/integration.test.ts | 60 +++++-- .../test/{unit => node}/connection.test.ts | 4 +- packages/storage/test/node/stream.test.ts | 76 +++++++++ packages/storage/test/unit/connection.ts | 12 +- packages/storage/test/unit/request.test.ts | 4 +- packages/storage/test/unit/requests.test.ts | 17 +- packages/storage/test/unit/testshared.ts | 7 +- 23 files changed, 811 insertions(+), 197 deletions(-) rename packages/storage/test/{unit => node}/connection.test.ts (89%) create mode 100644 packages/storage/test/node/stream.test.ts diff --git a/packages/storage/.run/All Tests.run.xml b/packages/storage/.run/All Tests.run.xml index c74dd195587..1830936c7e7 100644 --- a/packages/storage/.run/All Tests.run.xml +++ b/packages/storage/.run/All Tests.run.xml @@ -11,9 +11,9 @@ bdd - --require ts-node/register/type-check --require index.node.ts + --require ts-node/register/type-check --require src/index.node.ts PATTERN test/{,!(browser)/**/}*.test.ts - \ No newline at end of file + diff --git a/packages/storage/src/api.browser.ts b/packages/storage/src/api.browser.ts index acb501f62d5..cf39444832d 100644 --- a/packages/storage/src/api.browser.ts +++ b/packages/storage/src/api.browser.ts @@ -16,6 +16,8 @@ */ import { StorageReference } from './public-types'; +import { Reference, getBlobInternal } from './reference'; +import { getModularInstance } from '@firebase/util'; /** * Downloads the data at the object's location. Returns an error if the object @@ -33,12 +35,12 @@ import { StorageReference } from './public-types'; * retrieve. * @returns A Promise that resolves with a Blob containing the object's bytes */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function getBlob( +export function getBlob( ref: StorageReference, maxDownloadSizeBytes?: number ): Promise { - throw new Error('Not implemented'); + ref = getModularInstance(ref); + return getBlobInternal(ref as Reference, maxDownloadSizeBytes); } /** @@ -53,14 +55,9 @@ function getBlob( * retrieve. * @returns A stream with the object's data as bytes */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function getStream( +export function getStream( ref: StorageReference, maxDownloadSizeBytes?: number ): NodeJS.ReadableStream { throw new Error('getStream() is only supported by NodeJS builds'); } - -// TODO(getbytes): Export getBlob/getStream - -export {}; diff --git a/packages/storage/src/api.node.ts b/packages/storage/src/api.node.ts index 4ab8ef2e412..b413260d5bd 100644 --- a/packages/storage/src/api.node.ts +++ b/packages/storage/src/api.node.ts @@ -16,6 +16,8 @@ */ import { StorageReference } from './public-types'; +import { Reference, getStreamInternal } from './reference'; +import { getModularInstance } from '@firebase/util'; /** * Downloads the data at the object's location. Returns an error if the object @@ -34,7 +36,7 @@ import { StorageReference } from './public-types'; * @returns A Promise that resolves with a Blob containing the object's bytes */ // eslint-disable-next-line @typescript-eslint/no-unused-vars -function getBlob( +export function getBlob( ref: StorageReference, maxDownloadSizeBytes?: number ): Promise { @@ -53,14 +55,10 @@ function getBlob( * retrieve. * @returns A stream with the object's data as bytes */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function getStream( +export function getStream( ref: StorageReference, maxDownloadSizeBytes?: number ): NodeJS.ReadableStream { - throw new Error('Not implemented'); + ref = getModularInstance(ref); + return getStreamInternal(ref as Reference, maxDownloadSizeBytes); } - -// TODO(getbytes): Export getBlob/getStream - -export {}; diff --git a/packages/storage/src/api.ts b/packages/storage/src/api.ts index ad662a1011c..20eae852bac 100644 --- a/packages/storage/src/api.ts +++ b/packages/storage/src/api.ts @@ -47,7 +47,8 @@ import { getDownloadURL as getDownloadURLInternal, deleteObject as deleteObjectInternal, Reference, - _getChild as _getChildInternal + _getChild as _getChildInternal, + getBytesInternal } from './reference'; import { STORAGE_TYPE } from './constants'; import { EmulatorMockTokenOptions, getModularInstance } from '@firebase/util'; @@ -76,6 +77,28 @@ export { } from './implementation/taskenums'; export { StringFormat }; +/** + * Downloads the data at the object's location. Returns an error if the object + * is not found. + * + * To use this functionality, you have to whitelist your app's origin in your + * Cloud Storage bucket. See also + * https://cloud.google.com/storage/docs/configuring-cors + * + * @public + * @param ref - StorageReference where data should be download. + * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to + * retrieve. + * @returns A Promise containing the object's bytes + */ +export function getBytes( + ref: StorageReference, + maxDownloadSizeBytes?: number +): Promise { + ref = getModularInstance(ref); + return getBytesInternal(ref as Reference, maxDownloadSizeBytes); +} + /** * Uploads data to this object's location. * The upload is not resumable. diff --git a/packages/storage/src/implementation/connection.ts b/packages/storage/src/implementation/connection.ts index ba8c2c3edb6..40616eb1819 100644 --- a/packages/storage/src/implementation/connection.ts +++ b/packages/storage/src/implementation/connection.ts @@ -21,11 +21,18 @@ export type Headers = Record; /** * A lightweight wrapper around XMLHttpRequest with a * goog.net.XhrIo-like interface. + * + * ResponseType is generally either `string`, `ArrayBuffer` or `ReadableSteam`. + * You can create a new connection by invoking `newTextConnection()`, + * `newBytesConnection()` or `newStreamConnection()`. */ -export interface Connection { +export interface Connection { /** - * This method should never reject. In case of encountering an error, set an error code internally which can be accessed - * by calling getErrorCode() by callers. + * Sends a request to the provided URL. + * + * This method never rejects its promise. In case of encountering an error, + * it sets an error code internally which can be accessed by calling + * getErrorCode() by callers. */ send( url: string, @@ -38,7 +45,9 @@ export interface Connection { getStatus(): number; - getResponseText(): string; + getResponse(): ResponseType; + + getErrorText(): string; /** * Abort the request. diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index 7eaf3f04439..530234cde4f 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -20,18 +20,12 @@ * abstract representations. */ -import { start, stop, id as backoffId } from './backoff'; -import { - StorageError, - unknown, - appDeleted, - canceled, - retryLimitExceeded -} from './error'; -import { RequestHandler, RequestInfo } from './requestinfo'; +import { id as backoffId, start, stop } from './backoff'; +import { appDeleted, canceled, retryLimitExceeded, unknown } from './error'; +import { ErrorHandler, RequestHandler, RequestInfo } from './requestinfo'; import { isJustDef } from './type'; import { makeQueryString } from './url'; -import { Headers, Connection, ErrorCode } from './connection'; +import { Connection, ErrorCode, Headers } from './connection'; export interface Request { getPromise(): Promise; @@ -46,15 +40,24 @@ export interface Request { cancel(appDelete?: boolean): void; } -class NetworkRequest implements Request { - private pendingConnection_: Connection | null = null; +/** + * Handles network logic for all Storage Requests, including error reporting and + * retries with backoff. + * + * @param I - the type of the backend's network response (always `string` or + * `ArrayBuffer`). + * @param - O the output type used by the rest of the SDK. The conversion + * happens in the specified `callback_`. + */ +class NetworkRequest implements Request { + private pendingConnection_: Connection | null = null; private backoffId_: backoffId | null = null; - private resolve_!: (value?: T | PromiseLike) => void; + private resolve_!: (value?: O | PromiseLike) => void; // eslint-disable-next-line @typescript-eslint/no-explicit-any private reject_!: (reason?: any) => void; private canceled_: boolean = false; private appDelete_: boolean = false; - promise_: Promise; + private promise_: Promise; constructor( private url_: string, @@ -63,14 +66,14 @@ class NetworkRequest implements Request { private body_: string | Blob | Uint8Array | null, private successCodes_: number[], private additionalRetryCodes_: number[], - private callback_: RequestHandler, - private errorCallback_: RequestHandler | null, + private callback_: RequestHandler, + private errorCallback_: ErrorHandler | null, private timeout_: number, private progressCallback_: ((p1: number, p2: number) => void) | null, - private connectionFactory_: () => Connection + private connectionFactory_: () => Connection ) { this.promise_ = new Promise((resolve, reject) => { - this.resolve_ = resolve as (value?: T | PromiseLike) => void; + this.resolve_ = resolve as (value?: O | PromiseLike) => void; this.reject_ = reject; this.start_(); }); @@ -135,17 +138,14 @@ class NetworkRequest implements Request { */ const backoffDone: ( requestWentThrough: boolean, - status: RequestEndStatus + status: RequestEndStatus ) => void = (requestWentThrough, status) => { const resolve = this.resolve_; const reject = this.reject_; - const connection = status.connection as Connection; + const connection = status.connection as Connection; if (status.wasSuccessCode) { try { - const result = this.callback_( - connection, - connection.getResponseText() - ); + const result = this.callback_(connection, connection.getResponse()); if (isJustDef(result)) { resolve(result); } else { @@ -157,7 +157,7 @@ class NetworkRequest implements Request { } else { if (connection !== null) { const err = unknown(); - err.serverResponse = connection.getResponseText(); + err.serverResponse = connection.getErrorText(); if (this.errorCallback_) { reject(this.errorCallback_(connection, err)); } else { @@ -182,7 +182,7 @@ class NetworkRequest implements Request { } /** @inheritDoc */ - getPromise(): Promise { + getPromise(): Promise { return this.promise_; } @@ -219,7 +219,7 @@ class NetworkRequest implements Request { * A collection of information about the result of a network request. * @param opt_canceled - Defaults to false. */ -export class RequestEndStatus { +export class RequestEndStatus { /** * True if the request was canceled. */ @@ -227,7 +227,7 @@ export class RequestEndStatus { constructor( public wasSuccessCode: boolean, - public connection: Connection | null, + public connection: Connection | null, canceled?: boolean ) { this.canceled = !!canceled; @@ -266,14 +266,14 @@ export function addAppCheckHeader_( } } -export function makeRequest( - requestInfo: RequestInfo, +export function makeRequest( + requestInfo: RequestInfo, appId: string | null, authToken: string | null, appCheckToken: string | null, - requestFactory: () => Connection, + requestFactory: () => Connection, firebaseVersion?: string -): Request { +): Request { const queryPart = makeQueryString(requestInfo.urlParams); const url = requestInfo.url + queryPart; const headers = Object.assign({}, requestInfo.headers); @@ -281,7 +281,7 @@ export function makeRequest( addAuthHeader_(headers, authToken); addVersionHeader_(headers, firebaseVersion); addAppCheckHeader_(headers, appCheckToken); - return new NetworkRequest( + return new NetworkRequest( url, requestInfo.method, headers, diff --git a/packages/storage/src/implementation/requestinfo.ts b/packages/storage/src/implementation/requestinfo.ts index 2451410f826..68ad2ccc503 100644 --- a/packages/storage/src/implementation/requestinfo.ts +++ b/packages/storage/src/implementation/requestinfo.ts @@ -28,16 +28,33 @@ export interface UrlParams { * A function that converts a server response to the API type expected by the * SDK. * - * @param I - the type of the backend's network response + * @param I - the type of the backend's network response (always `string` or + * `ArrayBuffer`). * @param O - the output response type used by the rest of the SDK. */ -export type RequestHandler = (connection: Connection, response: I) => O; +export type RequestHandler = ( + connection: Connection, + response: I +) => O; -export class RequestInfo { +/** A function to handle an error. */ +export type ErrorHandler = ( + connection: Connection, + response: StorageError +) => StorageError; + +/** + * Contains a fully specified request. + * + * @param I - the type of the backend's network response (always `string` or + * `ArrayBuffer`). + * @param O - the output response type used by the rest of the SDK. + */ +export class RequestInfo { urlParams: UrlParams = {}; headers: Headers = {}; body: Blob | string | Uint8Array | null = null; - errorHandler: RequestHandler | null = null; + errorHandler: ErrorHandler | null = null; /** * Called with the current number of bytes uploaded and total size (-1 if not @@ -57,7 +74,7 @@ export class RequestInfo { * Note: The XhrIo passed to this function may be reused after this callback * returns. Do not keep a reference to it in any way. */ - public handler: RequestHandler, + public handler: RequestHandler, public timeout: number ) {} } diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 8b62702156f..9206abb3638 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -41,7 +41,7 @@ import { toResourceString } from './metadata'; import { fromResponseString } from './list'; -import { RequestInfo, UrlParams } from './requestinfo'; +import { RequestHandler, RequestInfo, UrlParams } from './requestinfo'; import { isString } from './type'; import { makeUrl } from './url'; import { Connection } from './connection'; @@ -59,8 +59,8 @@ export function handlerCheck(cndn: boolean): void { export function metadataHandler( service: FirebaseStorageImpl, mappings: Mappings -): (p1: Connection, p2: string) => Metadata { - function handler(xhr: Connection, text: string): Metadata { +): (p1: Connection, p2: string) => Metadata { + function handler(xhr: Connection, text: string): Metadata { const metadata = fromResourceString(service, text, mappings); handlerCheck(metadata !== null); return metadata as Metadata; @@ -71,8 +71,8 @@ export function metadataHandler( export function listHandler( service: FirebaseStorageImpl, bucket: string -): (p1: Connection, p2: string) => ListResult { - function handler(xhr: Connection, text: string): ListResult { +): (p1: Connection, p2: string) => ListResult { + function handler(xhr: Connection, text: string): ListResult { const listResult = fromResponseString(service, bucket, text); handlerCheck(listResult !== null); return listResult as ListResult; @@ -83,8 +83,8 @@ export function listHandler( export function downloadUrlHandler( service: FirebaseStorageImpl, mappings: Mappings -): (p1: Connection, p2: string) => string | null { - function handler(xhr: Connection, text: string): string | null { +): (p1: Connection, p2: string) => string | null { + function handler(xhr: Connection, text: string): string | null { const metadata = fromResourceString(service, text, mappings); handlerCheck(metadata !== null); return downloadUrlFromResourceString( @@ -99,14 +99,17 @@ export function downloadUrlHandler( export function sharedErrorHandler( location: Location -): (p1: Connection, p2: StorageError) => StorageError { - function errorHandler(xhr: Connection, err: StorageError): StorageError { +): (p1: Connection, p2: StorageError) => StorageError { + function errorHandler( + xhr: Connection, + err: StorageError + ): StorageError { let newErr; if (xhr.getStatus() === 401) { if ( // This exact message string is the only consistent part of the // server's error response that identifies it as an App Check error. - xhr.getResponseText().includes('Firebase App Check token is invalid') + xhr.getErrorText().includes('Firebase App Check token is invalid') ) { newErr = unauthorizedApp(); } else { @@ -131,10 +134,13 @@ export function sharedErrorHandler( export function objectErrorHandler( location: Location -): (p1: Connection, p2: StorageError) => StorageError { +): (p1: Connection, p2: StorageError) => StorageError { const shared = sharedErrorHandler(location); - function errorHandler(xhr: Connection, err: StorageError): StorageError { + function errorHandler( + xhr: Connection, + err: StorageError + ): StorageError { let newErr = shared(xhr, err); if (xhr.getStatus() === 404) { newErr = objectNotFound(location.path); @@ -149,7 +155,7 @@ export function getMetadata( service: FirebaseStorageImpl, location: Location, mappings: Mappings -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol); const method = 'GET'; @@ -170,7 +176,7 @@ export function list( delimiter?: string, pageToken?: string | null, maxResults?: number | null -): RequestInfo { +): RequestInfo { const urlParams: UrlParams = {}; if (location.isRoot) { urlParams['prefix'] = ''; @@ -201,11 +207,88 @@ export function list( return requestInfo; } +export function getBytes( + service: FirebaseStorageImpl, + location: Location, + maxDownloadSizeBytes?: number +): RequestInfo { + return createDownloadRequest( + location, + service, + getBytesHandler(), + maxDownloadSizeBytes + ); +} + +export function getBytesHandler(): RequestHandler { + return (xhr: Connection, data: ArrayBuffer) => data; +} + +export function getBlob( + service: FirebaseStorageImpl, + location: Location, + maxDownloadSizeBytes?: number +): RequestInfo { + return createDownloadRequest( + location, + service, + getBlobHandler(), + maxDownloadSizeBytes + ); +} + +export function getBlobHandler(): RequestHandler { + return (xhr: Connection, data: Blob) => data; +} + +export function getStream( + service: FirebaseStorageImpl, + location: Location, + maxDownloadSizeBytes?: number +): RequestInfo { + return createDownloadRequest( + location, + service, + getStreamHandler(), + maxDownloadSizeBytes + ); +} + +export function getStreamHandler(): RequestHandler< + NodeJS.ReadableStream, + NodeJS.ReadableStream +> { + return ( + xhr: Connection, + data: NodeJS.ReadableStream + ) => data; +} + +/** Creates a new request to download an object. */ +function createDownloadRequest( + location: Location, + service: FirebaseStorageImpl, + handler: RequestHandler, + maxDownloadSizeBytes?: number +): RequestInfo { + const urlPart = location.fullServerUrl(); + const url = makeUrl(urlPart, service.host, service._protocol) + '?alt=media'; + const method = 'GET'; + const timeout = service.maxOperationRetryTime; + const requestInfo = new RequestInfo(url, method, handler, timeout); + requestInfo.errorHandler = objectErrorHandler(location); + if (maxDownloadSizeBytes !== undefined) { + requestInfo.headers['Range'] = `bytes=0-${maxDownloadSizeBytes}`; + requestInfo.successCodes = [200 /* OK */, 206 /* Partial Content */]; + } + return requestInfo; +} + export function getDownloadUrl( service: FirebaseStorageImpl, location: Location, mappings: Mappings -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol); const method = 'GET'; @@ -225,7 +308,7 @@ export function updateMetadata( location: Location, metadata: Partial, mappings: Mappings -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol); const method = 'PATCH'; @@ -247,13 +330,13 @@ export function updateMetadata( export function deleteObject( service: FirebaseStorageImpl, location: Location -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol); const method = 'DELETE'; const timeout = service.maxOperationRetryTime; - function handler(_xhr: Connection, _text: string): void {} + function handler(_xhr: Connection, _text: string): void {} const requestInfo = new RequestInfo(url, method, handler, timeout); requestInfo.successCodes = [200, 204]; requestInfo.errorHandler = objectErrorHandler(location); @@ -294,7 +377,7 @@ export function multipartUpload( mappings: Mappings, blob: FbsBlob, metadata?: Metadata | null -): RequestInfo { +): RequestInfo { const urlPart = location.bucketOnlyServerUrl(); const headers: { [prop: string]: string } = { 'X-Goog-Upload-Protocol': 'multipart' @@ -368,7 +451,7 @@ export class ResumableUploadStatus { } export function checkResumeHeader_( - xhr: Connection, + xhr: Connection, allowed?: string[] ): string { let status: string | null = null; @@ -388,7 +471,7 @@ export function createResumableUpload( mappings: Mappings, blob: FbsBlob, metadata?: Metadata | null -): RequestInfo { +): RequestInfo { const urlPart = location.bucketOnlyServerUrl(); const metadataForUpload = metadataForUpload_(location, blob, metadata); const urlParams: UrlParams = { name: metadataForUpload['fullPath']! }; @@ -404,7 +487,7 @@ export function createResumableUpload( const body = toResourceString(metadataForUpload, mappings); const timeout = service.maxUploadRetryTime; - function handler(xhr: Connection): string { + function handler(xhr: Connection): string { checkResumeHeader_(xhr); let url; try { @@ -431,10 +514,10 @@ export function getResumableUploadStatus( location: Location, url: string, blob: FbsBlob -): RequestInfo { +): RequestInfo { const headers = { 'X-Goog-Upload-Command': 'query' }; - function handler(xhr: Connection): ResumableUploadStatus { + function handler(xhr: Connection): ResumableUploadStatus { const status = checkResumeHeader_(xhr, ['active', 'final']); let sizeString: string | null = null; try { @@ -484,7 +567,7 @@ export function continueResumableUpload( mappings: Mappings, status?: ResumableUploadStatus | null, progressCallback?: ((p1: number, p2: number) => void) | null -): RequestInfo { +): RequestInfo { // TODO(andysoto): standardize on internal asserts // assert(!(opt_status && opt_status.finalized)); const status_ = new ResumableUploadStatus(0, 0); @@ -516,7 +599,10 @@ export function continueResumableUpload( throw cannotSliceBlob(); } - function handler(xhr: Connection, text: string): ResumableUploadStatus { + function handler( + xhr: Connection, + text: string + ): ResumableUploadStatus { // TODO(andysoto): Verify the MD5 of each uploaded range: // the 'x-range-md5' header comes back with status code 308 responses. // We'll only be able to bail out though, because you can't re-upload a diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 087f1d31544..7627fa30f89 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -16,27 +16,28 @@ */ import { - Headers, Connection, - ErrorCode + ErrorCode, + Headers } from '../../implementation/connection'; import { internalError } from '../../implementation/error'; /** An override for the text-based Connection. Used in tests. */ -let connectionFactoryOverride: (() => Connection) | null = null; +let textFactoryOverride: (() => 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. */ -export class XhrConnection implements Connection { - private xhr_: XMLHttpRequest; +abstract class XhrConnection implements Connection { + protected xhr_: XMLHttpRequest; private errorCode_: ErrorCode; private sendPromise_: Promise; - private sent_: boolean = false; + protected sent_: boolean = false; constructor() { this.xhr_ = new XMLHttpRequest(); + this.initXhr(); this.errorCode_ = ErrorCode.NO_ERROR; this.sendPromise_ = new Promise(resolve => { this.xhr_.addEventListener('abort', () => { @@ -53,6 +54,8 @@ export class XhrConnection implements Connection { }); } + abstract initXhr(): void; + send( url: string, method: string, @@ -97,7 +100,9 @@ export class XhrConnection implements Connection { } } - getResponseText(): string { + abstract getResponse(): ResponseType; + + getErrorText(): string { if (!this.sent_) { throw internalError('cannot .getResponseText() before sending'); } @@ -126,12 +131,117 @@ export class XhrConnection implements Connection { } } -export function newConnection(): Connection { - return connectionFactoryOverride - ? connectionFactoryOverride() - : new XhrConnection(); +export class XhrTextConnection extends XhrConnection { + initXhr(): void { + this.xhr_.responseType = 'text'; + } + + getResponse(): string { + if (!this.sent_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.xhr_.response; + } +} + +export function newTextConnection(): Connection { + return textFactoryOverride ? textFactoryOverride() : new XhrTextConnection(); +} + +export class XhrBytesConnection extends XhrConnection { + private data_?: ArrayBuffer; + + initXhr(): void { + // We use Blob here instead of ArrayBuffer to ensure that this code works + // in Opera. + this.xhr_.responseType = 'blob'; + } + + getErrorText(): string { + if (!this.sent_) { + throw internalError('cannot .getResponseText() before sending'); + } + return new TextDecoder().decode(this.data_); + } + + getResponse(): ArrayBuffer { + if (!this.sent_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.data_!; + } + + send( + url: string, + method: string, + body?: ArrayBufferView | Blob | string, + headers?: Headers + ): Promise { + return super.send(url, method, body, headers).then(async () => { + if (this.getErrorCode() === ErrorCode.NO_ERROR) { + this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); + } + }); + } +} + +export function newBytesConnection(): Connection { + return new XhrBytesConnection(); +} + +const MAX_ERROR_MSG_LENGTH = 512; + +export class XhrBlobConnection extends XhrConnection { + private data_?: Blob; + private text_?: string; + + initXhr(): void { + this.xhr_.responseType = 'blob'; + } + + getErrorText(): string { + if (!this.sent_) { + throw internalError('cannot .getResponseText() before sending'); + } + return this.text_!; + } + + getResponse(): Blob { + if (!this.sent_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.data_!; + } + + send( + url: string, + method: string, + body?: ArrayBufferView | Blob | string, + headers?: Headers + ): Promise { + return super.send(url, method, body, headers).then(async () => { + if (this.getErrorCode() === ErrorCode.NO_ERROR) { + this.data_ = this.xhr_.response; + this.text_ = await this.data_!.slice( + 0, + MAX_ERROR_MSG_LENGTH, + 'string' + ).text(); + } + }); + } +} + +export function newBlobConnection(): Connection { + return new XhrBlobConnection(); +} + +export function newStreamConnection(): Connection { + throw new Error('Streams are only supported on Node'); } -export function injectTestConnection(factory: (() => Connection) | null): void { - connectionFactoryOverride = factory; +export function injectTestConnection( + factory: (() => Connection) | null +): void { + textFactoryOverride = factory; } diff --git a/packages/storage/src/platform/connection.ts b/packages/storage/src/platform/connection.ts index 93863e3a70b..f1732312e2d 100644 --- a/packages/storage/src/platform/connection.ts +++ b/packages/storage/src/platform/connection.ts @@ -16,16 +16,36 @@ */ import { Connection } from '../implementation/connection'; import { - newConnection as nodeNewConnection, + newTextConnection as nodeNewTextConnection, + newBytesConnection as nodeNewBytesConnection, + newBlobConnection as nodeNewBlobConnection, + newStreamConnection as nodeNewStreamConnection, injectTestConnection as nodeInjectTestConnection } from './node/connection'; -export function newConnection(): Connection { +export function injectTestConnection( + factory: (() => Connection) | null +): void { // This file is only used under ts-node. - return nodeNewConnection(); + nodeInjectTestConnection(factory); } -export function injectTestConnection(factory: (() => Connection) | null): void { +export function newTextConnection(): Connection { // This file is only used under ts-node. - nodeInjectTestConnection(factory); + return nodeNewTextConnection(); +} + +export function newBytesConnection(): Connection { + // This file is only used under ts-node. + return nodeNewBytesConnection(); +} + +export function newBlobConnection(): Connection { + // This file is only used under ts-node. + return nodeNewBlobConnection(); +} + +export function newStreamConnection(): Connection { + // This file is only used under ts-node. + return nodeNewStreamConnection(); } diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index 5bbadc98141..6506c97a238 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -15,12 +15,12 @@ * limitations under the License. */ -import { ErrorCode, Connection } from '../../implementation/connection'; +import { Connection, ErrorCode } from '../../implementation/connection'; import { internalError } from '../../implementation/error'; -import nodeFetch, { FetchError } from 'node-fetch'; +import nodeFetch, { Headers } from 'node-fetch'; /** An override for the text-based Connection. Used in tests. */ -let connectionFactoryOverride: (() => Connection) | null = null; +let textFactoryOverride: (() => Connection) | null = null; /** * Network layer that works in Node. @@ -28,20 +28,22 @@ let connectionFactoryOverride: (() => Connection) | null = null; * This network implementation should not be used in browsers as it does not * support progress updates. */ -export class FetchConnection implements Connection { - private errorCode_: ErrorCode; - private statusCode_: number | undefined; - private body_: string | undefined; - private headers_: Headers | undefined; - private sent_: boolean = false; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private fetch_: typeof window.fetch = nodeFetch as any; +abstract class FetchConnection + implements Connection +{ + protected errorCode_: ErrorCode; + protected statusCode_: number | undefined; + protected body_: ArrayBuffer | undefined; + protected errorText_ = ''; + protected headers_: Headers | undefined; + protected sent_: boolean = false; + private fetch_ = nodeFetch; constructor() { this.errorCode_ = ErrorCode.NO_ERROR; } - send( + async send( url: string, method: string, body?: ArrayBufferView | Blob | string, @@ -52,24 +54,22 @@ export class FetchConnection implements Connection { } this.sent_ = true; - return this.fetch_(url, { - method, - headers: headers || {}, - body - }).then( - resp => { - this.headers_ = resp.headers; - this.statusCode_ = resp.status; - return resp.text().then(body => { - this.body_ = body; - }); - }, - (_err: FetchError) => { - this.errorCode_ = ErrorCode.NETWORK_ERROR; - // emulate XHR which sets status to 0 when encountering a network error - this.statusCode_ = 0; - } - ); + try { + const response = await this.fetch_(url, { + method, + headers: headers || {}, + body: body as ArrayBufferView | string + }); + this.headers_ = response.headers; + this.statusCode_ = response.status; + this.errorCode_ = ErrorCode.NO_ERROR; + this.body_ = await response.arrayBuffer(); + } catch (e) { + this.errorText_ = e.message; + // emulate XHR which sets status to 0 when encountering a network error + this.statusCode_ = 0; + this.errorCode_ = ErrorCode.NETWORK_ERROR; + } } getErrorCode(): ErrorCode { @@ -86,13 +86,10 @@ export class FetchConnection implements Connection { return this.statusCode_; } - getResponseText(): string { - if (this.body_ === undefined) { - throw internalError( - 'cannot .getResponseText() before receiving response' - ); - } - return this.body_; + abstract getResponse(): ResponseType; + + getErrorText(): string { + return this.errorText_; } abort(): void { @@ -120,12 +117,90 @@ export class FetchConnection implements Connection { } } -export function newConnection(): Connection { - return connectionFactoryOverride - ? connectionFactoryOverride() - : new FetchConnection(); +export class FetchTextConnection extends FetchConnection { + getResponse(): string { + if (!this.body_) { + throw internalError( + 'cannot .getResponseText() before receiving response' + ); + } + return Buffer.from(this.body_).toString('utf-8'); + } +} + +export function newTextConnection(): Connection { + return textFactoryOverride + ? textFactoryOverride() + : new FetchTextConnection(); +} + +export class FetchBytesConnection extends FetchConnection { + getResponse(): ArrayBuffer { + if (!this.body_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.body_; + } +} + +export function newBytesConnection(): Connection { + return new FetchBytesConnection(); +} + +export class FetchStreamConnection extends FetchConnection { + private stream_: NodeJS.ReadableStream | null = null; + + async send( + url: string, + method: string, + body?: ArrayBufferView | Blob | string, + headers?: Record + ): Promise { + if (this.sent_) { + throw internalError('cannot .send() more than once'); + } + this.sent_ = true; + + try { + const response = await nodeFetch(url, { + method, + headers: headers || {}, + body: body as ArrayBufferView | string + }); + this.headers_ = response.headers; + this.statusCode_ = response.status; + this.errorCode_ = ErrorCode.NO_ERROR; + this.stream_ = response.body; + } catch (e) { + this.errorText_ = e.message; + // emulate XHR which sets status to 0 when encountering a network error + this.statusCode_ = 0; + this.errorCode_ = ErrorCode.NETWORK_ERROR; + } + } + + getResponse(): NodeJS.ReadableStream { + if (!this.stream_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.stream_; + } + + getErrorText(): string { + return this.errorText_; + } +} + +export function newStreamConnection(): Connection { + return new FetchStreamConnection(); +} + +export function newBlobConnection(): Connection { + throw new Error('Blobs are not supported on Node'); } -export function injectTestConnection(factory: (() => Connection) | null): void { - connectionFactoryOverride = factory; +export function injectTestConnection( + factory: (() => Connection) | null +): void { + textFactoryOverride = factory; } diff --git a/packages/storage/src/reference.ts b/packages/storage/src/reference.ts index a89fd5a66b1..ee6bcc45f00 100644 --- a/packages/storage/src/reference.ts +++ b/packages/storage/src/reference.ts @@ -19,6 +19,8 @@ * @fileoverview Defines the Firebase StorageReference class. */ +import { Transform, TransformOptions, PassThrough } from 'stream'; + import { FbsBlob } from './implementation/blob'; import { Location } from './implementation/location'; import { getMappings } from './implementation/metadata'; @@ -29,7 +31,10 @@ import { updateMetadata as requestsUpdateMetadata, getDownloadUrl as requestsGetDownloadUrl, deleteObject as requestsDeleteObject, - multipartUpload + multipartUpload, + getBytes, + getBlob, + getStream } from './implementation/requests'; import { ListOptions, UploadResult } from './public-types'; import { StringFormat, dataFromString } from './implementation/string'; @@ -39,7 +44,12 @@ import { ListResult } from './list'; import { UploadTask } from './task'; import { invalidRootOperation, noDownloadURL } from './implementation/error'; import { validateNumber } from './implementation/type'; -import { newConnection } from './platform/connection'; +import { + newBytesConnection, + newTextConnection, + newBlobConnection, + newStreamConnection +} from './platform/connection'; /** * Provides methods to interact with a bucket in the Firebase Storage service. @@ -143,6 +153,92 @@ export class Reference { } } +/** + * Download the bytes at the object's location. + * @returns A Promise containing the downloaded bytes. + */ +export function getBytesInternal( + ref: Reference, + maxDownloadSizeBytes?: number +): Promise { + ref._throwIfRoot('getBytes'); + const requestInfo = getBytes( + ref.storage, + ref._location, + maxDownloadSizeBytes + ); + return ref.storage + .makeRequestWithTokens(requestInfo, newBytesConnection) + .then(bytes => + maxDownloadSizeBytes !== undefined + ? // GCS may not honor the Range header for small files + bytes.slice(0, maxDownloadSizeBytes) + : bytes + ); +} + +/** + * Download the bytes at the object's location. + * @returns A Promise containing the downloaded blob. + */ +export function getBlobInternal( + ref: Reference, + maxDownloadSizeBytes?: number +): Promise { + ref._throwIfRoot('getBlob'); + const requestInfo = getBlob(ref.storage, ref._location, maxDownloadSizeBytes); + return ref.storage + .makeRequestWithTokens(requestInfo, newBlobConnection) + .then(blob => + maxDownloadSizeBytes !== undefined + ? // GCS may not honor the Range header for small files + blob.slice(0, maxDownloadSizeBytes) + : blob + ); +} + +/** Stream the bytes at the object's location. */ +export function getStreamInternal( + ref: Reference, + maxDownloadSizeBytes?: number +): NodeJS.ReadableStream { + ref._throwIfRoot('getStream'); + const requestInfo = getStream( + ref.storage, + ref._location, + maxDownloadSizeBytes + ); + + /** A transformer that passes through the first n bytes. */ + const newMaxSizeTransform: (n: number) => TransformOptions = n => { + let missingBytes = n; + return { + transform(chunk, encoding, callback) { + // GCS may not honor the Range header for small files + if (chunk.length < missingBytes) { + this.push(chunk); + missingBytes -= chunk.length; + } else { + this.push(chunk.slice(0, missingBytes)); + this.emit('end'); + } + callback(); + } + } as TransformOptions; + }; + + const result = + maxDownloadSizeBytes !== undefined + ? new Transform(newMaxSizeTransform(maxDownloadSizeBytes)) + : new PassThrough(); + + ref.storage + .makeRequestWithTokens(requestInfo, newStreamConnection) + .then(stream => stream.pipe(result)) + .catch(e => result.destroy(e)); + return result; +} + /** * Uploads data to this object's location. * The upload is not resumable. @@ -166,7 +262,7 @@ export function uploadBytes( metadata ); return ref.storage - .makeRequestWithTokens(requestInfo, newConnection) + .makeRequestWithTokens(requestInfo, newTextConnection) .then(finalMetadata => { return { metadata: finalMetadata, @@ -312,7 +408,7 @@ export function list( op.pageToken, op.maxResults ); - return ref.storage.makeRequestWithTokens(requestInfo, newConnection); + return ref.storage.makeRequestWithTokens(requestInfo, newTextConnection); } /** @@ -329,7 +425,7 @@ export function getMetadata(ref: Reference): Promise { ref._location, getMappings() ); - return ref.storage.makeRequestWithTokens(requestInfo, newConnection); + return ref.storage.makeRequestWithTokens(requestInfo, newTextConnection); } /** @@ -354,7 +450,7 @@ export function updateMetadata( metadata, getMappings() ); - return ref.storage.makeRequestWithTokens(requestInfo, newConnection); + return ref.storage.makeRequestWithTokens(requestInfo, newTextConnection); } /** @@ -371,7 +467,7 @@ export function getDownloadURL(ref: Reference): Promise { getMappings() ); return ref.storage - .makeRequestWithTokens(requestInfo, newConnection) + .makeRequestWithTokens(requestInfo, newTextConnection) .then(url => { if (url === null) { throw noDownloadURL(); @@ -389,7 +485,7 @@ export function getDownloadURL(ref: Reference): Promise { export function deleteObject(ref: Reference): Promise { ref._throwIfRoot('deleteObject'); const requestInfo = requestsDeleteObject(ref.storage, ref._location); - return ref.storage.makeRequestWithTokens(requestInfo, newConnection); + return ref.storage.makeRequestWithTokens(requestInfo, newTextConnection); } /** diff --git a/packages/storage/src/service.ts b/packages/storage/src/service.ts index c1e2bd080d4..f0bb1f9f9a1 100644 --- a/packages/storage/src/service.ts +++ b/packages/storage/src/service.ts @@ -298,12 +298,12 @@ export class FirebaseStorageImpl implements FirebaseStorage { * @param requestInfo - HTTP RequestInfo object * @param authToken - Firebase auth token */ - _makeRequest( - requestInfo: RequestInfo, - requestFactory: () => Connection, + _makeRequest( + requestInfo: RequestInfo, + requestFactory: () => Connection, authToken: string | null, appCheckToken: string | null - ): Request { + ): Request { if (!this._deleted) { const request = makeRequest( requestInfo, @@ -325,10 +325,10 @@ export class FirebaseStorageImpl implements FirebaseStorage { } } - async makeRequestWithTokens( - requestInfo: RequestInfo, - requestFactory: () => Connection - ): Promise { + async makeRequestWithTokens( + requestInfo: RequestInfo, + requestFactory: () => Connection + ): Promise { const [authToken, appCheckToken] = await Promise.all([ this._getAuthToken(), this._getAppCheckToken() diff --git a/packages/storage/src/task.ts b/packages/storage/src/task.ts index 3cf0fb9f0e8..62689856153 100644 --- a/packages/storage/src/task.ts +++ b/packages/storage/src/task.ts @@ -52,7 +52,7 @@ import { multipartUpload } from './implementation/requests'; import { Reference } from './reference'; -import { newConnection } from './platform/connection'; +import { newTextConnection } from './platform/connection'; /** * Represents a blob being uploaded. Can be used to pause/resume/cancel the @@ -208,7 +208,7 @@ export class UploadTask { ); const createRequest = this._ref.storage._makeRequest( requestInfo, - newConnection, + newTextConnection, authToken, appCheckToken ); @@ -234,7 +234,7 @@ export class UploadTask { ); const statusRequest = this._ref.storage._makeRequest( requestInfo, - newConnection, + newTextConnection, authToken, appCheckToken ); @@ -281,7 +281,7 @@ export class UploadTask { } const uploadRequest = this._ref.storage._makeRequest( requestInfo, - newConnection, + newTextConnection, authToken, appCheckToken ); @@ -318,7 +318,7 @@ export class UploadTask { ); const metadataRequest = this._ref.storage._makeRequest( requestInfo, - newConnection, + newTextConnection, authToken, appCheckToken ); @@ -342,7 +342,7 @@ export class UploadTask { ); const multipartRequest = this._ref.storage._makeRequest( requestInfo, - newConnection, + newTextConnection, authToken, appCheckToken ); diff --git a/packages/storage/test/browser/blob.test.ts b/packages/storage/test/browser/blob.test.ts index eafdfacc184..b6c56eafa66 100644 --- a/packages/storage/test/browser/blob.test.ts +++ b/packages/storage/test/browser/blob.test.ts @@ -15,13 +15,31 @@ * limitations under the License. */ -import { assert } from 'chai'; +import { assert, expect } from 'chai'; import * as sinon from 'sinon'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { FirebaseApp, deleteApp } from '@firebase/app'; + import { FbsBlob } from '../../src/implementation/blob'; import * as type from '../../src/implementation/type'; import * as testShared from '../unit/testshared'; +import { createApp, createStorage } from '../integration/integration.test'; +import { getBlob, ref, uploadBytes } from '../../src'; +import * as types from '../../src/public-types'; describe('Firebase Storage > Blob', () => { + let app: FirebaseApp; + let storage: types.FirebaseStorage; + + beforeEach(async () => { + app = await createApp(); + storage = createStorage(app); + }); + + afterEach(async () => { + await deleteApp(app); + }); + let stubs: sinon.SinonStub[] = []; before(() => { const definedStub = sinon.stub(type, 'isNativeBlobDefined'); @@ -47,6 +65,7 @@ describe('Firebase Storage > Blob', () => { new Uint8Array([2, 3, 4, 5]) ); }); + it('Blobs are merged with strings correctly', () => { const blob = new FbsBlob(new Uint8Array([1, 2, 3, 4])); const merged = FbsBlob.getBlob('what', blob, '\ud83d\ude0a ')!; @@ -70,4 +89,34 @@ describe('Firebase Storage > Blob', () => { assert.equal(20, concatenated!.size()); }); + + it('can get blob', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 3, 128, 255])); + const blob = await getBlob(reference); + const bytes = await blob.arrayBuffer(); + expect(new Uint8Array(bytes)).to.deep.equal( + new Uint8Array([0, 1, 3, 128, 255]) + ); + }); + + it('can get the first n-bytes of a blob', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 5])); + const blob = await getBlob(reference, 2); + const bytes = await blob.arrayBuffer(); + expect(new Uint8Array(bytes)).to.deep.equal(new Uint8Array([0, 1])); + }); + + it('getBlob() throws for missing file', async () => { + const reference = ref(storage, 'public/exp-bytes-missing'); + try { + await getBlob(reference); + expect.fail(); + } catch (e) { + expect(e.message).to.satisfy((v: string) => + v.match(/Object 'public\/exp-bytes-missing' does not exist/) + ); + } + }); }); diff --git a/packages/storage/test/browser/connection.test.ts b/packages/storage/test/browser/connection.test.ts index d89397696f3..b869c9ee31b 100644 --- a/packages/storage/test/browser/connection.test.ts +++ b/packages/storage/test/browser/connection.test.ts @@ -18,12 +18,12 @@ import { expect } from 'chai'; import { SinonFakeXMLHttpRequest, useFakeXMLHttpRequest } from 'sinon'; import { ErrorCode } from '../../src/implementation/connection'; -import { XhrConnection } from '../../src/platform/browser/connection'; +import { XhrBytesConnection } from '../../src/platform/browser/connection'; describe('Connections', () => { it('XhrConnection.send() should not reject on network errors', async () => { const fakeXHR = useFakeXMLHttpRequest(); - const connection = new XhrConnection(); + const connection = new XhrBytesConnection(); const sendPromise = connection.send('testurl', 'GET'); // simulate a network error ((connection as any).xhr_ as SinonFakeXMLHttpRequest).error(); diff --git a/packages/storage/test/integration/integration.test.ts b/packages/storage/test/integration/integration.test.ts index c1f015c352e..8729570ad92 100644 --- a/packages/storage/test/integration/integration.test.ts +++ b/packages/storage/test/integration/integration.test.ts @@ -29,8 +29,9 @@ import { deleteObject, getMetadata, updateMetadata, - listAll -} from '../../src/index'; + listAll, + getBytes +} from '../../src'; import { use, expect } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; @@ -46,19 +47,28 @@ export const STORAGE_BUCKET = PROJECT_CONFIG.storageBucket; export const API_KEY = PROJECT_CONFIG.apiKey; export const AUTH_DOMAIN = PROJECT_CONFIG.authDomain; -describe('FirebaseStorage Integration tests', () => { +export async function createApp(): Promise { + const app = initializeApp({ + apiKey: API_KEY, + projectId: PROJECT_ID, + storageBucket: STORAGE_BUCKET, + authDomain: AUTH_DOMAIN + }); + await signInAnonymously(getAuth(app)); + return app; +} + +export function createStorage(app: FirebaseApp): types.FirebaseStorage { + return getStorage(app); +} + +describe('FirebaseStorage Exp', () => { let app: FirebaseApp; let storage: types.FirebaseStorage; beforeEach(async () => { - app = initializeApp({ - apiKey: API_KEY, - projectId: PROJECT_ID, - storageBucket: STORAGE_BUCKET, - authDomain: AUTH_DOMAIN - }); - await signInAnonymously(getAuth(app)); - storage = getStorage(app); + app = await createApp(); + storage = createStorage(app); }); afterEach(async () => { @@ -71,6 +81,34 @@ describe('FirebaseStorage Integration tests', () => { expect(snap.metadata.timeCreated).to.exist; }); + it('can get bytes', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 3, 128, 255])); + const bytes = await getBytes(reference); + expect(new Uint8Array(bytes)).to.deep.equal( + new Uint8Array([0, 1, 3, 128, 255]) + ); + }); + + it('can get first n bytes', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 3])); + const bytes = await getBytes(reference, 2); + expect(new Uint8Array(bytes)).to.deep.equal(new Uint8Array([0, 1])); + }); + + it('getBytes() throws for missing file', async () => { + const reference = ref(storage, 'public/exp-bytes-missing'); + try { + await getBytes(reference); + expect.fail(); + } catch (e) { + expect(e.message).to.satisfy((v: string) => + v.match(/Object 'public\/exp-bytes-missing' does not exist/) + ); + } + }); + it('can upload bytes (resumable)', async () => { const reference = ref(storage, 'public/exp-bytesresumable'); const snap = await uploadBytesResumable( diff --git a/packages/storage/test/unit/connection.test.ts b/packages/storage/test/node/connection.test.ts similarity index 89% rename from packages/storage/test/unit/connection.test.ts rename to packages/storage/test/node/connection.test.ts index d9190da7330..32c499f0209 100644 --- a/packages/storage/test/unit/connection.test.ts +++ b/packages/storage/test/node/connection.test.ts @@ -18,11 +18,11 @@ import { stub } from 'sinon'; import { expect } from 'chai'; import { ErrorCode } from '../../src/implementation/connection'; -import { FetchConnection } from '../../src/platform/node/connection'; +import { FetchBytesConnection } from '../../src/platform/node/connection'; describe('Connections', () => { it('FetchConnection.send() should not reject on network errors', async () => { - const connection = new FetchConnection(); + const connection = new FetchBytesConnection(); // need the casting here because fetch_ is a private member stub(connection as any, 'fetch_').rejects(); diff --git a/packages/storage/test/node/stream.test.ts b/packages/storage/test/node/stream.test.ts new file mode 100644 index 00000000000..bbde160692a --- /dev/null +++ b/packages/storage/test/node/stream.test.ts @@ -0,0 +1,76 @@ +/** + * @license + * Copyright 2021 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 { expect } from 'chai'; +import { createApp, createStorage } from '../integration/integration.test'; +import { FirebaseApp, deleteApp } from '@firebase/app'; +import { getStream, ref, uploadBytes } from '../../src/index.node'; +import * as types from '../../src/public-types'; + +async function readData(reader: NodeJS.ReadableStream): Promise { + return new Promise((resolve, reject) => { + const data: number[] = []; + reader.on('error', e => reject(e)); + reader.on('data', chunk => data.push(...Array.from(chunk as Buffer))); + reader.on('end', () => resolve(data)); + }); +} + +describe('Firebase Storage > getStream', () => { + let app: FirebaseApp; + let storage: types.FirebaseStorage; + + beforeEach(async () => { + app = await createApp(); + storage = createStorage(app); + }); + + afterEach(async () => { + await deleteApp(app); + }); + + it('can get stream', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 3, 128, 255])); + const stream = await getStream(reference); + const data = await readData(stream); + expect(new Uint8Array(data)).to.deep.equal( + new Uint8Array([0, 1, 3, 128, 255]) + ); + }); + + it('can get first n bytes of stream', async () => { + const reference = ref(storage, 'public/exp-bytes'); + await uploadBytes(reference, new Uint8Array([0, 1, 3])); + const stream = await getStream(reference, 2); + const data = await readData(stream); + expect(new Uint8Array(data)).to.deep.equal(new Uint8Array([0, 1])); + }); + + it('getStream() throws for missing file', async () => { + const reference = ref(storage, 'public/exp-bytes-missing'); + const stream = getStream(reference); + try { + await readData(stream); + expect.fail(); + } catch (e) { + expect(e.message).to.satisfy((v: string) => + v.match(/Object 'public\/exp-bytes-missing' does not exist/) + ); + } + }); +}); diff --git a/packages/storage/test/unit/connection.ts b/packages/storage/test/unit/connection.ts index 11c27d29a6a..018f65f3671 100644 --- a/packages/storage/test/unit/connection.ts +++ b/packages/storage/test/unit/connection.ts @@ -35,7 +35,7 @@ export enum State { DONE = 2 } -export class TestingConnection implements Connection { +export class TestingConnection implements Connection { private state: State; private sendPromise: Promise; private resolve!: () => void; @@ -107,7 +107,11 @@ export class TestingConnection implements Connection { return this.status; } - getResponseText(): string { + getResponse(): string { + return this.responseText; + } + + getErrorText(): string { return this.responseText; } @@ -135,6 +139,8 @@ export class TestingConnection implements Connection { } } -export function newTestConnection(sendHook?: SendHook | null): Connection { +export function newTestConnection( + sendHook?: SendHook | null +): Connection { return new TestingConnection(sendHook ?? null); } diff --git a/packages/storage/test/unit/request.test.ts b/packages/storage/test/unit/request.test.ts index 633e828cc49..69b3dc41e1d 100644 --- a/packages/storage/test/unit/request.test.ts +++ b/packages/storage/test/unit/request.test.ts @@ -44,7 +44,7 @@ describe('Firebase Storage > Request', () => { } const spiedSend = sinon.spy(newSend); - function handler(connection: Connection, text: string): string { + function handler(connection: Connection, text: string): string { assert.equal(text, response); assert.equal(connection.getResponseHeader(responseHeader), responseValue); assert.equal(connection.getStatus(), status); @@ -92,7 +92,7 @@ describe('Firebase Storage > Request', () => { } const spiedSend = sinon.spy(newSend); - function handler(connection: Connection, text: string): string { + function handler(connection: Connection, text: string): string { return text; } diff --git a/packages/storage/test/unit/requests.test.ts b/packages/storage/test/unit/requests.test.ts index 25a02ce0337..8a7cff0ca4a 100644 --- a/packages/storage/test/unit/requests.test.ts +++ b/packages/storage/test/unit/requests.test.ts @@ -32,7 +32,8 @@ import { getResumableUploadStatus, ResumableUploadStatus, continueResumableUpload, - RESUMABLE_UPLOAD_CHUNK_SIZE + RESUMABLE_UPLOAD_CHUNK_SIZE, + getBytes } from '../../src/implementation/requests'; import { makeUrl } from '../../src/implementation/url'; import { unknown, StorageErrorCode } from '../../src/implementation/error'; @@ -178,12 +179,14 @@ describe('Firebase Storage > Requests', () => { } } - function checkMetadataHandler(requestInfo: RequestInfo): void { + function checkMetadataHandler( + requestInfo: RequestInfo + ): void { const metadata = requestInfo.handler(fakeXhrIo({}), serverResourceString); assert.deepEqual(metadata, metadataFromServerResource); } - function checkNoOpHandler(requestInfo: RequestInfo): void { + function checkNoOpHandler(requestInfo: RequestInfo): void { try { requestInfo.handler(fakeXhrIo({}), ''); } catch (e) { @@ -344,6 +347,14 @@ describe('Firebase Storage > Requests', () => { const url = requestInfo.handler(fakeXhrIo({}), serverResourceString); assert.equal(url, downloadUrlFromServerResource); }); + it('getBytes handler', () => { + const requestInfo = getBytes(storageService, locationNormal); + const bytes = requestInfo.handler( + fakeXhrIo({}), + new Uint8Array([1, 128, 255]) + ); + assert.deepEqual(new Uint8Array(bytes), new Uint8Array([1, 128, 255])); + }); it('updateMetadata requestinfo', () => { const maps = [ [locationNormal, locationNormalUrl], diff --git a/packages/storage/test/unit/testshared.ts b/packages/storage/test/unit/testshared.ts index 2a35386b653..91871a1748e 100644 --- a/packages/storage/test/unit/testshared.ts +++ b/packages/storage/test/unit/testshared.ts @@ -110,7 +110,10 @@ export function makeFakeAppCheckProvider(tokenResult: { * Returns something that looks like an fbs.XhrIo with the given headers * and status. */ -export function fakeXhrIo(headers: Headers, status: number = 200): Connection { +export function fakeXhrIo( + headers: Headers, + status: number = 200 +): Connection { const lower: Headers = {}; for (const [key, value] of Object.entries(headers)) { lower[key.toLowerCase()] = value.toString(); @@ -130,7 +133,7 @@ export function fakeXhrIo(headers: Headers, status: number = 200): Connection { } }; - return fakeConnection as Connection; + return fakeConnection as Connection; } /** From bbe754ac920cf7f5369b15cd0f6f49e94946191e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 27 Oct 2021 14:51:11 -0600 Subject: [PATCH 02/23] Create clever-eggs-relate.md --- .changeset/clever-eggs-relate.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/clever-eggs-relate.md diff --git a/.changeset/clever-eggs-relate.md b/.changeset/clever-eggs-relate.md new file mode 100644 index 00000000000..80ca100d6e4 --- /dev/null +++ b/.changeset/clever-eggs-relate.md @@ -0,0 +1,5 @@ +--- +"@firebase/storage": patch +--- + +Adds getBytes(), getStream() and getBlob(), which allow direct file downloads from the SDK. From 241e1ffe9361a81c440923dd6ff106770b0b171b Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 27 Oct 2021 14:51:56 -0600 Subject: [PATCH 03/23] Update clever-eggs-relate.md --- .changeset/clever-eggs-relate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/clever-eggs-relate.md b/.changeset/clever-eggs-relate.md index 80ca100d6e4..39095c16065 100644 --- a/.changeset/clever-eggs-relate.md +++ b/.changeset/clever-eggs-relate.md @@ -2,4 +2,4 @@ "@firebase/storage": patch --- -Adds getBytes(), getStream() and getBlob(), which allow direct file downloads from the SDK. +Adds `getBytes()`, `getStream()` and `getBlob()`, which allow direct file downloads from the SDK. From 9ad21e014df9e4631325eaa06e6090a1c16f5fcb Mon Sep 17 00:00:00 2001 From: schmidt-sebastian Date: Wed, 27 Oct 2021 21:07:20 +0000 Subject: [PATCH 04/23] Update API reports --- common/api-review/storage.api.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/common/api-review/storage.api.md b/common/api-review/storage.api.md index cbc4486e0c8..0daaf2ef274 100644 --- a/common/api-review/storage.api.md +++ b/common/api-review/storage.api.md @@ -82,9 +82,9 @@ export class _FirebaseStorageImpl implements FirebaseStorage { // Warning: (ae-forgotten-export) The symbol "Request" needs to be exported by the entry point index.d.ts // // (undocumented) - _makeRequest(requestInfo: RequestInfo_2, requestFactory: () => Connection, 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, requestFactory: () => Connection): Promise; + makeRequestWithTokens(requestInfo: RequestInfo_2, requestFactory: () => Connection): Promise; _makeStorageReference(loc: _Location): _Reference; get maxOperationRetryTime(): number; set maxOperationRetryTime(time: number); @@ -112,6 +112,12 @@ export interface FullMetadata extends UploadMetadata { updated: string; } +// @public +export function getBlob(ref: StorageReference, maxDownloadSizeBytes?: number): Promise; + +// @public +export function getBytes(ref: StorageReference, maxDownloadSizeBytes?: number): Promise; + // @internal (undocumented) export function _getChild(ref: StorageReference, childPath: string): _Reference; @@ -124,6 +130,9 @@ export function getMetadata(ref: StorageReference): Promise; // @public export function getStorage(app?: FirebaseApp, bucketUrl?: string): FirebaseStorage; +// @public +export function getStream(ref: StorageReference, maxDownloadSizeBytes?: number): NodeJS.ReadableStream; + // Warning: (ae-forgotten-export) The symbol "StorageError" needs to be exported by the entry point index.d.ts // // @internal (undocumented) From d377aa77c27c547878e3d144878553ee7a5889ee Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 27 Oct 2021 15:07:56 -0600 Subject: [PATCH 05/23] Small fixes --- packages/storage/src/implementation/request.ts | 4 ++-- .../storage/src/implementation/requestinfo.ts | 8 ++++---- .../storage/src/platform/browser/connection.ts | 18 +++++++----------- .../storage/src/platform/node/connection.ts | 8 ++------ 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index 530234cde4f..f9c700c9877 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -44,8 +44,8 @@ export interface Request { * Handles network logic for all Storage Requests, including error reporting and * retries with backoff. * - * @param I - the type of the backend's network response (always `string` or - * `ArrayBuffer`). + * @param I - the type of the backend's network response (always `string`, + * `ArrayBuffer` or `ReadableStream`). * @param - O the output type used by the rest of the SDK. The conversion * happens in the specified `callback_`. */ diff --git a/packages/storage/src/implementation/requestinfo.ts b/packages/storage/src/implementation/requestinfo.ts index 68ad2ccc503..cbca3de760f 100644 --- a/packages/storage/src/implementation/requestinfo.ts +++ b/packages/storage/src/implementation/requestinfo.ts @@ -28,8 +28,8 @@ export interface UrlParams { * A function that converts a server response to the API type expected by the * SDK. * - * @param I - the type of the backend's network response (always `string` or - * `ArrayBuffer`). + * @param I - the type of the backend's network response (always `string`, + * `ArrayBuffer` or `ReadableStream`). * @param O - the output response type used by the rest of the SDK. */ export type RequestHandler = ( @@ -46,8 +46,8 @@ export type ErrorHandler = ( /** * Contains a fully specified request. * - * @param I - the type of the backend's network response (always `string` or - * `ArrayBuffer`). + * @param I - the type of the backend's network response (always `string`, + * `ArrayBuffer` or `ReadableStream`). * @param O - the output response type used by the rest of the SDK. */ export class RequestInfo { diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 7627fa30f89..e78bbd64f81 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -178,9 +178,7 @@ export class XhrBytesConnection extends XhrConnection { headers?: Headers ): Promise { return super.send(url, method, body, headers).then(async () => { - if (this.getErrorCode() === ErrorCode.NO_ERROR) { - this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); - } + this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); }); } } @@ -220,14 +218,12 @@ export class XhrBlobConnection extends XhrConnection { headers?: Headers ): Promise { return super.send(url, method, body, headers).then(async () => { - if (this.getErrorCode() === ErrorCode.NO_ERROR) { - this.data_ = this.xhr_.response; - this.text_ = await this.data_!.slice( - 0, - MAX_ERROR_MSG_LENGTH, - 'string' - ).text(); - } + this.data_ = this.xhr_.response; + this.text_ = await this.data_!.slice( + 0, + MAX_ERROR_MSG_LENGTH, + 'string' + ).text(); }); } } diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index 6506c97a238..3bbd69c3f50 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -98,9 +98,7 @@ abstract class FetchConnection getResponseHeader(header: string): string | null { if (!this.headers_) { - throw internalError( - 'cannot .getResponseText() before receiving response' - ); + throw internalError('cannot .getResponse() before receiving response'); } return this.headers_.get(header); } @@ -120,9 +118,7 @@ abstract class FetchConnection export class FetchTextConnection extends FetchConnection { getResponse(): string { if (!this.body_) { - throw internalError( - 'cannot .getResponseText() before receiving response' - ); + throw internalError('cannot .getResponse() before receiving response'); } return Buffer.from(this.body_).toString('utf-8'); } From 9d4258382138dd654585f13e8e07343206f25db1 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 27 Oct 2021 16:16:56 -0600 Subject: [PATCH 06/23] Browser error handling --- .../src/platform/browser/connection.ts | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index e78bbd64f81..93ce1fae531 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -104,9 +104,9 @@ abstract class XhrConnection implements Connection { getErrorText(): string { if (!this.sent_) { - throw internalError('cannot .getResponseText() before sending'); + throw internalError('cannot .getErrorText() before sending'); } - return this.xhr_.responseText; + return this.xhr_.statusText; } /** Aborts the request. */ @@ -157,13 +157,6 @@ export class XhrBytesConnection extends XhrConnection { this.xhr_.responseType = 'blob'; } - getErrorText(): string { - if (!this.sent_) { - throw internalError('cannot .getResponseText() before sending'); - } - return new TextDecoder().decode(this.data_); - } - getResponse(): ArrayBuffer { if (!this.sent_) { throw internalError('cannot .getResponse() before sending'); @@ -178,7 +171,9 @@ export class XhrBytesConnection extends XhrConnection { headers?: Headers ): Promise { return super.send(url, method, body, headers).then(async () => { - this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); + if (this.getErrorCode() === ErrorCode.NO_ERROR) { + this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); + } }); } } @@ -218,12 +213,14 @@ export class XhrBlobConnection extends XhrConnection { headers?: Headers ): Promise { return super.send(url, method, body, headers).then(async () => { - this.data_ = this.xhr_.response; - this.text_ = await this.data_!.slice( - 0, - MAX_ERROR_MSG_LENGTH, - 'string' - ).text(); + if (this.getErrorCode() === ErrorCode.NO_ERROR) { + this.data_ = this.xhr_.response; + this.text_ = await this.data_!.slice( + 0, + MAX_ERROR_MSG_LENGTH, + 'string' + ).text(); + } }); } } From b300fe93c95f2e58ae9b9654921df32539e8ee26 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Sun, 7 Nov 2021 14:18:15 -0700 Subject: [PATCH 07/23] Update connection.ts --- .../src/platform/browser/connection.ts | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 93ce1fae531..ca858175ef5 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -185,25 +185,22 @@ export function newBytesConnection(): Connection { const MAX_ERROR_MSG_LENGTH = 512; export class XhrBlobConnection extends XhrConnection { - private data_?: Blob; - private text_?: string; - initXhr(): void { this.xhr_.responseType = 'blob'; } getErrorText(): string { if (!this.sent_) { - throw internalError('cannot .getResponseText() before sending'); + throw internalError('cannot .getErrorText() before sending'); } - return this.text_!; + return this.xhr_.responseText; } getResponse(): Blob { if (!this.sent_) { throw internalError('cannot .getResponse() before sending'); } - return this.data_!; + return this.xhr_.response; } send( @@ -212,16 +209,7 @@ export class XhrBlobConnection extends XhrConnection { body?: ArrayBufferView | Blob | string, headers?: Headers ): Promise { - return super.send(url, method, body, headers).then(async () => { - if (this.getErrorCode() === ErrorCode.NO_ERROR) { - this.data_ = this.xhr_.response; - this.text_ = await this.data_!.slice( - 0, - MAX_ERROR_MSG_LENGTH, - 'string' - ).text(); - } - }); + return super.send(url, method, body, headers); } } From 718c5370371bd2f2d6bc35472d26269d66a36094 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Sun, 7 Nov 2021 14:22:26 -0700 Subject: [PATCH 08/23] Update connection.ts --- packages/storage/src/platform/browser/connection.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index ca858175ef5..73e9b118d67 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -182,8 +182,6 @@ export function newBytesConnection(): Connection { return new XhrBytesConnection(); } -const MAX_ERROR_MSG_LENGTH = 512; - export class XhrBlobConnection extends XhrConnection { initXhr(): void { this.xhr_.responseType = 'blob'; From 9838a74fd2c031fb19e09117123a02a1c26fd536 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 09:21:59 -0700 Subject: [PATCH 09/23] Update packages/storage/src/platform/connection.ts Co-authored-by: Feiyang --- packages/storage/src/platform/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/platform/connection.ts b/packages/storage/src/platform/connection.ts index f1732312e2d..d1f26723c7d 100644 --- a/packages/storage/src/platform/connection.ts +++ b/packages/storage/src/platform/connection.ts @@ -36,7 +36,7 @@ export function newTextConnection(): Connection { } export function newBytesConnection(): Connection { - // This file is only used under ts-node. + // This file is only used in Node.js tests using ts-node. return nodeNewBytesConnection(); } From bbfba2289eb65a4230a433ef8dc4ec13bbe35ace Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 09:22:15 -0700 Subject: [PATCH 10/23] Update packages/storage/src/platform/connection.ts Co-authored-by: Feiyang --- packages/storage/src/platform/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/platform/connection.ts b/packages/storage/src/platform/connection.ts index d1f26723c7d..2f82f80909d 100644 --- a/packages/storage/src/platform/connection.ts +++ b/packages/storage/src/platform/connection.ts @@ -41,7 +41,7 @@ export function newBytesConnection(): Connection { } export function newBlobConnection(): Connection { - // This file is only used under ts-node. + // This file is only used in Node.js tests using ts-node. return nodeNewBlobConnection(); } From e4d2cc95c5cc680c6ac4d8d96dfb462511249e63 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 09:22:29 -0700 Subject: [PATCH 11/23] Update packages/storage/src/platform/connection.ts Co-authored-by: Feiyang --- packages/storage/src/platform/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/platform/connection.ts b/packages/storage/src/platform/connection.ts index 2f82f80909d..a647c19cacf 100644 --- a/packages/storage/src/platform/connection.ts +++ b/packages/storage/src/platform/connection.ts @@ -46,6 +46,6 @@ export function newBlobConnection(): Connection { } export function newStreamConnection(): Connection { - // This file is only used under ts-node. + // This file is only used in Node.js tests using ts-node. return nodeNewStreamConnection(); } From 33963937e737ec6cf0b6fd31a557498fcf5ce178 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 09:22:58 -0700 Subject: [PATCH 12/23] Update packages/storage/src/platform/node/connection.ts Co-authored-by: Feiyang --- packages/storage/src/platform/node/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index 3bbd69c3f50..b8cb1e085db 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -158,7 +158,7 @@ export class FetchStreamConnection extends FetchConnection Date: Tue, 9 Nov 2021 09:54:22 -0700 Subject: [PATCH 13/23] Update clever-eggs-relate.md --- .changeset/clever-eggs-relate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/clever-eggs-relate.md b/.changeset/clever-eggs-relate.md index 39095c16065..52bcfa4d1a6 100644 --- a/.changeset/clever-eggs-relate.md +++ b/.changeset/clever-eggs-relate.md @@ -1,5 +1,5 @@ --- -"@firebase/storage": patch +"@firebase/storage": minor --- Adds `getBytes()`, `getStream()` and `getBlob()`, which allow direct file downloads from the SDK. From ce4b6177689b869f546116da394936dbe573d2a3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 10:12:19 -0700 Subject: [PATCH 14/23] Review --- .../test/unit/specs/query_spec.test.ts | 21 +++++++++++++++++++ .../storage/src/implementation/connection.ts | 12 ++++++++--- .../storage/src/implementation/request.ts | 11 +++++----- .../storage/src/implementation/requestinfo.ts | 14 ++++++------- .../storage/src/implementation/requests.ts | 12 +++++------ .../src/platform/browser/connection.ts | 13 ++++++------ .../storage/src/platform/node/connection.ts | 14 ++++++++----- 7 files changed, 63 insertions(+), 34 deletions(-) diff --git a/packages/firestore/test/unit/specs/query_spec.test.ts b/packages/firestore/test/unit/specs/query_spec.test.ts index 5e9736db95b..286b0603d08 100644 --- a/packages/firestore/test/unit/specs/query_spec.test.ts +++ b/packages/firestore/test/unit/specs/query_spec.test.ts @@ -100,6 +100,27 @@ describeSpec('Queries:', [], () => { }); }); + Caused by: java.lang.AssertionError: expected:< + [DocumentViewChange(Document{key=cg/1, version=SnapshotVersion(seconds=0, nanos=1000000), re + adTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=SYNCED, value=ObjectValue{internalValue={val:1}}},ADDED), + DocumentViewChange(Document{key=cg/2, version=SnapshotVersion(seconds=0, nanos=0), readTime=SnapshotVersion(seconds=0, nanos=0), + type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, value=ObjectValue{internalValue={val:2}}},ADDED), + \DocumentViewChange(Document{key=not-cg/nope/cg/3, version=SnapshotVersion(seconds=0, nanos=0), + readTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, + value=ObjectValue{internalValue={val:1}}},ADDED)]> + + + but was:<[DocumentViewChange(Document{key=cg/1, version=SnapshotVersion(seconds=0, nanos=1000000), + readTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=SYNCED, + value=ObjectValue{internalValue={val:1}}},ADDED), DocumentViewChange(Document{key=cg/2, + version=SnapshotVersion(seconds=0, nanos=0), readTime=SnapshotVersion(seconds=0, nanos=0), + type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, value=ObjectValue{internalValue={val:2}}},ADDED)]> + at org.junit.Assert.fail(Assert.java:88) + at org.junit.Assert.failNotEquals(Assert.java:834) + at org.junit.Assert.assertEquals(Assert.java:118) + at org.junit.Assert.assertEquals(Assert.java:144) + + specTest( 'Latency-compensated updates are included in query results', [], diff --git a/packages/storage/src/implementation/connection.ts b/packages/storage/src/implementation/connection.ts index 40616eb1819..440a6966939 100644 --- a/packages/storage/src/implementation/connection.ts +++ b/packages/storage/src/implementation/connection.ts @@ -18,15 +18,21 @@ /** Network headers */ export type Headers = Record; +/** Response type exposed by the networking APIs. */ +export type ConnectionType = + | string + | ArrayBuffer + | Blob + | NodeJS.ReadableStream; + /** * A lightweight wrapper around XMLHttpRequest with a * goog.net.XhrIo-like interface. * - * ResponseType is generally either `string`, `ArrayBuffer` or `ReadableSteam`. * You can create a new connection by invoking `newTextConnection()`, * `newBytesConnection()` or `newStreamConnection()`. */ -export interface Connection { +export interface Connection { /** * Sends a request to the provided URL. * @@ -45,7 +51,7 @@ export interface Connection { getStatus(): number; - getResponse(): ResponseType; + getResponse(): T; getErrorText(): string; diff --git a/packages/storage/src/implementation/request.ts b/packages/storage/src/implementation/request.ts index f9c700c9877..226b1ea08de 100644 --- a/packages/storage/src/implementation/request.ts +++ b/packages/storage/src/implementation/request.ts @@ -25,7 +25,7 @@ import { appDeleted, canceled, retryLimitExceeded, unknown } from './error'; import { ErrorHandler, RequestHandler, RequestInfo } from './requestinfo'; import { isJustDef } from './type'; import { makeQueryString } from './url'; -import { Connection, ErrorCode, Headers } from './connection'; +import { Connection, ErrorCode, Headers, ConnectionType } from './connection'; export interface Request { getPromise(): Promise; @@ -44,12 +44,11 @@ export interface Request { * Handles network logic for all Storage Requests, including error reporting and * retries with backoff. * - * @param I - the type of the backend's network response (always `string`, - * `ArrayBuffer` or `ReadableStream`). + * @param I - the type of the backend's network response. * @param - O the output type used by the rest of the SDK. The conversion * happens in the specified `callback_`. */ -class NetworkRequest implements Request { +class NetworkRequest implements Request { private pendingConnection_: Connection | null = null; private backoffId_: backoffId | null = null; private resolve_!: (value?: O | PromiseLike) => void; @@ -219,7 +218,7 @@ class NetworkRequest implements Request { * A collection of information about the result of a network request. * @param opt_canceled - Defaults to false. */ -export class RequestEndStatus { +export class RequestEndStatus { /** * True if the request was canceled. */ @@ -266,7 +265,7 @@ export function addAppCheckHeader_( } } -export function makeRequest( +export function makeRequest( requestInfo: RequestInfo, appId: string | null, authToken: string | null, diff --git a/packages/storage/src/implementation/requestinfo.ts b/packages/storage/src/implementation/requestinfo.ts index cbca3de760f..708589bb07a 100644 --- a/packages/storage/src/implementation/requestinfo.ts +++ b/packages/storage/src/implementation/requestinfo.ts @@ -15,7 +15,7 @@ * limitations under the License. */ import { StorageError } from './error'; -import { Headers, Connection } from './connection'; +import { Headers, Connection, ConnectionType } from './connection'; /** * Type for url params stored in RequestInfo. @@ -28,29 +28,27 @@ export interface UrlParams { * A function that converts a server response to the API type expected by the * SDK. * - * @param I - the type of the backend's network response (always `string`, - * `ArrayBuffer` or `ReadableStream`). + * @param I - the type of the backend's network response . * @param O - the output response type used by the rest of the SDK. */ -export type RequestHandler = ( +export type RequestHandler = ( connection: Connection, response: I ) => O; /** A function to handle an error. */ export type ErrorHandler = ( - connection: Connection, + connection: Connection, response: StorageError ) => StorageError; /** * Contains a fully specified request. * - * @param I - the type of the backend's network response (always `string`, - * `ArrayBuffer` or `ReadableStream`). + * @param I - the type of the backend's network response. * @param O - the output response type used by the rest of the SDK. */ -export class RequestInfo { +export class RequestInfo { urlParams: UrlParams = {}; headers: Headers = {}; body: Blob | string | Uint8Array | null = null; diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 9206abb3638..1a4a8b70add 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -44,7 +44,7 @@ import { fromResponseString } from './list'; import { RequestHandler, RequestInfo, UrlParams } from './requestinfo'; import { isString } from './type'; import { makeUrl } from './url'; -import { Connection } from './connection'; +import { Connection, ConnectionType } from './connection'; import { FirebaseStorageImpl } from '../service'; /** @@ -99,9 +99,9 @@ export function downloadUrlHandler( export function sharedErrorHandler( location: Location -): (p1: Connection, p2: StorageError) => StorageError { +): (p1: Connection, p2: StorageError) => StorageError { function errorHandler( - xhr: Connection, + xhr: Connection, err: StorageError ): StorageError { let newErr; @@ -134,11 +134,11 @@ export function sharedErrorHandler( export function objectErrorHandler( location: Location -): (p1: Connection, p2: StorageError) => StorageError { +): (p1: Connection, p2: StorageError) => StorageError { const shared = sharedErrorHandler(location); function errorHandler( - xhr: Connection, + xhr: Connection, err: StorageError ): StorageError { let newErr = shared(xhr, err); @@ -265,7 +265,7 @@ export function getStreamHandler(): RequestHandler< } /** Creates a new request to download an object. */ -function createDownloadRequest( +function createDownloadRequest( location: Location, service: FirebaseStorageImpl, handler: RequestHandler, diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 73e9b118d67..093d7d5b666 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -17,6 +17,7 @@ import { Connection, + ConnectionType, ErrorCode, Headers } from '../../implementation/connection'; @@ -29,7 +30,9 @@ let textFactoryOverride: (() => 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. */ -abstract class XhrConnection implements Connection { +abstract class XhrConnection + implements Connection +{ protected xhr_: XMLHttpRequest; private errorCode_: ErrorCode; private sendPromise_: Promise; @@ -100,7 +103,7 @@ abstract class XhrConnection implements Connection { } } - abstract getResponse(): ResponseType; + abstract getResponse(): T; getErrorText(): string { if (!this.sent_) { @@ -152,9 +155,7 @@ export class XhrBytesConnection extends XhrConnection { private data_?: ArrayBuffer; initXhr(): void { - // We use Blob here instead of ArrayBuffer to ensure that this code works - // in Opera. - this.xhr_.responseType = 'blob'; + this.xhr_.responseType = 'arraybuffer'; } getResponse(): ArrayBuffer { @@ -172,7 +173,7 @@ export class XhrBytesConnection extends XhrConnection { ): Promise { return super.send(url, method, body, headers).then(async () => { if (this.getErrorCode() === ErrorCode.NO_ERROR) { - this.data_ = await (this.xhr_.response as Blob).arrayBuffer(); + this.data_ = this.xhr_.response as ArrayBuffer; } }); } diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index b8cb1e085db..d86b3160cb4 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -15,7 +15,11 @@ * limitations under the License. */ -import { Connection, ErrorCode } from '../../implementation/connection'; +import { + Connection, + ConnectionType, + ErrorCode +} from '../../implementation/connection'; import { internalError } from '../../implementation/error'; import nodeFetch, { Headers } from 'node-fetch'; @@ -28,8 +32,8 @@ let textFactoryOverride: (() => Connection) | null = null; * This network implementation should not be used in browsers as it does not * support progress updates. */ -abstract class FetchConnection - implements Connection +abstract class FetchConnection + implements Connection { protected errorCode_: ErrorCode; protected statusCode_: number | undefined; @@ -37,7 +41,7 @@ abstract class FetchConnection protected errorText_ = ''; protected headers_: Headers | undefined; protected sent_: boolean = false; - private fetch_ = nodeFetch; + protected fetch_ = nodeFetch; constructor() { this.errorCode_ = ErrorCode.NO_ERROR; @@ -86,7 +90,7 @@ abstract class FetchConnection return this.statusCode_; } - abstract getResponse(): ResponseType; + abstract getResponse(): T; getErrorText(): string { return this.errorText_; From f5f71e5bffad4db2663620d307cef42e22e5c8e5 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 12:38:56 -0700 Subject: [PATCH 15/23] Update .changeset/clever-eggs-relate.md Co-authored-by: Feiyang --- .changeset/clever-eggs-relate.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/clever-eggs-relate.md b/.changeset/clever-eggs-relate.md index 52bcfa4d1a6..472cff21374 100644 --- a/.changeset/clever-eggs-relate.md +++ b/.changeset/clever-eggs-relate.md @@ -1,5 +1,6 @@ --- "@firebase/storage": minor +"firebase": minor --- Adds `getBytes()`, `getStream()` and `getBlob()`, which allow direct file downloads from the SDK. From fce32febcf8c9be7689fdcf3a003753024067010 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 12:39:19 -0700 Subject: [PATCH 16/23] Undo --- .../test/unit/specs/query_spec.test.ts | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/packages/firestore/test/unit/specs/query_spec.test.ts b/packages/firestore/test/unit/specs/query_spec.test.ts index 286b0603d08..5e9736db95b 100644 --- a/packages/firestore/test/unit/specs/query_spec.test.ts +++ b/packages/firestore/test/unit/specs/query_spec.test.ts @@ -100,27 +100,6 @@ describeSpec('Queries:', [], () => { }); }); - Caused by: java.lang.AssertionError: expected:< - [DocumentViewChange(Document{key=cg/1, version=SnapshotVersion(seconds=0, nanos=1000000), re - adTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=SYNCED, value=ObjectValue{internalValue={val:1}}},ADDED), - DocumentViewChange(Document{key=cg/2, version=SnapshotVersion(seconds=0, nanos=0), readTime=SnapshotVersion(seconds=0, nanos=0), - type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, value=ObjectValue{internalValue={val:2}}},ADDED), - \DocumentViewChange(Document{key=not-cg/nope/cg/3, version=SnapshotVersion(seconds=0, nanos=0), - readTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, - value=ObjectValue{internalValue={val:1}}},ADDED)]> - - - but was:<[DocumentViewChange(Document{key=cg/1, version=SnapshotVersion(seconds=0, nanos=1000000), - readTime=SnapshotVersion(seconds=0, nanos=0), type=FOUND_DOCUMENT, documentState=SYNCED, - value=ObjectValue{internalValue={val:1}}},ADDED), DocumentViewChange(Document{key=cg/2, - version=SnapshotVersion(seconds=0, nanos=0), readTime=SnapshotVersion(seconds=0, nanos=0), - type=FOUND_DOCUMENT, documentState=HAS_LOCAL_MUTATIONS, value=ObjectValue{internalValue={val:2}}},ADDED)]> - at org.junit.Assert.fail(Assert.java:88) - at org.junit.Assert.failNotEquals(Assert.java:834) - at org.junit.Assert.assertEquals(Assert.java:118) - at org.junit.Assert.assertEquals(Assert.java:144) - - specTest( 'Latency-compensated updates are included in query results', [], From b0a2c6c282550ecf509cfe0917087dafb713418f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 13:59:44 -0700 Subject: [PATCH 17/23] Cleanup --- .../src/platform/browser/connection.ts | 57 ++----------------- .../storage/src/platform/node/connection.ts | 7 --- packages/storage/src/service.ts | 6 +- packages/storage/test/unit/testshared.ts | 8 ++- 4 files changed, 15 insertions(+), 63 deletions(-) diff --git a/packages/storage/src/platform/browser/connection.ts b/packages/storage/src/platform/browser/connection.ts index 093d7d5b666..442962dafe8 100644 --- a/packages/storage/src/platform/browser/connection.ts +++ b/packages/storage/src/platform/browser/connection.ts @@ -103,7 +103,12 @@ abstract class XhrConnection } } - abstract getResponse(): T; + getResponse(): T { + if (!this.sent_) { + throw internalError('cannot .getResponse() before sending'); + } + return this.xhr_.response; + } getErrorText(): string { if (!this.sent_) { @@ -138,13 +143,6 @@ export class XhrTextConnection extends XhrConnection { initXhr(): void { this.xhr_.responseType = 'text'; } - - getResponse(): string { - if (!this.sent_) { - throw internalError('cannot .getResponse() before sending'); - } - return this.xhr_.response; - } } export function newTextConnection(): Connection { @@ -157,26 +155,6 @@ export class XhrBytesConnection extends XhrConnection { initXhr(): void { this.xhr_.responseType = 'arraybuffer'; } - - getResponse(): ArrayBuffer { - if (!this.sent_) { - throw internalError('cannot .getResponse() before sending'); - } - return this.data_!; - } - - send( - url: string, - method: string, - body?: ArrayBufferView | Blob | string, - headers?: Headers - ): Promise { - return super.send(url, method, body, headers).then(async () => { - if (this.getErrorCode() === ErrorCode.NO_ERROR) { - this.data_ = this.xhr_.response as ArrayBuffer; - } - }); - } } export function newBytesConnection(): Connection { @@ -187,29 +165,6 @@ export class XhrBlobConnection extends XhrConnection { initXhr(): void { this.xhr_.responseType = 'blob'; } - - getErrorText(): string { - if (!this.sent_) { - throw internalError('cannot .getErrorText() before sending'); - } - return this.xhr_.responseText; - } - - getResponse(): Blob { - if (!this.sent_) { - throw internalError('cannot .getResponse() before sending'); - } - return this.xhr_.response; - } - - send( - url: string, - method: string, - body?: ArrayBufferView | Blob | string, - headers?: Headers - ): Promise { - return super.send(url, method, body, headers); - } } export function newBlobConnection(): Connection { diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index d86b3160cb4..4d07d1e2465 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -111,9 +111,6 @@ abstract class FetchConnection // Not supported } - /** - * @override - */ removeUploadProgressListener(listener: (p1: ProgressEvent) => void): void { // Not supported } @@ -185,10 +182,6 @@ export class FetchStreamConnection extends FetchConnection { diff --git a/packages/storage/src/service.ts b/packages/storage/src/service.ts index f0bb1f9f9a1..3034543e531 100644 --- a/packages/storage/src/service.ts +++ b/packages/storage/src/service.ts @@ -39,7 +39,7 @@ import { import { validateNumber } from './implementation/type'; import { FirebaseStorage } from './public-types'; import { createMockUserToken, EmulatorMockTokenOptions } from '@firebase/util'; -import { Connection } from './implementation/connection'; +import { Connection, ConnectionType } from './implementation/connection'; export function isUrl(path?: string): boolean { return /^[A-Za-z]+:\/\//.test(path as string); @@ -298,7 +298,7 @@ export class FirebaseStorageImpl implements FirebaseStorage { * @param requestInfo - HTTP RequestInfo object * @param authToken - Firebase auth token */ - _makeRequest( + _makeRequest( requestInfo: RequestInfo, requestFactory: () => Connection, authToken: string | null, @@ -325,7 +325,7 @@ export class FirebaseStorageImpl implements FirebaseStorage { } } - async makeRequestWithTokens( + async makeRequestWithTokens( requestInfo: RequestInfo, requestFactory: () => Connection ): Promise { diff --git a/packages/storage/test/unit/testshared.ts b/packages/storage/test/unit/testshared.ts index 0a30bfa68c9..1ff22a03b5b 100644 --- a/packages/storage/test/unit/testshared.ts +++ b/packages/storage/test/unit/testshared.ts @@ -22,7 +22,11 @@ use(chaiAsPromised); 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 { + Headers, + Connection, + ConnectionType +} from '../../src/implementation/connection'; import { newTestConnection, TestingConnection } from './connection'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { @@ -110,7 +114,7 @@ export function makeFakeAppCheckProvider(tokenResult: { * Returns something that looks like an fbs.XhrIo with the given headers * and status. */ -export function fakeXhrIo( +export function fakeXhrIo( headers: Headers, status: number = 200 ): Connection { From 850a6a5de27b31fd026a7f2224084d16f773d7a5 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Nov 2021 14:10:13 -0700 Subject: [PATCH 18/23] Simplify --- .../storage/src/implementation/requestinfo.ts | 2 +- .../storage/src/implementation/requests.ts | 51 ++++--------------- .../storage/src/platform/node/connection.ts | 4 +- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/packages/storage/src/implementation/requestinfo.ts b/packages/storage/src/implementation/requestinfo.ts index 708589bb07a..18af96d708f 100644 --- a/packages/storage/src/implementation/requestinfo.ts +++ b/packages/storage/src/implementation/requestinfo.ts @@ -28,7 +28,7 @@ export interface UrlParams { * A function that converts a server response to the API type expected by the * SDK. * - * @param I - the type of the backend's network response . + * @param I - the type of the backend's network response * @param O - the output response type used by the rest of the SDK. */ export type RequestHandler = ( diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 1a4a8b70add..0e66cc08ca0 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -212,16 +212,7 @@ export function getBytes( location: Location, maxDownloadSizeBytes?: number ): RequestInfo { - return createDownloadRequest( - location, - service, - getBytesHandler(), - maxDownloadSizeBytes - ); -} - -export function getBytesHandler(): RequestHandler { - return (xhr: Connection, data: ArrayBuffer) => data; + return createDownloadRequest(location, service, maxDownloadSizeBytes); } export function getBlob( @@ -229,16 +220,7 @@ export function getBlob( location: Location, maxDownloadSizeBytes?: number ): RequestInfo { - return createDownloadRequest( - location, - service, - getBlobHandler(), - maxDownloadSizeBytes - ); -} - -export function getBlobHandler(): RequestHandler { - return (xhr: Connection, data: Blob) => data; + return createDownloadRequest(location, service, maxDownloadSizeBytes); } export function getStream( @@ -246,36 +228,25 @@ export function getStream( location: Location, maxDownloadSizeBytes?: number ): RequestInfo { - return createDownloadRequest( - location, - service, - getStreamHandler(), - maxDownloadSizeBytes - ); -} - -export function getStreamHandler(): RequestHandler< - NodeJS.ReadableStream, - NodeJS.ReadableStream -> { - return ( - xhr: Connection, - data: NodeJS.ReadableStream - ) => data; + return createDownloadRequest(location, service, maxDownloadSizeBytes); } /** Creates a new request to download an object. */ -function createDownloadRequest( +function createDownloadRequest( location: Location, service: FirebaseStorageImpl, - handler: RequestHandler, maxDownloadSizeBytes?: number -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol) + '?alt=media'; const method = 'GET'; const timeout = service.maxOperationRetryTime; - const requestInfo = new RequestInfo(url, method, handler, timeout); + const requestInfo = new RequestInfo( + url, + method, + (_: Connection, data: I) => data, + timeout + ); requestInfo.errorHandler = objectErrorHandler(location); if (maxDownloadSizeBytes !== undefined) { requestInfo.headers['Range'] = `bytes=0-${maxDownloadSizeBytes}`; diff --git a/packages/storage/src/platform/node/connection.ts b/packages/storage/src/platform/node/connection.ts index 4d07d1e2465..a656a9f6f84 100644 --- a/packages/storage/src/platform/node/connection.ts +++ b/packages/storage/src/platform/node/connection.ts @@ -102,7 +102,9 @@ abstract class FetchConnection getResponseHeader(header: string): string | null { if (!this.headers_) { - throw internalError('cannot .getResponse() before receiving response'); + throw internalError( + 'cannot .getResponseHeader() before receiving response' + ); } return this.headers_.get(header); } From 7637d206cfd78f348bc6503b5e9ac3cd394c0dad Mon Sep 17 00:00:00 2001 From: schmidt-sebastian Date: Tue, 9 Nov 2021 21:19:38 +0000 Subject: [PATCH 19/23] Update API reports --- common/api-review/storage.api.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/api-review/storage.api.md b/common/api-review/storage.api.md index 0daaf2ef274..b893b9f256d 100644 --- a/common/api-review/storage.api.md +++ b/common/api-review/storage.api.md @@ -77,14 +77,15 @@ export class _FirebaseStorageImpl implements FirebaseStorage { _getAuthToken(): Promise; get host(): string; set host(host: string); + // Warning: (ae-forgotten-export) The symbol "ConnectionType" needs to be exported by the entry point index.d.ts // 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, requestFactory: () => Connection, 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, requestFactory: () => Connection): Promise; + makeRequestWithTokens(requestInfo: RequestInfo_2, requestFactory: () => Connection): Promise; _makeStorageReference(loc: _Location): _Reference; get maxOperationRetryTime(): number; set maxOperationRetryTime(time: number); From 7955731813338298b4e825315e22fa5f836857bb Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Nov 2021 11:44:42 -0700 Subject: [PATCH 20/23] Update requests.ts --- packages/storage/src/implementation/requests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 0e66cc08ca0..0ef41d2980b 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -41,7 +41,7 @@ import { toResourceString } from './metadata'; import { fromResponseString } from './list'; -import { RequestHandler, RequestInfo, UrlParams } from './requestinfo'; +import { RequestInfo, UrlParams } from './requestinfo'; import { isString } from './type'; import { makeUrl } from './url'; import { Connection, ConnectionType } from './connection'; From 3aa8b21c4fecd9ea0ae577d7f26c8f0087ef9a3e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Nov 2021 11:55:33 -0700 Subject: [PATCH 21/23] Cleanup --- common/api-review/storage.api.md | 5 +- .../storage/src/implementation/requests.ts | 29 +---------- packages/storage/src/reference.ts | 52 ++++++++----------- 3 files changed, 28 insertions(+), 58 deletions(-) diff --git a/common/api-review/storage.api.md b/common/api-review/storage.api.md index 0daaf2ef274..b893b9f256d 100644 --- a/common/api-review/storage.api.md +++ b/common/api-review/storage.api.md @@ -77,14 +77,15 @@ export class _FirebaseStorageImpl implements FirebaseStorage { _getAuthToken(): Promise; get host(): string; set host(host: string); + // Warning: (ae-forgotten-export) The symbol "ConnectionType" needs to be exported by the entry point index.d.ts // 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, requestFactory: () => Connection, 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, requestFactory: () => Connection): Promise; + makeRequestWithTokens(requestInfo: RequestInfo_2, requestFactory: () => Connection): Promise; _makeStorageReference(loc: _Location): _Reference; get maxOperationRetryTime(): number; set maxOperationRetryTime(time: number); diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index 0e66cc08ca0..f02e8e14f9a 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -207,36 +207,11 @@ export function list( return requestInfo; } -export function getBytes( +export function getBytes( service: FirebaseStorageImpl, location: Location, maxDownloadSizeBytes?: number -): RequestInfo { - return createDownloadRequest(location, service, maxDownloadSizeBytes); -} - -export function getBlob( - service: FirebaseStorageImpl, - location: Location, - maxDownloadSizeBytes?: number -): RequestInfo { - return createDownloadRequest(location, service, maxDownloadSizeBytes); -} - -export function getStream( - service: FirebaseStorageImpl, - location: Location, - maxDownloadSizeBytes?: number -): RequestInfo { - return createDownloadRequest(location, service, maxDownloadSizeBytes); -} - -/** Creates a new request to download an object. */ -function createDownloadRequest( - location: Location, - service: FirebaseStorageImpl, - maxDownloadSizeBytes?: number -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol) + '?alt=media'; const method = 'GET'; diff --git a/packages/storage/src/reference.ts b/packages/storage/src/reference.ts index ee6bcc45f00..8c253320c27 100644 --- a/packages/storage/src/reference.ts +++ b/packages/storage/src/reference.ts @@ -19,36 +19,34 @@ * @fileoverview Defines the Firebase StorageReference class. */ -import { Transform, TransformOptions, PassThrough } from 'stream'; +import {PassThrough, Transform, TransformOptions} from 'stream'; -import { FbsBlob } from './implementation/blob'; -import { Location } from './implementation/location'; -import { getMappings } from './implementation/metadata'; -import { child, parent, lastComponent } from './implementation/path'; +import {FbsBlob} from './implementation/blob'; +import {Location} from './implementation/location'; +import {getMappings} from './implementation/metadata'; +import {child, lastComponent, parent} from './implementation/path'; import { - list as requestsList, - getMetadata as requestsGetMetadata, - updateMetadata as requestsUpdateMetadata, - getDownloadUrl as requestsGetDownloadUrl, deleteObject as requestsDeleteObject, - multipartUpload, getBytes, - getBlob, - getStream + getDownloadUrl as requestsGetDownloadUrl, + getMetadata as requestsGetMetadata, + list as requestsList, + multipartUpload, + updateMetadata as requestsUpdateMetadata } from './implementation/requests'; -import { ListOptions, UploadResult } from './public-types'; -import { StringFormat, dataFromString } from './implementation/string'; -import { Metadata } from './metadata'; -import { FirebaseStorageImpl } from './service'; -import { ListResult } from './list'; -import { UploadTask } from './task'; -import { invalidRootOperation, noDownloadURL } from './implementation/error'; -import { validateNumber } from './implementation/type'; +import {ListOptions, UploadResult} from './public-types'; +import {dataFromString, StringFormat} from './implementation/string'; +import {Metadata} from './metadata'; +import {FirebaseStorageImpl} from './service'; +import {ListResult} from './list'; +import {UploadTask} from './task'; +import {invalidRootOperation, noDownloadURL} from './implementation/error'; +import {validateNumber} from './implementation/type'; import { - newBytesConnection, - newTextConnection, newBlobConnection, - newStreamConnection + newBytesConnection, + newStreamConnection, + newTextConnection } from './platform/connection'; /** @@ -186,7 +184,7 @@ export function getBlobInternal( maxDownloadSizeBytes?: number ): Promise { ref._throwIfRoot('getBlob'); - const requestInfo = getBlob(ref.storage, ref._location, maxDownloadSizeBytes); + const requestInfo = getBytes(ref.storage, ref._location, maxDownloadSizeBytes); return ref.storage .makeRequestWithTokens(requestInfo, newBlobConnection) .then(blob => @@ -203,11 +201,7 @@ export function getStreamInternal( maxDownloadSizeBytes?: number ): NodeJS.ReadableStream { ref._throwIfRoot('getStream'); - const requestInfo = getStream( - ref.storage, - ref._location, - maxDownloadSizeBytes - ); + const requestInfo = getBytes(ref.storage, ref._location, maxDownloadSizeBytes); /** A transformer that passes through the first n bytes. */ const newMaxSizeTransform: (n: number) => TransformOptions = n => { From 15b14138fcbd943579c523c3454d3fc39957b29d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Nov 2021 11:56:25 -0700 Subject: [PATCH 22/23] Lint --- .../storage/src/implementation/requests.ts | 2 +- packages/storage/src/reference.ts | 38 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/storage/src/implementation/requests.ts b/packages/storage/src/implementation/requests.ts index c9361b77490..ab3b4e26827 100644 --- a/packages/storage/src/implementation/requests.ts +++ b/packages/storage/src/implementation/requests.ts @@ -211,7 +211,7 @@ export function getBytes( service: FirebaseStorageImpl, location: Location, maxDownloadSizeBytes?: number -): RequestInfo { +): RequestInfo { const urlPart = location.fullServerUrl(); const url = makeUrl(urlPart, service.host, service._protocol) + '?alt=media'; const method = 'GET'; diff --git a/packages/storage/src/reference.ts b/packages/storage/src/reference.ts index 8c253320c27..1657ed2383e 100644 --- a/packages/storage/src/reference.ts +++ b/packages/storage/src/reference.ts @@ -19,12 +19,12 @@ * @fileoverview Defines the Firebase StorageReference class. */ -import {PassThrough, Transform, TransformOptions} from 'stream'; +import { PassThrough, Transform, TransformOptions } from 'stream'; -import {FbsBlob} from './implementation/blob'; -import {Location} from './implementation/location'; -import {getMappings} from './implementation/metadata'; -import {child, lastComponent, parent} from './implementation/path'; +import { FbsBlob } from './implementation/blob'; +import { Location } from './implementation/location'; +import { getMappings } from './implementation/metadata'; +import { child, lastComponent, parent } from './implementation/path'; import { deleteObject as requestsDeleteObject, getBytes, @@ -34,14 +34,14 @@ import { multipartUpload, updateMetadata as requestsUpdateMetadata } from './implementation/requests'; -import {ListOptions, UploadResult} from './public-types'; -import {dataFromString, StringFormat} from './implementation/string'; -import {Metadata} from './metadata'; -import {FirebaseStorageImpl} from './service'; -import {ListResult} from './list'; -import {UploadTask} from './task'; -import {invalidRootOperation, noDownloadURL} from './implementation/error'; -import {validateNumber} from './implementation/type'; +import { ListOptions, UploadResult } from './public-types'; +import { dataFromString, StringFormat } from './implementation/string'; +import { Metadata } from './metadata'; +import { FirebaseStorageImpl } from './service'; +import { ListResult } from './list'; +import { UploadTask } from './task'; +import { invalidRootOperation, noDownloadURL } from './implementation/error'; +import { validateNumber } from './implementation/type'; import { newBlobConnection, newBytesConnection, @@ -184,7 +184,11 @@ export function getBlobInternal( maxDownloadSizeBytes?: number ): Promise { ref._throwIfRoot('getBlob'); - const requestInfo = getBytes(ref.storage, ref._location, maxDownloadSizeBytes); + const requestInfo = getBytes( + ref.storage, + ref._location, + maxDownloadSizeBytes + ); return ref.storage .makeRequestWithTokens(requestInfo, newBlobConnection) .then(blob => @@ -201,7 +205,11 @@ export function getStreamInternal( maxDownloadSizeBytes?: number ): NodeJS.ReadableStream { ref._throwIfRoot('getStream'); - const requestInfo = getBytes(ref.storage, ref._location, maxDownloadSizeBytes); + const requestInfo = getBytes( + ref.storage, + ref._location, + maxDownloadSizeBytes + ); /** A transformer that passes through the first n bytes. */ const newMaxSizeTransform: (n: number) => TransformOptions = n => { From d119bfbf9a57d171fcd8270f6f277634b69da118 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 11 Nov 2021 10:14:25 -0700 Subject: [PATCH 23/23] Typo --- packages/storage/src/api.browser.ts | 4 ++-- packages/storage/src/api.node.ts | 4 ++-- packages/storage/src/api.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/storage/src/api.browser.ts b/packages/storage/src/api.browser.ts index cf39444832d..0ccf31bbdfb 100644 --- a/packages/storage/src/api.browser.ts +++ b/packages/storage/src/api.browser.ts @@ -30,7 +30,7 @@ import { getModularInstance } from '@firebase/util'; * This API is not available in Node. * * @public - * @param ref - StorageReference where data should be download. + * @param ref - StorageReference where data should be downloaded. * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to * retrieve. * @returns A Promise that resolves with a Blob containing the object's bytes @@ -50,7 +50,7 @@ export function getBlob( * This API is only available in Node. * * @public - * @param ref - StorageReference where data should be download. + * @param ref - StorageReference where data should be downloaded. * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to * retrieve. * @returns A stream with the object's data as bytes diff --git a/packages/storage/src/api.node.ts b/packages/storage/src/api.node.ts index b413260d5bd..790147d26fa 100644 --- a/packages/storage/src/api.node.ts +++ b/packages/storage/src/api.node.ts @@ -30,7 +30,7 @@ import { getModularInstance } from '@firebase/util'; * This API is not available in Node. * * @public - * @param ref - StorageReference where data should be download. + * @param ref - StorageReference where data should be downloaded. * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to * retrieve. * @returns A Promise that resolves with a Blob containing the object's bytes @@ -50,7 +50,7 @@ export function getBlob( * This API is only available in Node. * * @public - * @param ref - StorageReference where data should be download. + * @param ref - StorageReference where data should be downloaded. * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to * retrieve. * @returns A stream with the object's data as bytes diff --git a/packages/storage/src/api.ts b/packages/storage/src/api.ts index 20eae852bac..a489cacccb8 100644 --- a/packages/storage/src/api.ts +++ b/packages/storage/src/api.ts @@ -86,7 +86,7 @@ export { StringFormat }; * https://cloud.google.com/storage/docs/configuring-cors * * @public - * @param ref - StorageReference where data should be download. + * @param ref - StorageReference where data should be downloaded. * @param maxDownloadSizeBytes - If set, the maximum allowed size in bytes to * retrieve. * @returns A Promise containing the object's bytes