Add test for getHighestListenSequenceNumber() recovery (#3083)

firebase · May 21, 2020 · babdcfd · babdcfd
1 parent 9d87d70
commit babdcfd
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 41 deletions.
diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts
@@ -45,13 +45,11 @@ import {
   DbPrimaryClient,
   DbPrimaryClientKey,
   DbTargetDocument,
-  DbTargetGlobal,
   SCHEMA_VERSION,
   SchemaConverter
 } from './indexeddb_schema';
 import {
   documentTargetStore,
-  getHighestListenSequenceNumber,
   IndexedDbTargetCache
 } from './indexeddb_target_cache';
 import { LocalSerializer } from './local_serializer';
@@ -306,10 +304,10 @@ export class IndexedDbPersistence implements Persistence {
 
         this.scheduleClientMetadataAndPrimaryLeaseRefreshes();
 
-        return this.simpleDb.runTransaction(
+        return this.runTransaction(
+          'getHighestListenSequenceNumber',
           'readonly',
-          [DbTargetGlobal.store],
-          txn => getHighestListenSequenceNumber(txn)
+          txn => this.targetCache.getHighestSequenceNumber(txn)
         );
       })
       .then(highestListenSequenceNumber => {

diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts
@@ -28,7 +28,6 @@ import {
   EncodedResourcePath
 } from './encoded_resource_path';
 import { removeMutationBatch } from './indexeddb_mutation_queue';
-import { getHighestListenSequenceNumber } from './indexeddb_target_cache';
 import { dbDocumentSize } from './indexeddb_remote_document_cache';
 import { LocalSerializer } from './local_serializer';
 import { MemoryCollectionParentIndex } from './memory_index_manager';
@@ -231,16 +230,23 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
     const documentsStore = txn.store<DbRemoteDocumentKey, DbRemoteDocument>(
       DbRemoteDocument.store
     );
+    const globalTargetStore = txn.store<DbTargetGlobalKey, DbTargetGlobal>(
+      DbTargetGlobal.store
+    );
 
-    return getHighestListenSequenceNumber(txn).next(currentSequenceNumber => {
+    return globalTargetStore.get(DbTargetGlobal.key).next(metadata => {
+      debugAssert(
+        !!metadata,
+        'Metadata should have been written during the version 3 migration'
+      );
       const writeSentinelKey = (
         path: ResourcePath
       ): PersistencePromise<void> => {
         return documentTargetStore.put(
           new DbTargetDocument(
             0,
             encodeResourcePath(path),
-            currentSequenceNumber
+            metadata!.highestListenSequenceNumber!
           )
         );
       };

diff --git a/packages/firestore/src/local/indexeddb_target_cache.ts b/packages/firestore/src/local/indexeddb_target_cache.ts
@@ -29,8 +29,7 @@ import {
 } from './encoded_resource_path';
 import {
   IndexedDbLruDelegate,
-  IndexedDbPersistence,
-  IndexedDbTransaction
+  IndexedDbPersistence
 } from './indexeddb_persistence';
 import {
   DbTarget,
@@ -46,7 +45,7 @@ import { PersistenceTransaction } from './persistence';
 import { PersistencePromise } from './persistence_promise';
 import { TargetCache } from './target_cache';
 import { TargetData } from './target_data';
-import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db';
+import { SimpleDbStore } from './simple_db';
 import { Target } from '../core/target';
 
 export class IndexedDbTargetCache implements TargetCache {
@@ -90,8 +89,8 @@ export class IndexedDbTargetCache implements TargetCache {
   getHighestSequenceNumber(
     transaction: PersistenceTransaction
   ): PersistencePromise<ListenSequenceNumber> {
-    return getHighestListenSequenceNumber(
-      (transaction as IndexedDbTransaction).simpleDbTransaction
+    return this.retrieveMetadata(transaction).next(
+      targetGlobal => targetGlobal.highestListenSequenceNumber
     );
   }
 
@@ -192,9 +191,12 @@ export class IndexedDbTargetCache implements TargetCache {
   private retrieveMetadata(
     transaction: PersistenceTransaction
   ): PersistencePromise<DbTargetGlobal> {
-    return retrieveMetadata(
-      (transaction as IndexedDbTransaction).simpleDbTransaction
-    );
+    return globalTargetStore(transaction)
+      .get(DbTargetGlobal.key)
+      .next(metadata => {
+        hardAssert(metadata !== null, 'Missing metadata row.');
+        return metadata;
+      });
   }
 
   private saveMetadata(
@@ -422,27 +424,6 @@ function globalTargetStore(
   );
 }
 
-function retrieveMetadata(
-  txn: SimpleDbTransaction
-): PersistencePromise<DbTargetGlobal> {
-  const globalStore = SimpleDb.getStore<DbTargetGlobalKey, DbTargetGlobal>(
-    txn,
-    DbTargetGlobal.store
-  );
-  return globalStore.get(DbTargetGlobal.key).next(metadata => {
-    hardAssert(metadata !== null, 'Missing metadata row.');
-    return metadata;
-  });
-}
-
-export function getHighestListenSequenceNumber(
-  txn: SimpleDbTransaction
-): PersistencePromise<ListenSequenceNumber> {
-  return retrieveMetadata(txn).next(
-    targetGlobal => targetGlobal.highestListenSequenceNumber
-  );
-}
-
 /**
  * Helper to get a typed SimpleDbStore for the document target object store.
  */

diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts
@@ -15,7 +15,9 @@
  * limitations under the License.
  */
 
-import { expect } from 'chai';
+import * as chaiAsPromised from 'chai-as-promised';
+
+import { expect, use } from 'chai';
 import { Query } from '../../../src/core/query';
 import { SnapshotVersion } from '../../../src/core/snapshot_version';
 import {
@@ -74,6 +76,8 @@ import {
   TEST_SERIALIZER
 } from './persistence_test_helpers';
 
+use(chaiAsPromised);
+
 /* eslint-disable no-restricted-globals */
 
 function withDb(
@@ -1165,6 +1169,19 @@ describe('IndexedDb: allowTabSynchronization', () => {
     );
   });
 
+  it('blocks start() when getHighestListenSequenceNumber() fails', async () => {
+    await withUnstartedCustomPersistence(
+      'clientA',
+      /* multiClient= */ false,
+      async db1 => {
+        db1.injectFailures = ['getHighestListenSequenceNumber'];
+        await expect(db1.start()).to.eventually.be.rejectedWith(
+          'IndexedDB transaction failed'
+        );
+      }
+    );
+  });
+
   it('ignores intermittent IndexedDbTransactionError during lease refresh', async () => {
     await withPersistence('clientA', async (db, _, queue) => {
       db.injectFailures = ['updateClientMetadataAndTryBecomePrimary'];

diff --git a/packages/firestore/test/unit/specs/recovery_spec.test.ts b/packages/firestore/test/unit/specs/recovery_spec.test.ts
@@ -464,12 +464,12 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
           fromCache: true,
           hasPendingWrites: true
         })
-        .failDatabaseTransactions({ 'Handle user change': true })
+        .failDatabaseTransactions('Handle user change')
         .changeUser('user2')
         .recoverDatabase()
         .runTimer(TimerId.AsyncQueueRetry)
         .expectEvents(query, { removed: [doc1], fromCache: true })
-        .failDatabaseTransactions({ 'Handle user change': true })
+        .failDatabaseTransactions('Handle user change')
         .changeUser('user1')
         .recoverDatabase()
         .runTimer(TimerId.AsyncQueueRetry)

diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts
@@ -1225,7 +1225,8 @@ export type PersistenceAction =
   | 'Get target data'
   | 'Get new document changes'
   | 'Synchronize last document change read time'
-  | 'updateClientMetadataAndTryBecomePrimary';
+  | 'updateClientMetadataAndTryBecomePrimary'
+  | 'getHighestListenSequenceNumber';
 
 /**
  * Union type for each step. The step consists of exactly one `field`