diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index dabc3435025..f7c7fa93d87 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -101,7 +101,7 @@ const MAX_PRIMARY_ELIGIBLE_AGE_MS = 5000; const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000; /** User-facing error when the primary lease is required but not available. */ const PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG = - 'Another tab has exclusive access to the persistence layer. ' + + 'Failed to obtain exclusive access to the persistence layer. ' + 'To allow shared access, make sure to invoke ' + '`enablePersistence()` with `synchronizeTabs:true` in all tabs.'; const UNSUPPORTED_PLATFORM_ERROR_MSG = @@ -191,11 +191,12 @@ export class IndexedDbPersistence implements Persistence { private readonly document: Document | null; private readonly window: Window; - // Technically these types should be `| undefined` because they are + // Technically `simpleDb` should be `| undefined` because it is // initialized asynchronously by start(), but that would be more misleading // than useful. private simpleDb!: SimpleDb; - private listenSequence!: ListenSequence; + + private listenSequence: ListenSequence | null = null; private _started = false; private isPrimary = false; @@ -287,7 +288,15 @@ export class IndexedDbPersistence implements Persistence { // having the persistence lock), so it's the first thing we should do. return this.updateClientMetadataAndTryBecomePrimary(); }) - .then(() => { + .then(e => { + if (!this.isPrimary && !this.allowTabSynchronization) { + // Fail `start()` if `synchronizeTabs` is disabled and we cannot + // obtain the primary lease. + throw new FirestoreError( + Code.FAILED_PRECONDITION, + PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG + ); + } this.attachVisibilityHandler(); this.attachWindowUnloadHook(); @@ -375,8 +384,10 @@ export class IndexedDbPersistence implements Persistence { * primary lease. */ private updateClientMetadataAndTryBecomePrimary(): Promise { - return this.simpleDb - .runTransaction('readwrite', ALL_STORES, txn => { + return this.runTransaction( + 'updateClientMetadataAndTryBecomePrimary', + 'readwrite', + txn => { const metadataStore = clientMetadataStore(txn); return metadataStore .put( @@ -409,10 +420,18 @@ export class IndexedDbPersistence implements Persistence { return /* canActAsPrimary= */ false; } }); - }) + } + ) .catch(e => { if (!this.allowTabSynchronization) { - throw e; + if (e.name === 'IndexedDbTransactionError') { + logDebug(LOG_TAG, 'Failed to extend owner lease: ', e); + // Proceed with the existing state. Any subsequent access to + // IndexedDB will verify the lease. + return this.isPrimary; + } else { + throw e; + } } logDebug( @@ -433,7 +452,7 @@ export class IndexedDbPersistence implements Persistence { } private verifyPrimaryLease( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const store = primaryClientStore(txn); return store.get(DbPrimaryClient.key).next(primaryClient => { @@ -442,7 +461,7 @@ export class IndexedDbPersistence implements Persistence { } private removeClientMetadata( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const metadataStore = clientMetadataStore(txn); return metadataStore.delete(this.clientId); @@ -536,7 +555,7 @@ export class IndexedDbPersistence implements Persistence { * (foreground) client should become leaseholder instead. */ private canActAsPrimary( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const store = primaryClientStore(txn); return store @@ -645,15 +664,13 @@ export class IndexedDbPersistence implements Persistence { } this.detachVisibilityHandler(); this.detachWindowUnloadHook(); - await this.simpleDb.runTransaction( - 'readwrite', - [DbPrimaryClient.store, DbClientMetadata.store], - txn => { - return this.releasePrimaryLeaseIfHeld(txn).next(() => - this.removeClientMetadata(txn) - ); - } - ); + await this.runTransaction('shutdown', 'readwrite', txn => { + return this.releasePrimaryLeaseIfHeld(txn).next(() => + this.removeClientMetadata(txn) + ); + }).catch(e => { + logDebug(LOG_TAG, 'Proceeding with shutdown despite failure: ', e); + }); this.simpleDb.close(); // Remove the entry marking the client as zombied from LocalStorage since @@ -684,19 +701,15 @@ export class IndexedDbPersistence implements Persistence { * PORTING NOTE: This is only used for Web multi-tab. */ getActiveClients(): Promise { - return this.simpleDb.runTransaction( - 'readonly', - [DbClientMetadata.store], - txn => { - return clientMetadataStore(txn) - .loadAll() - .next(clients => - this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map( - clientMetadata => clientMetadata.clientId - ) - ); - } - ); + return this.runTransaction('getActiveClients', 'readonly', txn => { + return clientMetadataStore(txn) + .loadAll() + .next(clients => + this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map( + clientMetadata => clientMetadata.clientId + ) + ); + }); } static async clearPersistence(persistenceKey: string): Promise { @@ -767,7 +780,9 @@ export class IndexedDbPersistence implements Persistence { .runTransaction(simpleDbMode, ALL_STORES, simpleDbTxn => { persistenceTransaction = new IndexedDbTransaction( simpleDbTxn, - this.listenSequence.next() + this.listenSequence + ? this.listenSequence.next() + : ListenSequence.INVALID ); if (mode === 'readwrite-primary') { @@ -776,12 +791,12 @@ export class IndexedDbPersistence implements Persistence { // executing transactionOperation(). This ensures that even if the // transactionOperation takes a long time, we'll use a recent // leaseTimestampMs in the extended (or newly acquired) lease. - return this.verifyPrimaryLease(simpleDbTxn) + return this.verifyPrimaryLease(persistenceTransaction) .next(holdsPrimaryLease => { if (holdsPrimaryLease) { return /* holdsPrimaryLease= */ true; } - return this.canActAsPrimary(simpleDbTxn); + return this.canActAsPrimary(persistenceTransaction); }) .next(holdsPrimaryLease => { if (!holdsPrimaryLease) { @@ -800,14 +815,14 @@ export class IndexedDbPersistence implements Persistence { return transactionOperation(persistenceTransaction); }) .next(result => { - return this.acquireOrExtendPrimaryLease(simpleDbTxn).next( - () => result - ); + return this.acquireOrExtendPrimaryLease( + persistenceTransaction + ).next(() => result); }); } else { - return this.verifyAllowTabSynchronization(simpleDbTxn).next(() => - transactionOperation(persistenceTransaction) - ); + return this.verifyAllowTabSynchronization( + persistenceTransaction + ).next(() => transactionOperation(persistenceTransaction)); } }) .then(result => { @@ -823,7 +838,7 @@ export class IndexedDbPersistence implements Persistence { // TODO(b/114226234): Remove this check when `synchronizeTabs` can no longer // be turned off. private verifyAllowTabSynchronization( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const store = primaryClientStore(txn); return store.get(DbPrimaryClient.key).next(currentPrimary => { @@ -836,7 +851,10 @@ export class IndexedDbPersistence implements Persistence { !this.isClientZombied(currentPrimary.ownerId); if (currentLeaseIsValid && !this.isLocalClient(currentPrimary)) { - if (!currentPrimary!.allowTabSynchronization) { + if ( + !this.allowTabSynchronization || + !currentPrimary!.allowTabSynchronization + ) { throw new FirestoreError( Code.FAILED_PRECONDITION, PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG @@ -851,7 +869,7 @@ export class IndexedDbPersistence implements Persistence { * method does not verify that the client is eligible for this lease. */ private acquireOrExtendPrimaryLease( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const newPrimary = new DbPrimaryClient( this.clientId, @@ -887,7 +905,7 @@ export class IndexedDbPersistence implements Persistence { /** Checks the primary lease and removes it if we are the current primary. */ private releasePrimaryLeaseIfHeld( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): PersistencePromise { const store = primaryClientStore(txn); return store.get(DbPrimaryClient.key).next(primaryClient => { @@ -1052,18 +1070,22 @@ export class IndexedDbPersistence implements Persistence { * Helper to get a typed SimpleDbStore for the primary client object store. */ function primaryClientStore( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): SimpleDbStore { - return txn.store(DbPrimaryClient.store); + return IndexedDbPersistence.getStore( + txn, + DbPrimaryClient.store + ); } /** * Helper to get a typed SimpleDbStore for the client metadata object store. */ function clientMetadataStore( - txn: SimpleDbTransaction + txn: PersistenceTransaction ): SimpleDbStore { - return txn.store( + return IndexedDbPersistence.getStore( + txn, DbClientMetadata.store ); } diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts index 2b4168ced98..6a7bfcf8df2 100644 --- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts +++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts @@ -61,10 +61,11 @@ import { TargetData, TargetPurpose } from '../../../src/local/target_data'; import { PlatformSupport } from '../../../src/platform/platform'; import { firestoreV1ApiClientInterfaces } from '../../../src/protos/firestore_proto_api'; import { JsonProtoSerializer } from '../../../src/remote/serializer'; -import { AsyncQueue } from '../../../src/util/async_queue'; +import { AsyncQueue, TimerId } from '../../../src/util/async_queue'; import { FirestoreError } from '../../../src/util/error'; import { doc, filter, path, version } from '../../util/helpers'; import { SharedFakeWebStorage, TestPlatform } from '../../util/test_platform'; +import { MockIndexedDbPersistence } from '../specs/spec_test_components'; import { INDEXEDDB_TEST_DATABASE_NAME, MOCK_SEQUENCE_NUMBER_SYNCER, @@ -108,11 +109,11 @@ function withDb( }); } -async function withCustomPersistence( +async function withUnstartedCustomPersistence( clientId: ClientId, multiClient: boolean, fn: ( - persistence: IndexedDbPersistence, + persistence: MockIndexedDbPersistence, platform: TestPlatform, queue: AsyncQueue ) => Promise @@ -126,7 +127,7 @@ async function withCustomPersistence( PlatformSupport.getPlatform(), new SharedFakeWebStorage() ); - const persistence = new IndexedDbPersistence( + const persistence = new MockIndexedDbPersistence( multiClient, TEST_PERSISTENCE_PREFIX, clientId, @@ -137,15 +138,33 @@ async function withCustomPersistence( MOCK_SEQUENCE_NUMBER_SYNCER ); - await persistence.start(); await fn(persistence, platform, queue); - await persistence.shutdown(); +} + +function withCustomPersistence( + clientId: ClientId, + multiClient: boolean, + fn: ( + persistence: MockIndexedDbPersistence, + platform: TestPlatform, + queue: AsyncQueue + ) => Promise +): Promise { + return withUnstartedCustomPersistence( + clientId, + multiClient, + async (persistence, platform, queue) => { + await persistence.start(); + await fn(persistence, platform, queue); + await persistence.shutdown(); + } + ); } async function withPersistence( clientId: ClientId, fn: ( - persistence: IndexedDbPersistence, + persistence: MockIndexedDbPersistence, platform: TestPlatform, queue: AsyncQueue ) => Promise @@ -156,7 +175,7 @@ async function withPersistence( async function withMultiClientPersistence( clientId: ClientId, fn: ( - persistence: IndexedDbPersistence, + persistence: MockIndexedDbPersistence, platform: TestPlatform, queue: AsyncQueue ) => Promise @@ -1046,7 +1065,7 @@ describe('IndexedDb: canActAsPrimary', () => { } and ${thatVisibility}`; it(testName, () => { - return withPersistence( + return withMultiClientPersistence( 'thatClient', async (thatPersistence, thatPlatform, thatQueue) => { thatPlatform.raiseVisibilityEvent(thatVisibility); @@ -1057,7 +1076,7 @@ describe('IndexedDb: canActAsPrimary', () => { // the lease until it expires. await clearPrimaryLease(); - await withPersistence( + await withMultiClientPersistence( 'thisClient', async (thisPersistence, thisPlatform, thisQueue) => { thisPlatform.raiseVisibilityEvent(thisVisibility); @@ -1120,12 +1139,51 @@ describe('IndexedDb: allowTabSynchronization', () => { after(() => SimpleDb.delete(INDEXEDDB_TEST_DATABASE_NAME)); + it('blocks start() on IndexedDbTransactionError when synchronization is disabled ', async () => { + await withUnstartedCustomPersistence( + 'clientA', + /* multiClient= */ false, + async db => { + db.injectFailures = true; + await expect(db.start()).to.eventually.be.rejectedWith( + 'Failed to obtain exclusive access to the persistence layer.' + ); + await db.shutdown(); + } + ); + }); + + it('allows start() with IndexedDbTransactionError when synchronization is enabled ', async () => { + await withUnstartedCustomPersistence( + 'clientA', + /* multiClient= */ true, + async db => { + db.injectFailures = true; + await db.start(); + await db.shutdown(); + } + ); + }); + + it('ignores intermittent IndexedDbTransactionError during lease refresh', async () => { + await withPersistence('clientA', async (db, _, queue) => { + db.injectFailures = true; + await queue.runDelayedOperationsEarly(TimerId.ClientMetadataRefresh); + await queue.enqueue(() => { + db.injectFailures = false; + return db.runTransaction('check success', 'readwrite-primary', () => + PersistencePromise.resolve() + ); + }); + }); + }); + it('rejects access when synchronization is disabled', async () => { - await withPersistence('clientA', async db1 => { + await withMultiClientPersistence('clientA', async db1 => { await expect( withPersistence('clientB', db2 => Promise.resolve()) ).to.eventually.be.rejectedWith( - 'Another tab has exclusive access to the persistence layer.' + 'Failed to obtain exclusive access to the persistence layer.' ); }); });