firebase · schmidt-sebastian · Mar 20, 2020 · Mar 20, 2020
diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts
@@ -325,7 +325,7 @@ export class IndexedDbPersistence implements Persistence {
         this.scheduleClientMetadataAndPrimaryLeaseRefreshes();
 
         return this.simpleDb.runTransaction(
-          'readonly-idempotent',
+          'readonly',
           [DbTargetGlobal.store],
           txn => getHighestListenSequenceNumber(txn)
         );
@@ -388,7 +388,7 @@ export class IndexedDbPersistence implements Persistence {
    */
   private updateClientMetadataAndTryBecomePrimary(): Promise<void> {
     return this.simpleDb
-      .runTransaction('readwrite-idempotent', ALL_STORES, txn => {
+      .runTransaction('readwrite', ALL_STORES, txn => {
         const metadataStore = clientMetadataStore(txn);
         return metadataStore
           .put(
@@ -474,7 +474,7 @@ export class IndexedDbPersistence implements Persistence {
 
       const inactiveClients = await this.runTransaction(
         'maybeGarbageCollectMultiClientState',
-        'readwrite-primary-idempotent',
+        'readwrite-primary',
         txn => {
           const metadataStore = IndexedDbPersistence.getStore<
             DbClientMetadataKey,
@@ -658,7 +658,7 @@ export class IndexedDbPersistence implements Persistence {
     this.detachVisibilityHandler();
     this.detachWindowUnloadHook();
     await this.simpleDb.runTransaction(
-      'readwrite-idempotent',
+      'readwrite',
       [DbPrimaryClient.store, DbClientMetadata.store],
       txn => {
         return this.releasePrimaryLeaseIfHeld(txn).next(() =>
@@ -690,7 +690,7 @@ export class IndexedDbPersistence implements Persistence {
 
   getActiveClients(): Promise<ClientId[]> {
     return this.simpleDb.runTransaction(
-      'readonly-idempotent',
+      'readonly',
       [DbClientMetadata.store],
       txn => {
         return clientMetadataStore(txn)
@@ -762,16 +762,7 @@ export class IndexedDbPersistence implements Persistence {
   ): Promise<T> {
     log.debug(LOG_TAG, 'Starting transaction:', action);
 
-    // TODO(schmidt-sebastian): Simplify once all transactions are idempotent.
-    const idempotent = mode.endsWith('idempotent');
-    const readonly = mode.startsWith('readonly');
-    const simpleDbMode = readonly
-      ? idempotent
-        ? 'readonly-idempotent'
-        : 'readonly'
-      : idempotent
-      ? 'readwrite-idempotent'
-      : 'readwrite';
+    const simpleDbMode = mode === 'readonly' ? 'readonly' : 'readwrite';
 
     let persistenceTransaction: PersistenceTransaction;
 
@@ -784,10 +775,7 @@ export class IndexedDbPersistence implements Persistence {
           this.listenSequence.next()
         );
 
-        if (
-          mode === 'readwrite-primary' ||
-          mode === 'readwrite-primary-idempotent'
-        ) {
+        if (mode === 'readwrite-primary') {
           // While we merely verify that we have (or can acquire) the lease
           // immediately, we wait to extend the primary lease until after
           // executing transactionOperation(). This ensures that even if the

diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts
@@ -235,7 +235,7 @@ export class LocalStore {
 
     const result = await this.persistence.runTransaction(
       'Handle user change',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         // Swap out the mutation queue, grabbing the pending mutation batches
         // before and after.
@@ -310,10 +310,7 @@ export class LocalStore {
     let existingDocs: MaybeDocumentMap;
 
     return this.persistence
-      .runTransaction(
-        'Locally write mutations',
-        'readwrite-idempotent',
-        txn => {
+      .runTransaction('Locally write mutations', 'readwrite', txn => {
         // Load and apply all existing mutations. This lets us compute the
         // current base state for all non-idempotent transforms before applying
         // any additional user-provided writes.
@@ -353,8 +350,7 @@ export class LocalStore {
             mutations
           );
         });
-        }
-      )
+      })
       .then(batch => {
         const changes = batch.applyToLocalDocumentSet(existingDocs);
         return { batchId: batch.batchId, changes };
@@ -366,7 +362,7 @@ export class LocalStore {
   lookupMutationDocuments(batchId: BatchId): Promise<MaybeDocumentMap | null> {
     return this.persistence.runTransaction(
       'Lookup mutation documents',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         return this.mutationQueue
           .lookupMutationKeys(txn, batchId)
@@ -403,7 +399,7 @@ export class LocalStore {
   ): Promise<MaybeDocumentMap> {
     return this.persistence.runTransaction(
       'Acknowledge batch',
-      'readwrite-primary-idempotent',
+      'readwrite-primary',
       txn => {
         const affected = batchResult.batch.keys();
         const documentBuffer = this.remoteDocuments.newChangeBuffer({
@@ -430,7 +426,7 @@ export class LocalStore {
   rejectBatch(batchId: BatchId): Promise<MaybeDocumentMap> {
     return this.persistence.runTransaction(
       'Reject batch',
-      'readwrite-primary-idempotent',
+      'readwrite-primary',
       txn => {
         let affectedKeys: DocumentKeySet;
         return this.mutationQueue
@@ -457,7 +453,7 @@ export class LocalStore {
   getHighestUnacknowledgedBatchId(): Promise<BatchId> {
     return this.persistence.runTransaction(
       'Get highest unacknowledged batch id',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         return this.mutationQueue.getHighestUnacknowledgedBatchId(txn);
       }
@@ -468,7 +464,7 @@ export class LocalStore {
   getLastStreamToken(): Promise<ByteString> {
     return this.persistence.runTransaction(
       'Get last stream token',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         return this.mutationQueue.getLastStreamToken(txn);
       }
@@ -483,7 +479,7 @@ export class LocalStore {
   setLastStreamToken(streamToken: ByteString): Promise<void> {
     return this.persistence.runTransaction(
       'Set last stream token',
-      'readwrite-primary-idempotent',
+      'readwrite-primary',
       txn => {
         return this.mutationQueue.setLastStreamToken(txn, streamToken);
       }
@@ -497,7 +493,7 @@ export class LocalStore {
   getLastRemoteSnapshotVersion(): Promise<SnapshotVersion> {
     return this.persistence.runTransaction(
       'Get last remote snapshot version',
-      'readonly-idempotent',
+      'readonly',
       txn => this.targetCache.getLastRemoteSnapshotVersion(txn)
     );
   }
@@ -515,10 +511,7 @@ export class LocalStore {
     let newTargetDataByTargetMap = this.targetDataByTarget;
 
     return this.persistence
-      .runTransaction(
-        'Apply remote event',
-        'readwrite-primary-idempotent',
-        txn => {
+      .runTransaction('Apply remote event', 'readwrite-primary', txn => {
         const documentBuffer = this.remoteDocuments.newChangeBuffer({
           trackRemovals: true // Make sure document removals show up in `getNewDocumentChanges()`
         });
@@ -672,8 +665,7 @@ export class LocalStore {
               changedDocs
             );
           });
-        }
-      )
+      })
       .then(changedDocs => {
         this.targetDataByTarget = newTargetDataByTargetMap;
         return changedDocs;
@@ -763,7 +755,7 @@ export class LocalStore {
     }
     return this.persistence.runTransaction(
       'notifyLocalViewChanges',
-      'readwrite-idempotent',
+      'readwrite',
       txn => {
         return PersistencePromise.forEach(
           viewChanges,
@@ -788,7 +780,7 @@ export class LocalStore {
   nextMutationBatch(afterBatchId?: BatchId): Promise<MutationBatch | null> {
     return this.persistence.runTransaction(
       'Get next mutation batch',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         if (afterBatchId === undefined) {
           afterBatchId = BATCHID_UNKNOWN;
@@ -806,13 +798,9 @@ export class LocalStore {
    * found - used for testing.
    */
   readDocument(key: DocumentKey): Promise<MaybeDocument | null> {
-    return this.persistence.runTransaction(
-      'read document',
-      'readonly-idempotent',
-      txn => {
+    return this.persistence.runTransaction('read document', 'readonly', txn => {
       return this.localDocuments.getDocument(txn, key);
-      }
-    );
+    });
   }
 
   /**
@@ -825,7 +813,7 @@ export class LocalStore {
    */
   allocateTarget(target: Target): Promise<TargetData> {
     return this.persistence
-      .runTransaction('Allocate target', 'readwrite-idempotent', txn => {
+      .runTransaction('Allocate target', 'readwrite', txn => {
         let targetData: TargetData;
         return this.targetCache
           .getTargetData(txn, target)
@@ -900,9 +888,7 @@ export class LocalStore {
       `Tried to release nonexistent target: ${targetId}`
     );
 
-    const mode = keepPersistedTargetData
-      ? 'readwrite-idempotent'
-      : 'readwrite-primary-idempotent';
+    const mode = keepPersistedTargetData ? 'readwrite' : 'readwrite-primary';
     return this.persistence
       .runTransaction('Release target', mode, txn => {
         // References for documents sent via Watch are automatically removed
@@ -950,10 +936,7 @@ export class LocalStore {
     let lastLimboFreeSnapshotVersion = SnapshotVersion.MIN;
     let remoteKeys = documentKeySet();
 
-    return this.persistence.runTransaction(
-      'Execute query',
-      'readonly-idempotent',
-      txn => {
+    return this.persistence.runTransaction('Execute query', 'readonly', txn => {
       return this.getTargetData(txn, query.toTarget())
         .next(targetData => {
           if (targetData) {
@@ -979,8 +962,7 @@ export class LocalStore {
         .next(documents => {
           return { documents, remoteKeys };
         });
-      }
-    );
+    });
   }
 
   /**
@@ -990,7 +972,7 @@ export class LocalStore {
   remoteDocumentKeys(targetId: TargetId): Promise<DocumentKeySet> {
     return this.persistence.runTransaction(
       'Remote document keys',
-      'readonly-idempotent',
+      'readonly',
       txn => {
         return this.targetCache.getMatchingKeysForTargetId(txn, targetId);
       }
@@ -1060,7 +1042,7 @@ export class LocalStore {
   collectGarbage(garbageCollector: LruGarbageCollector): Promise<LruResults> {
     return this.persistence.runTransaction(
       'Collect garbage',
-      'readwrite-primary-idempotent',
+      'readwrite-primary',
       txn => garbageCollector.collect(txn, this.targetDataByTarget)
     );
   }
@@ -1074,7 +1056,7 @@ export class LocalStore {
     } else {
       return this.persistence.runTransaction(
         'Get target data',
-        'readonly-idempotent',
+        'readonly',
         txn => {
           return this.targetCache
             .getTargetDataForTarget(txn, targetId)
@@ -1093,7 +1075,7 @@ export class LocalStore {
   // PORTING NOTE: Multi-tab only.
   getNewDocumentChanges(): Promise<MaybeDocumentMap> {
     return this.persistence
-      .runTransaction('Get new document changes', 'readonly-idempotent', txn =>
+      .runTransaction('Get new document changes', 'readonly', txn =>
         this.remoteDocuments.getNewDocumentChanges(
           txn,
           this.lastDocumentChangeReadTime
@@ -1114,7 +1096,7 @@ export class LocalStore {
   async synchronizeLastDocumentChangeReadTime(): Promise<void> {
     this.lastDocumentChangeReadTime = await this.persistence.runTransaction(
       'Synchronize last document change read time',
-      'readonly-idempotent',
+      'readonly',
       txn => this.remoteDocuments.getLastReadTime(txn)
     );
   }

diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts
@@ -63,10 +63,7 @@ export abstract class PersistenceTransaction {
 export type PersistenceTransactionMode =
   | 'readonly'
   | 'readwrite'
-  | 'readwrite-primary'
-  | 'readonly-idempotent'
-  | 'readwrite-idempotent'
-  | 'readwrite-primary-idempotent';
+  | 'readwrite-primary';
 
 /**
  * Callback type for primary state notifications. This callback can be

diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts
@@ -32,11 +32,7 @@ const LOG_TAG = 'SimpleDb';
 const TRANSACTION_RETRY_COUNT = 3;
 
 // The different modes supported by `SimpleDb.runTransaction()`
-type SimpleDbTransactionMode =
-  | 'readonly'
-  | 'readwrite'
-  | 'readonly-idempotent'
-  | 'readwrite-idempotent';
+type SimpleDbTransactionMode = 'readonly' | 'readwrite';
 
 export interface SimpleDbSchemaConverter {
   createOrUpgrade(
@@ -276,8 +272,7 @@ export class SimpleDb {
     objectStores: string[],
     transactionFn: (transaction: SimpleDbTransaction) => PersistencePromise<T>
   ): Promise<T> {
-    const readonly = mode.startsWith('readonly');
-    const idempotent = mode.endsWith('idempotent');
+    const readonly = mode === 'readonly';
     let attemptNumber = 0;
 
     while (true) {
@@ -318,7 +313,6 @@ export class SimpleDb {
         // Note: We cannot use an instanceof check for FirestoreException, since the
         // exception is wrapped in a generic error by our async/await handling.
         const retryable =
-          idempotent &&
           error.name !== 'FirebaseError' &&
           attemptNumber < TRANSACTION_RETRY_COUNT;
         debug(

diff --git a/packages/firestore/test/unit/generate_spec_json.js b/packages/firestore/test/unit/generate_spec_json.js
@@ -65,7 +65,7 @@ function main(args) {
     var testName = specName.replace(/^specs\//, '');
     var filename = testName.replace(/[^A-Za-z\d]/g, '_') + '.json';
     var outputFile = outputPath + '/' + filename;
-    console.log("Generating " + outputFile);
+    console.log('Generating ' + outputFile);
     writeToJSON(testFiles[i], outputFile);
   }
 

diff --git a/packages/firestore/test/unit/local/encoded_resource_path.test.ts b/packages/firestore/test/unit/local/encoded_resource_path.test.ts
@@ -218,7 +218,7 @@ function runTransaction<T>(
     transaction: SimpleDbTransaction
   ) => PersistencePromise<T>
 ): Promise<T> {
-  return db.runTransaction<T>('readwrite-idempotent', ['test'], txn => {
+  return db.runTransaction<T>('readwrite', ['test'], txn => {
     return fn(txn.store<string, boolean>('test'), txn);
   });
 }
diff --git a/packages/firestore/test/unit/local/index_free_query_engine.test.ts b/packages/firestore/test/unit/local/index_free_query_engine.test.ts
@@ -144,10 +144,7 @@ describe('IndexFreeQueryEngine', () => {
       'Encountered runQuery() call not wrapped in expectIndexFreeQuery()/expectFullCollectionQuery()'
     );
 
-    return persistence.runTransaction(
-      'runQuery',
-      'readonly-idempotent',
-      txn => {
+    return persistence.runTransaction('runQuery', 'readonly', txn => {
       return targetCache
         .getMatchingKeysForTargetId(txn, TEST_TARGET_ID)
         .next(remoteKeys => {
@@ -167,8 +164,7 @@ describe('IndexFreeQueryEngine', () => {
               ).snapshot!.docs;
             });
         });
-      }
-    );
+    });
   }
 
   beforeEach(async () => {