diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index fe7afecac82..906d540efa8 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -23,11 +23,7 @@ import { validateArgType, validateExactNumberOfArgs } from '../util/input_validation'; -import { primitiveComparator } from '../util/misc'; -import { - binaryStringFromUint8Array, - uint8ArrayFromBinaryString -} from '../util/byte_string'; +import { ByteString } from '../util/byte_string'; /** Helper function to assert Uint8Array is available at runtime. */ function assertUint8ArrayAvailable(): void { @@ -57,15 +53,13 @@ function assertBase64Available(): void { * using the hack above to make sure no-one outside this module can call it. */ export class Blob { - // Prefix with underscore to signal this is a private variable in JS and - // prevent it showing up for autocompletion. - // A binary string is a string with each char as Unicode code point in the - // range of [0, 255], essentially simulating a byte array. - private _binaryString: string; + // Prefix with underscore to signal that we consider this not part of the + // public API and to prevent it from showing up for autocompletion. + _byteString: ByteString; - private constructor(binaryString: string) { + constructor(byteString: ByteString) { assertBase64Available(); - this._binaryString = binaryString; + this._byteString = byteString; } static fromBase64String(base64: string): Blob { @@ -73,8 +67,7 @@ export class Blob { validateArgType('Blob.fromBase64String', 'string', 1, base64); assertBase64Available(); try { - const binaryString = PlatformSupport.getPlatform().atob(base64); - return new Blob(binaryString); + return new Blob(ByteString.fromBase64String(base64)); } catch (e) { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -89,21 +82,19 @@ export class Blob { if (!(array instanceof Uint8Array)) { throw invalidClassError('Blob.fromUint8Array', 'Uint8Array', 1, array); } - const binaryString = binaryStringFromUint8Array(array); - return new Blob(binaryString); + return new Blob(ByteString.fromUint8Array(array)); } toBase64(): string { validateExactNumberOfArgs('Blob.toBase64', arguments, 0); assertBase64Available(); - return PlatformSupport.getPlatform().btoa(this._binaryString); + return this._byteString.toBase64(); } toUint8Array(): Uint8Array { validateExactNumberOfArgs('Blob.toUint8Array', arguments, 0); assertUint8ArrayAvailable(); - const buffer = uint8ArrayFromBinaryString(this._binaryString); - return buffer; + return this._byteString.toUint8Array(); } toString(): string { @@ -111,20 +102,7 @@ export class Blob { } isEqual(other: Blob): boolean { - return this._binaryString === other._binaryString; - } - - _approximateByteSize(): number { - // Assume UTF-16 encoding in memory (see StringValue.approximateByteSize()) - return this._binaryString.length * 2; - } - - /** - * Actually private to JS consumers of our API, so this function is prefixed - * with an underscore. - */ - _compareTo(other: Blob): number { - return primitiveComparator(this._binaryString, other._binaryString); + return this._byteString.isEqual(other._byteString); } } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 6fc11907499..157f21eeba7 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -19,6 +19,7 @@ import * as firestore from '@firebase/firestore-types'; import { FirebaseApp } from '@firebase/app-types'; import { FirebaseService, _FirebaseApp } from '@firebase/app-types/private'; +import { Blob } from './blob'; import { DatabaseId, DatabaseInfo } from '../core/database_info'; import { ListenOptions } from '../core/event_manager'; import { @@ -44,6 +45,7 @@ import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { ArrayValue, + BlobValue, FieldValue, FieldValueOptions, ObjectValue, @@ -1483,6 +1485,8 @@ export class DocumentSnapshot ); } return new DocumentReference(key, this._firestore, this._converter); + } else if (value instanceof BlobValue) { + return new Blob(value.internalValue); } else { return value.value(options); } diff --git a/packages/firestore/src/api/user_data_converter.ts b/packages/firestore/src/api/user_data_converter.ts index f4b0094d8bd..4341526959d 100644 --- a/packages/firestore/src/api/user_data_converter.ts +++ b/packages/firestore/src/api/user_data_converter.ts @@ -721,7 +721,7 @@ export class UserDataConverter { } else if (value instanceof GeoPoint) { return new GeoPointValue(value); } else if (value instanceof Blob) { - return new BlobValue(value); + return new BlobValue(value._byteString); } else if (value instanceof DocumentKeyReference) { return new RefValue(value.databaseId, value.key); } else { diff --git a/packages/firestore/src/model/field_value.ts b/packages/firestore/src/model/field_value.ts index c1cda3f22ce..6e8c863850b 100644 --- a/packages/firestore/src/model/field_value.ts +++ b/packages/firestore/src/model/field_value.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { Blob } from '../api/blob'; import { SnapshotOptions } from '../api/database'; import { GeoPoint } from '../api/geo_point'; import { Timestamp } from '../api/timestamp'; @@ -25,6 +24,7 @@ import { primitiveComparator } from '../util/misc'; import { DocumentKey } from './document_key'; import { FieldMask } from './mutation'; import { FieldPath } from './path'; +import { ByteString } from '../util/byte_string'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; @@ -455,11 +455,11 @@ export class ServerTimestampValue extends FieldValue { export class BlobValue extends FieldValue { typeOrder = TypeOrder.BlobValue; - constructor(readonly internalValue: Blob) { + constructor(readonly internalValue: ByteString) { super(); } - value(options?: FieldValueOptions): Blob { + value(options?: FieldValueOptions): ByteString { return this.internalValue; } @@ -472,13 +472,13 @@ export class BlobValue extends FieldValue { compareTo(other: FieldValue): number { if (other instanceof BlobValue) { - return this.internalValue._compareTo(other.internalValue); + return this.internalValue.compareTo(other.internalValue); } return this.defaultCompareTo(other); } approximateByteSize(): number { - return this.internalValue._approximateByteSize(); + return this.internalValue.approximateByteSize(); } } diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 66479919259..7e9acd74109 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -296,23 +296,23 @@ export class JsonProtoSerializer { } /** - * Parse the blob from the protos into the internal Blob class. Note that the - * typings assume all blobs are strings, but they are actually Uint8Arrays - * on Node. + * Parse the blob from the protos into the internal ByteString class. Note + * that the typings assume all blobs are strings, but they are actually + * Uint8Arrays on Node. */ - private fromBlob(blob: string | Uint8Array): Blob { + private fromBlob(blob: string | Uint8Array): ByteString { if (typeof blob === 'string') { assert( this.options.useProto3Json, 'Expected bytes to be passed in as Uint8Array, but got a string instead.' ); - return Blob.fromBase64String(blob); + return ByteString.fromBase64String(blob); } else { assert( !this.options.useProto3Json, 'Expected bytes to be passed in as Uint8Array, but got a string instead.' ); - return Blob.fromUint8Array(blob); + return ByteString.fromUint8Array(blob); } } diff --git a/packages/firestore/src/util/byte_string.ts b/packages/firestore/src/util/byte_string.ts index b6c0c3b9c00..79e2f1c6d70 100644 --- a/packages/firestore/src/util/byte_string.ts +++ b/packages/firestore/src/util/byte_string.ts @@ -16,6 +16,7 @@ */ import { PlatformSupport } from '../platform/platform'; +import { primitiveComparator } from './misc'; /** * Immutable class that represents a "proto" byte string. @@ -28,11 +29,7 @@ import { PlatformSupport } from '../platform/platform'; export class ByteString { static readonly EMPTY_BYTE_STRING = new ByteString(''); - private readonly _binaryString: string; - - private constructor(binaryString: string) { - this._binaryString = binaryString; - } + private constructor(private readonly binaryString: string) {} static fromBase64String(base64: string): ByteString { const binaryString = PlatformSupport.getPlatform().atob(base64); @@ -45,19 +42,23 @@ export class ByteString { } toBase64(): string { - return PlatformSupport.getPlatform().btoa(this._binaryString); + return PlatformSupport.getPlatform().btoa(this.binaryString); } toUint8Array(): Uint8Array { - return uint8ArrayFromBinaryString(this._binaryString); + return uint8ArrayFromBinaryString(this.binaryString); } approximateByteSize(): number { - return this._binaryString.length * 2; + return this.binaryString.length * 2; + } + + compareTo(other: ByteString): number { + return primitiveComparator(this.binaryString, other.binaryString); } isEqual(other: ByteString): boolean { - return this._binaryString === other._binaryString; + return this.binaryString === other.binaryString; } } diff --git a/packages/firestore/test/unit/api/blob.test.ts b/packages/firestore/test/unit/api/blob.test.ts index 3a04f88d133..292ad46bb5b 100644 --- a/packages/firestore/test/unit/api/blob.test.ts +++ b/packages/firestore/test/unit/api/blob.test.ts @@ -17,12 +17,7 @@ import { expect } from 'chai'; import { Blob, PublicBlob } from '../../../src/api/blob'; -import { - blob, - expectCorrectComparisons, - expectEqual, - expectNotEqual -} from '../../util/helpers'; +import { blob, expectEqual, expectNotEqual } from '../../util/helpers'; describe('Blob', () => { const base64Mappings: { [base64: string]: number[] } = { @@ -70,26 +65,6 @@ describe('Blob', () => { expect(Blob.fromBase64String('') instanceof PublicBlob).to.equal(true); }); - it('compares correctly', () => { - const values = [ - blob(0), - blob(0, 1), - blob(0, 1, 2), - blob(0, 2), - blob(0, 255), - blob(1), - blob(1, 0), - blob(1, 2), - blob(1, 255), - blob(2), - blob(255) - ]; - - expectCorrectComparisons(values, (left: Blob, right: Blob) => { - return left._compareTo(right); - }); - }); - it('support equality checking with isEqual()', () => { expectEqual(blob(1, 2, 3), blob(1, 2, 3)); expectNotEqual(blob(1, 2, 3), blob(4, 5, 6)); diff --git a/packages/firestore/test/unit/model/field_value.test.ts b/packages/firestore/test/unit/model/field_value.test.ts index b42201403f8..9fcd46b8a9b 100644 --- a/packages/firestore/test/unit/model/field_value.test.ts +++ b/packages/firestore/test/unit/model/field_value.test.ts @@ -345,8 +345,11 @@ describe('FieldValue', () => { // Doubles and Integers order the same but are not considered equal. [new fieldValue.DoubleValue(1)], [wrap(1.1), new fieldValue.DoubleValue(1.1)], - [wrap(blob(0, 1, 2)), new fieldValue.BlobValue(blob(0, 1, 2))], - [new fieldValue.BlobValue(blob(0, 1))], + [ + wrap(blob(0, 1, 2)), + new fieldValue.BlobValue(blob(0, 1, 2)._byteString) + ], + [new fieldValue.BlobValue(blob(0, 1)._byteString)], [wrap('string'), new fieldValue.StringValue('string')], [new fieldValue.StringValue('strin')], // latin small letter e + combining acute accent diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index 52fc0097ece..4058b5a1e55 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -19,7 +19,6 @@ import { expect } from 'chai'; import * as Long from 'long'; import * as ProtobufJS from 'protobufjs'; -import { Blob } from '../../../../src/api/blob'; import { PublicFieldValue as FieldValue } from '../../../../src/api/field_value'; import { GeoPoint } from '../../../../src/api/geo_point'; import { Timestamp } from '../../../../src/api/timestamp'; @@ -353,7 +352,7 @@ describe('Serializer', () => { it('converts BlobValue to Uint8Array', () => { const bytes = [0, 1, 2, 3, 4, 5]; - const example = Blob.fromUint8Array(new Uint8Array(bytes)); + const example = ByteString.fromUint8Array(new Uint8Array(bytes)); const expected = new Uint8Array(bytes); verifyFieldValueRoundTrip({ @@ -366,7 +365,7 @@ describe('Serializer', () => { it('converts BlobValue to Base64 string (useProto3Json=true)', () => { const base64 = 'AAECAwQF'; verifyFieldValueRoundTrip({ - value: new fieldValue.BlobValue(Blob.fromBase64String(base64)), + value: new fieldValue.BlobValue(ByteString.fromBase64String(base64)), valueType: 'bytesValue', jsonValue: base64, useProto3Json: true