Use ByteString in model types

Author: schmidt-sebastian

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,24 +53,21 @@ 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 {
     validateExactNumberOfArgs('Blob.fromBase64String', arguments, 1);
     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,42 +82,27 @@ 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 {
     return 'Blob(base64: ' + this.toBase64() + ')';
   }
 
   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);
   }
 }
 

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<T = firestore.DocumentData>
         );
       }
       return new DocumentReference(key, this._firestore, this._converter);
+    } else if (value instanceof BlobValue) {
+      return new Blob(value.internalValue);
     } else {
       return value.value(options);
     }

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 {

packages/firestore/src/model/field_value.ts

@@ -25,6 +25,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 +456,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 +473,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();
   }
 }
 

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);
     }
   }
 

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;
   }
 }
 

packages/firestore/test/unit/api/blob.test.ts

@@ -70,26 +70,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));

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

packages/firestore/test/unit/remote/node/serializer.test.ts

@@ -353,7 +353,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 +366,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