Allow failing of individual transactions

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_persistence.ts

@@ -63,6 +63,7 @@ import {
 } from './lru_garbage_collector';
 import {
   Persistence,
+  PersistenceAction,
   PersistenceTransaction,
   PersistenceTransactionMode,
   PRIMARY_LEASE_LOST_ERROR_MSG,
@@ -749,7 +750,7 @@ export class IndexedDbPersistence implements Persistence {
   }
 
   runTransaction<T>(
-    action: string,
+    action: PersistenceAction,
     mode: PersistenceTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction

packages/firestore/src/local/persistence.ts

@@ -31,6 +31,35 @@ export const PRIMARY_LEASE_LOST_ERROR_MSG =
   'The current tab is not in the required state to perform this operation. ' +
   'It might be necessary to refresh the browser tab.';
 
+/**
+ * The cumulative list of actions run against Persistence. This is used by the
+ * Spec tests to fail specific types of actions.
+ */
+export type PersistenceAction =
+  | 'Get next mutation batch'
+  | 'read document'
+  | 'Allocate target'
+  | 'Release target'
+  | 'Execute query'
+  | 'Handle user change'
+  | 'Locally write mutations'
+  | 'Acknowledge batch'
+  | 'Reject batch'
+  | 'Get highest unacknowledged batch id'
+  | 'Get last stream token'
+  | 'Set last stream token'
+  | 'Get last remote snapshot version'
+  | 'Set last remote snapshot version'
+  | 'Apply remote event'
+  | 'notifyLocalViewChanges'
+  | 'Remote document keys'
+  | 'Collect garbage'
+  | 'maybeGarbageCollectMultiClientState'
+  | 'Lookup mutation documents'
+  | 'Get target data'
+  | 'Get new document changes'
+  | 'Synchronize last document change read time';
+
 /**
  * A base class representing a persistence transaction, encapsulating both the
  * transaction's sequence numbers as well as a list of onCommitted listeners.
@@ -249,7 +278,7 @@ export interface Persistence {
    * @return A promise that is resolved once the transaction completes.
    */
   runTransaction<T>(
-    action: string,
+    action: PersistenceAction,
     mode: PersistenceTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction

packages/firestore/test/unit/specs/recovery_spec.test.ts

@@ -34,7 +34,11 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
           .client(1)
           .expectPrimaryState(false)
           .userSets('collection/a', { v: 1 })
-          .failDatabase()
+          .failDatabaseTransactions({
+            'Locally write mutations': true,
+            'Synchronize last document change read time': true,
+            'Lookup mutation documents': true
+          })
           .client(0)
           .writeAcks('collection/a', 1, { expectUserCallback: false })
           .client(1)
@@ -64,7 +68,11 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
         .client(1)
         .expectPrimaryState(false)
         .userListens(query)
-        .failDatabase()
+        .failDatabaseTransactions({
+          'Allocate target': true,
+          'Lookup mutation documents': true,
+          'Get new document changes': true
+        })
         .client(0)
         .expectListen(query)
         .watchAcksFull(query, 1000)
@@ -84,7 +92,10 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
       return (
         client(0)
           .expectPrimaryState(true)
-          .failDatabase()
+          .failDatabaseTransactions({
+            'Allocate target': true,
+            'Get target data': true
+          })
           .client(1)
           .userListens(query)
           .client(0)
@@ -94,7 +105,10 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
           .recoverDatabase()
           .runTimer(TimerId.AsyncQueueRetry)
           .expectListen(query)
-          .failDatabase()
+          .failDatabaseTransactions({
+            'Allocate target': true,
+            'Release target': true
+          })
           .client(1)
           .userUnlistens(query)
           .client(0)
@@ -113,7 +127,12 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
     return spec()
       .userSets('collection/key1', { foo: 'a' })
       .expectNumOutstandingWrites(1)
-      .failDatabase()
+      .failDatabaseTransactions({
+        'Locally write mutations': true,
+        notifyLocalViewChanges: true,
+        'Get next mutation batch': true,
+        'Get last stream token': true
+      })
       .userSets('collection/key2', { bar: 'b' })
       .expectUserCallbacks({ rejected: ['collection/key2'] })
       .recoverDatabase()
@@ -149,7 +168,11 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
         fromCache: true,
         hasPendingWrites: true
       })
-      .failDatabase()
+      .failDatabaseTransactions({
+        'Locally write mutations': true,
+        notifyLocalViewChanges: true,
+        'Get next mutation batch': true
+      })
       .userSets('collection/key2', { foo: 'b' })
       .expectUserCallbacks({ rejected: ['collection/key2'] })
       .recoverDatabase()

packages/firestore/test/unit/specs/spec_builder.ts

@@ -45,6 +45,7 @@ import {
   parseQuery,
   runSpec,
   SpecConfig,
+  SpecDatabaseFailures,
   SpecDocument,
   SpecQuery,
   SpecQueryFilter,
@@ -435,12 +436,15 @@ export class SpecBuilder {
     return this;
   }
 
-  /** Fails all database operations until `recoverDatabase()` is called. */
-  failDatabase(): this {
+  /**
+   * Fails the specified database transaction until `recoverDatabase()` is
+   * called.
+   */
+  failDatabaseTransactions(failureMode: SpecDatabaseFailures): this {
     this.nextStep();
     this.injectFailures = true;
     this.currentStep = {
-      failDatabase: true
+      failDatabase: failureMode
     };
     return this;
   }

packages/firestore/test/unit/specs/spec_test_components.ts

@@ -23,6 +23,7 @@ import {
 import {
   GarbageCollectionScheduler,
   Persistence,
+  PersistenceAction,
   PersistenceTransaction,
   PersistenceTransactionMode
 } from '../../../src/local/persistence';
@@ -36,6 +37,7 @@ import {
   MemoryPersistence
 } from '../../../src/local/memory_persistence';
 import { LruParams } from '../../../src/local/lru_garbage_collector';
+import { SpecDatabaseFailures } from './spec_test_runner';
 import { Connection, Stream } from '../../../src/remote/connection';
 import { StreamBridge } from '../../../src/remote/stream_bridge';
 import * as api from '../../../src/protos/firestore_proto_api';
@@ -56,22 +58,27 @@ import { expect } from 'chai';
  * transaction failures.
  */
 export class MockMemoryPersistence extends MemoryPersistence {
-  injectFailures = false;
+  injectFailures?: SpecDatabaseFailures;
 
   runTransaction<T>(
-    action: string,
+    action: PersistenceAction,
     mode: PersistenceTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
     if (this.injectFailures) {
-      return Promise.reject(
-        new IndexedDbTransactionError(new Error('Simulated retryable error'))
-      );
-    } else {
-      return super.runTransaction(action, mode, transactionOperation);
+      if (this.injectFailures[action] === undefined) {
+        throw fail('Failure mode not specified for action: ' + action);
+      }
+      if (this.injectFailures[action]) {
+        return Promise.reject(
+          new IndexedDbTransactionError(new Error('Simulated retryable error'))
+        );
+      }
     }
+
+    return super.runTransaction(action, mode, transactionOperation);
   }
 }
 
@@ -80,22 +87,26 @@ export class MockMemoryPersistence extends MemoryPersistence {
  * transaction failures.
  */
 export class MockIndexedDbPersistence extends IndexedDbPersistence {
-  injectFailures = false;
+  injectFailures?: SpecDatabaseFailures;
 
   runTransaction<T>(
-    action: string,
+    action: PersistenceAction,
     mode: PersistenceTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
     if (this.injectFailures) {
-      return Promise.reject(
-        new IndexedDbTransactionError(new Error('Simulated retryable error'))
-      );
-    } else {
-      return super.runTransaction(action, mode, transactionOperation);
+      if (this.injectFailures[action] === undefined) {
+        throw fail('Failure mode not specified for action: ' + action);
+      } else if (this.injectFailures[action]) {
+        return Promise.reject(
+          new IndexedDbTransactionError(new Error('Simulated retryable error'))
+        );
+      }
     }
+
+    return super.runTransaction(action, mode, transactionOperation);
   }
 }
 

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -113,6 +113,7 @@ import {
   SharedWriteTracker
 } from './spec_test_components';
 import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence';
+import { PersistenceAction } from '../../../src/local/persistence';
 
 const ARBITRARY_SEQUENCE_NUMBER = 2;
 
@@ -341,8 +342,8 @@ abstract class TestRunner {
     } else if ('changeUser' in step) {
       return this.doChangeUser(step.changeUser!);
     } else if ('failDatabase' in step) {
-      return step.failDatabase!
-        ? this.doFailDatabase()
+      return step.failDatabase
+        ? this.doFailDatabase(step.failDatabase!)
         : this.doRecoverDatabase();
     } else {
       return fail('Unknown step: ' + JSON.stringify(step));
@@ -446,7 +447,7 @@ abstract class TestRunner {
       () => this.rejectedDocs.push(...documentKeys)
     );
 
-    if (!this.persistence.injectFailures) {
+    if (this.persistence.injectFailures?.['Locally write mutations'] !== true) {
       this.sharedWrites.push(mutations);
     }
 
@@ -717,12 +718,14 @@ abstract class TestRunner {
     );
   }
 
-  private async doFailDatabase(): Promise<void> {
-    this.persistence.injectFailures = true;
+  private async doFailDatabase(
+    failActions: SpecDatabaseFailures
+  ): Promise<void> {
+    this.persistence.injectFailures = failActions;
   }
 
   private async doRecoverDatabase(): Promise<void> {
-    this.persistence.injectFailures = false;
+    this.persistence.injectFailures = undefined;
   }
 
   private validateExpectedSnapshotEvents(
@@ -1181,6 +1184,13 @@ export interface SpecConfig {
   maxConcurrentLimboResolutions?: number;
 }
 
+/** Specifies failure or success for a list of database actions. */
+export type SpecDatabaseFailures = Partial<
+  {
+    readonly [key in PersistenceAction]: boolean;
+  }
+>;
+
 /**
  * Union type for each step. The step consists of exactly one `field`
  * set and optionally expected events in the `expect` field.
@@ -1225,8 +1235,8 @@ export interface SpecStep {
   /** Fail a write */
   failWrite?: SpecWriteFailure;
 
-  /** Fail all database transactions. */
-  failDatabase?: boolean;
+  /** Fails the listed database actions. */
+  failDatabase?: false | SpecDatabaseFailures;
 
   /**
    * Run a queued timer task (without waiting for the delay to expire). See