diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index a0db6ea58cd..b9bad2d4cd3 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -70,7 +70,6 @@ import { } from '../util/input_validation'; import { logError, setLogLevel, LogLevel, getLogLevel } from '../util/log'; import { AutoId } from '../util/misc'; -import * as objUtils from '../util/obj'; import { Deferred, Rejecter, Resolver } from '../util/promise'; import { FieldPath as ExternalFieldPath } from './field_path'; @@ -166,7 +165,7 @@ class FirestoreSettings { this.host = settings.host; validateNamedOptionalType('settings', 'boolean', 'ssl', settings.ssl); - this.ssl = objUtils.defaulted(settings.ssl, DEFAULT_SSL); + this.ssl = settings.ssl ?? DEFAULT_SSL; } validateOptionNames('settings', settings, [ 'host', @@ -222,10 +221,8 @@ class FirestoreSettings { Please audit all existing usages of Date when you enable the new behavior.`); } - this.timestampsInSnapshots = objUtils.defaulted( - settings.timestampsInSnapshots, - DEFAULT_TIMESTAMPS_IN_SNAPSHOTS - ); + this.timestampsInSnapshots = + settings.timestampsInSnapshots ?? DEFAULT_TIMESTAMPS_IN_SNAPSHOTS; validateNamedOptionalType( 'settings', @@ -341,9 +338,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { validateExactNumberOfArgs('Firestore.settings', arguments, 1); validateArgType('Firestore.settings', 'object', 1, settingsLiteral); - if ( - objUtils.contains(settingsLiteral as objUtils.Dict<{}>, 'persistence') - ) { + if (contains(settingsLiteral, 'persistence')) { throw new FirestoreError( Code.INVALID_ARGUMENT, '"persistence" is now specified with a separate call to ' + @@ -398,12 +393,10 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { "firestore.enablePersistence() call to use 'synchronizeTabs'." ); } - synchronizeTabs = objUtils.defaulted( - settings.synchronizeTabs !== undefined - ? settings.synchronizeTabs - : settings.experimentalTabSynchronization, - DEFAULT_SYNCHRONIZE_TABS - ); + synchronizeTabs = + settings.synchronizeTabs ?? + settings.experimentalTabSynchronization ?? + DEFAULT_SYNCHRONIZE_TABS; } return this.configureClient(this._persistenceProvider, { @@ -558,15 +551,14 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { } private static databaseIdFromApp(app: FirebaseApp): DatabaseId { - const options = app.options as objUtils.Dict<{}>; - if (!objUtils.contains(options, 'projectId')) { + if (!contains(app.options, 'projectId')) { throw new FirestoreError( Code.INVALID_ARGUMENT, '"projectId" not provided in firebase.initializeApp.' ); } - const projectId = options['projectId']; + const projectId = app.options.projectId; if (!projectId || typeof projectId !== 'string') { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -2628,6 +2620,10 @@ function applyFirestoreDataConverter( return [convertedValue, functionName]; } +function contains(obj: object, key: string): obj is { key : unknown } { + return Object.prototype.hasOwnProperty.call(obj, key); +} + // Export the classes with a private constructor (it will fail if invoked // at runtime). Note that this still allows instanceof checks. diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 71a69549df0..9b0cdcfb6e7 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -45,7 +45,6 @@ import { QueryTargetState, SharedClientStateSyncer } from '../local/shared_client_state_syncer'; -import * as objUtils from '../util/obj'; import { SortedSet } from '../util/sorted_set'; import { ListenSequence } from './listen_sequence'; import { Query, LimitType } from './query'; @@ -147,7 +146,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { private queryViewsByQuery = new ObjectMap(q => q.canonicalId() ); - private queriesByTarget: { [targetId: number]: Query[] } = {}; + private queriesByTarget = new Map(); /** * The keys of documents that are in limbo for which we haven't yet started a * limbo resolution query. @@ -164,9 +163,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { * Keeps track of the information about an active limbo resolution for each * active target ID that was started for the purpose of limbo resolution. */ - private activeLimboResolutionsByTarget: { - [targetId: number]: LimboResolution; - } = {}; + private activeLimboResolutionsByTarget = new Map(); private limboDocumentRefs = new ReferenceSet(); /** Stores user completion handlers, indexed by User and BatchId. */ private mutationUserCallbacks = {} as { @@ -285,10 +282,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { const data = new QueryView(query, targetId, view); this.queryViewsByQuery.set(query, data); - if (!this.queriesByTarget[targetId]) { - this.queriesByTarget[targetId] = []; + if (this.queriesByTarget.has(targetId)) { + this.queriesByTarget.get(targetId)!.push(query); + } else { + this.queriesByTarget.set(targetId, [query]); } - this.queriesByTarget[targetId].push(query); return viewChange.snapshot!; } @@ -322,10 +320,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { // Only clean up the query view and target if this is the only query mapped // to the target. - const queries = this.queriesByTarget[queryView.targetId]; + const queries = this.queriesByTarget.get(queryView.targetId)!; if (queries.length > 1) { - this.queriesByTarget[queryView.targetId] = queries.filter( - q => !q.isEqual(query) + this.queriesByTarget.set( + queryView.targetId, + queries.filter(q => !q.isEqual(query)) ); this.queryViewsByQuery.delete(query); return; @@ -413,10 +412,10 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { try { const changes = await this.localStore.applyRemoteEvent(remoteEvent); // Update `receivedDocument` as appropriate for any limbo targets. - objUtils.forEach(remoteEvent.targetChanges, (targetId, targetChange) => { - const limboResolution = this.activeLimboResolutionsByTarget[ - Number(targetId) - ]; + remoteEvent.targetChanges.forEach((targetChange, targetId) => { + const limboResolution = this.activeLimboResolutionsByTarget.get( + targetId + ); if (limboResolution) { // Since this is a limbo resolution lookup, it's for a single document // and it could be added, modified, or removed, but not a combination. @@ -495,7 +494,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { // PORTING NOTE: Multi-tab only. this.sharedClientState.updateQueryState(targetId, 'rejected', err); - const limboResolution = this.activeLimboResolutionsByTarget[targetId]; + const limboResolution = this.activeLimboResolutionsByTarget.get(targetId); const limboKey = limboResolution && limboResolution.key; if (limboKey) { // Since this query failed, we won't want to manually unlisten to it. @@ -503,7 +502,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.activeLimboTargetsByKey = this.activeLimboTargetsByKey.remove( limboKey ); - delete this.activeLimboResolutionsByTarget[targetId]; + this.activeLimboResolutionsByTarget.delete(targetId); this.pumpEnqueuedLimboResolutions(); // TODO(klimt): We really only should do the following on permission @@ -523,7 +522,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { const resolvedLimboDocuments = documentKeySet().add(limboKey); const event = new RemoteEvent( SnapshotVersion.MIN, - /* targetChanges= */ {}, + /* targetChanges= */ new Map(), /* targetMismatches= */ new SortedSet(primitiveComparator), documentUpdates, resolvedLimboDocuments @@ -720,19 +719,19 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.sharedClientState.removeLocalQueryTarget(targetId); assert( - this.queriesByTarget[targetId] && - this.queriesByTarget[targetId].length !== 0, + this.queriesByTarget.has(targetId) && + this.queriesByTarget.get(targetId)!.length !== 0, `There are no queries mapped to target id ${targetId}` ); - for (const query of this.queriesByTarget[targetId]) { + for (const query of this.queriesByTarget.get(targetId)!) { this.queryViewsByQuery.delete(query); if (error) { this.syncEngineListener!.onWatchError(query, error); } } - delete this.queriesByTarget[targetId]; + this.queriesByTarget.delete(targetId); if (this.isPrimary) { const limboKeys = this.limboDocumentRefs.referencesForId(targetId); @@ -758,7 +757,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.remoteStore.unlisten(limboTargetId); this.activeLimboTargetsByKey = this.activeLimboTargetsByKey.remove(key); - delete this.activeLimboResolutionsByTarget[limboTargetId]; + this.activeLimboResolutionsByTarget.delete(limboTargetId); this.pumpEnqueuedLimboResolutions(); } @@ -810,8 +809,9 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { ) { const key = this.enqueuedLimboResolutions.shift()!; const limboTargetId = this.limboTargetIdGenerator.next(); - this.activeLimboResolutionsByTarget[limboTargetId] = new LimboResolution( - key + this.activeLimboResolutionsByTarget.set( + limboTargetId, + new LimboResolution(key) ); this.activeLimboTargetsByKey = this.activeLimboTargetsByKey.insert( key, @@ -868,7 +868,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { }) .then((viewDocChanges: ViewDocumentChanges) => { const targetChange = - remoteEvent && remoteEvent.targetChanges[queryView.targetId]; + remoteEvent && remoteEvent.targetChanges.get(queryView.targetId); const viewChange = queryView.view.applyChanges( viewDocChanges, /* updateLimboDocuments= */ this.isPrimary === true, @@ -957,7 +957,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { const activeTargets: TargetId[] = []; let p = Promise.resolve(); - objUtils.forEachNumber(this.queriesByTarget, (targetId, _) => { + this.queriesByTarget.forEach((_, targetId) => { if (this.sharedClientState.isLocalQueryTarget(targetId)) { activeTargets.push(targetId); } else { @@ -981,11 +981,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { // PORTING NOTE: Multi-tab only. private resetLimboDocuments(): void { - objUtils.forEachNumber(this.activeLimboResolutionsByTarget, targetId => { + this.activeLimboResolutionsByTarget.forEach((_, targetId) => { this.remoteStore.unlisten(targetId); }); this.limboDocumentRefs.removeAllReferences(); - this.activeLimboResolutionsByTarget = []; + this.activeLimboResolutionsByTarget = new Map(); this.activeLimboTargetsByKey = new SortedMap( DocumentKey.comparator ); @@ -1004,7 +1004,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { const newViewSnapshots: ViewSnapshot[] = []; for (const targetId of targets) { let targetData: TargetData; - const queries = this.queriesByTarget[targetId]; + const queries = this.queriesByTarget.get(targetId); if (queries && queries.length !== 0) { // For queries that have a local View, we need to update their state @@ -1096,7 +1096,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { return; } - if (this.queriesByTarget[targetId]) { + if (this.queriesByTarget.has(targetId)) { switch (state) { case 'current': case 'not-current': { @@ -1136,7 +1136,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { for (const targetId of added) { assert( - !this.queriesByTarget[targetId], + !this.queriesByTarget.has(targetId), 'Trying to add an already active target' ); const target = await this.localStore.getTarget(targetId); @@ -1153,7 +1153,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { for (const targetId of removed) { // Check that the target is still active since the target might have been // removed if it has been rejected by the backend. - if (!this.queriesByTarget[targetId]) { + if (!this.queriesByTarget.has(targetId)) { continue; } @@ -1183,12 +1183,12 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { } getRemoteKeysForTarget(targetId: TargetId): DocumentKeySet { - const limboResolution = this.activeLimboResolutionsByTarget[targetId]; + const limboResolution = this.activeLimboResolutionsByTarget.get(targetId); if (limboResolution && limboResolution.receivedDocument) { return documentKeySet().add(limboResolution.key); } else { let keySet = documentKeySet(); - const queries = this.queriesByTarget[targetId]; + const queries = this.queriesByTarget.get(targetId); if (!queries) { return keySet; } diff --git a/packages/firestore/src/local/encoded_resource_path.ts b/packages/firestore/src/local/encoded_resource_path.ts index 4c9d504d8d1..31ebf89402e 100644 --- a/packages/firestore/src/local/encoded_resource_path.ts +++ b/packages/firestore/src/local/encoded_resource_path.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -72,7 +72,7 @@ const encodedEscape = '\u0011'; /** * Encodes a resource path into a IndexedDb-compatible string form. */ -export function encode(path: ResourcePath): EncodedResourcePath { +export function encodeResourcePath(path: ResourcePath): EncodedResourcePath { let result = ''; for (let i = 0; i < path.length; i++) { if (result.length > 0) { @@ -114,7 +114,7 @@ function encodeSeparator(result: string): string { * decoding resource names from the server; those are One Platform format * strings. */ -export function decode(path: EncodedResourcePath): ResourcePath { +export function decodeResourcePath(path: EncodedResourcePath): ResourcePath { // Event the empty path must encode as a path of at least length 2. A path // with exactly 2 must be the empty path. const length = path.length; diff --git a/packages/firestore/src/local/indexeddb_index_manager.ts b/packages/firestore/src/local/indexeddb_index_manager.ts index 74bcf90d56d..55a5b01a367 100644 --- a/packages/firestore/src/local/indexeddb_index_manager.ts +++ b/packages/firestore/src/local/indexeddb_index_manager.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2019 Google Inc. + * Copyright 2019 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,7 +18,10 @@ import { ResourcePath } from '../model/path'; import { assert } from '../util/assert'; import { immediateSuccessor } from '../util/misc'; -import { decode, encode } from './encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath +} from './encoded_resource_path'; import { IndexManager } from './index_manager'; import { IndexedDbPersistence } from './indexeddb_persistence'; import { DbCollectionParent, DbCollectionParentKey } from './indexeddb_schema'; @@ -64,7 +67,7 @@ export class IndexedDbIndexManager implements IndexManager { const collectionParent: DbCollectionParent = { collectionId, - parent: encode(parentPath) + parent: encodeResourcePath(parentPath) }; return collectionParentsStore(transaction).put(collectionParent); } @@ -93,7 +96,7 @@ export class IndexedDbIndexManager implements IndexManager { if (entry.collectionId !== collectionId) { break; } - parentPaths.push(decode(entry.parent)); + parentPaths.push(decodeResourcePath(entry.parent)); } return parentPaths; }); diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 86a4ed146fb..84db52698a4 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,8 +29,7 @@ import { primitiveComparator } from '../util/misc'; import { ByteString } from '../util/byte_string'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; - -import * as EncodedResourcePath from './encoded_resource_path'; +import { decodeResourcePath } from './encoded_resource_path'; import { IndexManager } from './index_manager'; import { IndexedDbPersistence, @@ -336,7 +335,7 @@ export class IndexedDbMutationQueue implements MutationQueue { // the rows for documentKey will occur before any rows for // documents nested in a subcollection beneath documentKey so we // can stop as soon as we hit any such row. - const path = EncodedResourcePath.decode(encodedPath); + const path = decodeResourcePath(encodedPath); if (userID !== this.userId || !documentKey.path.isEqual(path)) { control.done(); return; @@ -389,7 +388,7 @@ export class IndexedDbMutationQueue implements MutationQueue { // the rows for documentKey will occur before any rows for // documents nested in a subcollection beneath documentKey so we // can stop as soon as we hit any such row. - const path = EncodedResourcePath.decode(encodedPath); + const path = decodeResourcePath(encodedPath); if (userID !== this.userId || !documentKey.path.isEqual(path)) { control.done(); return; @@ -447,7 +446,7 @@ export class IndexedDbMutationQueue implements MutationQueue { return documentMutationsStore(transaction) .iterate({ range: indexStart }, (indexKey, _, control) => { const [userID, encodedPath, batchID] = indexKey; - const path = EncodedResourcePath.decode(encodedPath); + const path = decodeResourcePath(encodedPath); if (userID !== this.userId || !queryPath.isPrefixOf(path)) { control.done(); return; @@ -544,7 +543,7 @@ export class IndexedDbMutationQueue implements MutationQueue { control.done(); return; } else { - const path = EncodedResourcePath.decode(key[1]); + const path = decodeResourcePath(key[1]); danglingMutationReferences.push(path); } }) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index c2215694e2f..246732f67a7 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -28,8 +28,11 @@ import { AsyncQueue, TimerId } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { logDebug, logError } from '../util/log'; import { CancelablePromise } from '../util/promise'; - -import { decode, encode, EncodedResourcePath } from './encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath, + EncodedResourcePath +} from './encoded_resource_path'; import { IndexedDbIndexManager } from './indexeddb_index_manager'; import { IndexedDbMutationQueue, @@ -1244,7 +1247,7 @@ export class IndexedDbLruDelegate implements ReferenceDelegate, LruDelegate { // if nextToReport is valid, report it, this is a new key so the // last one must not be a member of any targets. if (nextToReport !== ListenSequence.INVALID) { - f(new DocumentKey(decode(nextPath)), nextToReport); + f(new DocumentKey(decodeResourcePath(nextPath)), nextToReport); } // set nextToReport to be this sequence number. It's the next one we // might report, if we don't find any targets for this document. @@ -1264,7 +1267,7 @@ export class IndexedDbLruDelegate implements ReferenceDelegate, LruDelegate { // need to check if the last key we iterated over was an orphaned // document and report it. if (nextToReport !== ListenSequence.INVALID) { - f(new DocumentKey(decode(nextPath)), nextToReport); + f(new DocumentKey(decodeResourcePath(nextPath)), nextToReport); } }); } @@ -1275,7 +1278,7 @@ export class IndexedDbLruDelegate implements ReferenceDelegate, LruDelegate { } function sentinelKey(key: DocumentKey): [TargetId, EncodedResourcePath] { - return [0, encode(key.path)]; + return [0, encodeResourcePath(key.path)]; } /** @@ -1286,7 +1289,7 @@ function sentinelRow( key: DocumentKey, sequenceNumber: ListenSequenceNumber ): DbTargetDocument { - return new DbTargetDocument(0, encode(key.path), sequenceNumber); + return new DbTargetDocument(0, encodeResourcePath(key.path), sequenceNumber); } function writeSentinelKey( diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts index b7bd71377e5..26e18770072 100644 --- a/packages/firestore/src/local/indexeddb_schema.ts +++ b/packages/firestore/src/local/indexeddb_schema.ts @@ -22,7 +22,11 @@ import { assert } from '../util/assert'; import { SnapshotVersion } from '../core/snapshot_version'; import { BATCHID_UNKNOWN } from '../model/mutation_batch'; -import { decode, encode, EncodedResourcePath } from './encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath, + EncodedResourcePath +} from './encoded_resource_path'; import { removeMutationBatch } from './indexeddb_mutation_queue'; import { getHighestListenSequenceNumber } from './indexeddb_target_cache'; import { dbDocumentSize } from './indexeddb_remote_document_cache'; @@ -233,7 +237,11 @@ export class SchemaConverter implements SimpleDbSchemaConverter { path: ResourcePath ): PersistencePromise => { return documentTargetStore.put( - new DbTargetDocument(0, encode(path), currentSequenceNumber) + new DbTargetDocument( + 0, + encodeResourcePath(path), + currentSequenceNumber + ) ); }; @@ -280,7 +288,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter { const parentPath = collectionPath.popLast(); return collectionParentsStore.put({ collectionId, - parent: encode(parentPath) + parent: encodeResourcePath(parentPath) }); } }; @@ -299,7 +307,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter { DbDocumentMutation.store ) .iterate({ keysOnly: true }, ([userID, encodedPath, batchId], _) => { - const path = decode(encodedPath); + const path = decodeResourcePath(encodedPath); return addEntry(path.popLast()); }); }); @@ -318,7 +326,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter { } function sentinelKey(path: ResourcePath): DbTargetDocumentKey { - return [0, encode(path)]; + return [0, encodeResourcePath(path)]; } /** @@ -559,7 +567,7 @@ export class DbDocumentMutation { userId: string, path: ResourcePath ): [string, EncodedResourcePath] { - return [userId, encode(path)]; + return [userId, encodeResourcePath(path)]; } /** @@ -571,7 +579,7 @@ export class DbDocumentMutation { path: ResourcePath, batchId: BatchId ): DbDocumentMutationKey { - return [userId, encode(path), batchId]; + return [userId, encodeResourcePath(path), batchId]; } /** diff --git a/packages/firestore/src/local/indexeddb_target_cache.ts b/packages/firestore/src/local/indexeddb_target_cache.ts index edbdc04b275..ecac7133f00 100644 --- a/packages/firestore/src/local/indexeddb_target_cache.ts +++ b/packages/firestore/src/local/indexeddb_target_cache.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,9 +22,11 @@ import { DocumentKeySet, documentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { assert } from '../util/assert'; import { immediateSuccessor } from '../util/misc'; - import { TargetIdGenerator } from '../core/target_id_generator'; -import * as EncodedResourcePath from './encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath +} from './encoded_resource_path'; import { IndexedDbLruDelegate, IndexedDbPersistence, @@ -280,7 +282,7 @@ export class IndexedDbTargetCache implements TargetCache { const promises: Array> = []; const store = documentTargetStore(txn); keys.forEach(key => { - const path = EncodedResourcePath.encode(key.path); + const path = encodeResourcePath(key.path); promises.push(store.put(new DbTargetDocument(targetId, path))); promises.push(this.referenceDelegate.addReference(txn, key)); }); @@ -296,7 +298,7 @@ export class IndexedDbTargetCache implements TargetCache { // IndexedDb. const store = documentTargetStore(txn); return PersistencePromise.forEach(keys, (key: DocumentKey) => { - const path = EncodedResourcePath.encode(key.path); + const path = encodeResourcePath(key.path); return PersistencePromise.waitFor([ store.delete([targetId, path]), this.referenceDelegate.removeReference(txn, key) @@ -333,7 +335,7 @@ export class IndexedDbTargetCache implements TargetCache { return store .iterate({ range, keysOnly: true }, (key, _, control) => { - const path = EncodedResourcePath.decode(key[1]); + const path = decodeResourcePath(key[1]); const docKey = new DocumentKey(path); result = result.add(docKey); }) @@ -344,7 +346,7 @@ export class IndexedDbTargetCache implements TargetCache { txn: PersistenceTransaction, key: DocumentKey ): PersistencePromise { - const path = EncodedResourcePath.encode(key.path); + const path = encodeResourcePath(key.path); const range = IDBKeyRange.bound( [path], [immediateSuccessor(path)], diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index f2901007ccd..3ecb81413da 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -32,7 +32,11 @@ import { ByteString } from '../util/byte_string'; import { documentKeySet, DocumentKeySet } from '../model/collections'; import { Target } from '../core/target'; -import { decode, encode, EncodedResourcePath } from './encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath, + EncodedResourcePath +} from './encoded_resource_path'; import { DbMutationBatch, DbNoDocument, @@ -181,7 +185,7 @@ export class LocalSerializer { const encodedKeys: EncodedResourcePath[] = []; keys.forEach(key => { - encodedKeys.push(encode(key.path)); + encodedKeys.push(encodeResourcePath(key.path)); }); return encodedKeys; @@ -192,7 +196,7 @@ export class LocalSerializer { let keys = documentKeySet(); for (const documentKey of encodedPaths) { - keys = keys.add(new DocumentKey(decode(documentKey))); + keys = keys.add(new DocumentKey(decodeResourcePath(documentKey))); } return keys; diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index e3006b812d0..10ba821799a 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -41,7 +41,6 @@ import { assert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; import { primitiveComparator } from '../util/misc'; -import * as objUtils from '../util/obj'; import { ObjectMap } from '../util/obj_map'; import { SortedMap } from '../util/sorted_map'; @@ -520,56 +519,53 @@ export class LocalStore { newTargetDataByTargetMap = this.targetDataByTarget; const promises = [] as Array>; - objUtils.forEachNumber( - remoteEvent.targetChanges, - (targetId: TargetId, change: TargetChange) => { - const oldTargetData = newTargetDataByTargetMap.get(targetId); - if (!oldTargetData) { - return; - } + remoteEvent.targetChanges.forEach((change, targetId) => { + const oldTargetData = newTargetDataByTargetMap.get(targetId); + if (!oldTargetData) { + return; + } - // Only update the remote keys if the target is still active. This - // ensures that we can persist the updated target data along with - // the updated assignment. - promises.push( - this.targetCache - .removeMatchingKeys(txn, change.removedDocuments, targetId) - .next(() => { - return this.targetCache.addMatchingKeys( - txn, - change.addedDocuments, - targetId - ); - }) + // Only update the remote keys if the target is still active. This + // ensures that we can persist the updated target data along with + // the updated assignment. + promises.push( + this.targetCache + .removeMatchingKeys(txn, change.removedDocuments, targetId) + .next(() => { + return this.targetCache.addMatchingKeys( + txn, + change.addedDocuments, + targetId + ); + }) + ); + + const resumeToken = change.resumeToken; + // Update the resume token if the change includes one. + if (resumeToken.approximateByteSize() > 0) { + const newTargetData = oldTargetData + .withResumeToken(resumeToken, remoteVersion) + .withSequenceNumber(txn.currentSequenceNumber); + newTargetDataByTargetMap = newTargetDataByTargetMap.insert( + targetId, + newTargetData ); - const resumeToken = change.resumeToken; - // Update the resume token if the change includes one. - if (resumeToken.approximateByteSize() > 0) { - const newTargetData = oldTargetData - .withResumeToken(resumeToken, remoteVersion) - .withSequenceNumber(txn.currentSequenceNumber); - newTargetDataByTargetMap = newTargetDataByTargetMap.insert( - targetId, - newTargetData + // Update the target data if there are target changes (or if + // sufficient time has passed since the last update). + if ( + LocalStore.shouldPersistTargetData( + oldTargetData, + newTargetData, + change + ) + ) { + promises.push( + this.targetCache.updateTargetData(txn, newTargetData) ); - - // Update the target data if there are target changes (or if - // sufficient time has passed since the last update). - if ( - LocalStore.shouldPersistTargetData( - oldTargetData, - newTargetData, - change - ) - ) { - promises.push( - this.targetCache.updateTargetData(txn, newTargetData) - ); - } } } - ); + }); let changedDocs = maybeDocumentMap(); let updatedKeys = documentKeySet(); diff --git a/packages/firestore/src/local/memory_persistence.ts b/packages/firestore/src/local/memory_persistence.ts index 95eb7fc788d..442547193dc 100644 --- a/packages/firestore/src/local/memory_persistence.ts +++ b/packages/firestore/src/local/memory_persistence.ts @@ -21,16 +21,14 @@ import { DocumentKey } from '../model/document_key'; import { assert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; -import * as obj from '../util/obj'; import { ObjectMap } from '../util/obj_map'; -import { encode } from './encoded_resource_path'; +import { encodeResourcePath } from './encoded_resource_path'; import { ActiveTargets, LruDelegate, LruGarbageCollector, LruParams } from './lru_garbage_collector'; - import { DatabaseInfo } from '../core/database_info'; import { PersistenceSettings } from '../core/firestore_client'; import { ListenSequence } from '../core/listen_sequence'; @@ -193,9 +191,9 @@ export class MemoryPersistence implements Persistence { key: DocumentKey ): PersistencePromise { return PersistencePromise.or( - obj - .values(this.mutationQueues) - .map(queue => () => queue.containsKey(transaction, key)) + Object.values(this.mutationQueues).map(queue => () => + queue.containsKey(transaction, key) + ) ); } } @@ -331,7 +329,7 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate { private orphanedSequenceNumbers: ObjectMap< DocumentKey, ListenSequenceNumber - > = new ObjectMap(k => encode(k.path)); + > = new ObjectMap(k => encodeResourcePath(k.path)); readonly garbageCollector: LruGarbageCollector; diff --git a/packages/firestore/src/local/shared_client_state.ts b/packages/firestore/src/local/shared_client_state.ts index 5999221d946..643a574ee0e 100644 --- a/packages/firestore/src/local/shared_client_state.ts +++ b/packages/firestore/src/local/shared_client_state.ts @@ -29,8 +29,8 @@ import { Platform } from '../platform/platform'; import { assert } from '../util/assert'; import { AsyncQueue } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; +import { forEach } from '../util/obj'; import { logError, logDebug } from '../util/log'; -import * as objUtils from '../util/obj'; import { SortedSet } from '../util/sorted_set'; import { isSafeInteger } from '../util/types'; import { @@ -612,7 +612,7 @@ export class WebStorageSharedClientState implements SharedClientState { getAllActiveQueryTargets(): TargetIdSet { let activeTargets = targetIdSet(); - objUtils.forEach(this.activeClients, (key, value) => { + forEach(this.activeClients, (key, value) => { activeTargets = activeTargets.unionWith(value.activeTargetIds); }); return activeTargets; diff --git a/packages/firestore/src/model/values.ts b/packages/firestore/src/model/values.ts index 26d0c863779..e95dbee3aa9 100644 --- a/packages/firestore/src/model/values.ts +++ b/packages/firestore/src/model/values.ts @@ -19,7 +19,7 @@ import * as api from '../protos/firestore_proto_api'; import { TypeOrder } from './field_value'; import { assert, fail } from '../util/assert'; -import { forEach, keys, size } from '../util/obj'; +import { forEach, objectSize } from '../util/obj'; import { ByteString } from '../util/byte_string'; import { isNegativeZero } from '../util/types'; import { DocumentKey } from './document_key'; @@ -162,7 +162,7 @@ function objectEquals(left: api.Value, right: api.Value): boolean { const leftMap = left.mapValue!.fields || {}; const rightMap = right.mapValue!.fields || {}; - if (size(leftMap) !== size(rightMap)) { + if (objectSize(leftMap) !== objectSize(rightMap)) { return false; } @@ -320,9 +320,9 @@ function compareArrays(left: api.ArrayValue, right: api.ArrayValue): number { function compareMaps(left: api.MapValue, right: api.MapValue): number { const leftMap = left.fields || {}; - const leftKeys = keys(leftMap); + const leftKeys = Object.keys(leftMap); const rightMap = right.fields || {}; - const rightKeys = keys(rightMap); + const rightKeys = Object.keys(rightMap); // Even though MapValues are likely sorted correctly based on their insertion // order (e.g. when received from the backend), local modifications can bring @@ -401,7 +401,7 @@ function canonifyReference(referenceValue: string): string { function canonifyMap(mapValue: api.MapValue): string { // Iteration order in JavaScript is not guaranteed. To ensure that we generate // matching canonical IDs for identical maps, we need to sort the keys. - const sortedKeys = keys(mapValue.fields || {}).sort(); + const sortedKeys = Object.keys(mapValue.fields || {}).sort(); let result = '{'; let first = true; diff --git a/packages/firestore/src/platform/config.ts b/packages/firestore/src/platform/config.ts index 42f1f60bebe..f8dee256720 100644 --- a/packages/firestore/src/platform/config.ts +++ b/packages/firestore/src/platform/config.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; +import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { Component, ComponentType, Provider } from '@firebase/component'; @@ -37,7 +37,6 @@ import { FieldPath } from '../api/field_path'; import { PublicFieldValue } from '../api/field_value'; import { GeoPoint } from '../api/geo_point'; import { Timestamp } from '../api/timestamp'; -import { shallowCopy } from '../util/obj'; const firestoreNamespace = { Firestore: PublicFirestore, @@ -80,6 +79,6 @@ export function configureForFirebase( return firestoreFactory(app, container.getProvider('auth-internal')); }, ComponentType.PUBLIC - ).setServiceProps(shallowCopy(firestoreNamespace)) + ).setServiceProps({ ...firestoreNamespace }) ); } diff --git a/packages/firestore/src/platform_node/load_protos.ts b/packages/firestore/src/platform_node/load_protos.ts index 70e095b8004..823388e19b2 100644 --- a/packages/firestore/src/platform_node/load_protos.ts +++ b/packages/firestore/src/platform_node/load_protos.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,15 +15,15 @@ * limitations under the License. */ -import * as protoLoader from '@grpc/proto-loader'; -import * as grpc from 'grpc'; -import * as path from 'path'; +import { loadSync } from '@grpc/proto-loader'; +import { loadPackageDefinition, GrpcObject } from 'grpc'; +import { join, resolve, isAbsolute } from 'path'; // only used in tests // eslint-disable-next-line import/no-extraneous-dependencies -import * as ProtobufJS from 'protobufjs'; +import { IConversionOptions, Root } from 'protobufjs'; /** Used by tests so we can match @grpc/proto-loader behavior. */ -export const protoLoaderOptions: ProtobufJS.IConversionOptions = { +export const protoLoaderOptions: IConversionOptions = { longs: String, enums: String, defaults: true, @@ -35,43 +35,37 @@ export const protoLoaderOptions: ProtobufJS.IConversionOptions = { * * @returns The GrpcObject representing our protos. */ -export function loadProtos(): grpc.GrpcObject { - const root = path.resolve( +export function loadProtos(): GrpcObject { + const root = resolve( __dirname, process.env.FIRESTORE_PROTO_ROOT || '../protos' ); - const firestoreProtoFile = path.join( - root, - 'google/firestore/v1/firestore.proto' - ); + const firestoreProtoFile = join(root, 'google/firestore/v1/firestore.proto'); - const packageDefinition = protoLoader.loadSync(firestoreProtoFile, { + const packageDefinition = loadSync(firestoreProtoFile, { ...protoLoaderOptions, includeDirs: [root] }); - return grpc.loadPackageDefinition(packageDefinition); + return loadPackageDefinition(packageDefinition); } /** Used by tests so we can directly create ProtobufJS proto message objects from JSON protos. */ -export function loadRawProtos(): ProtobufJS.Root { - const root = path.resolve( +export function loadRawProtos(): Root { + const root = resolve( __dirname, process.env.FIRESTORE_PROTO_ROOT || '../protos' ); - const firestoreProtoFile = path.join( - root, - 'google/firestore/v1/firestore.proto' - ); + const firestoreProtoFile = join(root, 'google/firestore/v1/firestore.proto'); - const protoRoot = new ProtobufJS.Root(); + const protoRoot = new Root(); // Override the resolvePath function to look for protos in the 'root' // directory. protoRoot.resolvePath = (origin: string, target: string) => { - if (path.isAbsolute(target)) { + if (isAbsolute(target)) { return target; } - return path.join(root, target); + return join(root, target); }; protoRoot.loadSync(firestoreProtoFile); diff --git a/packages/firestore/src/platform_node/node_platform.ts b/packages/firestore/src/platform_node/node_platform.ts index 9fd86703ee6..5b6a30e5400 100644 --- a/packages/firestore/src/platform_node/node_platform.ts +++ b/packages/firestore/src/platform_node/node_platform.ts @@ -16,7 +16,7 @@ */ import { randomBytes } from 'crypto'; -import * as util from 'util'; +import { inspect } from 'util'; import { DatabaseId, DatabaseInfo } from '../core/database_info'; import { Platform } from '../platform/platform'; @@ -58,7 +58,7 @@ export class NodePlatform implements Platform { formatJSON(value: unknown): string { // util.inspect() results in much more readable output than JSON.stringify() - return util.inspect(value, { depth: 100 }); + return inspect(value, { depth: 100 }); } atob(encoded: string): string { diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index d74ee93a7e2..af9ed6eaa06 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,7 +41,7 @@ export class RemoteEvent { /** * A map from target to changes to the target. See TargetChange. */ - readonly targetChanges: { [targetId: number]: TargetChange }, + readonly targetChanges: Map, /** * A set of targets that is known to be inconsistent. Listens for these * targets should be re-established without resume tokens. @@ -69,12 +69,14 @@ export class RemoteEvent { targetId: TargetId, current: boolean ): RemoteEvent { - const targetChanges = { - [targetId]: TargetChange.createSynthesizedTargetChangeForCurrentChange( + const targetChanges = new Map(); + targetChanges.set( + targetId, + TargetChange.createSynthesizedTargetChangeForCurrentChange( targetId, current ) - }; + ); return new RemoteEvent( SnapshotVersion.MIN, targetChanges, diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 916cc0d7e10..f4f64f06dc1 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -29,8 +29,6 @@ import { import { assert } from '../util/assert'; import { FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; -import * as objUtils from '../util/obj'; - import { DocumentKeySet } from '../model/collections'; import { AsyncQueue } from '../util/async_queue'; import { ConnectivityMonitor, NetworkStatus } from './connectivity_monitor'; @@ -106,7 +104,7 @@ export class RemoteStore implements TargetMetadataProvider { * to the server. The targets removed with unlistens are removed eagerly * without waiting for confirmation from the listen stream. */ - private listenTargets: { [targetId: number]: TargetData } = {}; + private listenTargets = new Map(); private connectivityMonitor: ConnectivityMonitor; private watchStream: PersistentListenStream; @@ -242,12 +240,12 @@ export class RemoteStore implements TargetMetadataProvider { * is a no-op if the target of given `TargetData` is already being listened to. */ listen(targetData: TargetData): void { - if (objUtils.contains(this.listenTargets, targetData.targetId)) { + if (this.listenTargets.has(targetData.targetId)) { return; } // Mark this as something the client is currently listening for. - this.listenTargets[targetData.targetId] = targetData; + this.listenTargets.set(targetData.targetId, targetData); if (this.shouldStartWatchStream()) { // The listen will be sent in onWatchStreamOpen @@ -263,16 +261,16 @@ export class RemoteStore implements TargetMetadataProvider { */ unlisten(targetId: TargetId): void { assert( - objUtils.contains(this.listenTargets, targetId), + this.listenTargets.has(targetId), `unlisten called on target no currently watched: ${targetId}` ); - delete this.listenTargets[targetId]; + this.listenTargets.delete(targetId); if (this.watchStream.isOpen()) { this.sendUnwatchRequest(targetId); } - if (objUtils.isEmpty(this.listenTargets)) { + if (this.listenTargets.size === 0) { if (this.watchStream.isOpen()) { this.watchStream.markIdle(); } else if (this.canUseNetwork()) { @@ -286,7 +284,7 @@ export class RemoteStore implements TargetMetadataProvider { /** {@link TargetMetadataProvider.getTargetDataForTarget} */ getTargetDataForTarget(targetId: TargetId): TargetData | null { - return this.listenTargets[targetId] || null; + return this.listenTargets.get(targetId) || null; } /** {@link TargetMetadataProvider.getRemoteKeysForTarget} */ @@ -332,7 +330,7 @@ export class RemoteStore implements TargetMetadataProvider { return ( this.canUseNetwork() && !this.watchStream.isStarted() && - !objUtils.isEmpty(this.listenTargets) + this.listenTargets.size > 0 ); } @@ -345,7 +343,7 @@ export class RemoteStore implements TargetMetadataProvider { } private async onWatchStreamOpen(): Promise { - objUtils.forEachNumber(this.listenTargets, (targetId, targetData) => { + this.listenTargets.forEach((targetData, targetId) => { this.sendWatchRequest(targetData); }); } @@ -430,14 +428,14 @@ 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) => { + remoteEvent.targetChanges.forEach((change, targetId) => { if (change.resumeToken.approximateByteSize() > 0) { - const targetData = this.listenTargets[targetId]; + const targetData = this.listenTargets.get(targetId); // A watched target might have been removed already. if (targetData) { - this.listenTargets[targetId] = targetData.withResumeToken( - change.resumeToken, - snapshotVersion + this.listenTargets.set( + targetId, + targetData.withResumeToken(change.resumeToken, snapshotVersion) ); } } @@ -446,7 +444,7 @@ export class RemoteStore implements TargetMetadataProvider { // Re-establish listens for the targets that have been invalidated by // existence filter mismatches. remoteEvent.targetMismatches.forEach(targetId => { - const targetData = this.listenTargets[targetId]; + const targetData = this.listenTargets.get(targetId); if (!targetData) { // A watched target might have been removed already. return; @@ -454,9 +452,12 @@ 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( - ByteString.EMPTY_BYTE_STRING, - targetData.snapshotVersion + this.listenTargets.set( + targetId, + targetData.withResumeToken( + ByteString.EMPTY_BYTE_STRING, + targetData.snapshotVersion + ) ); // Cause a hard reset by unwatching and rewatching immediately, but @@ -488,8 +489,8 @@ export class RemoteStore implements TargetMetadataProvider { watchChange.targetIds.forEach(targetId => { promiseChain = promiseChain.then(async () => { // A watched target might have been removed already. - if (objUtils.contains(this.listenTargets, targetId)) { - delete this.listenTargets[targetId]; + if (this.listenTargets.has(targetId)) { + this.listenTargets.delete(targetId); this.watchChangeAggregator!.removeTarget(targetId); return this.syncEngine.rejectListen(targetId, error); } diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index c7bf0e7162b..1313853f0de 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -30,7 +30,6 @@ import { assert, fail } from '../util/assert'; import { FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; import { primitiveComparator } from '../util/misc'; -import * as objUtils from '../util/obj'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; @@ -263,7 +262,7 @@ export class WatchChangeAggregator { constructor(private metadataProvider: TargetMetadataProvider) {} /** The internal state of all tracked targets. */ - private targetStates: { [targetId: number]: TargetState } = {}; + private targetStates = new Map(); /** Keeps track of the documents to update since the last raised snapshot. */ private pendingDocumentUpdates = maybeDocumentMap(); @@ -368,7 +367,7 @@ export class WatchChangeAggregator { if (targetChange.targetIds.length > 0) { targetChange.targetIds.forEach(fn); } else { - objUtils.forEachNumber(this.targetStates, fn); + this.targetStates.forEach((_, targetId) => fn(targetId)); } } @@ -421,9 +420,9 @@ export class WatchChangeAggregator { * provided snapshot version. Resets the accumulated changes before returning. */ createRemoteEvent(snapshotVersion: SnapshotVersion): RemoteEvent { - const targetChanges: { [targetId: number]: TargetChange } = {}; + const targetChanges = new Map(); - objUtils.forEachNumber(this.targetStates, (targetId, targetState) => { + this.targetStates.forEach((targetState, targetId) => { const targetData = this.targetDataForActiveTarget(targetId); if (targetData) { if (targetState.current && targetData.target.isDocumentQuery()) { @@ -450,7 +449,7 @@ export class WatchChangeAggregator { } if (targetState.hasPendingChanges) { - targetChanges[targetId] = targetState.toTargetChange(); + targetChanges.set(targetId, targetState.toTargetChange()); targetState.clearPendingChanges(); } } @@ -567,7 +566,7 @@ export class WatchChangeAggregator { } removeTarget(targetId: TargetId): void { - delete this.targetStates[targetId]; + this.targetStates.delete(targetId); } /** @@ -596,11 +595,12 @@ export class WatchChangeAggregator { } private ensureTargetState(targetId: TargetId): TargetState { - if (!this.targetStates[targetId]) { - this.targetStates[targetId] = new TargetState(); + let result = this.targetStates.get(targetId); + if (!result) { + result = new TargetState(); + this.targetStates.set(targetId, result); } - - return this.targetStates[targetId]; + return result; } private ensureDocumentTargetMapping(key: DocumentKey): SortedSet { @@ -635,7 +635,7 @@ export class WatchChangeAggregator { * is still interested in that has no outstanding target change requests). */ protected targetDataForActiveTarget(targetId: TargetId): TargetData | null { - const targetState = this.targetStates[targetId]; + const targetState = this.targetStates.get(targetId); return targetState && targetState.isPending ? null : this.metadataProvider.getTargetDataForTarget(targetId); @@ -648,10 +648,10 @@ export class WatchChangeAggregator { */ private resetTarget(targetId: TargetId): void { assert( - !this.targetStates[targetId].isPending, + !this.targetStates.get(targetId)!.isPending, 'Should only reset active targets' ); - this.targetStates[targetId] = new TargetState(); + this.targetStates.set(targetId, new TargetState()); // Trigger removal for any documents currently mapped to this target. // These removals will be part of the initial snapshot if Watch does not diff --git a/packages/firestore/src/util/input_validation.ts b/packages/firestore/src/util/input_validation.ts index 52dc20d7b7b..1ee0031255b 100644 --- a/packages/firestore/src/util/input_validation.ts +++ b/packages/firestore/src/util/input_validation.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ */ import { fail } from './assert'; import { Code, FirestoreError } from './error'; -import * as obj from './obj'; +import { Dict, forEach } from './obj'; /** Types accepted by validateType() and related methods for validation. */ export type ValidationType = @@ -421,7 +421,7 @@ export function validateOptionNames( options: object, optionNames: string[] ): void { - obj.forEach(options as obj.Dict, (key, _) => { + forEach(options as Dict, (key, _) => { if (optionNames.indexOf(key) < 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, diff --git a/packages/firestore/src/util/obj.ts b/packages/firestore/src/util/obj.ts index 19a11d8c0df..780ef9dc08a 100644 --- a/packages/firestore/src/util/obj.ts +++ b/packages/firestore/src/util/obj.ts @@ -19,18 +19,9 @@ import { assert } from './assert'; export interface Dict { [stringKey: string]: V; - [numberKey: number]: V; } -export function contains(obj: Dict, key: string | number): boolean { - return Object.prototype.hasOwnProperty.call(obj, key); -} - -export function get(obj: Dict, key: string | number): V | null { - return Object.prototype.hasOwnProperty.call(obj, key) ? obj[key] : null; -} - -export function size(obj: object): number { +export function objectSize(obj: object): number { let count = 0; for (const key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { @@ -40,37 +31,6 @@ export function size(obj: object): number { return count; } -/** Returns the given value if it's defined or the defaultValue otherwise. */ -export function defaulted(value: V | undefined, defaultValue: V): V { - return value !== undefined ? value : defaultValue; -} - -export function forEachNumber( - obj: Dict, - fn: (key: number, val: V) => void -): void { - for (const key in obj) { - if (Object.prototype.hasOwnProperty.call(obj, key)) { - const num = Number(key); - if (!isNaN(num)) { - fn(num, obj[key]); - } - } - } -} - -export function values(obj: Dict): V[] { - const vs: V[] = []; - forEach(obj, (_, v) => vs.push(v)); - return vs; -} - -export function keys(obj: Dict): string[] { - const ks: string[] = []; - forEach(obj, k => ks.push(k)); - return ks; -} - export function forEach( obj: Dict, fn: (key: string, val: V) => void @@ -82,17 +42,6 @@ export function forEach( } } -export function lookupOrInsert( - obj: Dict, - key: string | number, - valFn: () => V -): V { - if (!contains(obj, key)) { - obj[key] = valFn(); - } - return obj[key]; -} - export function isEmpty(obj: Dict): boolean { assert( obj != null && typeof obj === 'object', @@ -105,11 +54,3 @@ export function isEmpty(obj: Dict): boolean { } return true; } - -export function shallowCopy(obj: Dict): Dict { - assert( - obj && typeof obj === 'object', - 'shallowCopy() expects object parameter.' - ); - return { ...obj }; -} diff --git a/packages/firestore/src/util/obj_map.ts b/packages/firestore/src/util/obj_map.ts index 7582b6f3505..773eb6e5662 100644 --- a/packages/firestore/src/util/obj_map.ts +++ b/packages/firestore/src/util/obj_map.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ */ import { Equatable } from './misc'; -import * as objUtil from './obj'; +import { forEach, isEmpty } from './obj'; type Entry = [K, V]; @@ -98,7 +98,7 @@ export class ObjectMap, ValueType> { } forEach(fn: (key: KeyType, val: ValueType) => void): void { - objUtil.forEach(this.inner, (_, entries) => { + forEach(this.inner, (_, entries) => { for (const [k, v] of entries) { fn(k, v); } @@ -106,6 +106,6 @@ export class ObjectMap, ValueType> { } isEmpty(): boolean { - return objUtil.isEmpty(this.inner); + return isEmpty(this.inner); } } diff --git a/packages/firestore/test/unit/local/encoded_resource_path.test.ts b/packages/firestore/test/unit/local/encoded_resource_path.test.ts index 9bd7d2eccc3..2d02e7c1e13 100644 --- a/packages/firestore/test/unit/local/encoded_resource_path.test.ts +++ b/packages/firestore/test/unit/local/encoded_resource_path.test.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,11 @@ */ import { expect } from 'chai'; -import * as EncodedResourcePath from '../../../src/local/encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath, + prefixSuccessor +} from '../../../src/local/encoded_resource_path'; import { PersistencePromise } from '../../../src/local/persistence_promise'; import { SimpleDb, @@ -138,15 +142,13 @@ function assertPrefixSuccessorEquals( expected: string, path: ResourcePath ): void { - expect( - EncodedResourcePath.prefixSuccessor(EncodedResourcePath.encode(path)) - ).to.deep.equal(expected); + expect(prefixSuccessor(encodeResourcePath(path))).to.deep.equal(expected); } function assertEncoded(expected: string, path: ResourcePath): Promise { - const encoded = EncodedResourcePath.encode(path); + const encoded = encodeResourcePath(path); expect(encoded).to.deep.equal(expected); - const decoded = EncodedResourcePath.decode(encoded); + const decoded = decodeResourcePath(encoded); expect(decoded.toArray()).to.deep.equal(path.toArray()); let store: SimpleDbStore; @@ -168,7 +170,7 @@ async function assertOrdered(paths: ResourcePath[]): Promise { // Compute the encoded forms of all the given paths const encoded: string[] = []; for (const path of paths) { - encoded.push(EncodedResourcePath.encode(path)); + encoded.push(encodeResourcePath(path)); } // Insert those all into a table, but backwards diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts index 7f7b4a7a684..cad44923885 100644 --- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts +++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts @@ -18,7 +18,10 @@ import { expect } from 'chai'; import { Query } from '../../../src/core/query'; import { SnapshotVersion } from '../../../src/core/snapshot_version'; -import { decode, encode } from '../../../src/local/encoded_resource_path'; +import { + decodeResourcePath, + encodeResourcePath +} from '../../../src/local/encoded_resource_path'; import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence'; import { DbCollectionParent, @@ -597,7 +600,7 @@ describe('IndexedDbSchema: createOrUpgradeDb', () => { targetDocumentStore.put( new DbTargetDocument( 0, - encode(document.key.path), + encodeResourcePath(document.key.path), oldSequenceNumber ) ) @@ -626,7 +629,7 @@ describe('IndexedDbSchema: createOrUpgradeDb', () => { return targetDocumentStore.iterate( { range }, ([_, path], targetDocument) => { - const decoded = decode(path); + const decoded = decodeResourcePath(path); const lastSegment = decoded.lastSegment(); const docNum = +lastSegment.split('_')[1]; const expected = @@ -713,7 +716,7 @@ describe('IndexedDbSchema: createOrUpgradeDb', () => { parents = []; actualParents[collectionId] = parents; } - parents.push(decode(parent).toString()); + parents.push(decodeResourcePath(parent).toString()); } expect(actualParents).to.deep.equal(expectedParents); diff --git a/packages/firestore/test/unit/local/web_storage_shared_client_state.test.ts b/packages/firestore/test/unit/local/web_storage_shared_client_state.test.ts index 80581409dd9..b790fe61bb4 100644 --- a/packages/firestore/test/unit/local/web_storage_shared_client_state.test.ts +++ b/packages/firestore/test/unit/local/web_storage_shared_client_state.test.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2018 Google Inc. + * Copyright 2018 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,7 +41,7 @@ import { PlatformSupport } from '../../../src/platform/platform'; import { AsyncQueue } from '../../../src/util/async_queue'; import { FirestoreError } from '../../../src/util/error'; import { AutoId } from '../../../src/util/misc'; -import * as objUtils from '../../../src/util/obj'; +import { objectSize } from '../../../src/util/obj'; import { SortedSet } from '../../../src/util/sorted_set'; import { clearWebStorage, @@ -103,7 +103,7 @@ class TestSharedClientSyncer implements SharedClientStateSyncer { get sharedClientState(): TestSharedClientState { return { - mutationCount: objUtils.size(this.mutationState), + mutationCount: objectSize(this.mutationState), mutationState: this.mutationState, targetIds: this.activeTargets, targetState: this.queryState, diff --git a/packages/firestore/test/unit/remote/remote_event.test.ts b/packages/firestore/test/unit/remote/remote_event.test.ts index 486bdbeefdd..ea3328379e4 100644 --- a/packages/firestore/test/unit/remote/remote_event.test.ts +++ b/packages/firestore/test/unit/remote/remote_event.test.ts @@ -29,7 +29,6 @@ import { WatchTargetChange, WatchTargetChangeState } from '../../../src/remote/watch_change'; -import * as objUtils from '../../../src/util/obj'; import { deletedDoc, doc, @@ -37,10 +36,10 @@ import { keys, targetData, resumeTokenForSnapshot, - size, updateMapping, version, - key + key, + forEachNumber } from '../../util/helpers'; import { ByteString } from '../../../src/util/byte_string'; @@ -106,7 +105,7 @@ describe('RemoteEvent', () => { const targetIds: TargetId[] = []; if (options.targets) { - objUtils.forEachNumber(options.targets, targetId => { + forEachNumber(options.targets, targetId => { targetIds.push(targetId); }); } @@ -118,14 +117,11 @@ describe('RemoteEvent', () => { }); if (options.outstandingResponses) { - objUtils.forEachNumber( - options.outstandingResponses, - (targetId, count) => { - for (let i = 0; i < count; ++i) { - aggregator.recordPendingTargetRequest(targetId); - } + forEachNumber(options.outstandingResponses, (targetId, count) => { + for (let i = 0; i < count; ++i) { + aggregator.recordPendingTargetRequest(targetId); } - ); + }); } if (options.changes) { @@ -185,25 +181,25 @@ describe('RemoteEvent', () => { expectEqual(event.documentUpdates.get(existingDoc.key), existingDoc); expectEqual(event.documentUpdates.get(newDoc.key), newDoc); - expect(size(event.targetChanges)).to.equal(6); + expect(event.targetChanges.size).to.equal(6); const mapping1 = updateMapping(version(3), [newDoc], [existingDoc], []); - expectTargetChangeEquals(event.targetChanges[1], mapping1); + expectTargetChangeEquals(event.targetChanges.get(1)!, mapping1); const mapping2 = updateMapping(version(3), [], [existingDoc], []); - expectTargetChangeEquals(event.targetChanges[2], mapping2); + expectTargetChangeEquals(event.targetChanges.get(2)!, mapping2); const mapping3 = updateMapping(version(3), [], [existingDoc], []); - expectTargetChangeEquals(event.targetChanges[3], mapping3); + expectTargetChangeEquals(event.targetChanges.get(3)!, mapping3); const mapping4 = updateMapping(version(3), [newDoc], [], [existingDoc]); - expectTargetChangeEquals(event.targetChanges[4], mapping4); + expectTargetChangeEquals(event.targetChanges.get(4)!, mapping4); const mapping5 = updateMapping(version(3), [], [], [existingDoc]); - expectTargetChangeEquals(event.targetChanges[5], mapping5); + expectTargetChangeEquals(event.targetChanges.get(5)!, mapping5); const mapping6 = updateMapping(version(3), [], [], [existingDoc]); - expectTargetChangeEquals(event.targetChanges[6], mapping6); + expectTargetChangeEquals(event.targetChanges.get(6)!, mapping6); }); it('will ignore events for pending targets', () => { @@ -233,7 +229,7 @@ describe('RemoteEvent', () => { expect(event.documentUpdates.size).to.equal(1); expectEqual(event.documentUpdates.get(doc2.key), doc2); // Target 1 is ignored because it was removed - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); }); it('will ignore events for removed targets', () => { @@ -255,7 +251,7 @@ describe('RemoteEvent', () => { // Doc 1 is ignored because it was not apart of an active target. expect(event.documentUpdates.size).to.equal(0); // Target 1 is ignored because it was removed - expect(size(event.targetChanges)).to.equal(0); + expect(event.targetChanges.size).to.equal(0); }); it('will keep reset mapping even with updates', () => { @@ -290,11 +286,11 @@ describe('RemoteEvent', () => { expectEqual(event.documentUpdates.get(doc2.key), doc2); expectEqual(event.documentUpdates.get(doc3.key), doc3); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); // Only doc3 is part of the new mapping. const expected = updateMapping(version(3), [doc3], [], [doc1]); - expectTargetChangeEquals(event.targetChanges[1], expected); + expectTargetChangeEquals(event.targetChanges.get(1)!, expected); }); it('will handle single reset', () => { @@ -312,11 +308,11 @@ describe('RemoteEvent', () => { expectEqual(event.snapshotVersion, version(3)); expect(event.documentUpdates.size).to.equal(0); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); // Reset mapping is empty. const expected = updateMapping(version(3), [], [], []); - expectTargetChangeEquals(event.targetChanges[1], expected); + expectTargetChangeEquals(event.targetChanges.get(1)!, expected); }); it('will handle target add and removal in same batch', () => { @@ -340,13 +336,13 @@ describe('RemoteEvent', () => { expect(event.documentUpdates.size).to.equal(1); expectEqual(event.documentUpdates.get(doc1b.key), doc1b); - expect(size(event.targetChanges)).to.equal(2); + expect(event.targetChanges.size).to.equal(2); const mapping1 = updateMapping(version(3), [], [], [doc1b]); - expectTargetChangeEquals(event.targetChanges[1], mapping1); + expectTargetChangeEquals(event.targetChanges.get(1)!, mapping1); const mapping2 = updateMapping(version(3), [], [doc1b], []); - expectTargetChangeEquals(event.targetChanges[2], mapping2); + expectTargetChangeEquals(event.targetChanges.get(2)!, mapping2); }); it('target current change will mark the target current', () => { @@ -362,10 +358,10 @@ describe('RemoteEvent', () => { expectEqual(event.snapshotVersion, version(3)); expect(event.documentUpdates.size).to.equal(0); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); const mapping = updateMapping(version(3), [], [], [], true); - expectTargetChangeEquals(event.targetChanges[1], mapping); + expectTargetChangeEquals(event.targetChanges.get(1)!, mapping); }); it('target added change will reset previous state', () => { @@ -402,17 +398,17 @@ describe('RemoteEvent', () => { // target 1 and 3 are affected (1 because of re-add), target 2 is not // because of remove. - expect(size(event.targetChanges)).to.equal(2); + expect(event.targetChanges.size).to.equal(2); // doc1 was before the remove, so it does not show up in the mapping. // Current was before the remove. const mapping1 = updateMapping(version(3), [], [doc2], [], false); - expectTargetChangeEquals(event.targetChanges[1], mapping1); + expectTargetChangeEquals(event.targetChanges.get(1)!, mapping1); // Doc1 was before the remove. // Current was before the remove const mapping3 = updateMapping(version(3), [doc1], [], [doc2], true); - expectTargetChangeEquals(event.targetChanges[3], mapping3); + expectTargetChangeEquals(event.targetChanges.get(3)!, mapping3); }); it('no change will still mark the affected targets', () => { @@ -428,9 +424,9 @@ describe('RemoteEvent', () => { expectEqual(event.snapshotVersion, version(3)); expect(event.documentUpdates.size).to.equal(0); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); const expected = updateMapping(version(3), [], [], [], false); - expectTargetChangeEquals(event.targetChanges[1], expected); + expectTargetChangeEquals(event.targetChanges.get(1)!, expected); }); it('existence filters clears target mapping', () => { @@ -450,7 +446,7 @@ describe('RemoteEvent', () => { let event = aggregator.createRemoteEvent(version(3)); expect(event.documentUpdates.size).to.equal(2); - expect(size(event.targetChanges)).to.equal(2); + expect(event.targetChanges.size).to.equal(2); // The existence filter mismatch will remove the document from target 1, // but not synthesize a document delete. @@ -461,10 +457,10 @@ describe('RemoteEvent', () => { event = aggregator.createRemoteEvent(version(3)); expect(event.documentUpdates.size).to.equal(0); expect(event.targetMismatches.size).to.equal(1); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); const expected = updateMapping(SnapshotVersion.MIN, [], [], [doc1], false); - expectTargetChangeEquals(event.targetChanges[1], expected); + expectTargetChangeEquals(event.targetChanges.get(1)!, expected); }); it('existence filters removes current changes', () => { @@ -495,7 +491,7 @@ describe('RemoteEvent', () => { const event = aggregator.createRemoteEvent(version(3)); expect(event.documentUpdates.size).to.equal(1); expect(event.targetMismatches.size).to.equal(1); - expect(event.targetChanges[1].current).to.be.false; + expect(event.targetChanges.get(1)!.current).to.be.false; }); it('handles document update', () => { @@ -538,7 +534,7 @@ describe('RemoteEvent', () => { expectEqual(event.documentUpdates.get(doc3.key), doc3); // Target is unchanged - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); const mapping1 = updateMapping( version(3), @@ -546,7 +542,7 @@ describe('RemoteEvent', () => { [updatedDoc2], [deletedDoc1] ); - expectTargetChangeEquals(event.targetChanges[1], mapping1); + expectTargetChangeEquals(event.targetChanges.get(1)!, mapping1); }); it('only raises events for updated targets', () => { @@ -568,16 +564,16 @@ describe('RemoteEvent', () => { let event = aggregator.createRemoteEvent(version(2)); expect(event.documentUpdates.size).to.equal(2); - expect(size(event.targetChanges)).to.equal(2); + expect(event.targetChanges.size).to.equal(2); aggregator.addDocumentToTarget(2, updatedDoc2); event = aggregator.createRemoteEvent(version(2)); expect(event.documentUpdates.size).to.equal(1); - expect(size(event.targetChanges)).to.equal(1); + expect(event.targetChanges.size).to.equal(1); const mapping1 = updateMapping(version(3), [], [updatedDoc2], []); - expectTargetChangeEquals(event.targetChanges[2], mapping1); + expectTargetChangeEquals(event.targetChanges.get(2)!, mapping1); }); it('synthesizes deletes', () => { @@ -646,7 +642,7 @@ describe('RemoteEvent', () => { changes: [newDocChange, existingDocChange] }); - const updateChange = event.targetChanges[updateTargetId]; + const updateChange = event.targetChanges.get(updateTargetId)!; expect(updateChange.addedDocuments.has(newDoc.key)).to.be.true; expect(updateChange.addedDocuments.has(existingDoc.key)).to.be.false; expect(updateChange.modifiedDocuments.has(newDoc.key)).to.be.false; diff --git a/packages/firestore/test/unit/remote/serializer.test.ts b/packages/firestore/test/unit/remote/serializer.test.ts index a32502935b2..be7e2335f3b 100644 --- a/packages/firestore/test/unit/remote/serializer.test.ts +++ b/packages/firestore/test/unit/remote/serializer.test.ts @@ -56,7 +56,7 @@ import { WatchTargetChangeState } from '../../../src/remote/watch_change'; import { Code, FirestoreError } from '../../../src/util/error'; -import * as obj from '../../../src/util/obj'; +import { forEach } from '../../../src/util/obj'; import { addEqualityMatcher } from '../../util/equality_matcher'; import { bound, @@ -1280,7 +1280,7 @@ describe('Serializer', () => { it('contains all Operators', () => { // giant hack // eslint-disable-next-line @typescript-eslint/no-explicit-any - obj.forEach(Operator as any, (name, op) => { + forEach(Operator as any, (name, op) => { if (op instanceof Operator) { expect(s.toOperatorName(op), 'for name').to.exist; expect(s.fromOperatorName(s.toOperatorName(op))).to.deep.equal(op); @@ -1295,7 +1295,7 @@ describe('Serializer', () => { it('contains all Directions', () => { // giant hack // eslint-disable-next-line @typescript-eslint/no-explicit-any - obj.forEach(Direction as any, (name, dir) => { + forEach(Direction as any, (name, dir) => { if (dir instanceof Direction) { expect(s.toDirection(dir), 'for ' + name).to.exist; expect(s.fromDirection(s.toDirection(dir))).to.deep.equal(dir); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index ba2f6da2603..0715bb18af4 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -34,7 +34,7 @@ import { import { assert, fail } from '../../../src/util/assert'; import { Code } from '../../../src/util/error'; -import * as objUtils from '../../../src/util/obj'; +import { forEach } from '../../../src/util/obj'; import { isNullOrUndefined } from '../../../src/util/types'; import { TestSnapshotVersion, testUserDataWriter } from '../../util/helpers'; @@ -63,8 +63,13 @@ export interface LimboMap { [key: string]: TargetId; } +export interface ActiveTargetSpec { + queries: SpecQuery[]; + resumeToken: string; +} + export interface ActiveTargetMap { - [targetId: string]: { queries: SpecQuery[]; resumeToken: string }; + [targetId: string]: ActiveTargetSpec; } /** @@ -237,7 +242,7 @@ export class SpecBuilder { this.addQueryToActiveTargets(targetId, query, resumeToken); this.currentStep = { userListen: [targetId, SpecBuilder.queryToSpec(query)], - expectedState: { activeTargets: objUtils.shallowCopy(this.activeTargets) } + expectedState: { activeTargets: { ...this.activeTargets } } }; return this; } @@ -257,9 +262,7 @@ export class SpecBuilder { const currentStep = this.currentStep!; currentStep.expectedState = currentStep.expectedState || {}; - currentStep.expectedState.activeTargets = objUtils.shallowCopy( - this.activeTargets - ); + currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } @@ -279,7 +282,7 @@ export class SpecBuilder { this.currentStep = { userUnlisten: [targetId, SpecBuilder.queryToSpec(query)], - expectedState: { activeTargets: objUtils.shallowCopy(this.activeTargets) } + expectedState: { activeTargets: { ...this.activeTargets } } }; return this; } @@ -434,9 +437,7 @@ export class SpecBuilder { this.addQueryToActiveTargets(this.getTargetId(query), query, resumeToken); }); currentStep.expectedState = currentStep.expectedState || {}; - currentStep.expectedState.activeTargets = objUtils.shallowCopy( - this.activeTargets - ); + currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } @@ -450,14 +451,14 @@ export class SpecBuilder { // Clear any preexisting limbo watch targets, which we'll re-create as // necessary from the provided keys below. - objUtils.forEach(this.limboMapping, (key, targetId) => { + forEach(this.limboMapping, (key, targetId) => { delete this.activeTargets[targetId]; }); keys.forEach(key => { const path = key.path.canonicalString(); // Create limbo target ID mapping if it was not in limbo yet - if (!objUtils.contains(this.limboMapping, path)) { + if (!this.limboMapping[path]) { this.limboMapping[path] = this.limboIdGenerator.next(); } // Limbo doc queries are always without resume token @@ -472,9 +473,7 @@ export class SpecBuilder { currentStep.expectedState.activeLimboDocs = keys.map(k => SpecBuilder.keyToSpec(k) ); - currentStep.expectedState.activeTargets = objUtils.shallowCopy( - this.activeTargets - ); + currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } @@ -617,7 +616,7 @@ export class SpecBuilder { if (cause) { delete this.activeTargets[this.getTargetId(query)]; this.currentStep.expectedState = { - activeTargets: objUtils.shallowCopy(this.activeTargets) + activeTargets: { ...this.activeTargets } }; } return this; @@ -807,9 +806,7 @@ export class SpecBuilder { const currentStep = this.currentStep!; currentStep.expectedState = currentStep.expectedState || {}; - currentStep.expectedState.activeTargets = objUtils.shallowCopy( - this.activeTargets - ); + currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } @@ -829,9 +826,7 @@ export class SpecBuilder { const currentStep = this.currentStep!; currentStep.expectedState = currentStep.expectedState || {}; - currentStep.expectedState.activeTargets = objUtils.shallowCopy( - this.activeTargets - ); + currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index b0a45d3afee..9f869cdcb77 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -86,7 +86,7 @@ import { assert, fail } from '../../../src/util/assert'; import { AsyncQueue, TimerId } from '../../../src/util/async_queue'; import { FirestoreError } from '../../../src/util/error'; import { primitiveComparator } from '../../../src/util/misc'; -import * as obj from '../../../src/util/obj'; +import { forEach, objectSize } from '../../../src/util/obj'; import { ObjectMap } from '../../../src/util/obj_map'; import { Deferred, sequence } from '../../../src/util/promise'; import { @@ -116,6 +116,7 @@ import { import { MULTI_CLIENT_TAG } from './describe_spec'; import { ByteString } from '../../../src/util/byte_string'; import { SortedSet } from '../../../src/util/sorted_set'; +import { ActiveTargetMap, ActiveTargetSpec } from './spec_builder'; const ARBITRARY_SEQUENCE_NUMBER = 2; @@ -412,9 +413,7 @@ abstract class TestRunner { private expectedActiveLimboDocs: DocumentKey[]; private expectedEnqueuedLimboDocs: DocumentKey[]; - private expectedActiveTargets: { - [targetId: number]: { queries: SpecQuery[]; resumeToken: string }; - }; + private expectedActiveTargets: Map; private networkEnabled = true; @@ -462,10 +461,9 @@ abstract class TestRunner { this.useGarbageCollection = config.useGarbageCollection; this.numClients = config.numClients; this.maxConcurrentLimboResolutions = config.maxConcurrentLimboResolutions; - this.expectedActiveLimboDocs = []; this.expectedEnqueuedLimboDocs = []; - this.expectedActiveTargets = {}; + this.expectedActiveTargets = new Map(); this.acknowledgedDocs = []; this.rejectedDocs = []; this.snapshotsInSyncListeners = []; @@ -1055,7 +1053,10 @@ abstract class TestRunner { ); } if ('activeTargets' in expectedState) { - this.expectedActiveTargets = expectedState.activeTargets!; + this.expectedActiveTargets.clear(); + forEach(expectedState.activeTargets!, (key, value) => { + this.expectedActiveTargets.set(Number(key), value); + }); } if ('isPrimary' in expectedState) { expect(this.isPrimaryClient).to.eq( @@ -1108,9 +1109,10 @@ abstract class TestRunner { let actualLimboDocs = this.syncEngine.activeLimboDocumentResolutions(); // Validate that each active limbo doc has an expected active target actualLimboDocs.forEach((key, targetId) => { - const targetIds: number[] = []; - obj.forEachNumber(this.expectedActiveTargets, id => targetIds.push(id)); - expect(obj.contains(this.expectedActiveTargets, targetId)).to.equal( + const targetIds = new Array(this.expectedActiveTargets.keys()).map( + n => '' + n + ); + expect(this.expectedActiveTargets.has(targetId)).to.equal( true, `Found limbo doc ${key.toString()}, but its target ID ${targetId} ` + `was not in the set of expected active target IDs ` + @@ -1169,15 +1171,15 @@ abstract class TestRunner { // TODO(mrschmidt): Refactor so this is only executed after primary tab // change - if (!obj.isEmpty(this.expectedActiveTargets)) { + if (this.expectedActiveTargets.size > 0) { await this.connection.waitForWatchOpen(); await this.queue.drain(); } - const actualTargets = obj.shallowCopy(this.connection.activeTargets); - obj.forEachNumber(this.expectedActiveTargets, (targetId, expected) => { - expect(obj.contains(actualTargets, targetId)).to.equal( - true, + const actualTargets = { ...this.connection.activeTargets }; + this.expectedActiveTargets.forEach((expected, targetId) => { + expect(actualTargets[targetId]).to.not.equal( + undefined, 'Expected active target not found: ' + JSON.stringify(expected) ); const actualTarget = actualTargets[targetId]; @@ -1208,7 +1210,7 @@ abstract class TestRunner { ); delete actualTargets[targetId]; }); - expect(obj.size(actualTargets)).to.equal( + expect(objectSize(actualTargets)).to.equal( 0, 'Unexpected active targets: ' + JSON.stringify(actualTargets) ); @@ -1706,9 +1708,7 @@ export interface StateExpectation { /** * Current expected active targets. Verified in each step until overwritten. */ - activeTargets?: { - [targetId: number]: { queries: SpecQuery[]; resumeToken: string }; - }; + activeTargets?: ActiveTargetMap; /** * Expected set of callbacks for previously written docs. */ diff --git a/packages/firestore/test/unit/util/sorted_map.test.ts b/packages/firestore/test/unit/util/sorted_map.test.ts index 959f72b3ad9..6a3c653829c 100644 --- a/packages/firestore/test/unit/util/sorted_map.test.ts +++ b/packages/firestore/test/unit/util/sorted_map.test.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google Inc. + * Copyright 2017 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,7 @@ import { expect } from 'chai'; import { primitiveComparator } from '../../../src/util/misc'; -import * as obj from '../../../src/util/obj'; +import { forEach } from '../../../src/util/obj'; import { LLRBNode, SortedMap } from '../../../src/util/sorted_map'; function shuffle(arr: number[]): void { @@ -363,7 +363,7 @@ describe('SortedMap Tests', () => { max: number ): void => { const keys: number[] = []; - obj.forEach(tree, k => keys.push(Number(k))); + forEach(tree, k => keys.push(Number(k))); keys.sort(); expect(keys.length).to.equal(max); diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 16ae40d2569..be0a7e661ec 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -84,7 +84,7 @@ import { } from '../../src/remote/watch_change'; import { assert, fail } from '../../src/util/assert'; import { primitiveComparator } from '../../src/util/misc'; -import { Dict, forEach } from '../../src/util/obj'; +import { Dict } from '../../src/util/obj'; import { SortedMap } from '../../src/util/sorted_map'; import { SortedSet } from '../../src/util/sorted_set'; import { query } from './api_helpers'; @@ -512,7 +512,7 @@ export function byteStringFromString(value: string): ByteString { return ByteString.fromBase64String(base64); } -/** +/** * Decodes a base 64 decoded string. * * Note that this is typed to accept Uint8Arrays to match the types used @@ -812,13 +812,20 @@ export function expectEqualitySets( } } -/** Returns the number of keys in this object. */ -export function size(obj: JsonObject): number { - let c = 0; - forEach(obj, () => c++); - return c; -} - export function expectFirestoreError(err: Error): void { expect(err.name).to.equal('FirebaseError'); } + +export function forEachNumber( + obj: Dict, + fn: (key: number, val: V) => void +): void { + for (const key in obj) { + if (Object.prototype.hasOwnProperty.call(obj, key)) { + const num = Number(key); + if (!isNaN(num)) { + fn(num, obj[key]); + } + } + } +}