From fd1810f570814fb9c6a6552cf20479f6f4e08d16 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Fri, 14 Feb 2020 17:54:44 -0800 Subject: [PATCH 01/12] first pass --- packages/firestore/src/core/types.ts | 6 +- .../src/local/indexeddb_mutation_queue.ts | 26 +++------ .../firestore/src/local/local_serializer.ts | 21 ++----- packages/firestore/src/local/local_store.ts | 6 +- packages/firestore/src/local/target_data.ts | 4 +- packages/firestore/src/platform/platform.ts | 11 ---- .../src/platform_browser/browser_platform.ts | 2 - .../src/platform_node/node_platform.ts | 2 - .../firestore/src/remote/persistent_stream.ts | 4 +- packages/firestore/src/remote/remote_store.ts | 2 +- packages/firestore/src/remote/serializer.ts | 56 +++++++++++-------- packages/firestore/src/remote/watch_change.ts | 2 +- .../firestore/src/util/proto_byte_string.ts | 37 ++++++++++++ .../test/unit/local/local_store.test.ts | 27 ++++++--- .../test/unit/local/mutation_queue.test.ts | 9 ++- .../test/unit/local/test_mutation_queue.ts | 13 ++--- .../test/unit/remote/node/serializer.test.ts | 11 ++-- .../test/unit/remote/remote_event.test.ts | 2 +- .../firestore/test/unit/specs/spec_builder.ts | 10 +++- .../test/unit/specs/spec_test_runner.ts | 23 +++++--- packages/firestore/test/util/helpers.ts | 7 ++- packages/firestore/test/util/test_platform.ts | 5 -- 22 files changed, 157 insertions(+), 129 deletions(-) create mode 100644 packages/firestore/src/util/proto_byte_string.ts diff --git a/packages/firestore/src/core/types.ts b/packages/firestore/src/core/types.ts index 98c367af4c4..810993511b3 100644 --- a/packages/firestore/src/core/types.ts +++ b/packages/firestore/src/core/types.ts @@ -15,6 +15,8 @@ * limitations under the License. */ +import { Blob } from '../api/blob'; + /** * BatchID is a locally assigned ID for a batch of mutations that have been * applied. @@ -29,9 +31,7 @@ export type TargetId = number; export type ListenSequenceNumber = number; -// TODO(b/35918695): In GRPC / node, tokens are Uint8Array. In WebChannel, -// they're strings. We should probably (de-)serialize to a common internal type. -export type ProtoByteString = Uint8Array | string; +export type ProtoByteString = Blob; /** The different states of a mutation batch. */ export type MutationBatchState = 'pending' | 'acknowledged' | 'rejected'; diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index f91023b8ed0..9fe985f5688 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -47,7 +47,8 @@ import { LocalSerializer } from './local_serializer'; import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; -import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db'; +import { SimpleDbStore, SimpleDbTransaction } from './simple_db'; +import { Blob } from '../api/blob'; /** A mutation queue for a specific user, backed by IndexedDB. */ export class IndexedDbMutationQueue implements MutationQueue { @@ -124,7 +125,9 @@ export class IndexedDbMutationQueue implements MutationQueue { streamToken: ProtoByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { - metadata.lastStreamToken = convertStreamToken(streamToken); + // Convert the streamToken to base64 in order to store it as a string that can + // be reconstructed into a Blob type. + metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); @@ -134,7 +137,7 @@ export class IndexedDbMutationQueue implements MutationQueue { transaction: PersistenceTransaction ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next( - metadata => metadata.lastStreamToken + metadata => Blob.fromBase64String(metadata.lastStreamToken) ); } @@ -143,7 +146,9 @@ export class IndexedDbMutationQueue implements MutationQueue { streamToken: ProtoByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { - metadata.lastStreamToken = convertStreamToken(streamToken); + // Convert the streamToken to base64 in order to store it as a string that can + // be reconstructed into a Blob type. + metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); } @@ -671,19 +676,6 @@ export function removeMutationBatch( return PersistencePromise.waitFor(promises).next(() => removedDocuments); } -function convertStreamToken(token: ProtoByteString): string { - if (token instanceof Uint8Array) { - // TODO(b/78771403): Convert tokens to strings during deserialization - assert( - SimpleDb.isMockPersistence(), - 'Persisting non-string stream tokens is only supported with mock persistence.' - ); - return token.toString(); - } else { - return token; - } -} - /** * Helper to get a typed SimpleDbStore for the mutations object store. */ diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 82a7eab7a97..993d695ed9e 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -42,8 +42,8 @@ import { DbTimestampKey, DbUnknownDocument } from './indexeddb_schema'; -import { SimpleDb } from './simple_db'; import { TargetData, TargetPurpose } from './target_data'; +import { Blob } from '../api/blob'; /** Serializer for values stored in the LocalStore. */ export class LocalSerializer { @@ -207,8 +207,6 @@ export class LocalSerializer { dbTarget.lastLimboFreeSnapshotVersion !== undefined ? this.fromDbTimestamp(dbTarget.lastLimboFreeSnapshotVersion) : SnapshotVersion.MIN; - // TODO(b/140573486): Convert to platform representation - const resumeToken = dbTarget.resumeToken; let target: Target; if (isDocumentQuery(dbTarget.query)) { @@ -223,7 +221,7 @@ export class LocalSerializer { dbTarget.lastListenSequenceNumber, version, lastLimboFreeSnapshotVersion, - resumeToken + Blob.fromBase64String(dbTarget.resumeToken) ); } @@ -247,18 +245,9 @@ export class LocalSerializer { queryProto = this.remoteSerializer.toQueryTarget(targetData.target); } - let resumeToken: string; - - if (targetData.resumeToken instanceof Uint8Array) { - // TODO(b/78771403): Convert tokens to strings during deserialization - assert( - SimpleDb.isMockPersistence(), - 'Persisting non-string stream tokens is only supported with mock persistence .' - ); - resumeToken = targetData.resumeToken.toString(); - } else { - resumeToken = targetData.resumeToken; - } + // We can't store the resumeToken as a Blob in IndexedDb, so we convert it + // to a base64 string for storage. + const resumeToken = targetData.resumeToken.toBase64(); // lastListenSequenceNumber is always 0 until we do real GC. return new DbTarget( diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 1c319b4dea5..50de92fad52 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -551,7 +551,7 @@ export class LocalStore { const resumeToken = change.resumeToken; // Update the resume token if the change includes one. - if (resumeToken.length > 0) { + if (resumeToken._approximateByteSize() > 0) { const newTargetData = oldTargetData .withResumeToken(resumeToken, remoteVersion) .withSequenceNumber(txn.currentSequenceNumber); @@ -696,12 +696,12 @@ export class LocalStore { change: TargetChange ): boolean { assert( - newTargetData.resumeToken.length > 0, + newTargetData.resumeToken._approximateByteSize() > 0, 'Attempted to persist target data with no resume token' ); // Always persist target data if we don't already have a resume token. - if (oldTargetData.resumeToken.length === 0) { + if (oldTargetData.resumeToken._approximateByteSize() === 0) { return true; } diff --git a/packages/firestore/src/local/target_data.ts b/packages/firestore/src/local/target_data.ts index 06b168322c4..b57008b84c8 100644 --- a/packages/firestore/src/local/target_data.ts +++ b/packages/firestore/src/local/target_data.ts @@ -18,7 +18,7 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; import { ListenSequenceNumber, ProtoByteString, TargetId } from '../core/types'; -import { emptyByteString } from '../platform/platform'; +import { emptyByteString } from '../util/proto_byte_string'; /** An enumeration of the different purposes we have for targets. */ export enum TargetPurpose { @@ -128,7 +128,7 @@ export class TargetData { this.lastLimboFreeSnapshotVersion.isEqual( other.lastLimboFreeSnapshotVersion ) && - this.resumeToken === other.resumeToken && + this.resumeToken.isEqual(other.resumeToken) && this.target.isEqual(other.target) ); } diff --git a/packages/firestore/src/platform/platform.ts b/packages/firestore/src/platform/platform.ts index bd54878e3cf..2263fed1d6b 100644 --- a/packages/firestore/src/platform/platform.ts +++ b/packages/firestore/src/platform/platform.ts @@ -16,7 +16,6 @@ */ import { DatabaseId, DatabaseInfo } from '../core/database_info'; -import { ProtoByteString } from '../core/types'; import { Connection } from '../remote/connection'; import { JsonProtoSerializer } from '../remote/serializer'; import { fail } from '../util/assert'; @@ -52,8 +51,6 @@ export interface Platform { /** True if and only if the Base64 conversion functions are available. */ readonly base64Available: boolean; - - readonly emptyByteString: ProtoByteString; } /** @@ -77,11 +74,3 @@ export class PlatformSupport { return PlatformSupport.platform; } } - -/** - * Returns the representation of an empty "proto" byte string for the - * platform. - */ -export function emptyByteString(): ProtoByteString { - return PlatformSupport.getPlatform().emptyByteString; -} diff --git a/packages/firestore/src/platform_browser/browser_platform.ts b/packages/firestore/src/platform_browser/browser_platform.ts index 2c6b523d028..c047def3773 100644 --- a/packages/firestore/src/platform_browser/browser_platform.ts +++ b/packages/firestore/src/platform_browser/browser_platform.ts @@ -28,8 +28,6 @@ import { WebChannelConnection } from './webchannel_connection'; export class BrowserPlatform implements Platform { readonly base64Available: boolean; - readonly emptyByteString = ''; - constructor() { this.base64Available = typeof atob !== 'undefined'; } diff --git a/packages/firestore/src/platform_node/node_platform.ts b/packages/firestore/src/platform_node/node_platform.ts index 8e66c72d3ee..8937f3fbe4d 100644 --- a/packages/firestore/src/platform_node/node_platform.ts +++ b/packages/firestore/src/platform_node/node_platform.ts @@ -31,8 +31,6 @@ import { loadProtos } from './load_protos'; export class NodePlatform implements Platform { readonly base64Available = true; - readonly emptyByteString = new Uint8Array(0); - readonly document = null; get window(): Window | null { diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 6a6435bdba2..819f7916493 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -698,7 +698,7 @@ export class PersistentWriteStream extends PersistentStream< !!responseProto.streamToken, 'Got a write response without a stream token' ); - this.lastStreamToken = responseProto.streamToken; + this.lastStreamToken = this.serializer.fromBytes(responseProto.streamToken); if (!this.handshakeComplete_) { // The first response is always the handshake response @@ -748,7 +748,7 @@ export class PersistentWriteStream extends PersistentStream< 'Handshake must be complete before writing mutations' ); assert( - this.lastStreamToken.length > 0, + this.lastStreamToken._approximateByteSize() > 0, 'Trying to write mutation without a token' ); diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 73d6b930bcc..b040b6fb60f 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -431,7 +431,7 @@ export class RemoteStore implements TargetMetadataProvider { // Update in-memory resume tokens. LocalStore will update the // persistent view of these when applying the completed RemoteEvent. objUtils.forEachNumber(remoteEvent.targetChanges, (targetId, change) => { - if (change.resumeToken.length > 0) { + if (change.resumeToken._approximateByteSize() > 0) { const targetData = this.listenTargets[targetId]; // A watched target might have been removed already. if (targetData) { diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 40e1ddc88c6..97c0c00aee5 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -31,7 +31,7 @@ import { } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; -import { ProtoByteString, TargetId } from '../core/types'; +import { TargetId } from '../core/types'; import { TargetData, TargetPurpose } from '../local/target_data'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -140,20 +140,6 @@ export class JsonProtoSerializer { private options: SerializerOptions ) {} - private emptyByteString(): ProtoByteString { - if (this.options.useProto3Json) { - return ''; - } else { - return new Uint8Array(0); - } - } - - private unsafeCastProtoByteString(byteString: ProtoByteString): string { - // byteStrings can be either string or UInt8Array, but the typings say - // it's always a string. Cast as string to avoid type check failing - return byteString as string; - } - fromRpcStatus(status: api.Status): FirestoreError { const code = status.code === undefined @@ -278,13 +264,35 @@ export class JsonProtoSerializer { * This method cheats. It's typed as returning "string" because that's what * our generated proto interfaces say bytes must be. But it should return * an Uint8Array in Node. + * + * Visible for testing. */ - private toBytes(bytes: Blob): string { + toBytes(bytes: Blob): string { if (this.options.useProto3Json) { return bytes.toBase64(); } else { // The typings say it's a string, but it needs to be a Uint8Array in Node. - return this.unsafeCastProtoByteString(bytes.toUint8Array()); + // Cast as string to avoid type check failing. + return (bytes.toUint8Array() as unknown) as string; + } + } + + /** + * Returns a Blob based on the proto string value. + * DO NOT USE THIS FOR ANYTHING ELSE. + * This method cheats. Value is typed as "string" because that's what + * our generated proto interfaces say bytes must be, but it is actually + * an Uint8Array in Node. + */ + fromBytes(value: string | undefined): Blob { + if (this.options.useProto3Json) { + return Blob.fromBase64String(value ? value : ''); + } else { + // The typings say it's a string, but it will actually be a Uint8Array + // in Node. Cast to Uint8Array when creating the Blob. + return Blob.fromUint8Array( + value ? ((value as unknown) as Uint8Array) : new Uint8Array() + ); } } @@ -700,7 +708,7 @@ export class JsonProtoSerializer { targetChange: { targetChangeType: this.toWatchTargetChangeState(watchChange.state), targetIds: watchChange.targetIds, - resumeToken: this.unsafeCastProtoByteString(watchChange.resumeToken), + resumeToken: this.toBytes(watchChange.resumeToken), cause } }; @@ -718,8 +726,10 @@ export class JsonProtoSerializer { change.targetChange.targetChangeType || 'NO_CHANGE' ); const targetIds: TargetId[] = change.targetChange.targetIds || []; - const resumeToken = - change.targetChange.resumeToken || this.emptyByteString(); + + // Depending on the platform, the resumeToken could be a Uint8Array or + // string, even though it is typed as "string". + const resumeToken = this.fromBytes(change.targetChange.resumeToken); const causeProto = change.targetChange!.cause; const cause = causeProto && this.fromRpcStatus(causeProto); watchChange = new WatchTargetChange( @@ -1184,10 +1194,8 @@ export class JsonProtoSerializer { result.targetId = targetData.targetId; - if (targetData.resumeToken.length > 0) { - result.resumeToken = this.unsafeCastProtoByteString( - targetData.resumeToken - ); + if (targetData.resumeToken._approximateByteSize() > 0) { + result.resumeToken = this.toBytes(targetData.resumeToken); } return result; diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index d1940c0d13e..46cef15f541 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -162,7 +162,7 @@ class TargetState { * value. Empty resumeTokens are discarded. */ updateResumeToken(resumeToken: ProtoByteString): void { - if (resumeToken.length > 0) { + if (resumeToken._approximateByteSize() > 0) { this._hasPendingChanges = true; this._resumeToken = resumeToken; } diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts new file mode 100644 index 00000000000..54095fb0d85 --- /dev/null +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -0,0 +1,37 @@ +/** + * @license + * Copyright 2020 Google Inc. + * + * 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 { ProtoByteString } from '../core/types'; +import { PlatformSupport } from '../platform/platform'; +import { Blob } from '../api/blob'; + +/** + * Returns the representation of an empty "proto" byte string for the + * platform. + */ +export function emptyByteString(): ProtoByteString { + return Blob.fromBase64String(''); +} + +/** + * Returns the representation of a "proto" byte string (in base64) for the + * platform from the given hexadecimal string. + */ +export function byteStringFromString(value: string): ProtoByteString { + const base64 = PlatformSupport.getPlatform().btoa(value); + return Blob.fromBase64String(base64); +} diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 8e19580dc14..7f6b9884756 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -45,7 +45,10 @@ import { MutationBatchResult, BATCHID_UNKNOWN } from '../../../src/model/mutation_batch'; -import { emptyByteString } from '../../../src/platform/platform'; +import { + emptyByteString, + byteStringFromString +} from '../../../src/platform/platform'; import { RemoteEvent } from '../../../src/remote/remote_event'; import { WatchChangeAggregator, @@ -1124,7 +1127,7 @@ function genericLocalStoreTests( const query = Query.atPath(path('foo/bar')); const targetData = await localStore.allocateTarget(query.toTarget()); const targetId = targetData.targetId; - const resumeToken = 'abc'; + const resumeToken = byteStringFromString('abc'); const watchChange = new WatchTargetChange( WatchTargetChangeState.Current, [targetId], @@ -1156,12 +1159,12 @@ function genericLocalStoreTests( const query = Query.atPath(path('foo/bar')); const targetData = await localStore.allocateTarget(query.toTarget()); const targetId = targetData.targetId; - const resumeToken = 'abc'; + const resumeToken = byteStringFromString('abc'); const watchChange1 = new WatchTargetChange( WatchTargetChangeState.Current, [targetId], - resumeToken + byteStringFromString('abc') ); const aggregator1 = new WatchChangeAggregator({ getRemoteKeysForTarget: () => documentKeySet(), @@ -1527,7 +1530,11 @@ function genericLocalStoreTests( ) ) .after( - noChangeEvent(/* targetId= */ 2, /* snapshotVersion= */ 10, 'foo') + noChangeEvent( + /* targetId= */ 2, + /* snapshotVersion= */ 10, + /* resumeToken= */ byteStringFromString('foo') + ) ) .after(localViewChanges(2, /* fromCache= */ false, {})) .afterExecutingQuery(query) @@ -1552,7 +1559,11 @@ function genericLocalStoreTests( // Advance the query snapshot await localStore.applyRemoteEvent( - noChangeEvent(targetData.targetId, 10, 'resumeToken') + noChangeEvent( + /* targetId= */ targetData.targetId, + /* snapshotVersion= */ 10, + /* resumeToken= */ byteStringFromString('foo') + ) ); // At this point, we have not yet confirmed that the query is limbo free. @@ -1665,7 +1676,7 @@ function genericLocalStoreTests( noChangeEvent( /* targetId= */ 2, /* snapshotVersion= */ 10, - 'resumeToken' + /* resumeToken= */ byteStringFromString('foo') ) ) .after(localViewChanges(2, /* fromCache= */ false, {})) @@ -1727,7 +1738,7 @@ function genericLocalStoreTests( noChangeEvent( /* targetId= */ 2, /* snapshotVersion= */ 10, - 'resumeToken' + /* resumeToken= */ byteStringFromString('foo') ) ) .after(localViewChanges(2, /* fromCache= */ false, {})) diff --git a/packages/firestore/test/unit/local/mutation_queue.test.ts b/packages/firestore/test/unit/local/mutation_queue.test.ts index 9f4d3c26f05..0458928148e 100644 --- a/packages/firestore/test/unit/local/mutation_queue.test.ts +++ b/packages/firestore/test/unit/local/mutation_queue.test.ts @@ -23,7 +23,6 @@ import { Persistence } from '../../../src/local/persistence'; import { ReferenceSet } from '../../../src/local/reference_set'; import { documentKeySet } from '../../../src/model/collections'; import { MutationBatch } from '../../../src/model/mutation_batch'; -import { emptyByteString } from '../../../src/platform/platform'; import { expectEqualArrays, key, @@ -35,6 +34,10 @@ import { import { addEqualityMatcher } from '../../util/equality_matcher'; import * as persistenceHelpers from './persistence_test_helpers'; import { TestMutationQueue } from './test_mutation_queue'; +import { + emptyByteString, + byteStringFromString +} from '../../../src/util/proto_byte_string'; let persistence: Persistence; let mutationQueue: TestMutationQueue; @@ -307,8 +310,8 @@ function genericMutationQueueTests(): void { }); it('can save the last stream token', async () => { - const streamToken1 = 'token1'; - const streamToken2 = 'token2'; + const streamToken1 = byteStringFromString('token1'); + const streamToken2 = byteStringFromString('token2'); await mutationQueue.setLastStreamToken(streamToken1); diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index af26b06dc6c..459b188bbfe 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -25,6 +25,7 @@ import { DocumentKey } from '../../../src/model/document_key'; import { Mutation } from '../../../src/model/mutation'; import { MutationBatch } from '../../../src/model/mutation_batch'; import { SortedMap } from '../../../src/util/sorted_map'; +import { Blob } from '../../../src/api/blob'; /** * A wrapper around a MutationQueue that automatically creates a @@ -64,23 +65,17 @@ export class TestMutationQueue { ); } - getLastStreamToken(): Promise { + getLastStreamToken(): Promise { return this.persistence.runTransaction( 'getLastStreamToken', 'readonly-idempotent', txn => { - return this.queue.getLastStreamToken(txn).next(token => { - if (typeof token === 'string') { - return token; - } else { - throw new Error('Test mutation queue cannot handle Uint8Arrays'); - } - }); + return this.queue.getLastStreamToken(txn); } ); } - setLastStreamToken(streamToken: string): Promise { + setLastStreamToken(streamToken: Blob): Promise { return this.persistence.runTransaction( 'setLastStreamToken', 'readwrite-primary-idempotent', diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index ba009a08473..624883e5af9 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -83,6 +83,7 @@ import { wrap, wrapObject } from '../../../util/helpers'; +import { byteStringFromString } from '../../../../src/platform/platform'; describe('Serializer', () => { const partition = new DatabaseId('p', 'd'); @@ -1249,7 +1250,7 @@ describe('Serializer', () => { 4, SnapshotVersion.MIN, SnapshotVersion.MIN, - new Uint8Array([1, 2, 3]) + Blob.fromUint8Array(new Uint8Array([1, 2, 3])) ) ); const expected = { @@ -1347,14 +1348,14 @@ describe('Serializer', () => { const expected = new WatchTargetChange( WatchTargetChangeState.Removed, [1, 4], - 'token', + byteStringFromString('token'), new FirestoreError(Code.CANCELLED, 'message') ); const actual = s.fromWatchChange({ targetChange: { targetChangeType: 'REMOVE', targetIds: [1, 4], - resumeToken: 'token', + resumeToken: s.toBytes(byteStringFromString('token')), cause: { code: 1, message: 'message' } } }); @@ -1392,14 +1393,14 @@ describe('Serializer', () => { const expected = new WatchTargetChange( WatchTargetChangeState.Removed, [1, 4], - 'resume', + byteStringFromString('resume'), new FirestoreError(Code.CANCELLED, 'message') ); const actual = s.fromWatchChange({ targetChange: { targetChangeType: 'REMOVE', targetIds: [1, 4], - resumeToken: 'resume', + resumeToken: s.toBytes(byteStringFromString('resume')), cause: { code: 1, message: 'message' } } }); diff --git a/packages/firestore/test/unit/remote/remote_event.test.ts b/packages/firestore/test/unit/remote/remote_event.test.ts index b9361631f85..d2d95449a66 100644 --- a/packages/firestore/test/unit/remote/remote_event.test.ts +++ b/packages/firestore/test/unit/remote/remote_event.test.ts @@ -77,7 +77,7 @@ function expectTargetChangeEquals( expected: TargetChange ): void { expect(actual.current).to.equal(expected.current, 'TargetChange.current'); - expect(actual.resumeToken).to.equal( + expect(actual.resumeToken).to.deep.equal( expected.resumeToken, 'TargetChange.resumeToken' ); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index d988eaf91ce..fe626eff6f9 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -54,6 +54,7 @@ import { SpecWriteAck, SpecWriteFailure } from './spec_test_runner'; +import { PlatformSupport } from '../../../src/platform/platform'; // These types are used in a protected API by SpecBuilder and need to be // exported. @@ -951,18 +952,21 @@ export class SpecBuilder { // `query` is not added yet. this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query), ...activeQueries], - resumeToken: resumeToken || '' + // Convert to base64 string to be compatible with Blob format. + resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } else { this.activeTargets[targetId] = { queries: activeQueries, - resumeToken: resumeToken || '' + // Convert to base64 string to be compatible with Blob format. + resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } } else { this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query)], - resumeToken: resumeToken || '' + // Convert to base64 string to be compatible with Blob format. + resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 87adb5d630b..f0ea2d8b23c 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -30,7 +30,6 @@ import { SyncEngine } from '../../../src/core/sync_engine'; import { OnlineState, OnlineStateSource, - ProtoByteString, TargetId } from '../../../src/core/types'; import { @@ -62,10 +61,7 @@ import { DocumentOptions } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; import { JsonObject } from '../../../src/model/field_value'; import { Mutation } from '../../../src/model/mutation'; -import { - emptyByteString, - PlatformSupport -} from '../../../src/platform/platform'; +import { PlatformSupport } from '../../../src/platform/platform'; import * as api from '../../../src/protos/firestore_proto_api'; import { Connection, Stream } from '../../../src/remote/connection'; import { Datastore } from '../../../src/remote/datastore'; @@ -111,6 +107,10 @@ import { TEST_SERIALIZER } from '../local/persistence_test_helpers'; import { MULTI_CLIENT_TAG } from './describe_spec'; +import { + byteStringFromString, + emptyByteString +} from '../../../src/util/proto_byte_string'; const ARBITRARY_SEQUENCE_NUMBER = 2; @@ -723,7 +723,7 @@ abstract class TestRunner { private doWatchCurrent(currentTargets: SpecWatchCurrent): Promise { const targets = currentTargets[0]; - const resumeToken = currentTargets[1] as ProtoByteString; + const resumeToken = byteStringFromString(currentTargets[1]); const change = new WatchTargetChange( WatchTargetChangeState.Current, targets, @@ -831,7 +831,8 @@ abstract class TestRunner { const protoJSON: api.ListenResponse = { targetChange: { readTime: this.serializer.toVersion(version(watchSnapshot.version)), - resumeToken: watchSnapshot.resumeToken, + // Convert to base64 string to be compatible with Blob format. + resumeToken: this.platform.btoa(watchSnapshot.resumeToken || ''), targetIds: watchSnapshot.targetIds } }; @@ -1143,13 +1144,17 @@ abstract class TestRunner { ARBITRARY_SEQUENCE_NUMBER, SnapshotVersion.MIN, SnapshotVersion.MIN, - expected.resumeToken + byteStringFromString(expected.resumeToken) ) ); expect(actualTarget.query).to.deep.equal(expectedTarget.query); expect(actualTarget.targetId).to.equal(expectedTarget.targetId); expect(actualTarget.readTime).to.equal(expectedTarget.readTime); - expect(actualTarget.resumeToken).to.equal(expectedTarget.resumeToken); + // actualTarget's resumeToken is a hexadecimal string, but the serialized + // resumeToken will be a base64 string, so we need to convert it back. + expect(actualTarget.resumeToken || '').to.equal( + this.platform.atob(expectedTarget.resumeToken || '') + ); delete actualTargets[targetId]; }); expect(obj.size(actualTargets)).to.equal( diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 7ac49374986..ea29bb3f085 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -75,7 +75,6 @@ import { TransformMutation } from '../../src/model/mutation'; import { FieldPath, ResourcePath } from '../../src/model/path'; -import { emptyByteString } from '../../src/platform/platform'; import { RemoteEvent, TargetChange } from '../../src/remote/remote_event'; import { DocumentWatchChange, @@ -89,6 +88,10 @@ import { Dict, forEach } from '../../src/util/obj'; import { SortedMap } from '../../src/util/sorted_map'; import { SortedSet } from '../../src/util/sorted_set'; import { query } from './api_helpers'; +import { + emptyByteString, + byteStringFromString +} from '../../src/util/proto_byte_string'; export type TestSnapshotVersion = number; @@ -488,7 +491,7 @@ export function resumeTokenForSnapshot( if (snapshotVersion.isEqual(SnapshotVersion.MIN)) { return emptyByteString(); } else { - return snapshotVersion.toString(); + return byteStringFromString(snapshotVersion.toString()); } } diff --git a/packages/firestore/test/util/test_platform.ts b/packages/firestore/test/util/test_platform.ts index 3d09b59dd46..3ed3526ed89 100644 --- a/packages/firestore/test/util/test_platform.ts +++ b/packages/firestore/test/util/test_platform.ts @@ -16,7 +16,6 @@ */ import { DatabaseId, DatabaseInfo } from '../../src/core/database_info'; -import { ProtoByteString } from '../../src/core/types'; import { Platform } from '../../src/platform/platform'; import { Connection } from '../../src/remote/connection'; import { JsonProtoSerializer } from '../../src/remote/serializer'; @@ -234,10 +233,6 @@ export class TestPlatform implements Platform { return this.basePlatform.base64Available; } - get emptyByteString(): ProtoByteString { - return this.basePlatform.emptyByteString; - } - raiseVisibilityEvent(visibility: VisibilityState): void { if (this.mockDocument) { this.mockDocument.raiseVisibilityEvent(visibility); From 746bc3862491e5f78c22124bb22a65d8e398ba21 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Fri, 14 Feb 2020 18:09:14 -0800 Subject: [PATCH 02/12] change imports --- packages/firestore/src/local/memory_mutation_queue.ts | 2 +- packages/firestore/src/remote/persistent_stream.ts | 2 +- packages/firestore/src/remote/remote_event.ts | 2 +- packages/firestore/src/remote/remote_store.ts | 2 +- packages/firestore/src/remote/watch_change.ts | 2 +- packages/firestore/test/unit/local/local_store.test.ts | 5 +---- packages/firestore/test/unit/remote/node/serializer.test.ts | 2 +- packages/firestore/test/unit/remote/remote_event.test.ts | 2 +- 8 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index 1c9c62eb280..90375f9ef9a 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -22,7 +22,6 @@ import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; import { MutationBatch, BATCHID_UNKNOWN } from '../model/mutation_batch'; -import { emptyByteString } from '../platform/platform'; import { assert } from '../util/assert'; import { primitiveComparator } from '../util/misc'; import { SortedMap } from '../util/sorted_map'; @@ -33,6 +32,7 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { DocReference } from './reference_set'; +import { emptyByteString } from '../util/proto_byte_string'; export class MemoryMutationQueue implements MutationQueue { /** diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 819f7916493..d0c93f75f64 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -32,7 +32,7 @@ import { ExponentialBackoff } from './backoff'; import { Connection, Stream } from './connection'; import { JsonProtoSerializer } from './serializer'; import { WatchChange } from './watch_change'; -import { emptyByteString } from '../platform/platform'; +import { emptyByteString } from '../util/proto_byte_string'; const LOG_TAG = 'PersistentStream'; diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index ebbd84f539d..ade95e6231f 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -24,8 +24,8 @@ import { MaybeDocumentMap, targetIdSet } from '../model/collections'; -import { emptyByteString } from '../platform/platform'; import { SortedSet } from '../util/sorted_set'; +import { emptyByteString } from '../util/proto_byte_string'; /** * An event from the RemoteStore. It is split into targetChanges (changes to the diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index b040b6fb60f..f075ac99c80 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -26,7 +26,6 @@ import { MutationBatch, MutationBatchResult } from '../model/mutation_batch'; -import { emptyByteString } from '../platform/platform'; import { assert } from '../util/assert'; import { FirestoreError } from '../util/error'; import * as log from '../util/log'; @@ -52,6 +51,7 @@ import { WatchTargetChange, WatchTargetChangeState } from './watch_change'; +import { emptyByteString } from '../util/proto_byte_string'; const LOG_TAG = 'RemoteStore'; diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index 46cef15f541..45af59776b3 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -26,7 +26,6 @@ import { } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; -import { emptyByteString } from '../platform/platform'; import { assert, fail } from '../util/assert'; import { FirestoreError } from '../util/error'; import { debug } from '../util/log'; @@ -36,6 +35,7 @@ import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; import { RemoteEvent, TargetChange } from './remote_event'; +import { emptyByteString } from '../util/proto_byte_string'; /** * Internal representation of the watcher API protocol buffers. diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 7f6b9884756..da1052bb21d 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -45,10 +45,6 @@ import { MutationBatchResult, BATCHID_UNKNOWN } from '../../../src/model/mutation_batch'; -import { - emptyByteString, - byteStringFromString -} from '../../../src/platform/platform'; import { RemoteEvent } from '../../../src/remote/remote_event'; import { WatchChangeAggregator, @@ -81,6 +77,7 @@ import { import { FieldValue, IntegerValue } from '../../../src/model/field_value'; import { CountingQueryEngine } from './counting_query_engine'; import * as persistenceHelpers from './persistence_test_helpers'; +import { emptyByteString, byteStringFromString } from '../../../src/util/proto_byte_string'; class LocalStoreTester { private promiseChain: Promise = Promise.resolve(); diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index 624883e5af9..37435866f88 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -83,7 +83,7 @@ import { wrap, wrapObject } from '../../../util/helpers'; -import { byteStringFromString } from '../../../../src/platform/platform'; +import { byteStringFromString } from '../../../../src/util/proto_byte_string'; describe('Serializer', () => { const partition = new DatabaseId('p', 'd'); diff --git a/packages/firestore/test/unit/remote/remote_event.test.ts b/packages/firestore/test/unit/remote/remote_event.test.ts index d2d95449a66..8a78a2ffe02 100644 --- a/packages/firestore/test/unit/remote/remote_event.test.ts +++ b/packages/firestore/test/unit/remote/remote_event.test.ts @@ -21,7 +21,6 @@ import { TargetId } from '../../../src/core/types'; import { TargetData, TargetPurpose } from '../../../src/local/target_data'; import { DocumentKeySet, documentKeySet } from '../../../src/model/collections'; import { DocumentKey } from '../../../src/model/document_key'; -import { emptyByteString } from '../../../src/platform/platform'; import { ExistenceFilter } from '../../../src/remote/existence_filter'; import { RemoteEvent, TargetChange } from '../../../src/remote/remote_event'; import { @@ -43,6 +42,7 @@ import { updateMapping, version } from '../../util/helpers'; +import { emptyByteString } from '../../../src/util/proto_byte_string'; interface TargetMap { [targetId: string]: TargetData; From eaeed94a3092e81343129d95726a3d5fe173e57d Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Mon, 17 Feb 2020 20:22:49 -0800 Subject: [PATCH 03/12] also serialize streamTokens properly --- packages/firestore/src/remote/persistent_stream.ts | 4 +--- packages/firestore/test/unit/local/local_store.test.ts | 5 ++++- packages/firestore/test/unit/specs/spec_test_runner.ts | 5 ++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index d0c93f75f64..cad676e9d26 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -753,9 +753,7 @@ export class PersistentWriteStream extends PersistentStream< ); const request: WriteRequest = { - // Protos are typed with string, but we support UInt8Array on Node - // eslint-disable-next-line @typescript-eslint/no-explicit-any - streamToken: this.lastStreamToken as any, + streamToken: this.serializer.toBytes(this.lastStreamToken), writes: mutations.map(mutation => this.serializer.toMutation(mutation)) }; diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index da1052bb21d..c8d89fc8e48 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -77,7 +77,10 @@ import { import { FieldValue, IntegerValue } from '../../../src/model/field_value'; import { CountingQueryEngine } from './counting_query_engine'; import * as persistenceHelpers from './persistence_test_helpers'; -import { emptyByteString, byteStringFromString } from '../../../src/util/proto_byte_string'; +import { + emptyByteString, + byteStringFromString +} from '../../../src/util/proto_byte_string'; class LocalStoreTester { private promiseChain: Promise = Promise.resolve(); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index f0ea2d8b23c..7701cf07101 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -200,7 +200,10 @@ class MockConnection implements Connection { ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void { this.writeStream!.callOnMessage({ - streamToken: 'write-stream-token-' + this.nextWriteStreamToken, + // Convert to base64 string to be compatible with Blob format. + streamToken: PlatformSupport.getPlatform().btoa( + 'write-stream-token-' + this.nextWriteStreamToken + ), commitTime, writeResults: mutationResults }); From 076a3f1443391cab01fbfc2a7c0430872029fc4f Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 18 Feb 2020 17:31:18 -0800 Subject: [PATCH 04/12] Use ProtoByteString class instead of Blob --- packages/firestore/src/api/blob.ts | 18 ++--- packages/firestore/src/core/types.ts | 4 - .../src/local/indexeddb_mutation_queue.ts | 8 +- .../firestore/src/local/local_serializer.ts | 4 +- packages/firestore/src/local/local_store.ts | 9 ++- .../src/local/memory_mutation_queue.ts | 4 +- .../firestore/src/local/mutation_queue.ts | 3 +- packages/firestore/src/local/target_data.ts | 4 +- .../firestore/src/model/mutation_batch.ts | 3 +- .../firestore/src/remote/persistent_stream.ts | 6 +- packages/firestore/src/remote/remote_event.ts | 4 +- packages/firestore/src/remote/remote_store.ts | 2 +- packages/firestore/src/remote/serializer.ts | 11 +-- packages/firestore/src/remote/watch_change.ts | 6 +- .../firestore/src/util/proto_byte_string.ts | 80 ++++++++++++++++--- .../test/unit/local/local_store.test.ts | 8 +- .../test/unit/local/mutation_queue.test.ts | 8 +- .../test/unit/local/test_mutation_queue.ts | 8 +- .../test/unit/remote/node/serializer.test.ts | 7 +- .../test/unit/specs/spec_test_runner.ts | 8 +- packages/firestore/test/util/helpers.ts | 14 +++- 21 files changed, 137 insertions(+), 82 deletions(-) diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index 11c8a73816b..f52c33eed6c 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -24,6 +24,10 @@ import { validateExactNumberOfArgs } from '../util/input_validation'; import { primitiveComparator } from '../util/misc'; +import { + binaryStringFromUint8Array, + Uint8ArrayFromBinaryString +} from '../util/proto_byte_string'; /** Helper function to assert Uint8Array is available at runtime. */ function assertUint8ArrayAvailable(): void { @@ -85,14 +89,7 @@ export class Blob { if (!(array instanceof Uint8Array)) { throw invalidClassError('Blob.fromUint8Array', 'Uint8Array', 1, array); } - // We can't call array.map directly because it expects the return type to - // be a Uint8Array, whereas we can convert it to a regular array by invoking - // map on the Array prototype. - const binaryString = Array.prototype.map - .call(array, (char: number) => { - return String.fromCharCode(char); - }) - .join(''); + const binaryString = binaryStringFromUint8Array(array); return new Blob(binaryString); } @@ -105,10 +102,7 @@ export class Blob { toUint8Array(): Uint8Array { validateExactNumberOfArgs('Blob.toUint8Array', arguments, 0); assertUint8ArrayAvailable(); - const buffer = new Uint8Array(this._binaryString.length); - for (let i = 0; i < this._binaryString.length; i++) { - buffer[i] = this._binaryString.charCodeAt(i); - } + const buffer = Uint8ArrayFromBinaryString(this._binaryString); return buffer; } diff --git a/packages/firestore/src/core/types.ts b/packages/firestore/src/core/types.ts index 810993511b3..009700fd016 100644 --- a/packages/firestore/src/core/types.ts +++ b/packages/firestore/src/core/types.ts @@ -15,8 +15,6 @@ * limitations under the License. */ -import { Blob } from '../api/blob'; - /** * BatchID is a locally assigned ID for a batch of mutations that have been * applied. @@ -31,8 +29,6 @@ export type TargetId = number; export type ListenSequenceNumber = number; -export type ProtoByteString = Blob; - /** The different states of a mutation batch. */ export type MutationBatchState = 'pending' | 'acknowledged' | 'rejected'; diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 9fe985f5688..9574725bd4b 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -18,7 +18,7 @@ import { Timestamp } from '../api/timestamp'; import { User } from '../auth/user'; import { Query } from '../core/query'; -import { BatchId, ProtoByteString } from '../core/types'; +import { BatchId } from '../core/types'; import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; @@ -48,7 +48,7 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { SimpleDbStore, SimpleDbTransaction } from './simple_db'; -import { Blob } from '../api/blob'; +import { ProtoByteString } from '../util/proto_byte_string'; /** A mutation queue for a specific user, backed by IndexedDB. */ export class IndexedDbMutationQueue implements MutationQueue { @@ -137,7 +137,7 @@ export class IndexedDbMutationQueue implements MutationQueue { transaction: PersistenceTransaction ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next( - metadata => Blob.fromBase64String(metadata.lastStreamToken) + metadata => ProtoByteString.fromBase64String(metadata.lastStreamToken) ); } @@ -147,7 +147,7 @@ export class IndexedDbMutationQueue implements MutationQueue { ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a Blob type. + // be reconstructed into a Blob. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 993d695ed9e..9fb33a32ab5 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -43,7 +43,7 @@ import { DbUnknownDocument } from './indexeddb_schema'; import { TargetData, TargetPurpose } from './target_data'; -import { Blob } from '../api/blob'; +import { ProtoByteString } from '../util/proto_byte_string'; /** Serializer for values stored in the LocalStore. */ export class LocalSerializer { @@ -221,7 +221,7 @@ export class LocalSerializer { dbTarget.lastListenSequenceNumber, version, lastLimboFreeSnapshotVersion, - Blob.fromBase64String(dbTarget.resumeToken) + ProtoByteString.fromBase64String(dbTarget.resumeToken) ); } diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 50de92fad52..5db5323ddcf 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -20,7 +20,7 @@ import { User } from '../auth/user'; import { Query } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; -import { BatchId, ProtoByteString, TargetId } from '../core/types'; +import { BatchId, TargetId } from '../core/types'; import { DocumentKeySet, documentKeySet, @@ -62,6 +62,7 @@ import { RemoteDocumentCache } from './remote_document_cache'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; import { ClientId } from './shared_client_state'; import { TargetData, TargetPurpose } from './target_data'; +import { ProtoByteString } from '../util/proto_byte_string'; const LOG_TAG = 'LocalStore'; @@ -551,7 +552,7 @@ export class LocalStore { const resumeToken = change.resumeToken; // Update the resume token if the change includes one. - if (resumeToken._approximateByteSize() > 0) { + if (resumeToken.approximateByteSize() > 0) { const newTargetData = oldTargetData .withResumeToken(resumeToken, remoteVersion) .withSequenceNumber(txn.currentSequenceNumber); @@ -696,12 +697,12 @@ export class LocalStore { change: TargetChange ): boolean { assert( - newTargetData.resumeToken._approximateByteSize() > 0, + newTargetData.resumeToken.approximateByteSize() > 0, 'Attempted to persist target data with no resume token' ); // Always persist target data if we don't already have a resume token. - if (oldTargetData.resumeToken._approximateByteSize() === 0) { + if (oldTargetData.resumeToken.approximateByteSize() === 0) { return true; } diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index 90375f9ef9a..db20051ebb2 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -17,7 +17,7 @@ import { Timestamp } from '../api/timestamp'; import { Query } from '../core/query'; -import { BatchId, ProtoByteString } from '../core/types'; +import { BatchId } from '../core/types'; import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; @@ -32,7 +32,7 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { DocReference } from './reference_set'; -import { emptyByteString } from '../util/proto_byte_string'; +import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; export class MemoryMutationQueue implements MutationQueue { /** diff --git a/packages/firestore/src/local/mutation_queue.ts b/packages/firestore/src/local/mutation_queue.ts index a396ded8c3f..42ce1c7254e 100644 --- a/packages/firestore/src/local/mutation_queue.ts +++ b/packages/firestore/src/local/mutation_queue.ts @@ -17,7 +17,7 @@ import { Timestamp } from '../api/timestamp'; import { Query } from '../core/query'; -import { BatchId, ProtoByteString } from '../core/types'; +import { BatchId } from '../core/types'; import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; @@ -26,6 +26,7 @@ import { SortedMap } from '../util/sorted_map'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; +import { ProtoByteString } from '../util/proto_byte_string'; /** A queue of mutations to apply to the remote store. */ export interface MutationQueue { diff --git a/packages/firestore/src/local/target_data.ts b/packages/firestore/src/local/target_data.ts index b57008b84c8..db218833e61 100644 --- a/packages/firestore/src/local/target_data.ts +++ b/packages/firestore/src/local/target_data.ts @@ -17,8 +17,8 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; -import { ListenSequenceNumber, ProtoByteString, TargetId } from '../core/types'; -import { emptyByteString } from '../util/proto_byte_string'; +import { ListenSequenceNumber, TargetId } from '../core/types'; +import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; /** An enumeration of the different purposes we have for targets. */ export enum TargetPurpose { diff --git a/packages/firestore/src/model/mutation_batch.ts b/packages/firestore/src/model/mutation_batch.ts index 28e3b6ebe4f..41f8a04e939 100644 --- a/packages/firestore/src/model/mutation_batch.ts +++ b/packages/firestore/src/model/mutation_batch.ts @@ -17,7 +17,7 @@ import { Timestamp } from '../api/timestamp'; import { SnapshotVersion } from '../core/snapshot_version'; -import { BatchId, ProtoByteString } from '../core/types'; +import { BatchId } from '../core/types'; import { assert } from '../util/assert'; import * as misc from '../util/misc'; import { @@ -30,6 +30,7 @@ import { import { MaybeDocument } from './document'; import { DocumentKey } from './document_key'; import { Mutation, MutationResult } from './mutation'; +import { ProtoByteString } from '../util/proto_byte_string'; export const BATCHID_UNKNOWN = -1; diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index cad676e9d26..065ed7027d0 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -17,7 +17,7 @@ import { CredentialsProvider, Token } from '../api/credentials'; import { SnapshotVersion } from '../core/snapshot_version'; -import { ProtoByteString, TargetId } from '../core/types'; +import { TargetId } from '../core/types'; import { TargetData } from '../local/target_data'; import { Mutation, MutationResult } from '../model/mutation'; import * as api from '../protos/firestore_proto_api'; @@ -32,7 +32,7 @@ import { ExponentialBackoff } from './backoff'; import { Connection, Stream } from './connection'; import { JsonProtoSerializer } from './serializer'; import { WatchChange } from './watch_change'; -import { emptyByteString } from '../util/proto_byte_string'; +import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; const LOG_TAG = 'PersistentStream'; @@ -748,7 +748,7 @@ export class PersistentWriteStream extends PersistentStream< 'Handshake must be complete before writing mutations' ); assert( - this.lastStreamToken._approximateByteSize() > 0, + this.lastStreamToken.approximateByteSize() > 0, 'Trying to write mutation without a token' ); diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index ade95e6231f..6f47415197f 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -16,7 +16,7 @@ */ import { SnapshotVersion } from '../core/snapshot_version'; -import { ProtoByteString, TargetId } from '../core/types'; +import { TargetId } from '../core/types'; import { documentKeySet, DocumentKeySet, @@ -25,7 +25,7 @@ import { targetIdSet } from '../model/collections'; import { SortedSet } from '../util/sorted_set'; -import { emptyByteString } from '../util/proto_byte_string'; +import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; /** * An event from the RemoteStore. It is split into targetChanges (changes to the diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index f075ac99c80..771ddfca7d7 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -431,7 +431,7 @@ export class RemoteStore implements TargetMetadataProvider { // Update in-memory resume tokens. LocalStore will update the // persistent view of these when applying the completed RemoteEvent. objUtils.forEachNumber(remoteEvent.targetChanges, (targetId, change) => { - if (change.resumeToken._approximateByteSize() > 0) { + if (change.resumeToken.approximateByteSize() > 0) { const targetData = this.listenTargets[targetId]; // A watched target might have been removed already. if (targetData) { diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 97c0c00aee5..00df777c3ac 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -71,6 +71,7 @@ import { WatchTargetChange, WatchTargetChangeState } from './watch_change'; +import { ProtoByteString } from '../util/proto_byte_string'; const DIRECTIONS = (() => { const dirs: { [dir: string]: api.OrderDirection } = {}; @@ -267,7 +268,7 @@ export class JsonProtoSerializer { * * Visible for testing. */ - toBytes(bytes: Blob): string { + toBytes(bytes: Blob | ProtoByteString): string { if (this.options.useProto3Json) { return bytes.toBase64(); } else { @@ -284,13 +285,13 @@ export class JsonProtoSerializer { * our generated proto interfaces say bytes must be, but it is actually * an Uint8Array in Node. */ - fromBytes(value: string | undefined): Blob { + fromBytes(value: string | undefined): ProtoByteString { if (this.options.useProto3Json) { - return Blob.fromBase64String(value ? value : ''); + return ProtoByteString.fromBase64String(value ? value : ''); } else { // The typings say it's a string, but it will actually be a Uint8Array // in Node. Cast to Uint8Array when creating the Blob. - return Blob.fromUint8Array( + return ProtoByteString.fromUint8Array( value ? ((value as unknown) as Uint8Array) : new Uint8Array() ); } @@ -1194,7 +1195,7 @@ export class JsonProtoSerializer { result.targetId = targetData.targetId; - if (targetData.resumeToken._approximateByteSize() > 0) { + if (targetData.resumeToken.approximateByteSize() > 0) { result.resumeToken = this.toBytes(targetData.resumeToken); } diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index 45af59776b3..a568910a8d1 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -16,7 +16,7 @@ */ import { SnapshotVersion } from '../core/snapshot_version'; -import { ProtoByteString, TargetId } from '../core/types'; +import { TargetId } from '../core/types'; import { ChangeType } from '../core/view_snapshot'; import { TargetData, TargetPurpose } from '../local/target_data'; import { @@ -35,7 +35,7 @@ import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; import { RemoteEvent, TargetChange } from './remote_event'; -import { emptyByteString } from '../util/proto_byte_string'; +import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; /** * Internal representation of the watcher API protocol buffers. @@ -162,7 +162,7 @@ class TargetState { * value. Empty resumeTokens are discarded. */ updateResumeToken(resumeToken: ProtoByteString): void { - if (resumeToken._approximateByteSize() > 0) { + if (resumeToken.approximateByteSize() > 0) { this._hasPendingChanges = true; this._resumeToken = resumeToken; } diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 54095fb0d85..7ebce4ab964 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2020 Google Inc. + * Copyright 2020 Google LLC. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,23 +15,79 @@ * limitations under the License. */ -import { ProtoByteString } from '../core/types'; import { PlatformSupport } from '../platform/platform'; -import { Blob } from '../api/blob'; /** - * Returns the representation of an empty "proto" byte string for the - * platform. + * Immutable class that represents a "proto" byte string. + * + * Proto byte strings can either be base64 strings or Uint8Arrays when sent + * on the wire, but they are always typed to string. This class abstracts away + * this differentiation by holding the proto byte string in a common class that + * must be converted into a string before being sent as a proto. */ -export function emptyByteString(): ProtoByteString { - return Blob.fromBase64String(''); +export class ProtoByteString { + private _binaryString: string; + + private constructor(binaryString: string) { + this._binaryString = binaryString; + } + + static fromBase64String(base64: string): ProtoByteString { + const binaryString = PlatformSupport.getPlatform().atob(base64); + return new ProtoByteString(binaryString); + } + + static fromUint8Array(array: Uint8Array): ProtoByteString { + const binaryString = binaryStringFromUint8Array(array); + return new ProtoByteString(binaryString); + } + + toBase64(): string { + return PlatformSupport.getPlatform().btoa(this._binaryString); + } + + toUint8Array(): Uint8Array { + const buffer = Uint8ArrayFromBinaryString(this._binaryString); + return buffer; + } + + approximateByteSize(): number { + return this._binaryString.length * 2; + } + + isEqual(other: ProtoByteString): boolean { + return this._binaryString === other._binaryString; + } } /** - * Returns the representation of a "proto" byte string (in base64) for the - * platform from the given hexadecimal string. + * Helper function to convert an Uint8array to a binary string. */ -export function byteStringFromString(value: string): ProtoByteString { - const base64 = PlatformSupport.getPlatform().btoa(value); - return Blob.fromBase64String(base64); +export function binaryStringFromUint8Array(array: Uint8Array): string { + // We can't call array.map directly because it expects the return type to + // be a Uint8Array, whereas we can convert it to a regular array by invoking + // map on the Array prototype. + return Array.prototype.map + .call(array, (char: number) => { + return String.fromCharCode(char); + }) + .join(''); +} + +/** + * Helper function to convert a binary string to an Uint8Array. + */ +export function Uint8ArrayFromBinaryString(binaryString: string): Uint8Array { + const buffer = new Uint8Array(binaryString.length); + for (let i = 0; i < binaryString.length; i++) { + buffer[i] = binaryString.charCodeAt(i); + } + return buffer; +} + +/** + * Returns an empty ProtoByteString. + */ +export function emptyByteString(): ProtoByteString { + return ProtoByteString.fromBase64String(''); } diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index c8d89fc8e48..3fd884d78c8 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -71,16 +71,14 @@ import { TestSnapshotVersion, transformMutation, unknownDoc, - version + version, + byteStringFromString } from '../../util/helpers'; import { FieldValue, IntegerValue } from '../../../src/model/field_value'; import { CountingQueryEngine } from './counting_query_engine'; import * as persistenceHelpers from './persistence_test_helpers'; -import { - emptyByteString, - byteStringFromString -} from '../../../src/util/proto_byte_string'; +import { emptyByteString } from '../../../src/util/proto_byte_string'; class LocalStoreTester { private promiseChain: Promise = Promise.resolve(); diff --git a/packages/firestore/test/unit/local/mutation_queue.test.ts b/packages/firestore/test/unit/local/mutation_queue.test.ts index 0458928148e..5a6681aed3e 100644 --- a/packages/firestore/test/unit/local/mutation_queue.test.ts +++ b/packages/firestore/test/unit/local/mutation_queue.test.ts @@ -28,16 +28,14 @@ import { key, patchMutation, path, - setMutation + setMutation, + byteStringFromString } from '../../util/helpers'; import { addEqualityMatcher } from '../../util/equality_matcher'; import * as persistenceHelpers from './persistence_test_helpers'; import { TestMutationQueue } from './test_mutation_queue'; -import { - emptyByteString, - byteStringFromString -} from '../../../src/util/proto_byte_string'; +import { emptyByteString } from '../../../src/util/proto_byte_string'; let persistence: Persistence; let mutationQueue: TestMutationQueue; diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index 459b188bbfe..87bcaa31987 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -17,7 +17,7 @@ import { Timestamp } from '../../../src/api/timestamp'; import { Query } from '../../../src/core/query'; -import { BatchId, ProtoByteString } from '../../../src/core/types'; +import { BatchId } from '../../../src/core/types'; import { MutationQueue } from '../../../src/local/mutation_queue'; import { Persistence } from '../../../src/local/persistence'; import { DocumentKeySet } from '../../../src/model/collections'; @@ -25,7 +25,7 @@ import { DocumentKey } from '../../../src/model/document_key'; import { Mutation } from '../../../src/model/mutation'; import { MutationBatch } from '../../../src/model/mutation_batch'; import { SortedMap } from '../../../src/util/sorted_map'; -import { Blob } from '../../../src/api/blob'; +import { ProtoByteString } from '../../../src/util/proto_byte_string'; /** * A wrapper around a MutationQueue that automatically creates a @@ -65,7 +65,7 @@ export class TestMutationQueue { ); } - getLastStreamToken(): Promise { + getLastStreamToken(): Promise { return this.persistence.runTransaction( 'getLastStreamToken', 'readonly-idempotent', @@ -75,7 +75,7 @@ export class TestMutationQueue { ); } - setLastStreamToken(streamToken: Blob): Promise { + setLastStreamToken(streamToken: ProtoByteString): Promise { return this.persistence.runTransaction( 'setLastStreamToken', 'readwrite-primary-idempotent', diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index 37435866f88..c7a64b416b1 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -81,9 +81,10 @@ import { transformMutation, version, wrap, - wrapObject + wrapObject, + byteStringFromString } from '../../../util/helpers'; -import { byteStringFromString } from '../../../../src/util/proto_byte_string'; +import { ProtoByteString } from '../../../../src/util/proto_byte_string'; describe('Serializer', () => { const partition = new DatabaseId('p', 'd'); @@ -1250,7 +1251,7 @@ describe('Serializer', () => { 4, SnapshotVersion.MIN, SnapshotVersion.MIN, - Blob.fromUint8Array(new Uint8Array([1, 2, 3])) + ProtoByteString.fromUint8Array(new Uint8Array([1, 2, 3])) ) ); const expected = { diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 7701cf07101..62d885af65f 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -97,7 +97,8 @@ import { path, setMutation, TestSnapshotVersion, - version + version, + byteStringFromString } from '../../util/helpers'; import { SharedFakeWebStorage, TestPlatform } from '../../util/test_platform'; import { @@ -107,10 +108,7 @@ import { TEST_SERIALIZER } from '../local/persistence_test_helpers'; import { MULTI_CLIENT_TAG } from './describe_spec'; -import { - byteStringFromString, - emptyByteString -} from '../../../src/util/proto_byte_string'; +import { emptyByteString } from '../../../src/util/proto_byte_string'; const ARBITRARY_SEQUENCE_NUMBER = 2; diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index ea29bb3f085..4b161515c94 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -34,7 +34,7 @@ import { OrderBy } from '../../src/core/query'; import { SnapshotVersion } from '../../src/core/snapshot_version'; -import { ProtoByteString, TargetId } from '../../src/core/types'; +import { TargetId } from '../../src/core/types'; import { AddedLimboDocument, LimboDocumentChange, @@ -90,8 +90,9 @@ import { SortedSet } from '../../src/util/sorted_set'; import { query } from './api_helpers'; import { emptyByteString, - byteStringFromString + ProtoByteString } from '../../src/util/proto_byte_string'; +import { PlatformSupport } from '../../src/platform/platform'; export type TestSnapshotVersion = number; @@ -484,6 +485,15 @@ export function localViewChanges( return new LocalViewChanges(targetId, fromCache, addedKeys, removedKeys); } +/** + * Returns a ProtoByteString representation for the platform from the given hexadecimal + * string. + */ +export function byteStringFromString(value: string): ProtoByteString { + const base64 = PlatformSupport.getPlatform().btoa(value); + return ProtoByteString.fromBase64String(base64); +} + /** Creates a resume token to match the given snapshot version. */ export function resumeTokenForSnapshot( snapshotVersion: SnapshotVersion From 43c6d69eeabaca6852cf5127e846b35d1aa62a04 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 18 Feb 2020 17:43:53 -0800 Subject: [PATCH 05/12] update comments to not use blob --- packages/firestore/src/local/indexeddb_mutation_queue.ts | 4 ++-- packages/firestore/src/local/local_serializer.ts | 4 ++-- packages/firestore/src/remote/serializer.ts | 2 +- packages/firestore/test/unit/specs/spec_builder.ts | 6 +++--- packages/firestore/test/unit/specs/spec_test_runner.ts | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 9574725bd4b..f118caf7b10 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -126,7 +126,7 @@ export class IndexedDbMutationQueue implements MutationQueue { ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a Blob type. + // be reconstructed into a ProtoByteString. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); @@ -147,7 +147,7 @@ export class IndexedDbMutationQueue implements MutationQueue { ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a Blob. + // be reconstructed into a ProtoByteString. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 9fb33a32ab5..120ed81649b 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -245,8 +245,8 @@ export class LocalSerializer { queryProto = this.remoteSerializer.toQueryTarget(targetData.target); } - // We can't store the resumeToken as a Blob in IndexedDb, so we convert it - // to a base64 string for storage. + // We can't store the resumeToken as a ProtoByteString in IndexedDb, so we + // convert it to a base64 string for storage. const resumeToken = targetData.resumeToken.toBase64(); // lastListenSequenceNumber is always 0 until we do real GC. diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 00df777c3ac..7f09b2a2edc 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -279,7 +279,7 @@ export class JsonProtoSerializer { } /** - * Returns a Blob based on the proto string value. + * Returns a ProtoByteString based on the proto string value. * DO NOT USE THIS FOR ANYTHING ELSE. * This method cheats. Value is typed as "string" because that's what * our generated proto interfaces say bytes must be, but it is actually diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index fe626eff6f9..cd13e4ac5eb 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -952,20 +952,20 @@ export class SpecBuilder { // `query` is not added yet. this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query), ...activeQueries], - // Convert to base64 string to be compatible with Blob format. + // Convert to base64 string to be compatible with ProtoByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } else { this.activeTargets[targetId] = { queries: activeQueries, - // Convert to base64 string to be compatible with Blob format. + // Convert to base64 string to be compatible with ProtoByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } } else { this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query)], - // Convert to base64 string to be compatible with Blob format. + // Convert to base64 string to be compatible with ProtoByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 62d885af65f..9644dc9a59d 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -198,7 +198,7 @@ class MockConnection implements Connection { ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void { this.writeStream!.callOnMessage({ - // Convert to base64 string to be compatible with Blob format. + // Convert to base64 string to be compatible with ProtoByteString. streamToken: PlatformSupport.getPlatform().btoa( 'write-stream-token-' + this.nextWriteStreamToken ), @@ -832,7 +832,7 @@ abstract class TestRunner { const protoJSON: api.ListenResponse = { targetChange: { readTime: this.serializer.toVersion(version(watchSnapshot.version)), - // Convert to base64 string to be compatible with Blob format. + // Convert to base64 string to be compatible with ProtoByteString. resumeToken: this.platform.btoa(watchSnapshot.resumeToken || ''), targetIds: watchSnapshot.targetIds } From f1e826a927ece1356a9c5ec6c3d8769a80745fa2 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 10:07:41 -0800 Subject: [PATCH 06/12] rename ProtoByteString to ByteString --- .../src/local/indexeddb_mutation_queue.ts | 16 ++++++++-------- .../firestore/src/local/local_serializer.ts | 6 +++--- packages/firestore/src/local/local_store.ts | 6 +++--- .../src/local/memory_mutation_queue.ts | 10 +++++----- packages/firestore/src/local/mutation_queue.ts | 8 ++++---- packages/firestore/src/local/target_data.ts | 6 +++--- packages/firestore/src/model/mutation_batch.ts | 6 +++--- .../firestore/src/remote/persistent_stream.ts | 4 ++-- packages/firestore/src/remote/remote_event.ts | 4 ++-- packages/firestore/src/remote/serializer.ts | 12 ++++++------ packages/firestore/src/remote/watch_change.ts | 10 +++++----- .../firestore/src/util/proto_byte_string.ts | 18 +++++++++--------- .../test/unit/local/test_mutation_queue.ts | 8 ++++---- .../test/unit/remote/node/serializer.test.ts | 4 ++-- .../firestore/test/unit/specs/spec_builder.ts | 6 +++--- .../test/unit/specs/spec_test_runner.ts | 4 ++-- packages/firestore/test/util/helpers.ts | 12 ++++++------ 17 files changed, 70 insertions(+), 70 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index f118caf7b10..aa2fe829f3b 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -48,7 +48,7 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { SimpleDbStore, SimpleDbTransaction } from './simple_db'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** A mutation queue for a specific user, backed by IndexedDB. */ export class IndexedDbMutationQueue implements MutationQueue { @@ -122,11 +122,11 @@ export class IndexedDbMutationQueue implements MutationQueue { acknowledgeBatch( transaction: PersistenceTransaction, batch: MutationBatch, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a ProtoByteString. + // be reconstructed into a ByteString. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); @@ -135,19 +135,19 @@ export class IndexedDbMutationQueue implements MutationQueue { getLastStreamToken( transaction: PersistenceTransaction - ): PersistencePromise { - return this.getMutationQueueMetadata(transaction).next( - metadata => ProtoByteString.fromBase64String(metadata.lastStreamToken) + ): PersistencePromise { + return this.getMutationQueueMetadata(transaction).next( + metadata => ByteString.fromBase64String(metadata.lastStreamToken) ); } setLastStreamToken( transaction: PersistenceTransaction, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a ProtoByteString. + // be reconstructed into a ByteString. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 120ed81649b..4335f1bff02 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -43,7 +43,7 @@ import { DbUnknownDocument } from './indexeddb_schema'; import { TargetData, TargetPurpose } from './target_data'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** Serializer for values stored in the LocalStore. */ export class LocalSerializer { @@ -221,7 +221,7 @@ export class LocalSerializer { dbTarget.lastListenSequenceNumber, version, lastLimboFreeSnapshotVersion, - ProtoByteString.fromBase64String(dbTarget.resumeToken) + ByteString.fromBase64String(dbTarget.resumeToken) ); } @@ -245,7 +245,7 @@ export class LocalSerializer { queryProto = this.remoteSerializer.toQueryTarget(targetData.target); } - // We can't store the resumeToken as a ProtoByteString in IndexedDb, so we + // We can't store the resumeToken as a ByteString in IndexedDb, so we // convert it to a base64 string for storage. const resumeToken = targetData.resumeToken.toBase64(); diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 5db5323ddcf..4e64b243389 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -62,7 +62,7 @@ import { RemoteDocumentCache } from './remote_document_cache'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; import { ClientId } from './shared_client_state'; import { TargetData, TargetPurpose } from './target_data'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; const LOG_TAG = 'LocalStore'; @@ -465,7 +465,7 @@ export class LocalStore { } /** Returns the last recorded stream token for the current user. */ - getLastStreamToken(): Promise { + getLastStreamToken(): Promise { return this.persistence.runTransaction( 'Get last stream token', 'readonly-idempotent', @@ -480,7 +480,7 @@ export class LocalStore { * mutation batch. This is usually only useful after a stream handshake or in * response to an error that requires clearing the stream token. */ - setLastStreamToken(streamToken: ProtoByteString): Promise { + setLastStreamToken(streamToken: ByteString): Promise { return this.persistence.runTransaction( 'Set last stream token', 'readwrite-primary-idempotent', diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index db20051ebb2..0c0969a5857 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -32,7 +32,7 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { DocReference } from './reference_set'; -import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; +import { emptyByteString, ByteString } from '../util/proto_byte_string'; export class MemoryMutationQueue implements MutationQueue { /** @@ -48,7 +48,7 @@ export class MemoryMutationQueue implements MutationQueue { * responses the client has processed. Stream tokens are opaque checkpoint * markers whose only real value is their inclusion in the next request. */ - private lastStreamToken: ProtoByteString = emptyByteString(); + private lastStreamToken: ByteString = emptyByteString(); /** An ordered mapping between documents and the mutations batch IDs. */ private batchesByDocumentKey = new SortedSet(DocReference.compareByKey); @@ -65,7 +65,7 @@ export class MemoryMutationQueue implements MutationQueue { acknowledgeBatch( transaction: PersistenceTransaction, batch: MutationBatch, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise { const batchId = batch.batchId; const batchIndex = this.indexOfExistingBatchId(batchId, 'acknowledged'); @@ -90,13 +90,13 @@ export class MemoryMutationQueue implements MutationQueue { getLastStreamToken( transaction: PersistenceTransaction - ): PersistencePromise { + ): PersistencePromise { return PersistencePromise.resolve(this.lastStreamToken); } setLastStreamToken( transaction: PersistenceTransaction, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise { this.lastStreamToken = streamToken; return PersistencePromise.resolve(); diff --git a/packages/firestore/src/local/mutation_queue.ts b/packages/firestore/src/local/mutation_queue.ts index 42ce1c7254e..23206d4e40a 100644 --- a/packages/firestore/src/local/mutation_queue.ts +++ b/packages/firestore/src/local/mutation_queue.ts @@ -26,7 +26,7 @@ import { SortedMap } from '../util/sorted_map'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** A queue of mutations to apply to the remote store. */ export interface MutationQueue { @@ -39,18 +39,18 @@ export interface MutationQueue { acknowledgeBatch( transaction: PersistenceTransaction, batch: MutationBatch, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise; /** Returns the current stream token for this mutation queue. */ getLastStreamToken( transaction: PersistenceTransaction - ): PersistencePromise; + ): PersistencePromise; /** Sets the stream token for this mutation queue. */ setLastStreamToken( transaction: PersistenceTransaction, - streamToken: ProtoByteString + streamToken: ByteString ): PersistencePromise; /** diff --git a/packages/firestore/src/local/target_data.ts b/packages/firestore/src/local/target_data.ts index db218833e61..28dc669ee43 100644 --- a/packages/firestore/src/local/target_data.ts +++ b/packages/firestore/src/local/target_data.ts @@ -18,7 +18,7 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; import { ListenSequenceNumber, TargetId } from '../core/types'; -import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; +import { emptyByteString, ByteString } from '../util/proto_byte_string'; /** An enumeration of the different purposes we have for targets. */ export enum TargetPurpose { @@ -66,7 +66,7 @@ export class TargetData { * matches the target. The resume token essentially identifies a point in * time from which the server should resume sending results. */ - readonly resumeToken: ProtoByteString = emptyByteString() + readonly resumeToken: ByteString = emptyByteString() ) {} /** Creates a new target data instance with an updated sequence number. */ @@ -87,7 +87,7 @@ export class TargetData { * snapshot version. */ withResumeToken( - resumeToken: ProtoByteString, + resumeToken: ByteString, snapshotVersion: SnapshotVersion ): TargetData { return new TargetData( diff --git a/packages/firestore/src/model/mutation_batch.ts b/packages/firestore/src/model/mutation_batch.ts index 41f8a04e939..b8e7671e23f 100644 --- a/packages/firestore/src/model/mutation_batch.ts +++ b/packages/firestore/src/model/mutation_batch.ts @@ -30,7 +30,7 @@ import { import { MaybeDocument } from './document'; import { DocumentKey } from './document_key'; import { Mutation, MutationResult } from './mutation'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; export const BATCHID_UNKNOWN = -1; @@ -187,7 +187,7 @@ export class MutationBatchResult { readonly batch: MutationBatch, readonly commitVersion: SnapshotVersion, readonly mutationResults: MutationResult[], - readonly streamToken: ProtoByteString, + readonly streamToken: ByteString, /** * A pre-computed mapping from each mutated document to the resulting * version. @@ -204,7 +204,7 @@ export class MutationBatchResult { batch: MutationBatch, commitVersion: SnapshotVersion, results: MutationResult[], - streamToken: ProtoByteString + streamToken: ByteString ): MutationBatchResult { assert( batch.mutations.length === results.length, diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 065ed7027d0..3012500ece2 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -32,7 +32,7 @@ import { ExponentialBackoff } from './backoff'; import { Connection, Stream } from './connection'; import { JsonProtoSerializer } from './serializer'; import { WatchChange } from './watch_change'; -import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; +import { emptyByteString, ByteString } from '../util/proto_byte_string'; const LOG_TAG = 'PersistentStream'; @@ -661,7 +661,7 @@ export class PersistentWriteStream extends PersistentStream< * PersistentWriteStream manages propagating this value from responses to the * next request. */ - lastStreamToken: ProtoByteString = emptyByteString(); + lastStreamToken: ByteString = emptyByteString(); /** * Tracks whether or not a handshake has been successfully exchanged and diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index 6f47415197f..2f470601ee0 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -25,7 +25,7 @@ import { targetIdSet } from '../model/collections'; import { SortedSet } from '../util/sorted_set'; -import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; +import { emptyByteString, ByteString } from '../util/proto_byte_string'; /** * An event from the RemoteStore. It is split into targetChanges (changes to the @@ -101,7 +101,7 @@ export class TargetChange { * query. The resume token essentially identifies a point in time from which * the server should resume sending results. */ - readonly resumeToken: ProtoByteString, + readonly resumeToken: ByteString, /** * The "current" (synced) status of this target. Note that "current" * has special meaning in the RPC protocol that implies that a target is diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 7f09b2a2edc..76d14f5523c 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -71,7 +71,7 @@ import { WatchTargetChange, WatchTargetChangeState } from './watch_change'; -import { ProtoByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; const DIRECTIONS = (() => { const dirs: { [dir: string]: api.OrderDirection } = {}; @@ -268,7 +268,7 @@ export class JsonProtoSerializer { * * Visible for testing. */ - toBytes(bytes: Blob | ProtoByteString): string { + toBytes(bytes: Blob | ByteString): string { if (this.options.useProto3Json) { return bytes.toBase64(); } else { @@ -279,19 +279,19 @@ export class JsonProtoSerializer { } /** - * Returns a ProtoByteString based on the proto string value. + * Returns a ByteString based on the proto string value. * DO NOT USE THIS FOR ANYTHING ELSE. * This method cheats. Value is typed as "string" because that's what * our generated proto interfaces say bytes must be, but it is actually * an Uint8Array in Node. */ - fromBytes(value: string | undefined): ProtoByteString { + fromBytes(value: string | undefined): ByteString { if (this.options.useProto3Json) { - return ProtoByteString.fromBase64String(value ? value : ''); + return ByteString.fromBase64String(value ? value : ''); } else { // The typings say it's a string, but it will actually be a Uint8Array // in Node. Cast to Uint8Array when creating the Blob. - return ProtoByteString.fromUint8Array( + return ByteString.fromUint8Array( value ? ((value as unknown) as Uint8Array) : new Uint8Array() ); } diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index a568910a8d1..e5b5689bca0 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -35,7 +35,7 @@ import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; import { RemoteEvent, TargetChange } from './remote_event'; -import { emptyByteString, ProtoByteString } from '../util/proto_byte_string'; +import { emptyByteString, ByteString } from '../util/proto_byte_string'; /** * Internal representation of the watcher API protocol buffers. @@ -94,7 +94,7 @@ export class WatchTargetChange { * matches the target. The resume token essentially identifies a point in * time from which the server should resume sending results. */ - public resumeToken: ProtoByteString = emptyByteString(), + public resumeToken: ByteString = emptyByteString(), /** An RPC error indicating why the watch failed. */ public cause: FirestoreError | null = null ) {} @@ -120,7 +120,7 @@ class TargetState { > = snapshotChangesMap(); /** See public getters for explanations of these fields. */ - private _resumeToken: ProtoByteString = emptyByteString(); + private _resumeToken: ByteString = emptyByteString(); private _current = false; /** @@ -143,7 +143,7 @@ class TargetState { } /** The last resume token sent to us for this target. */ - get resumeToken(): ProtoByteString { + get resumeToken(): ByteString { return this._resumeToken; } @@ -161,7 +161,7 @@ class TargetState { * Applies the resume token to the TargetChange, but only when it has a new * value. Empty resumeTokens are discarded. */ - updateResumeToken(resumeToken: ProtoByteString): void { + updateResumeToken(resumeToken: ByteString): void { if (resumeToken.approximateByteSize() > 0) { this._hasPendingChanges = true; this._resumeToken = resumeToken; diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 7ebce4ab964..84d8ac6fa95 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -25,21 +25,21 @@ import { PlatformSupport } from '../platform/platform'; * this differentiation by holding the proto byte string in a common class that * must be converted into a string before being sent as a proto. */ -export class ProtoByteString { +export class ByteString { private _binaryString: string; private constructor(binaryString: string) { this._binaryString = binaryString; } - static fromBase64String(base64: string): ProtoByteString { + static fromBase64String(base64: string): ByteString { const binaryString = PlatformSupport.getPlatform().atob(base64); - return new ProtoByteString(binaryString); + return new ByteString(binaryString); } - static fromUint8Array(array: Uint8Array): ProtoByteString { + static fromUint8Array(array: Uint8Array): ByteString { const binaryString = binaryStringFromUint8Array(array); - return new ProtoByteString(binaryString); + return new ByteString(binaryString); } toBase64(): string { @@ -55,7 +55,7 @@ export class ProtoByteString { return this._binaryString.length * 2; } - isEqual(other: ProtoByteString): boolean { + isEqual(other: ByteString): boolean { return this._binaryString === other._binaryString; } } @@ -86,8 +86,8 @@ export function Uint8ArrayFromBinaryString(binaryString: string): Uint8Array { } /** - * Returns an empty ProtoByteString. + * Returns an empty ByteString. */ -export function emptyByteString(): ProtoByteString { - return ProtoByteString.fromBase64String(''); +export function emptyByteString(): ByteString { + return ByteString.fromBase64String(''); } diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index 87bcaa31987..ec9e44ee714 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -25,7 +25,7 @@ import { DocumentKey } from '../../../src/model/document_key'; import { Mutation } from '../../../src/model/mutation'; import { MutationBatch } from '../../../src/model/mutation_batch'; import { SortedMap } from '../../../src/util/sorted_map'; -import { ProtoByteString } from '../../../src/util/proto_byte_string'; +import { ByteString } from '../../../src/util/proto_byte_string'; /** * A wrapper around a MutationQueue that automatically creates a @@ -54,7 +54,7 @@ export class TestMutationQueue { acknowledgeBatch( batch: MutationBatch, - streamToken: ProtoByteString + streamToken: ByteString ): Promise { return this.persistence.runTransaction( 'acknowledgeThroughBatchId', @@ -65,7 +65,7 @@ export class TestMutationQueue { ); } - getLastStreamToken(): Promise { + getLastStreamToken(): Promise { return this.persistence.runTransaction( 'getLastStreamToken', 'readonly-idempotent', @@ -75,7 +75,7 @@ export class TestMutationQueue { ); } - setLastStreamToken(streamToken: ProtoByteString): Promise { + setLastStreamToken(streamToken: ByteString): Promise { return this.persistence.runTransaction( 'setLastStreamToken', 'readwrite-primary-idempotent', diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index c7a64b416b1..7e0ceb09f74 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -84,7 +84,7 @@ import { wrapObject, byteStringFromString } from '../../../util/helpers'; -import { ProtoByteString } from '../../../../src/util/proto_byte_string'; +import { ByteString } from '../../../../src/util/proto_byte_string'; describe('Serializer', () => { const partition = new DatabaseId('p', 'd'); @@ -1251,7 +1251,7 @@ describe('Serializer', () => { 4, SnapshotVersion.MIN, SnapshotVersion.MIN, - ProtoByteString.fromUint8Array(new Uint8Array([1, 2, 3])) + ByteString.fromUint8Array(new Uint8Array([1, 2, 3])) ) ); const expected = { diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index cd13e4ac5eb..5f9fa58526a 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -952,20 +952,20 @@ export class SpecBuilder { // `query` is not added yet. this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query), ...activeQueries], - // Convert to base64 string to be compatible with ProtoByteString. + // Convert to base64 string to be compatible with ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } else { this.activeTargets[targetId] = { queries: activeQueries, - // Convert to base64 string to be compatible with ProtoByteString. + // Convert to base64 string to be compatible with ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } } else { this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query)], - // Convert to base64 string to be compatible with ProtoByteString. + // Convert to base64 string to be compatible with ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 9644dc9a59d..a66c7605c88 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -198,7 +198,7 @@ class MockConnection implements Connection { ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void { this.writeStream!.callOnMessage({ - // Convert to base64 string to be compatible with ProtoByteString. + // Convert to base64 string to be compatible with ByteString. streamToken: PlatformSupport.getPlatform().btoa( 'write-stream-token-' + this.nextWriteStreamToken ), @@ -832,7 +832,7 @@ abstract class TestRunner { const protoJSON: api.ListenResponse = { targetChange: { readTime: this.serializer.toVersion(version(watchSnapshot.version)), - // Convert to base64 string to be compatible with ProtoByteString. + // Convert to base64 string to be compatible with ByteString. resumeToken: this.platform.btoa(watchSnapshot.resumeToken || ''), targetIds: watchSnapshot.targetIds } diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 4b161515c94..afa4f28e3f0 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -90,7 +90,7 @@ import { SortedSet } from '../../src/util/sorted_set'; import { query } from './api_helpers'; import { emptyByteString, - ProtoByteString + ByteString } from '../../src/util/proto_byte_string'; import { PlatformSupport } from '../../src/platform/platform'; @@ -292,7 +292,7 @@ export function targetData( export function noChangeEvent( targetId: number, snapshotVersion: number, - resumeToken: ProtoByteString = emptyByteString() + resumeToken: ByteString = emptyByteString() ): RemoteEvent { const aggregator = new WatchChangeAggregator({ getRemoteKeysForTarget: () => documentKeySet(), @@ -486,18 +486,18 @@ export function localViewChanges( } /** - * Returns a ProtoByteString representation for the platform from the given hexadecimal + * Returns a ByteString representation for the platform from the given hexadecimal * string. */ -export function byteStringFromString(value: string): ProtoByteString { +export function byteStringFromString(value: string): ByteString { const base64 = PlatformSupport.getPlatform().btoa(value); - return ProtoByteString.fromBase64String(base64); + return ByteString.fromBase64String(base64); } /** Creates a resume token to match the given snapshot version. */ export function resumeTokenForSnapshot( snapshotVersion: SnapshotVersion -): ProtoByteString { +): ByteString { if (snapshotVersion.isEqual(SnapshotVersion.MIN)) { return emptyByteString(); } else { From bd9e76f5df69317347781c71765c9e4e0c7df368 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 14:24:28 -0800 Subject: [PATCH 07/12] fix comments --- packages/firestore/src/api/blob.ts | 4 ++-- .../src/local/indexeddb_mutation_queue.ts | 6 +++--- .../firestore/src/local/local_serializer.ts | 2 +- .../src/local/memory_mutation_queue.ts | 4 ++-- packages/firestore/src/local/mutation_queue.ts | 2 +- packages/firestore/src/local/target_data.ts | 4 ++-- packages/firestore/src/model/mutation_batch.ts | 2 +- .../firestore/src/remote/persistent_stream.ts | 4 ++-- packages/firestore/src/remote/remote_event.ts | 4 ++-- packages/firestore/src/remote/remote_store.ts | 8 ++++---- packages/firestore/src/remote/serializer.ts | 18 ++++++++++-------- packages/firestore/src/remote/watch_change.ts | 6 +++--- .../firestore/src/util/proto_byte_string.ts | 15 +++++---------- .../test/unit/local/local_store.test.ts | 6 +++--- .../test/unit/local/mutation_queue.test.ts | 4 ++-- .../test/unit/remote/remote_event.test.ts | 4 ++-- .../test/unit/specs/spec_test_runner.ts | 4 ++-- packages/firestore/test/util/helpers.ts | 12 ++++-------- 18 files changed, 51 insertions(+), 58 deletions(-) diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index f52c33eed6c..b27215b8b9f 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -26,7 +26,7 @@ import { import { primitiveComparator } from '../util/misc'; import { binaryStringFromUint8Array, - Uint8ArrayFromBinaryString + uint8ArrayFromBinaryString } from '../util/proto_byte_string'; /** Helper function to assert Uint8Array is available at runtime. */ @@ -102,7 +102,7 @@ export class Blob { toUint8Array(): Uint8Array { validateExactNumberOfArgs('Blob.toUint8Array', arguments, 0); assertUint8ArrayAvailable(); - const buffer = Uint8ArrayFromBinaryString(this._binaryString); + const buffer = uint8ArrayFromBinaryString(this._binaryString); return buffer; } diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index aa2fe829f3b..54d4a443a9a 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -26,6 +26,7 @@ import { BATCHID_UNKNOWN, MutationBatch } from '../model/mutation_batch'; import { ResourcePath } from '../model/path'; import { assert, fail } from '../util/assert'; import { primitiveComparator } from '../util/misc'; +import { ByteString } from '../util/proto_byte_string'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; @@ -48,7 +49,6 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { SimpleDbStore, SimpleDbTransaction } from './simple_db'; -import { ByteString } from '../util/proto_byte_string'; /** A mutation queue for a specific user, backed by IndexedDB. */ export class IndexedDbMutationQueue implements MutationQueue { @@ -125,8 +125,8 @@ export class IndexedDbMutationQueue implements MutationQueue { streamToken: ByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { - // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a ByteString. + // We can't store the resumeToken as a ByteString in IndexedDB, so we + // convert it to a base64 string for storage. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 4335f1bff02..1f9e9a6b6c4 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -28,6 +28,7 @@ import { MutationBatch } from '../model/mutation_batch'; import * as api from '../protos/firestore_proto_api'; import { JsonProtoSerializer } from '../remote/serializer'; import { assert, fail } from '../util/assert'; +import { ByteString } from '../util/proto_byte_string'; import { documentKeySet, DocumentKeySet } from '../model/collections'; import { Target } from '../core/target'; @@ -43,7 +44,6 @@ import { DbUnknownDocument } from './indexeddb_schema'; import { TargetData, TargetPurpose } from './target_data'; -import { ByteString } from '../util/proto_byte_string'; /** Serializer for values stored in the LocalStore. */ export class LocalSerializer { diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index 0c0969a5857..5f27bea338b 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -24,6 +24,7 @@ import { Mutation } from '../model/mutation'; import { MutationBatch, BATCHID_UNKNOWN } from '../model/mutation_batch'; import { assert } from '../util/assert'; import { primitiveComparator } from '../util/misc'; +import { ByteString } from '../util/proto_byte_string'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; @@ -32,7 +33,6 @@ import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { DocReference } from './reference_set'; -import { emptyByteString, ByteString } from '../util/proto_byte_string'; export class MemoryMutationQueue implements MutationQueue { /** @@ -48,7 +48,7 @@ export class MemoryMutationQueue implements MutationQueue { * responses the client has processed. Stream tokens are opaque checkpoint * markers whose only real value is their inclusion in the next request. */ - private lastStreamToken: ByteString = emptyByteString(); + private lastStreamToken: ByteString = ByteString.EMPTY_BYTE_STRING; /** An ordered mapping between documents and the mutations batch IDs. */ private batchesByDocumentKey = new SortedSet(DocReference.compareByKey); diff --git a/packages/firestore/src/local/mutation_queue.ts b/packages/firestore/src/local/mutation_queue.ts index 23206d4e40a..253f3baf49d 100644 --- a/packages/firestore/src/local/mutation_queue.ts +++ b/packages/firestore/src/local/mutation_queue.ts @@ -22,11 +22,11 @@ import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; import { MutationBatch } from '../model/mutation_batch'; +import { ByteString } from '../util/proto_byte_string'; import { SortedMap } from '../util/sorted_map'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; -import { ByteString } from '../util/proto_byte_string'; /** A queue of mutations to apply to the remote store. */ export interface MutationQueue { diff --git a/packages/firestore/src/local/target_data.ts b/packages/firestore/src/local/target_data.ts index 28dc669ee43..ad210979641 100644 --- a/packages/firestore/src/local/target_data.ts +++ b/packages/firestore/src/local/target_data.ts @@ -18,7 +18,7 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { Target } from '../core/target'; import { ListenSequenceNumber, TargetId } from '../core/types'; -import { emptyByteString, ByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** An enumeration of the different purposes we have for targets. */ export enum TargetPurpose { @@ -66,7 +66,7 @@ export class TargetData { * matches the target. The resume token essentially identifies a point in * time from which the server should resume sending results. */ - readonly resumeToken: ByteString = emptyByteString() + readonly resumeToken: ByteString = ByteString.EMPTY_BYTE_STRING ) {} /** Creates a new target data instance with an updated sequence number. */ diff --git a/packages/firestore/src/model/mutation_batch.ts b/packages/firestore/src/model/mutation_batch.ts index b8e7671e23f..73986f7a3ec 100644 --- a/packages/firestore/src/model/mutation_batch.ts +++ b/packages/firestore/src/model/mutation_batch.ts @@ -20,6 +20,7 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { BatchId } from '../core/types'; import { assert } from '../util/assert'; import * as misc from '../util/misc'; +import { ByteString } from '../util/proto_byte_string'; import { documentKeySet, DocumentKeySet, @@ -30,7 +31,6 @@ import { import { MaybeDocument } from './document'; import { DocumentKey } from './document_key'; import { Mutation, MutationResult } from './mutation'; -import { ByteString } from '../util/proto_byte_string'; export const BATCHID_UNKNOWN = -1; diff --git a/packages/firestore/src/remote/persistent_stream.ts b/packages/firestore/src/remote/persistent_stream.ts index 3012500ece2..9f618e54e90 100644 --- a/packages/firestore/src/remote/persistent_stream.ts +++ b/packages/firestore/src/remote/persistent_stream.ts @@ -32,7 +32,7 @@ import { ExponentialBackoff } from './backoff'; import { Connection, Stream } from './connection'; import { JsonProtoSerializer } from './serializer'; import { WatchChange } from './watch_change'; -import { emptyByteString, ByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; const LOG_TAG = 'PersistentStream'; @@ -661,7 +661,7 @@ export class PersistentWriteStream extends PersistentStream< * PersistentWriteStream manages propagating this value from responses to the * next request. */ - lastStreamToken: ByteString = emptyByteString(); + lastStreamToken: ByteString = ByteString.EMPTY_BYTE_STRING; /** * Tracks whether or not a handshake has been successfully exchanged and diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index 2f470601ee0..0cc38050657 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -25,7 +25,7 @@ import { targetIdSet } from '../model/collections'; import { SortedSet } from '../util/sorted_set'; -import { emptyByteString, ByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** * An event from the RemoteStore. It is split into targetChanges (changes to the @@ -135,7 +135,7 @@ export class TargetChange { current: boolean ): TargetChange { return new TargetChange( - emptyByteString(), + ByteString.EMPTY_BYTE_STRING, current, documentKeySet(), documentKeySet(), diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 771ddfca7d7..e3f5cf544d1 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -51,7 +51,7 @@ import { WatchTargetChange, WatchTargetChangeState } from './watch_change'; -import { emptyByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; const LOG_TAG = 'RemoteStore'; @@ -455,7 +455,7 @@ export class RemoteStore implements TargetMetadataProvider { // Clear the resume token for the target, since we're in a known mismatch // state. this.listenTargets[targetId] = targetData.withResumeToken( - emptyByteString(), + ByteString.EMPTY_BYTE_STRING, targetData.snapshotVersion ); @@ -665,10 +665,10 @@ export class RemoteStore implements TargetMetadataProvider { 'RemoteStore error before completed handshake; resetting stream token: ', this.writeStream.lastStreamToken ); - this.writeStream.lastStreamToken = emptyByteString(); + this.writeStream.lastStreamToken = ByteString.EMPTY_BYTE_STRING; return this.localStore - .setLastStreamToken(emptyByteString()) + .setLastStreamToken(ByteString.EMPTY_BYTE_STRING) .catch(ignoreIfPrimaryLeaseLoss); } else { // Some other error, don't reset stream token. Our stream logic will diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 76d14f5523c..9d3e6b09da6 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -53,6 +53,7 @@ import * as api from '../protos/firestore_proto_api'; import { assert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import * as obj from '../util/obj'; +import { ByteString } from '../util/proto_byte_string'; import * as typeUtils from '../util/types'; import { @@ -71,7 +72,6 @@ import { WatchTargetChange, WatchTargetChangeState } from './watch_change'; -import { ByteString } from '../util/proto_byte_string'; const DIRECTIONS = (() => { const dirs: { [dir: string]: api.OrderDirection } = {}; @@ -272,8 +272,6 @@ export class JsonProtoSerializer { if (this.options.useProto3Json) { return bytes.toBase64(); } else { - // The typings say it's a string, but it needs to be a Uint8Array in Node. - // Cast as string to avoid type check failing. return (bytes.toUint8Array() as unknown) as string; } } @@ -285,15 +283,19 @@ export class JsonProtoSerializer { * our generated proto interfaces say bytes must be, but it is actually * an Uint8Array in Node. */ - fromBytes(value: string | undefined): ByteString { + fromBytes(value: string | Uint8Array | undefined): ByteString { if (this.options.useProto3Json) { + assert( + typeof value === 'string', + 'value must be a string when using proto3 Json ' + ); return ByteString.fromBase64String(value ? value : ''); } else { - // The typings say it's a string, but it will actually be a Uint8Array - // in Node. Cast to Uint8Array when creating the Blob. - return ByteString.fromUint8Array( - value ? ((value as unknown) as Uint8Array) : new Uint8Array() + assert( + value === undefined || value instanceof Uint8Array, + 'value must be undefined or Uint8Array' ); + return ByteString.fromUint8Array(value ? value : new Uint8Array()); } } diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index e5b5689bca0..ecde3f2b589 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -35,7 +35,7 @@ import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; import { RemoteEvent, TargetChange } from './remote_event'; -import { emptyByteString, ByteString } from '../util/proto_byte_string'; +import { ByteString } from '../util/proto_byte_string'; /** * Internal representation of the watcher API protocol buffers. @@ -94,7 +94,7 @@ export class WatchTargetChange { * matches the target. The resume token essentially identifies a point in * time from which the server should resume sending results. */ - public resumeToken: ByteString = emptyByteString(), + public resumeToken: ByteString = ByteString.EMPTY_BYTE_STRING, /** An RPC error indicating why the watch failed. */ public cause: FirestoreError | null = null ) {} @@ -120,7 +120,7 @@ class TargetState { > = snapshotChangesMap(); /** See public getters for explanations of these fields. */ - private _resumeToken: ByteString = emptyByteString(); + private _resumeToken: ByteString = ByteString.EMPTY_BYTE_STRING; private _current = false; /** diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 84d8ac6fa95..6676e58e6e9 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2020 Google LLC. + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,8 @@ import { PlatformSupport } from '../platform/platform'; * must be converted into a string before being sent as a proto. */ export class ByteString { + static EMPTY_BYTE_STRING = ByteString.fromBase64String(''); + private _binaryString: string; private constructor(binaryString: string) { @@ -47,7 +49,7 @@ export class ByteString { } toUint8Array(): Uint8Array { - const buffer = Uint8ArrayFromBinaryString(this._binaryString); + const buffer = uint8ArrayFromBinaryString(this._binaryString); return buffer; } @@ -77,17 +79,10 @@ export function binaryStringFromUint8Array(array: Uint8Array): string { /** * Helper function to convert a binary string to an Uint8Array. */ -export function Uint8ArrayFromBinaryString(binaryString: string): Uint8Array { +export function uint8ArrayFromBinaryString(binaryString: string): Uint8Array { const buffer = new Uint8Array(binaryString.length); for (let i = 0; i < binaryString.length; i++) { buffer[i] = binaryString.charCodeAt(i); } return buffer; } - -/** - * Returns an empty ByteString. - */ -export function emptyByteString(): ByteString { - return ByteString.fromBase64String(''); -} diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 3fd884d78c8..d4ce4913126 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -78,7 +78,7 @@ import { import { FieldValue, IntegerValue } from '../../../src/model/field_value'; import { CountingQueryEngine } from './counting_query_engine'; import * as persistenceHelpers from './persistence_test_helpers'; -import { emptyByteString } from '../../../src/util/proto_byte_string'; +import { ByteString } from '../../../src/util/proto_byte_string'; class LocalStoreTester { private promiseChain: Promise = Promise.resolve(); @@ -176,7 +176,7 @@ class LocalStoreTester { batch, ver, mutationResults, - /*streamToken=*/ emptyByteString() + /*streamToken=*/ ByteString.EMPTY_BYTE_STRING ); return this.localStore.acknowledgeBatch(write); @@ -1175,7 +1175,7 @@ function genericLocalStoreTests( const watchChange2 = new WatchTargetChange( WatchTargetChangeState.Current, [targetId], - emptyByteString() + ByteString.EMPTY_BYTE_STRING ); const aggregator2 = new WatchChangeAggregator({ getRemoteKeysForTarget: () => documentKeySet(), diff --git a/packages/firestore/test/unit/local/mutation_queue.test.ts b/packages/firestore/test/unit/local/mutation_queue.test.ts index 5a6681aed3e..eff81159d47 100644 --- a/packages/firestore/test/unit/local/mutation_queue.test.ts +++ b/packages/firestore/test/unit/local/mutation_queue.test.ts @@ -35,7 +35,7 @@ import { import { addEqualityMatcher } from '../../util/equality_matcher'; import * as persistenceHelpers from './persistence_test_helpers'; import { TestMutationQueue } from './test_mutation_queue'; -import { emptyByteString } from '../../../src/util/proto_byte_string'; +import { ByteString } from '../../../src/util/proto_byte_string'; let persistence: Persistence; let mutationQueue: TestMutationQueue; @@ -154,7 +154,7 @@ function genericMutationQueueTests(): void { const batch1 = await addMutationBatch(); expect(await mutationQueue.countBatches()).to.equal(1); - await mutationQueue.acknowledgeBatch(batch1, emptyByteString()); + await mutationQueue.acknowledgeBatch(batch1, ByteString.EMPTY_BYTE_STRING); await mutationQueue.removeMutationBatch(batch1); expect(await mutationQueue.countBatches()).to.equal(0); diff --git a/packages/firestore/test/unit/remote/remote_event.test.ts b/packages/firestore/test/unit/remote/remote_event.test.ts index 8a78a2ffe02..e207bdb9f2c 100644 --- a/packages/firestore/test/unit/remote/remote_event.test.ts +++ b/packages/firestore/test/unit/remote/remote_event.test.ts @@ -42,7 +42,7 @@ import { updateMapping, version } from '../../util/helpers'; -import { emptyByteString } from '../../../src/util/proto_byte_string'; +import { ByteString } from '../../../src/util/proto_byte_string'; interface TargetMap { [targetId: string]: TargetData; @@ -475,7 +475,7 @@ describe('RemoteEvent', () => { const markCurrent = new WatchTargetChange( WatchTargetChangeState.Current, [1], - emptyByteString() + ByteString.EMPTY_BYTE_STRING ); const aggregator = createAggregator({ diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index a66c7605c88..87956a582f2 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -108,7 +108,7 @@ import { TEST_SERIALIZER } from '../local/persistence_test_helpers'; import { MULTI_CLIENT_TAG } from './describe_spec'; -import { emptyByteString } from '../../../src/util/proto_byte_string'; +import { ByteString } from '../../../src/util/proto_byte_string'; const ARBITRARY_SEQUENCE_NUMBER = 2; @@ -751,7 +751,7 @@ abstract class TestRunner { const change = new WatchTargetChange( WatchTargetChangeState.Removed, removed.targetIds, - emptyByteString(), + ByteString.EMPTY_BYTE_STRING, cause || null ); if (cause) { diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index afa4f28e3f0..5475ef49096 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -88,10 +88,7 @@ import { Dict, forEach } from '../../src/util/obj'; import { SortedMap } from '../../src/util/sorted_map'; import { SortedSet } from '../../src/util/sorted_set'; import { query } from './api_helpers'; -import { - emptyByteString, - ByteString -} from '../../src/util/proto_byte_string'; +import { ByteString } from '../../src/util/proto_byte_string'; import { PlatformSupport } from '../../src/platform/platform'; export type TestSnapshotVersion = number; @@ -292,7 +289,7 @@ export function targetData( export function noChangeEvent( targetId: number, snapshotVersion: number, - resumeToken: ByteString = emptyByteString() + resumeToken: ByteString = ByteString.EMPTY_BYTE_STRING ): RemoteEvent { const aggregator = new WatchChangeAggregator({ getRemoteKeysForTarget: () => documentKeySet(), @@ -486,8 +483,7 @@ export function localViewChanges( } /** - * Returns a ByteString representation for the platform from the given hexadecimal - * string. + * Returns a ByteString representation for the platform from the given string. */ export function byteStringFromString(value: string): ByteString { const base64 = PlatformSupport.getPlatform().btoa(value); @@ -499,7 +495,7 @@ export function resumeTokenForSnapshot( snapshotVersion: SnapshotVersion ): ByteString { if (snapshotVersion.isEqual(SnapshotVersion.MIN)) { - return emptyByteString(); + return ByteString.EMPTY_BYTE_STRING; } else { return byteStringFromString(snapshotVersion.toString()); } From c90413bb865b80a37231eaee4a7de941e4e5f696 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 14:25:55 -0800 Subject: [PATCH 08/12] more efficient Uint8Array conversion --- packages/firestore/src/api/blob.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index 11c8a73816b..4cea49a2a6c 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -85,14 +85,10 @@ export class Blob { if (!(array instanceof Uint8Array)) { throw invalidClassError('Blob.fromUint8Array', 'Uint8Array', 1, array); } - // We can't call array.map directly because it expects the return type to - // be a Uint8Array, whereas we can convert it to a regular array by invoking - // map on the Array prototype. - const binaryString = Array.prototype.map - .call(array, (char: number) => { - return String.fromCharCode(char); - }) - .join(''); + let binaryString = ''; + for (let i = 0; i < array.length; ++i) { + binaryString += String.fromCharCode(array[i]); + } return new Blob(binaryString); } From 164cb45724a49a047cc8072c37621bd62f98e89c Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 16:15:27 -0800 Subject: [PATCH 09/12] move new uint8 conversion over --- packages/firestore/src/util/proto_byte_string.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 6676e58e6e9..3b06f0ffc46 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -66,14 +66,11 @@ export class ByteString { * Helper function to convert an Uint8array to a binary string. */ export function binaryStringFromUint8Array(array: Uint8Array): string { - // We can't call array.map directly because it expects the return type to - // be a Uint8Array, whereas we can convert it to a regular array by invoking - // map on the Array prototype. - return Array.prototype.map - .call(array, (char: number) => { - return String.fromCharCode(char); - }) - .join(''); + let binaryString = ''; + for (let i = 0; i < array.length; ++i) { + binaryString += String.fromCharCode(array[i]); + } + return binaryString; } /** From b57edaedf7ecd7c722f506636081338aef0d76b9 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 16:58:17 -0800 Subject: [PATCH 10/12] no more hexa, add readonly --- packages/firestore/src/util/proto_byte_string.ts | 2 +- packages/firestore/test/unit/specs/spec_test_runner.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 3b06f0ffc46..2d72cdd8f26 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -26,7 +26,7 @@ import { PlatformSupport } from '../platform/platform'; * must be converted into a string before being sent as a proto. */ export class ByteString { - static EMPTY_BYTE_STRING = ByteString.fromBase64String(''); + static readonly EMPTY_BYTE_STRING = new ByteString(''); private _binaryString: string; diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 87956a582f2..8b652b52e4f 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -1151,7 +1151,7 @@ abstract class TestRunner { expect(actualTarget.query).to.deep.equal(expectedTarget.query); expect(actualTarget.targetId).to.equal(expectedTarget.targetId); expect(actualTarget.readTime).to.equal(expectedTarget.readTime); - // actualTarget's resumeToken is a hexadecimal string, but the serialized + // actualTarget's resumeToken is a string, but the serialized // resumeToken will be a base64 string, so we need to convert it back. expect(actualTarget.resumeToken || '').to.equal( this.platform.atob(expectedTarget.resumeToken || '') From 7ae7c3864115ab8bc37bf9a83edf44f521efc0c8 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 19 Feb 2020 17:59:18 -0800 Subject: [PATCH 11/12] add undefined check to fromBytes --- packages/firestore/src/remote/serializer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 9d3e6b09da6..8b975666883 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -286,8 +286,8 @@ export class JsonProtoSerializer { fromBytes(value: string | Uint8Array | undefined): ByteString { if (this.options.useProto3Json) { assert( - typeof value === 'string', - 'value must be a string when using proto3 Json ' + value === undefined || typeof value === 'string', + 'value must be undefined or a string when using proto3 Json' ); return ByteString.fromBase64String(value ? value : ''); } else { From d46d8f1b23fa57fd6d47065617858ef8199ccc53 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Thu, 20 Feb 2020 09:25:44 -0800 Subject: [PATCH 12/12] update comments from review --- .../firestore/src/local/indexeddb_mutation_queue.ts | 6 +++--- packages/firestore/src/remote/serializer.ts | 6 ------ packages/firestore/src/util/proto_byte_string.ts | 13 ++++++------- packages/firestore/test/unit/specs/spec_builder.ts | 6 +++--- .../firestore/test/unit/specs/spec_test_runner.ts | 4 ++-- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 54d4a443a9a..66064c1b242 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -126,7 +126,7 @@ export class IndexedDbMutationQueue implements MutationQueue { ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { // We can't store the resumeToken as a ByteString in IndexedDB, so we - // convert it to a base64 string for storage. + // convert it to a Base64 string for storage. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); @@ -146,8 +146,8 @@ export class IndexedDbMutationQueue implements MutationQueue { streamToken: ByteString ): PersistencePromise { return this.getMutationQueueMetadata(transaction).next(metadata => { - // Convert the streamToken to base64 in order to store it as a string that can - // be reconstructed into a ByteString. + // We can't store the resumeToken as a ByteString in IndexedDB, so we + // convert it to a Base64 string for storage. metadata.lastStreamToken = streamToken.toBase64(); return mutationQueuesStore(transaction).put(metadata); }); diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 8b975666883..9ed0d043059 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -278,10 +278,6 @@ export class JsonProtoSerializer { /** * Returns a ByteString based on the proto string value. - * DO NOT USE THIS FOR ANYTHING ELSE. - * This method cheats. Value is typed as "string" because that's what - * our generated proto interfaces say bytes must be, but it is actually - * an Uint8Array in Node. */ fromBytes(value: string | Uint8Array | undefined): ByteString { if (this.options.useProto3Json) { @@ -730,8 +726,6 @@ export class JsonProtoSerializer { ); const targetIds: TargetId[] = change.targetChange.targetIds || []; - // Depending on the platform, the resumeToken could be a Uint8Array or - // string, even though it is typed as "string". const resumeToken = this.fromBytes(change.targetChange.resumeToken); const causeProto = change.targetChange!.cause; const cause = causeProto && this.fromRpcStatus(causeProto); diff --git a/packages/firestore/src/util/proto_byte_string.ts b/packages/firestore/src/util/proto_byte_string.ts index 2d72cdd8f26..b6c0c3b9c00 100644 --- a/packages/firestore/src/util/proto_byte_string.ts +++ b/packages/firestore/src/util/proto_byte_string.ts @@ -20,15 +20,15 @@ import { PlatformSupport } from '../platform/platform'; /** * Immutable class that represents a "proto" byte string. * - * Proto byte strings can either be base64 strings or Uint8Arrays when sent - * on the wire, but they are always typed to string. This class abstracts away - * this differentiation by holding the proto byte string in a common class that - * must be converted into a string before being sent as a proto. + * Proto byte strings can either be Base64-encoded strings or Uint8Arrays when + * sent on the wire. This class abstracts away this differentiation by holding + * the proto byte string in a common class that must be converted into a string + * before being sent as a proto. */ export class ByteString { static readonly EMPTY_BYTE_STRING = new ByteString(''); - private _binaryString: string; + private readonly _binaryString: string; private constructor(binaryString: string) { this._binaryString = binaryString; @@ -49,8 +49,7 @@ export class ByteString { } toUint8Array(): Uint8Array { - const buffer = uint8ArrayFromBinaryString(this._binaryString); - return buffer; + return uint8ArrayFromBinaryString(this._binaryString); } approximateByteSize(): number { diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 5f9fa58526a..cd026849e85 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -952,20 +952,20 @@ export class SpecBuilder { // `query` is not added yet. this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query), ...activeQueries], - // Convert to base64 string to be compatible with ByteString. + // Convert to base64 string so it can later be parsed into ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } else { this.activeTargets[targetId] = { queries: activeQueries, - // Convert to base64 string to be compatible with ByteString. + // Convert to base64 string so it can later be parsed into ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } } else { this.activeTargets[targetId] = { queries: [SpecBuilder.queryToSpec(query)], - // Convert to base64 string to be compatible with ByteString. + // Convert to base64 string so it can later be parsed into ByteString. resumeToken: PlatformSupport.getPlatform().btoa(resumeToken || '') }; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 8b652b52e4f..7c5affad206 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -198,7 +198,7 @@ class MockConnection implements Connection { ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void { this.writeStream!.callOnMessage({ - // Convert to base64 string to be compatible with ByteString. + // Convert to base64 string so it can later be parsed into ByteString. streamToken: PlatformSupport.getPlatform().btoa( 'write-stream-token-' + this.nextWriteStreamToken ), @@ -832,7 +832,7 @@ abstract class TestRunner { const protoJSON: api.ListenResponse = { targetChange: { readTime: this.serializer.toVersion(version(watchSnapshot.version)), - // Convert to base64 string to be compatible with ByteString. + // Convert to base64 string so it can later be parsed into ByteString. resumeToken: this.platform.btoa(watchSnapshot.resumeToken || ''), targetIds: watchSnapshot.targetIds }