From d518f80a713863fb1656a1298ca47982be88ca8f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 10 Dec 2020 10:41:22 -0700 Subject: [PATCH 1/3] Prefix all LocalStore functions with localStore --- packages/firestore/src/core/bundle_impl.ts | 12 +- .../firestore/src/core/component_provider.ts | 4 +- .../firestore/src/core/firestore_client.ts | 19 +- packages/firestore/src/core/sync_engine.ts | 128 +++++++++----- packages/firestore/src/local/local_store.ts | 1 + .../firestore/src/local/local_store_impl.ts | 56 +++--- packages/firestore/src/remote/remote_store.ts | 11 +- .../test/unit/local/local_store.test.ts | 167 ++++++++++-------- 8 files changed, 231 insertions(+), 167 deletions(-) diff --git a/packages/firestore/src/core/bundle_impl.ts b/packages/firestore/src/core/bundle_impl.ts index 041f11cf9b9..afb30758080 100644 --- a/packages/firestore/src/core/bundle_impl.ts +++ b/packages/firestore/src/core/bundle_impl.ts @@ -19,8 +19,8 @@ import { LoadBundleTaskProgress } from '@firebase/firestore-types'; import { LocalStore } from '../local/local_store'; import { - applyBundleDocuments, - saveNamedQuery + localStoreApplyBundledDocuments, + localStoreSaveNamedQuery } from '../local/local_store_impl'; import { documentKeySet, DocumentKeySet } from '../model/collections'; import { MaybeDocument, NoDocument } from '../model/document'; @@ -173,7 +173,7 @@ export class BundleLoader { ); debugAssert(!!this.bundleMetadata.id, 'Bundle ID must be set.'); - const changedDocuments = await applyBundleDocuments( + const changedDocuments = await localStoreApplyBundledDocuments( this.localStore, new BundleConverterImpl(this.serializer), this.documents, @@ -183,7 +183,11 @@ export class BundleLoader { const queryDocumentMap = this.getQueryDocumentMapping(this.documents); for (const q of this.queries) { - await saveNamedQuery(this.localStore, q, queryDocumentMap.get(q.name!)); + await localStoreSaveNamedQuery( + this.localStore, + q, + queryDocumentMap.get(q.name!) + ); } this.progress.taskState = 'Success'; diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index d7905cccf61..36461957cba 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -24,7 +24,7 @@ import { import { LocalStore } from '../local/local_store'; import { newLocalStore, - synchronizeLastDocumentChangeReadTime + localStoreSynchronizeLastDocumentChangeReadTime } from '../local/local_store_impl'; import { LruParams } from '../local/lru_garbage_collector'; import { LruScheduler } from '../local/lru_garbage_collector_impl'; @@ -172,7 +172,7 @@ export class IndexedDbOfflineComponentProvider extends MemoryOfflineComponentPro async initialize(cfg: ComponentConfiguration): Promise { await super.initialize(cfg); - await synchronizeLastDocumentChangeReadTime(this.localStore); + await localStoreSynchronizeLastDocumentChangeReadTime(this.localStore); await this.onlineComponentProvider.initialize(this, cfg); diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 2445b351113..bcb1cb59a39 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -25,10 +25,10 @@ import { import { User } from '../auth/user'; import { LocalStore } from '../local/local_store'; import { - executeQuery, - getNamedQuery, - handleUserChange, - readLocalDocument + localStoreExecuteQuery, + localStoreGetNamedQuery, + localStoreHandleUserChange, + localStoreReadDocument } from '../local/local_store_impl'; import { Persistence } from '../local/persistence'; import { Document, NoDocument } from '../model/document'; @@ -201,7 +201,10 @@ export async function setOfflineComponentProvider( client.setCredentialChangeListener(user => client.asyncQueue.enqueueRetryable(async () => { - await handleUserChange(offlineComponentProvider.localStore, user); + await localStoreHandleUserChange( + offlineComponentProvider.localStore, + user + ); }) ); @@ -493,7 +496,7 @@ async function readDocumentFromCache( result: Deferred ): Promise { try { - const maybeDoc = await readLocalDocument(localStore, docKey); + const maybeDoc = await localStoreReadDocument(localStore, docKey); if (maybeDoc instanceof Document) { result.resolve(maybeDoc); } else if (maybeDoc instanceof NoDocument) { @@ -595,7 +598,7 @@ async function executeQueryFromCache( result: Deferred ): Promise { try { - const queryResult = await executeQuery( + const queryResult = await localStoreExecuteQuery( localStore, query, /* usePreviousResults= */ true @@ -676,7 +679,7 @@ export function firestoreClientGetNamedQuery( queryName: string ): Promise { return client.asyncQueue.enqueue(async () => - getNamedQuery(await getLocalStore(client), queryName) + localStoreGetNamedQuery(await getLocalStore(client), queryName) ); } diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 4f4cd351cac..d96433b3d78 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -19,23 +19,23 @@ import { LoadBundleTask } from '../api/bundle'; import { User } from '../auth/user'; import { ignoreIfPrimaryLeaseLoss, LocalStore } from '../local/local_store'; import { - acknowledgeBatch, - allocateTarget, - applyRemoteEventToLocalCache, - executeQuery, - getActiveClientsFromPersistence, - getCachedTarget, - getHighestUnacknowledgedBatchId, - getNewDocumentChanges, - handleUserChange, - hasNewerBundle, - localWrite, - lookupMutationDocuments, - notifyLocalViewChanges, - rejectBatch, - releaseTarget, - removeCachedMutationBatchMetadata, - saveBundle + localStoreAcknowledgeBatch, + localStoreAllocateTarget, + localStoreApplyRemoteEventToLocalCache, + localStoreExecuteQuery, + localStoreGetActiveClients, + localStoreGetCachedTarget, + localStoreGetHighestUnacknowledgedBatchId, + localStoreGetNewDocumentChanges, + localStoreHandleUserChange, + localStoreHasNewerBundle, + localStoreWriteLocally, + localStoreLookupMutationDocuments, + localStoreNotifyLocalViewChanges, + localStoreRejectBatch, + localStoreReleaseTarget, + localStoreRemoveCachedMutationBatchMetadata, + localStoreSaveBundle } from '../local/local_store_impl'; import { LocalViewChanges } from '../local/local_view_changes'; import { ReferenceSet } from '../local/reference_set'; @@ -325,7 +325,7 @@ export async function syncEngineListen( syncEngineImpl.sharedClientState.addLocalQueryTarget(targetId); viewSnapshot = queryView.view.computeInitialSnapshot(); } else { - const targetData = await allocateTarget( + const targetData = await localStoreAllocateTarget( syncEngineImpl.localStore, queryToTarget(query) ); @@ -364,7 +364,7 @@ async function initializeViewAndComputeSnapshot( syncEngineImpl.applyDocChanges = (queryView, changes, remoteEvent) => applyDocChanges(syncEngineImpl, queryView, changes, remoteEvent); - const queryResult = await executeQuery( + const queryResult = await localStoreExecuteQuery( syncEngineImpl.localStore, query, /* usePreviousResults= */ true @@ -433,7 +433,7 @@ export async function syncEngineUnlisten( ); if (!targetRemainsActive) { - await releaseTarget( + await localStoreReleaseTarget( syncEngineImpl.localStore, queryView.targetId, /*keepPersistedTargetData=*/ false @@ -447,7 +447,7 @@ export async function syncEngineUnlisten( } } else { removeAndCleanupTarget(syncEngineImpl, queryView.targetId); - await releaseTarget( + await localStoreReleaseTarget( syncEngineImpl.localStore, queryView.targetId, /*keepPersistedTargetData=*/ true @@ -473,7 +473,10 @@ export async function syncEngineWrite( const syncEngineImpl = ensureWriteCallbacks(syncEngine); try { - const result = await localWrite(syncEngineImpl.localStore, batch); + const result = await localStoreWriteLocally( + syncEngineImpl.localStore, + batch + ); syncEngineImpl.sharedClientState.addPendingMutation(result.batchId); addMutationCallback(syncEngineImpl, result.batchId, userCallback); await emitNewSnapsAndNotifyLocalStore(syncEngineImpl, result.changes); @@ -498,7 +501,7 @@ export async function applyRemoteEvent( const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); try { - const changes = await applyRemoteEventToLocalCache( + const changes = await localStoreApplyRemoteEventToLocalCache( syncEngineImpl.localStore, remoteEvent ); @@ -652,7 +655,7 @@ export async function rejectListen( syncEngineImpl.activeLimboResolutionsByTarget.delete(targetId); pumpEnqueuedLimboResolutions(syncEngineImpl); } else { - await releaseTarget( + await localStoreReleaseTarget( syncEngineImpl.localStore, targetId, /* keepPersistedTargetData */ false @@ -670,7 +673,7 @@ export async function applySuccessfulWrite( const batchId = mutationBatchResult.batch.batchId; try { - const changes = await acknowledgeBatch( + const changes = await localStoreAcknowledgeBatch( syncEngineImpl.localStore, mutationBatchResult ); @@ -700,7 +703,10 @@ export async function rejectFailedWrite( const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); try { - const changes = await rejectBatch(syncEngineImpl.localStore, batchId); + const changes = await localStoreRejectBatch( + syncEngineImpl.localStore, + batchId + ); // The local store may or may not be able to apply the write result and // raise events immediately (depending on whether the watcher is caught up), @@ -738,7 +744,7 @@ export async function registerPendingWritesCallback( } try { - const highestBatchId = await getHighestUnacknowledgedBatchId( + const highestBatchId = await localStoreGetHighestUnacknowledgedBatchId( syncEngineImpl.localStore ); if (highestBatchId === BATCHID_UNKNOWN) { @@ -1036,7 +1042,10 @@ export async function emitNewSnapsAndNotifyLocalStore( await Promise.all(queriesProcessed); syncEngineImpl.syncEngineListener.onWatchChange!(newSnaps); - await notifyLocalViewChanges(syncEngineImpl.localStore, docChangesInAllViews); + await localStoreNotifyLocalViewChanges( + syncEngineImpl.localStore, + docChangesInAllViews + ); } async function applyDocChanges( @@ -1050,7 +1059,7 @@ async function applyDocChanges( // The query has a limit and some docs were removed, so we need // to re-run the query against the local store to make sure we // didn't lose any good docs that had been past the limit. - viewDocChanges = await executeQuery( + viewDocChanges = await localStoreExecuteQuery( syncEngineImpl.localStore, queryView.query, /* usePreviousResults= */ false @@ -1084,7 +1093,10 @@ export async function syncEngineHandleCredentialChange( if (userChanged) { logDebug(LOG_TAG, 'User change. New user:', user.toKey()); - const result = await handleUserChange(syncEngineImpl.localStore, user); + const result = await localStoreHandleUserChange( + syncEngineImpl.localStore, + user + ); syncEngineImpl.currentUser = user; // Fails tasks waiting for pending writes requested by previous user. @@ -1142,7 +1154,7 @@ async function synchronizeViewAndComputeSnapshot( queryView: QueryView ): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); - const queryResult = await executeQuery( + const queryResult = await localStoreExecuteQuery( syncEngineImpl.localStore, queryView.query, /* usePreviousResults= */ true @@ -1170,9 +1182,9 @@ export async function synchronizeWithChangedDocuments( ): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); - return getNewDocumentChanges(syncEngineImpl.localStore).then(changes => - emitNewSnapsAndNotifyLocalStore(syncEngineImpl, changes) - ); + return localStoreGetNewDocumentChanges( + syncEngineImpl.localStore + ).then(changes => emitNewSnapsAndNotifyLocalStore(syncEngineImpl, changes)); } /** Applies a mutation state to an existing batch. */ @@ -1184,7 +1196,7 @@ export async function applyBatchState( error?: FirestoreError ): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); - const documents = await lookupMutationDocuments( + const documents = await localStoreLookupMutationDocuments( syncEngineImpl.localStore, batchId ); @@ -1211,7 +1223,10 @@ export async function applyBatchState( // other clients. processUserCallback(syncEngineImpl, batchId, error ? error : null); triggerPendingWritesCallbacks(syncEngineImpl, batchId); - removeCachedMutationBatchMetadata(syncEngineImpl.localStore, batchId); + localStoreRemoveCachedMutationBatchMetadata( + syncEngineImpl.localStore, + batchId + ); } else { fail(`Unknown batchState: ${batchState}`); } @@ -1256,7 +1271,7 @@ export async function applyPrimaryState( } else { p = p.then(() => { removeAndCleanupTarget(syncEngineImpl, targetId); - return releaseTarget( + return localStoreReleaseTarget( syncEngineImpl.localStore, targetId, /*keepPersistedTargetData=*/ true @@ -1322,7 +1337,7 @@ async function synchronizeQueryViewsAndRaiseSnapshots( // from LocalStore (as the resume token and the snapshot version // might have changed) and reconcile their views with the persisted // state (the list of syncedDocuments may have gotten out of sync). - targetData = await allocateTarget( + targetData = await localStoreAllocateTarget( syncEngineImpl.localStore, queryToTarget(queries[0]) ); @@ -1349,9 +1364,15 @@ async function synchronizeQueryViewsAndRaiseSnapshots( ); // For queries that never executed on this client, we need to // allocate the target in LocalStore and initialize a new View. - const target = await getCachedTarget(syncEngineImpl.localStore, targetId); + const target = await localStoreGetCachedTarget( + syncEngineImpl.localStore, + targetId + ); debugAssert(!!target, `Target for id ${targetId} not found`); - targetData = await allocateTarget(syncEngineImpl.localStore, target); + targetData = await localStoreAllocateTarget( + syncEngineImpl.localStore, + target + ); await initializeViewAndComputeSnapshot( syncEngineImpl, synthesizeTargetToQuery(target!), @@ -1395,7 +1416,7 @@ function synthesizeTargetToQuery(target: Target): Query { // PORTING NOTE: Multi-Tab only. export function getActiveClients(syncEngine: SyncEngine): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); - return getActiveClientsFromPersistence(syncEngineImpl.localStore); + return localStoreGetActiveClients(syncEngineImpl.localStore); } /** Applies a query target change from a different tab. */ @@ -1418,7 +1439,9 @@ export async function applyTargetState( switch (state) { case 'current': case 'not-current': { - const changes = await getNewDocumentChanges(syncEngineImpl.localStore); + const changes = await localStoreGetNewDocumentChanges( + syncEngineImpl.localStore + ); const synthesizedRemoteEvent = RemoteEvent.createSynthesizedRemoteEventForCurrentChange( targetId, state === 'current' @@ -1431,7 +1454,7 @@ export async function applyTargetState( break; } case 'rejected': { - await releaseTarget( + await localStoreReleaseTarget( syncEngineImpl.localStore, targetId, /* keepPersistedTargetData */ true @@ -1463,9 +1486,15 @@ export async function applyActiveTargetsChange( continue; } - const target = await getCachedTarget(syncEngineImpl.localStore, targetId); + const target = await localStoreGetCachedTarget( + syncEngineImpl.localStore, + targetId + ); debugAssert(!!target, `Query data for active target ${targetId} not found`); - const targetData = await allocateTarget(syncEngineImpl.localStore, target); + const targetData = await localStoreAllocateTarget( + syncEngineImpl.localStore, + target + ); await initializeViewAndComputeSnapshot( syncEngineImpl, synthesizeTargetToQuery(target), @@ -1483,7 +1512,7 @@ export async function applyActiveTargetsChange( } // Release queries that are still active. - await releaseTarget( + await localStoreReleaseTarget( syncEngineImpl.localStore, targetId, /* keepPersistedTargetData */ false @@ -1561,7 +1590,10 @@ async function loadBundleImpl( ): Promise { try { const metadata = await reader.getMetadata(); - const skip = await hasNewerBundle(syncEngine.localStore, metadata); + const skip = await localStoreHasNewerBundle( + syncEngine.localStore, + metadata + ); if (skip) { await reader.close(); task._completeWith(bundleSuccessProgress(metadata)); @@ -1600,7 +1632,7 @@ async function loadBundleImpl( ); // Save metadata, so loading the same bundle will skip. - await saveBundle(syncEngine.localStore, metadata); + await localStoreSaveBundle(syncEngine.localStore, metadata); task._completeWith(result.progress); } catch (e) { logWarn(LOG_TAG, `Loading bundle failed with ${e}`); diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 112577222fc..b29f354c52f 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -20,6 +20,7 @@ import { logDebug } from '../util/log'; import { LruGarbageCollector, LruResults } from './lru_garbage_collector'; import { PRIMARY_LEASE_LOST_ERROR_MSG } from './persistence_transaction'; + export interface LocalStore { collectGarbage(garbageCollector: LruGarbageCollector): Promise; } diff --git a/packages/firestore/src/local/local_store_impl.ts b/packages/firestore/src/local/local_store_impl.ts index b8745ab4511..eca70eb4f2f 100644 --- a/packages/firestore/src/local/local_store_impl.ts +++ b/packages/firestore/src/local/local_store_impl.ts @@ -213,7 +213,7 @@ export function newLocalStore( */ // PORTING NOTE: Android and iOS only return the documents affected by the // change. -export async function handleUserChange( +export async function localStoreHandleUserChange( localStore: LocalStore, user: User ): Promise { @@ -290,7 +290,7 @@ export async function handleUserChange( } /* Accepts locally generated Mutations and commit them to storage. */ -export function localWrite( +export function localStoreWriteLocally( localStore: LocalStore, mutations: Mutation[] ): Promise { @@ -365,7 +365,7 @@ export function localWrite( * * @returns The resulting (modified) documents. */ -export function acknowledgeBatch( +export function localStoreAcknowledgeBatch( localStore: LocalStore, batchResult: MutationBatchResult ): Promise { @@ -397,7 +397,7 @@ export function acknowledgeBatch( * * @returns The resulting modified documents. */ -export function rejectBatch( +export function localStoreRejectBatch( localStore: LocalStore, batchId: BatchId ): Promise { @@ -428,7 +428,7 @@ export function rejectBatch( * * Returns `BATCHID_UNKNOWN` if the queue is empty. */ -export function getHighestUnacknowledgedBatchId( +export function localStoreGetHighestUnacknowledgedBatchId( localStore: LocalStore ): Promise { const localStoreImpl = debugCast(localStore, LocalStoreImpl); @@ -443,7 +443,7 @@ export function getHighestUnacknowledgedBatchId( * Returns the last consistent snapshot processed (used by the RemoteStore to * determine whether to buffer incoming snapshots from the backend). */ -export function getLastRemoteSnapshotVersion( +export function localStoreGetLastRemoteSnapshotVersion( localStore: LocalStore ): Promise { const localStoreImpl = debugCast(localStore, LocalStoreImpl); @@ -462,7 +462,7 @@ export function getLastRemoteSnapshotVersion( * LocalDocuments are re-calculated if there are remaining mutations in the * queue. */ -export function applyRemoteEventToLocalCache( +export function localStoreApplyRemoteEventToLocalCache( localStore: LocalStore, remoteEvent: RemoteEvent ): Promise { @@ -714,7 +714,7 @@ function shouldPersistTargetData( /** * Notifies local store of the changed views to locally pin documents. */ -export async function notifyLocalViewChanges( +export async function localStoreNotifyLocalViewChanges( localStore: LocalStore, viewChanges: LocalViewChanges[] ): Promise { @@ -791,7 +791,7 @@ export async function notifyLocalViewChanges( * @param afterBatchId - If provided, the batch to search after. * @returns The next mutation or null if there wasn't one. */ -export function nextMutationBatch( +export function localStoreGetNextMutationBatch( localStore: LocalStore, afterBatchId?: BatchId ): Promise { @@ -815,7 +815,7 @@ export function nextMutationBatch( * Reads the current value of a Document with a given key or null if not * found - used for testing. */ -export function readLocalDocument( +export function localStoreReadDocument( localStore: LocalStore, key: DocumentKey ): Promise { @@ -835,7 +835,7 @@ export function readLocalDocument( * Allocating an already allocated `Target` will return the existing `TargetData` * for that `Target`. */ -export function allocateTarget( +export function localStoreAllocateTarget( localStore: LocalStore, target: Target ): Promise { @@ -895,7 +895,7 @@ export function allocateTarget( * have not yet been persisted to the TargetCache. */ // Visible for testing. -export function getLocalTargetData( +export function localStoreGetTargetData( localStore: LocalStore, transaction: PersistenceTransaction, target: Target @@ -919,7 +919,7 @@ export function getLocalTargetData( * Releasing a non-existing `Target` is a no-op. */ // PORTING NOTE: `keepPersistedTargetData` is multi-tab only. -export async function releaseTarget( +export async function localStoreReleaseTarget( localStore: LocalStore, targetId: number, keepPersistedTargetData: boolean @@ -976,7 +976,7 @@ export async function releaseTarget( * @param usePreviousResults - Whether results from previous executions can * be used to optimize this query execution. */ -export function executeQuery( +export function localStoreExecuteQuery( localStore: LocalStore, query: Query, usePreviousResults: boolean @@ -989,7 +989,7 @@ export function executeQuery( 'Execute query', 'readonly', txn => { - return getLocalTargetData(localStoreImpl, txn, queryToTarget(query)) + return localStoreGetTargetData(localStoreImpl, txn, queryToTarget(query)) .next(targetData => { if (targetData) { lastLimboFreeSnapshotVersion = @@ -1066,7 +1066,7 @@ function applyWriteToRemoteDocuments( /** Returns the local view of the documents affected by a mutation batch. */ // PORTING NOTE: Multi-Tab only. -export function lookupMutationDocuments( +export function localStoreLookupMutationDocuments( localStore: LocalStore, batchId: BatchId ): Promise { @@ -1094,7 +1094,7 @@ export function lookupMutationDocuments( } // PORTING NOTE: Multi-Tab only. -export function removeCachedMutationBatchMetadata( +export function localStoreRemoveCachedMutationBatchMetadata( localStore: LocalStore, batchId: BatchId ): void { @@ -1106,7 +1106,7 @@ export function removeCachedMutationBatchMetadata( } // PORTING NOTE: Multi-Tab only. -export function getActiveClientsFromPersistence( +export function localStoreGetActiveClients( localStore: LocalStore ): Promise { const persistenceImpl = debugCast( @@ -1117,7 +1117,7 @@ export function getActiveClientsFromPersistence( } // PORTING NOTE: Multi-Tab only. -export function getCachedTarget( +export function localStoreGetCachedTarget( localStore: LocalStore, targetId: TargetId ): Promise { @@ -1149,7 +1149,7 @@ export function getCachedTarget( * since the prior call. */ // PORTING NOTE: Multi-Tab only. -export function getNewDocumentChanges( +export function localStoreGetNewDocumentChanges( localStore: LocalStore ): Promise { const localStoreImpl = debugCast(localStore, LocalStoreImpl); @@ -1173,7 +1173,7 @@ export function getNewDocumentChanges( * only return changes that happened after client initialization. */ // PORTING NOTE: Multi-Tab only. -export async function synchronizeLastDocumentChangeReadTime( +export async function localStoreSynchronizeLastDocumentChangeReadTime( localStore: LocalStore ): Promise { const localStoreImpl = debugCast(localStore, LocalStoreImpl); @@ -1209,7 +1209,7 @@ function umbrellaTarget(bundleName: string): Target { * LocalDocuments are re-calculated if there are remaining mutations in the * queue. */ -export async function applyBundleDocuments( +export async function localStoreApplyBundledDocuments( localStore: LocalStore, bundleConverter: BundleConverter, documents: BundledDocuments, @@ -1240,7 +1240,7 @@ export async function applyBundleDocuments( // Allocates a target to hold all document keys from the bundle, such that // they will not get garbage collected right away. - const umbrellaTargetData = await allocateTarget( + const umbrellaTargetData = await localStoreAllocateTarget( localStoreImpl, umbrellaTarget(bundleName) ); @@ -1284,7 +1284,7 @@ export async function applyBundleDocuments( * Returns a promise of a boolean to indicate if the given bundle has already * been loaded and the create time is newer than the current loading bundle. */ -export function hasNewerBundle( +export function localStoreHasNewerBundle( localStore: LocalStore, bundleMetadata: BundleMetadata ): Promise { @@ -1305,7 +1305,7 @@ export function hasNewerBundle( /** * Saves the given `BundleMetadata` to local persistence. */ -export function saveBundle( +export function localStoreSaveBundle( localStore: LocalStore, bundleMetadata: BundleMetadata ): Promise { @@ -1326,7 +1326,7 @@ export function saveBundle( * Returns a promise of a `NamedQuery` associated with given query name. Promise * resolves to undefined if no persisted data can be found. */ -export function getNamedQuery( +export function localStoreGetNamedQuery( localStore: LocalStore, queryName: string ): Promise { @@ -1342,7 +1342,7 @@ export function getNamedQuery( /** * Saves the given `NamedQuery` to local persistence. */ -export async function saveNamedQuery( +export async function localStoreSaveNamedQuery( localStore: LocalStore, query: ProtoNamedQuery, documents: DocumentKeySet = documentKeySet() @@ -1352,7 +1352,7 @@ export async function saveNamedQuery( // NOTE: this also means if no corresponding target exists, the new target // will remain active and will not get collected, unless users happen to // unlisten the query somehow. - const allocated = await allocateTarget( + const allocated = await localStoreAllocateTarget( localStore, queryToTarget(fromBundledQuery(query.bundledQuery!)) ); diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index ccc38a96eb1..3db431ca0d3 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -20,8 +20,8 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { OnlineState, TargetId } from '../core/types'; import { LocalStore } from '../local/local_store'; import { - getLastRemoteSnapshotVersion, - nextMutationBatch + localStoreGetLastRemoteSnapshotVersion, + localStoreGetNextMutationBatch } from '../local/local_store_impl'; import { isIndexedDbTransactionError } from '../local/simple_db'; import { TargetData, TargetPurpose } from '../local/target_data'; @@ -470,7 +470,7 @@ async function onWatchStreamChange( if (!snapshotVersion.isEqual(SnapshotVersion.min())) { try { - const lastRemoteSnapshotVersion = await getLastRemoteSnapshotVersion( + const lastRemoteSnapshotVersion = await localStoreGetLastRemoteSnapshotVersion( remoteStoreImpl.localStore ); if (snapshotVersion.compareTo(lastRemoteSnapshotVersion) >= 0) { @@ -514,7 +514,8 @@ async function disableNetworkUntilRecovery( // Use a simple read operation to determine if IndexedDB recovered. // Ideally, we would expose a health check directly on SimpleDb, but // RemoteStore only has access to persistence through LocalStore. - op = () => getLastRemoteSnapshotVersion(remoteStoreImpl.localStore); + op = () => + localStoreGetLastRemoteSnapshotVersion(remoteStoreImpl.localStore); } // Probe IndexedDB periodically and re-enable network @@ -659,7 +660,7 @@ export async function fillWritePipeline( while (canAddToWritePipeline(remoteStoreImpl)) { try { - const batch = await nextMutationBatch( + const batch = await localStoreGetNextMutationBatch( remoteStoreImpl.localStore, lastBatchIdRetrieved ); diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index e456ba95c22..a1d2e5a0f81 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -35,25 +35,25 @@ import { BatchId, TargetId } from '../../../src/core/types'; import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence'; import { LocalStore } from '../../../src/local/local_store'; import { - acknowledgeBatch, - allocateTarget, - applyBundleDocuments, - applyRemoteEventToLocalCache, - executeQuery, - getHighestUnacknowledgedBatchId, - getLocalTargetData, - getNamedQuery, - hasNewerBundle, - localWrite, + localStoreAcknowledgeBatch, + localStoreAllocateTarget, + localStoreApplyBundledDocuments, + localStoreApplyRemoteEventToLocalCache, + localStoreExecuteQuery, + localStoreGetHighestUnacknowledgedBatchId, + localStoreGetTargetData, + localStoreGetNamedQuery, + localStoreHasNewerBundle, + localStoreWriteLocally, LocalWriteResult, newLocalStore, - notifyLocalViewChanges, - readLocalDocument, - rejectBatch, - releaseTarget, - saveBundle, - saveNamedQuery, - synchronizeLastDocumentChangeReadTime + localStoreNotifyLocalViewChanges, + localStoreReadDocument, + localStoreRejectBatch, + localStoreReleaseTarget, + localStoreSaveBundle, + localStoreSaveNamedQuery, + localStoreSynchronizeLastDocumentChangeReadTime } from '../../../src/local/local_store_impl'; import { LocalViewChanges } from '../../../src/local/local_view_changes'; import { Persistence } from '../../../src/local/persistence'; @@ -172,7 +172,7 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain - .then(() => localWrite(this.localStore, mutations)) + .then(() => localStoreWriteLocally(this.localStore, mutations)) .then((result: LocalWriteResult) => { this.batches.push( new MutationBatch(result.batchId, Timestamp.now(), [], mutations) @@ -186,7 +186,9 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain - .then(() => applyRemoteEventToLocalCache(this.localStore, remoteEvent)) + .then(() => + localStoreApplyRemoteEventToLocalCache(this.localStore, remoteEvent) + ) .then((result: MaybeDocumentMap) => { this.lastChanges = result; }); @@ -201,7 +203,7 @@ class LocalStoreTester { this.promiseChain = this.promiseChain .then(() => - applyBundleDocuments( + localStoreApplyBundledDocuments( this.localStore, this.bundleConverter, documents, @@ -218,7 +220,7 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain.then(() => - saveNamedQuery( + localStoreSaveNamedQuery( this.localStore, testQuery.namedQuery, testQuery.matchingDocuments @@ -231,7 +233,7 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain.then(() => - notifyLocalViewChanges(this.localStore, [viewChanges]) + localStoreNotifyLocalViewChanges(this.localStore, [viewChanges]) ); return this; } @@ -258,7 +260,7 @@ class LocalStoreTester { ]; const write = MutationBatchResult.from(batch, ver, mutationResults); - return acknowledgeBatch(this.localStore, write); + return localStoreAcknowledgeBatch(this.localStore, write); }) .then((changes: MaybeDocumentMap) => { this.lastChanges = changes; @@ -270,7 +272,9 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain - .then(() => rejectBatch(this.localStore, this.batches.shift()!.batchId)) + .then(() => + localStoreRejectBatch(this.localStore, this.batches.shift()!.batchId) + ) .then((changes: MaybeDocumentMap) => { this.lastChanges = changes; }); @@ -285,7 +289,7 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain.then(() => - allocateTarget(this.localStore, target).then(result => { + localStoreAllocateTarget(this.localStore, target).then(result => { this.lastTargetId = result.targetId; }) ); @@ -296,7 +300,7 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain.then(() => - releaseTarget( + localStoreReleaseTarget( this.localStore, targetId, /*keepPersistedTargetData=*/ false @@ -309,11 +313,13 @@ class LocalStoreTester { this.prepareNextStep(); this.promiseChain = this.promiseChain.then(() => - executeQuery(this.localStore, query, /* usePreviousResults= */ true).then( - ({ documents }) => { - this.lastChanges = documents; - } - ) + localStoreExecuteQuery( + this.localStore, + query, + /* usePreviousResults= */ true + ).then(({ documents }) => { + this.lastChanges = documents; + }) ); return this; } @@ -416,7 +422,7 @@ class LocalStoreTester { toContain(doc: MaybeDocument): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { - return readLocalDocument(this.localStore, doc.key).then(result => { + return localStoreReadDocument(this.localStore, doc.key).then(result => { expectEqual( result, doc, @@ -431,7 +437,7 @@ class LocalStoreTester { toNotContain(keyStr: string): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => - readLocalDocument(this.localStore, key(keyStr)).then(result => { + localStoreReadDocument(this.localStore, key(keyStr)).then(result => { expect(result).to.be.null; }) ); @@ -448,9 +454,11 @@ class LocalStoreTester { toReturnHighestUnacknowledgeBatchId(expectedId: BatchId): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => - getHighestUnacknowledgedBatchId(this.localStore).then(actual => { - expect(actual).to.equal(expectedId); - }) + localStoreGetHighestUnacknowledgedBatchId(this.localStore).then( + actual => { + expect(actual).to.equal(expectedId); + } + ) ); return this; } @@ -483,28 +491,32 @@ class LocalStoreTester { expected: boolean ): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { - return hasNewerBundle(this.localStore, metadata).then(actual => { - expect(actual).to.equal(expected); - }); + return localStoreHasNewerBundle(this.localStore, metadata).then( + actual => { + expect(actual).to.equal(expected); + } + ); }); return this; } toHaveNamedQuery(namedQuery: NamedQuery): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { - return getNamedQuery(this.localStore, namedQuery.name).then(actual => { - expect(actual).to.exist; - expect(actual!.name).to.equal(namedQuery.name); - expect(namedQuery.readTime.isEqual(actual!.readTime)).to.be.true; - expect(queryEquals(actual!.query, namedQuery.query)).to.be.true; - }); + return localStoreGetNamedQuery(this.localStore, namedQuery.name).then( + actual => { + expect(actual).to.exist; + expect(actual!.name).to.equal(namedQuery.name); + expect(namedQuery.readTime.isEqual(actual!.readTime)).to.be.true; + expect(queryEquals(actual!.query, namedQuery.query)).to.be.true; + } + ); }); return this; } afterSavingBundle(metadata: ProtoBundleMetadata): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => - saveBundle(this.localStore, metadata) + localStoreSaveBundle(this.localStore, metadata) ); return this; } @@ -548,7 +560,7 @@ describe('LocalStore w/ IndexedDB Persistence', () => { User.UNAUTHENTICATED, JSON_SERIALIZER ); - await synchronizeLastDocumentChangeReadTime(localStore); + await localStoreSynchronizeLastDocumentChangeReadTime(localStore); return { queryEngine, persistence, localStore }; } @@ -1135,13 +1147,13 @@ function genericLocalStoreTests( it('can execute document queries', () => { const localStore = expectLocalStore().localStore; - return localWrite(localStore, [ + return localStoreWriteLocally(localStore, [ setMutation('foo/bar', { foo: 'bar' }), setMutation('foo/baz', { foo: 'baz' }), setMutation('foo/bar/Foo/Bar', { Foo: 'Bar' }) ]) .then(() => { - return executeQuery( + return localStoreExecuteQuery( localStore, query('foo/bar'), /* usePreviousResults= */ true @@ -1155,7 +1167,7 @@ function genericLocalStoreTests( it('can execute collection queries', () => { const localStore = expectLocalStore().localStore; - return localWrite(localStore, [ + return localStoreWriteLocally(localStore, [ setMutation('fo/bar', { fo: 'bar' }), setMutation('foo/bar', { foo: 'bar' }), setMutation('foo/baz', { foo: 'baz' }), @@ -1163,7 +1175,7 @@ function genericLocalStoreTests( setMutation('fooo/blah', { fooo: 'blah' }) ]) .then(() => { - return executeQuery( + return localStoreExecuteQuery( localStore, query('foo'), /* usePreviousResults= */ true @@ -1178,18 +1190,23 @@ function genericLocalStoreTests( it('can execute mixed collection queries', async () => { const query1 = query('foo'); - const targetData = await allocateTarget(localStore, queryToTarget(query1)); + const targetData = await localStoreAllocateTarget( + localStore, + queryToTarget(query1) + ); expect(targetData.targetId).to.equal(2); - await applyRemoteEventToLocalCache( + await localStoreApplyRemoteEventToLocalCache( localStore, docAddedRemoteEvent(doc('foo/baz', 10, { a: 'b' }), [2], []) ); - await applyRemoteEventToLocalCache( + await localStoreApplyRemoteEventToLocalCache( localStore, docUpdateRemoteEvent(doc('foo/bar', 20, { a: 'b' }), [2], []) ); - await localWrite(localStore, [setMutation('foo/bonk', { a: 'b' })]); - const { documents } = await executeQuery( + await localStoreWriteLocally(localStore, [ + setMutation('foo/bonk', { a: 'b' }) + ]); + const { documents } = await localStoreExecuteQuery( localStore, query1, /* usePreviousResults= */ true @@ -1243,7 +1260,10 @@ function genericLocalStoreTests( // eslint-disable-next-line no-restricted-properties (gcIsEager ? it.skip : it)('persists resume tokens', async () => { const query1 = query('foo/bar'); - const targetData = await allocateTarget(localStore, queryToTarget(query1)); + const targetData = await localStoreAllocateTarget( + localStore, + queryToTarget(query1) + ); const targetId = targetData.targetId; const resumeToken = byteStringFromString('abc'); const watchChange = new WatchTargetChange( @@ -1257,17 +1277,20 @@ function genericLocalStoreTests( }); aggregator.handleTargetChange(watchChange); const remoteEvent = aggregator.createRemoteEvent(version(1000)); - await applyRemoteEventToLocalCache(localStore, remoteEvent); + await localStoreApplyRemoteEventToLocalCache(localStore, remoteEvent); // Stop listening so that the query should become inactive (but persistent) - await releaseTarget( + await localStoreReleaseTarget( localStore, targetData.targetId, /*keepPersistedTargetData=*/ false ); // Should come back with the same resume token - const targetData2 = await allocateTarget(localStore, queryToTarget(query1)); + const targetData2 = await localStoreAllocateTarget( + localStore, + queryToTarget(query1) + ); expect(targetData2.resumeToken).to.deep.equal(resumeToken); }); @@ -1276,7 +1299,7 @@ function genericLocalStoreTests( 'does not replace resume token with empty resume token', async () => { const query1 = query('foo/bar'); - const targetData = await allocateTarget( + const targetData = await localStoreAllocateTarget( localStore, queryToTarget(query1) ); @@ -1294,7 +1317,7 @@ function genericLocalStoreTests( }); aggregator1.handleTargetChange(watchChange1); const remoteEvent1 = aggregator1.createRemoteEvent(version(1000)); - await applyRemoteEventToLocalCache(localStore, remoteEvent1); + await localStoreApplyRemoteEventToLocalCache(localStore, remoteEvent1); const watchChange2 = new WatchTargetChange( WatchTargetChangeState.Current, @@ -1307,17 +1330,17 @@ function genericLocalStoreTests( }); aggregator2.handleTargetChange(watchChange2); const remoteEvent2 = aggregator2.createRemoteEvent(version(2000)); - await applyRemoteEventToLocalCache(localStore, remoteEvent2); + await localStoreApplyRemoteEventToLocalCache(localStore, remoteEvent2); // Stop listening so that the query should become inactive (but persistent) - await releaseTarget( + await localStoreReleaseTarget( localStore, targetId, /*keepPersistedTargetData=*/ false ); // Should come back with the same resume token - const targetData2 = await allocateTarget( + const targetData2 = await localStoreAllocateTarget( localStore, queryToTarget(query1) ); @@ -1922,10 +1945,10 @@ function genericLocalStoreTests( const target = queryToTarget(query('foo')); - const targetData = await allocateTarget(localStore, target); + const targetData = await localStoreAllocateTarget(localStore, target); // Advance the query snapshot - await applyRemoteEventToLocalCache( + await localStoreApplyRemoteEventToLocalCache( localStore, noChangeEvent( /* targetId= */ targetData.targetId, @@ -1938,7 +1961,7 @@ function genericLocalStoreTests( let cachedTargetData = await persistence.runTransaction( 'getTargetData', 'readonly', - txn => getLocalTargetData(localStore, txn, target) + txn => localStoreGetTargetData(localStore, txn, target) ); expect( cachedTargetData!.lastLimboFreeSnapshotVersion.isEqual( @@ -1947,20 +1970,20 @@ function genericLocalStoreTests( ).to.be.true; // Mark the view synced, which updates the last limbo free snapshot version. - await notifyLocalViewChanges(localStore, [ + await localStoreNotifyLocalViewChanges(localStore, [ localViewChanges(2, /* fromCache= */ false, {}) ]); cachedTargetData = await persistence.runTransaction( 'getTargetData', 'readonly', - txn => getLocalTargetData(localStore, txn, target) + txn => localStoreGetTargetData(localStore, txn, target) ); expect(cachedTargetData!.lastLimboFreeSnapshotVersion.isEqual(version(10))) .to.be.true; // The last limbo free snapshot version is persisted even if we release the // query. - await releaseTarget( + await localStoreReleaseTarget( localStore, targetData.targetId, /* keepPersistedTargetData= */ false @@ -1970,7 +1993,7 @@ function genericLocalStoreTests( cachedTargetData = await persistence.runTransaction( 'getTargetData', 'readonly', - txn => getLocalTargetData(localStore, txn, target) + txn => localStoreGetTargetData(localStore, txn, target) ); expect( cachedTargetData!.lastLimboFreeSnapshotVersion.isEqual(version(10)) From 7e6dbd4a0ec86ba1988ff759e8658b664cc4f245 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 10 Dec 2020 10:47:25 -0700 Subject: [PATCH 2/3] Manually order --- packages/firestore/test/unit/local/local_store.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index a1d2e5a0f81..6a68e9ca7a6 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -46,14 +46,14 @@ import { localStoreHasNewerBundle, localStoreWriteLocally, LocalWriteResult, - newLocalStore, localStoreNotifyLocalViewChanges, localStoreReadDocument, localStoreRejectBatch, localStoreReleaseTarget, localStoreSaveBundle, localStoreSaveNamedQuery, - localStoreSynchronizeLastDocumentChangeReadTime + localStoreSynchronizeLastDocumentChangeReadTime, + newLocalStore } from '../../../src/local/local_store_impl'; import { LocalViewChanges } from '../../../src/local/local_view_changes'; import { Persistence } from '../../../src/local/persistence'; From 3321ae907988bcb84a32be4e53a1abd3fbb32e09 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 10 Dec 2020 15:09:20 -0700 Subject: [PATCH 3/3] Fix merge --- packages/firestore/src/core/firestore_client.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 93a282adf57..f8365adefef 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -201,10 +201,12 @@ export async function setOfflineComponentProvider( client.setCredentialChangeListener(user => { if (!currentUser.isEqual(user)) { currentUser = user; - await localStoreHandleUserChange( - offlineComponentProvider.localStore, - user - ); + client.asyncQueue.enqueueRetryable(async () => { + await localStoreHandleUserChange( + offlineComponentProvider.localStore, + user + ); + }); } });