diff --git a/.changeset/lazy-nails-guess.md b/.changeset/lazy-nails-guess.md new file mode 100644 index 00000000000..9a7e7ae82e4 --- /dev/null +++ b/.changeset/lazy-nails-guess.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': patch +--- + +Add `TransactionOptions` param to `runTransaction` method diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index f026ffc2840..477962a737f 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -254,7 +254,7 @@ export class QuerySnapshot { export function refEqual(left: DocumentReference | CollectionReference, right: DocumentReference | CollectionReference): boolean; // @public -export function runTransaction(firestore: Firestore, updateFunction: (transaction: Transaction) => Promise): Promise; +export function runTransaction(firestore: Firestore, updateFunction: (transaction: Transaction) => Promise, options?: TransactionOptions): Promise; // @public export function serverTimestamp(): FieldValue; @@ -331,6 +331,11 @@ export class Transaction { update(documentRef: DocumentReference, field: string | FieldPath, value: unknown, ...moreFieldsAndValues: unknown[]): this; } +// @public +export interface TransactionOptions { + readonly maxAttempts?: number; +} + // @public export type UnionToIntersection = (U extends unknown ? (k: U) => void : never) extends (k: infer I) => void ? I : never; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 39828bda9ff..80a772e4db7 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -385,7 +385,7 @@ export class QuerySnapshot { export function refEqual(left: DocumentReference | CollectionReference, right: DocumentReference | CollectionReference): boolean; // @public -export function runTransaction(firestore: Firestore, updateFunction: (transaction: Transaction) => Promise): Promise; +export function runTransaction(firestore: Firestore, updateFunction: (transaction: Transaction) => Promise, options?: TransactionOptions): Promise; // @public export function serverTimestamp(): FieldValue; @@ -475,6 +475,11 @@ export class Transaction { update(documentRef: DocumentReference, field: string | FieldPath, value: unknown, ...moreFieldsAndValues: unknown[]): this; } +// @public +export interface TransactionOptions { + readonly maxAttempts?: number; +} + // @public export type UnionToIntersection = (U extends unknown ? (k: U) => void : never) extends (k: infer I) => void ? I : never; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 42e1554c92e..ad7ec7d8294 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -111,6 +111,8 @@ export { export { WriteBatch, writeBatch } from '../src/lite-api/write_batch'; +export { TransactionOptions } from '../src/lite-api/transaction_options'; + export { Transaction, runTransaction } from '../src/lite-api/transaction'; export { setLogLevel, LogLevelString as LogLevel } from '../src/util/log'; diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 810289c0d14..8e774dbfb30 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -89,6 +89,8 @@ export { export { Unsubscribe, SnapshotListenOptions } from './api/reference_impl'; +export { TransactionOptions } from './api/transaction_options'; + export { runTransaction, Transaction } from './api/transaction'; export { diff --git a/packages/firestore/src/api/transaction.ts b/packages/firestore/src/api/transaction.ts index 798f95cf4fa..ee5377b2fe8 100644 --- a/packages/firestore/src/api/transaction.ts +++ b/packages/firestore/src/api/transaction.ts @@ -17,6 +17,11 @@ import { firestoreClientTransaction } from '../core/firestore_client'; import { Transaction as InternalTransaction } from '../core/transaction'; +import { + TransactionOptions as TranasactionOptionsInternal, + DEFAULT_TRANSACTION_OPTIONS, + validateTransactionOptions +} from '../core/transaction_options'; import { DocumentReference } from '../lite-api/reference'; import { Transaction as LiteTransaction } from '../lite-api/transaction'; import { validateReference } from '../lite-api/write_batch'; @@ -25,6 +30,7 @@ import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; import { ExpUserDataWriter } from './reference_impl'; import { DocumentSnapshot, SnapshotMetadata } from './snapshot'; +import { TransactionOptions } from './transaction_options'; /** * A reference to a transaction. @@ -92,11 +98,20 @@ export class Transaction extends LiteTransaction { */ export function runTransaction( firestore: Firestore, - updateFunction: (transaction: Transaction) => Promise + updateFunction: (transaction: Transaction) => Promise, + options?: TransactionOptions ): Promise { firestore = cast(firestore, Firestore); + const optionsWithDefaults: TranasactionOptionsInternal = { + ...DEFAULT_TRANSACTION_OPTIONS, + ...options + }; + validateTransactionOptions(optionsWithDefaults); const client = ensureFirestoreConfigured(firestore); - return firestoreClientTransaction(client, internalTransaction => - updateFunction(new Transaction(firestore, internalTransaction)) + return firestoreClientTransaction( + client, + internalTransaction => + updateFunction(new Transaction(firestore, internalTransaction)), + optionsWithDefaults ); } diff --git a/packages/firestore/src/api/transaction_options.ts b/packages/firestore/src/api/transaction_options.ts new file mode 100644 index 00000000000..e369259918a --- /dev/null +++ b/packages/firestore/src/api/transaction_options.ts @@ -0,0 +1,18 @@ +/** + * @license + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { TransactionOptions } from '../lite-api/transaction_options'; diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 858b40929ad..7175737c59b 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -82,6 +82,7 @@ import { syncEngineWrite } from './sync_engine_impl'; import { Transaction } from './transaction'; +import { TransactionOptions } from './transaction_options'; import { TransactionRunner } from './transaction_runner'; import { View } from './view'; import { ViewSnapshot } from './view_snapshot'; @@ -483,7 +484,8 @@ export function firestoreClientAddSnapshotsInSyncListener( */ export function firestoreClientTransaction( client: FirestoreClient, - updateFunction: (transaction: Transaction) => Promise + updateFunction: (transaction: Transaction) => Promise, + options: TransactionOptions ): Promise { const deferred = new Deferred(); client.asyncQueue.enqueueAndForget(async () => { @@ -491,6 +493,7 @@ export function firestoreClientTransaction( new TransactionRunner( client.asyncQueue, datastore, + options, updateFunction, deferred ).run(); diff --git a/packages/firestore/src/core/transaction_options.ts b/packages/firestore/src/core/transaction_options.ts new file mode 100644 index 00000000000..a1d5b5a40ae --- /dev/null +++ b/packages/firestore/src/core/transaction_options.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Code, FirestoreError } from '../util/error'; + +export const DEFAULT_TRANSACTION_OPTIONS: TransactionOptions = { + maxAttempts: 5 +}; + +/** + * Options to customize transaction behavior. + */ +export declare interface TransactionOptions { + /** Maximum number of attempts to commit, after which transaction fails. Default is 5. */ + readonly maxAttempts: number; +} + +export function validateTransactionOptions(options: TransactionOptions): void { + if (options.maxAttempts < 1) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Max attempts must be at least 1' + ); + } +} diff --git a/packages/firestore/src/core/transaction_runner.ts b/packages/firestore/src/core/transaction_runner.ts index a9e17327aa7..d46f581b768 100644 --- a/packages/firestore/src/core/transaction_runner.ts +++ b/packages/firestore/src/core/transaction_runner.ts @@ -24,23 +24,24 @@ import { Deferred } from '../util/promise'; import { isNullOrUndefined } from '../util/types'; import { Transaction } from './transaction'; - -export const DEFAULT_MAX_ATTEMPTS_COUNT = 5; +import { TransactionOptions } from './transaction_options'; /** * TransactionRunner encapsulates the logic needed to run and retry transactions * with backoff. */ export class TransactionRunner { - private attemptsRemaining = DEFAULT_MAX_ATTEMPTS_COUNT; + private attemptsRemaining: number; private backoff: ExponentialBackoff; constructor( private readonly asyncQueue: AsyncQueue, private readonly datastore: Datastore, + private readonly options: TransactionOptions, private readonly updateFunction: (transaction: Transaction) => Promise, private readonly deferred: Deferred ) { + this.attemptsRemaining = options.maxAttempts; this.backoff = new ExponentialBackoff( this.asyncQueue, TimerId.TransactionRetry diff --git a/packages/firestore/src/lite-api/transaction.ts b/packages/firestore/src/lite-api/transaction.ts index 38297e8528f..af602582c5f 100644 --- a/packages/firestore/src/lite-api/transaction.ts +++ b/packages/firestore/src/lite-api/transaction.ts @@ -18,6 +18,11 @@ import { getModularInstance } from '@firebase/util'; import { Transaction as InternalTransaction } from '../core/transaction'; +import { + DEFAULT_TRANSACTION_OPTIONS, + TransactionOptions as TranasactionOptionsInternal, + validateTransactionOptions +} from '../core/transaction_options'; import { TransactionRunner } from '../core/transaction_runner'; import { fail } from '../util/assert'; import { newAsyncQueue } from '../util/async_queue_impl'; @@ -39,6 +44,7 @@ import { LiteUserDataWriter } from './reference_impl'; import { DocumentSnapshot } from './snapshot'; +import { TransactionOptions } from './transaction_options'; import { newUserDataReader, parseSetData, @@ -266,14 +272,21 @@ export class Transaction { */ export function runTransaction( firestore: Firestore, - updateFunction: (transaction: Transaction) => Promise + updateFunction: (transaction: Transaction) => Promise, + options?: TransactionOptions ): Promise { firestore = cast(firestore, Firestore); const datastore = getDatastore(firestore); + const optionsWithDefaults: TranasactionOptionsInternal = { + ...DEFAULT_TRANSACTION_OPTIONS, + ...options + }; + validateTransactionOptions(optionsWithDefaults); const deferred = new Deferred(); new TransactionRunner( newAsyncQueue(), datastore, + optionsWithDefaults, internalTransaction => updateFunction(new Transaction(firestore, internalTransaction)), deferred diff --git a/packages/firestore/src/lite-api/transaction_options.ts b/packages/firestore/src/lite-api/transaction_options.ts new file mode 100644 index 00000000000..d31b9127fc3 --- /dev/null +++ b/packages/firestore/src/lite-api/transaction_options.ts @@ -0,0 +1,24 @@ +/** + * @license + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Options to customize transaction behavior. + */ +export declare interface TransactionOptions { + /** Maximum number of attempts to commit, after which transaction fails. Default is 5. */ + readonly maxAttempts?: number; +} diff --git a/packages/firestore/test/integration/api_internal/transaction.test.ts b/packages/firestore/test/integration/api_internal/transaction.test.ts index f8665094b6b..7eab6d7d53d 100644 --- a/packages/firestore/test/integration/api_internal/transaction.test.ts +++ b/packages/firestore/test/integration/api_internal/transaction.test.ts @@ -17,16 +17,16 @@ import { expect } from 'chai'; -import { DEFAULT_MAX_ATTEMPTS_COUNT } from '../../../src/core/transaction_runner'; +import { DEFAULT_TRANSACTION_OPTIONS } from '../../../src/core/transaction_options'; import { TimerId } from '../../../src/util/async_queue'; import { Deferred } from '../../util/promise'; import { collection, doc, - FirestoreError, getDoc, runTransaction, - setDoc + setDoc, + TransactionOptions } from '../util/firebase_export'; import { apiDescribe, withTestDb } from '../util/helpers'; import { asyncQueue } from '../util/internal_helpers'; @@ -34,7 +34,7 @@ import { asyncQueue } from '../util/internal_helpers'; apiDescribe( 'Database transactions (with internal API)', (persistence: boolean) => { - it('increment transactionally', () => { + it('should increment transactionally', async () => { // A set of concurrent transactions. const transactionPromises: Array> = []; const readPromises: Array> = []; @@ -42,55 +42,45 @@ apiDescribe( const barrier = new Deferred(); let started = 0; - return withTestDb(persistence, db => { + await withTestDb(persistence, async db => { asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry); const docRef = doc(collection(db, 'counters')); - return setDoc(docRef, { - count: 5 - }) - .then(() => { - // Make 3 transactions that will all increment. - for (let i = 0; i < 3; i++) { - const resolveRead = new Deferred(); - readPromises.push(resolveRead.promise); - transactionPromises.push( - runTransaction(db, transaction => { - return transaction.get(docRef).then(snapshot => { - expect(snapshot).to.exist; - started = started + 1; - resolveRead.resolve(); - return barrier.promise.then(() => { - transaction.set(docRef, { - count: snapshot.data()!['count'] + 1 - }); - }); - }); - }) - ); - } + await setDoc(docRef, { count: 5 }); + // Make 3 transactions that will all increment. + for (let i = 0; i < 3; i++) { + const resolveRead = new Deferred(); + readPromises.push(resolveRead.promise); + transactionPromises.push( + runTransaction(db, async transaction => { + const snapshot = await transaction.get(docRef); + expect(snapshot).to.exist; + started += 1; + resolveRead.resolve(); + await barrier.promise; + transaction.set(docRef, { + count: snapshot.data()!['count'] + 1 + }); + }) + ); + } - // Let all of the transactions fetch the old value and stop once. - return Promise.all(readPromises); - }) - .then(() => { - // Let all of the transactions continue and wait for them to - // finish. - expect(started).to.equal(3); - barrier.resolve(); - return Promise.all(transactionPromises); - }) - .then(() => { - // Now all transaction should be completed, so check the result. - return getDoc(docRef); - }) - .then(snapshot => { - expect(snapshot).to.exist; - expect(snapshot.data()!['count']).to.equal(8); - }); + // Let all of the transactions fetch the old value and stop once. + await Promise.all(readPromises); + + // Let all of the transactions continue and wait for them to + // finish. + expect(started).to.equal(3); + barrier.resolve(); + await Promise.all(transactionPromises); + + // Now all transaction should be completed, so check the result. + const snapshot = await getDoc(docRef); + expect(snapshot).to.exist; + expect(snapshot.data()!['count']).to.equal(8); }); }); - it('update transactionally', () => { + it('should update transactionally', async () => { // A set of concurrent transactions. const transactionPromises: Array> = []; const readPromises: Array> = []; @@ -98,96 +88,119 @@ apiDescribe( const barrier = new Deferred(); let counter = 0; - return withTestDb(persistence, db => { + await withTestDb(persistence, async db => { asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry); const docRef = doc(collection(db, 'counters')); - return setDoc(docRef, { + await setDoc(docRef, { count: 5, other: 'yes' - }) - .then(() => { - // Make 3 transactions that will all increment. - for (let i = 0; i < 3; i++) { - const resolveRead = new Deferred(); - readPromises.push(resolveRead.promise); - transactionPromises.push( - runTransaction(db, transaction => { - return transaction.get(docRef).then(snapshot => { - expect(snapshot).to.exist; - counter = counter + 1; - resolveRead.resolve(); - return barrier.promise.then(() => { - transaction.update(docRef, { - count: snapshot.data()!['count'] + 1 - }); - }); - }); - }) - ); - } + }); + // Make 3 transactions that will all increment. + for (let i = 0; i < 3; i++) { + const resolveRead = new Deferred(); + readPromises.push(resolveRead.promise); + transactionPromises.push( + runTransaction(db, async transaction => { + const snapshot = await transaction.get(docRef); + expect(snapshot).to.exist; + counter += 1; + resolveRead.resolve(); + await barrier.promise; + await transaction.update(docRef, { + count: snapshot.data()!['count'] + 1 + }); + }) + ); + } + + // Let all of the transactions fetch the old value and stop once. + await Promise.all(readPromises); + + // Let all of the transactions continue and wait for them to + // finish. There should be 3 initial transaction runs. + expect(counter).to.equal(3); + barrier.resolve(); + await Promise.all(transactionPromises); + + // Now all transaction should be completed, so check the result. + // There should be a maximum of 3 retries: once for the 2nd update, + // and twice for the 3rd update. + expect(counter).to.be.lessThan(7); + const snapshot = await getDoc(docRef); + expect(snapshot).to.exist; + expect(snapshot.data()!['count']).to.equal(8); + expect(snapshot.data()!['other']).to.equal('yes'); + }); + }); - // Let all of the transactions fetch the old value and stop once. - return Promise.all(readPromises); - }) - .then(() => { - // Let all of the transactions continue and wait for them to - // finish. There should be 3 initial transaction runs. - expect(counter).to.equal(3); - barrier.resolve(); - return Promise.all(transactionPromises); - }) - .then(() => { - // Now all transaction should be completed, so check the result. - // There should be a maximum of 3 retries: once for the 2nd update, - // and twice for the 3rd update. - expect(counter).to.be.lessThan(7); - return getDoc(docRef); - }) - .then(snapshot => { - expect(snapshot).to.exist; - expect(snapshot.data()!['count']).to.equal(8); - expect(snapshot.data()!['other']).to.equal('yes'); + it('should fail transaction (maxAttempts: default) when reading a doc twice with different versions', async () => { + await withTestDb(persistence, async db => { + asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry); + const docRef = doc(collection(db, 'counters')); + let counter = 0; + await setDoc(docRef, { count: 15 }); + try { + await runTransaction(db, async transaction => { + counter++; + // Get the docRef once. + await transaction.get(docRef); + // Do a write outside of the transaction. Because the transaction + // will retry, set the document to a different value each time. + await setDoc(docRef, { count: 1234 + counter }); + // Get the docRef again in the transaction with the new + // version. + await transaction.get(docRef); + // Now try to update the docRef from within the transaction. + // This should fail, because we read 15 earlier. + await transaction.set(docRef, { count: 16 }); }); + expect.fail('transaction should fail'); + } catch (err) { + expect(err).to.exist; + expect(err.code).to.equal('aborted'); + } + const snapshot = await getDoc(docRef); + expect(snapshot.data()!['count']).to.equal(1234 + counter); + expect(counter).to.equal(DEFAULT_TRANSACTION_OPTIONS.maxAttempts); }); }); - it('handle reading a doc twice with different versions', () => { - return withTestDb(persistence, db => { + it('should fail transaction (maxAttempts: 1) when reading a doc twice with different versions', async () => { + await withTestDb(persistence, async db => { asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry); const docRef = doc(collection(db, 'counters')); + const options: TransactionOptions = { + maxAttempts: 1 + }; let counter = 0; - return setDoc(docRef, { - count: 15 - }) - .then(() => { - return runTransaction(db, transaction => { + await setDoc(docRef, { count: 15 }); + try { + await runTransaction( + db, + async transaction => { counter++; // Get the docRef once. - return ( - transaction - .get(docRef) - // Do a write outside of the transaction. Because the transaction - // will retry, set the document to a different value each time. - .then(() => setDoc(docRef, { count: 1234 + counter })) - // Get the docRef again in the transaction with the new - // version. - .then(() => transaction.get(docRef)) - // Now try to update the docRef from within the transaction. - // This should fail, because we read 15 earlier. - .then(() => transaction.set(docRef, { count: 16 })) - ); - }); - }) - .then(() => expect.fail('transaction should fail')) - .catch((err: FirestoreError) => { - expect(err).to.exist; - expect(err.code).to.equal('aborted'); - }) - .then(() => getDoc(docRef)) - .then(snapshot => { - expect(snapshot.data()!['count']).to.equal(1234 + counter); - expect(counter).to.equal(DEFAULT_MAX_ATTEMPTS_COUNT); - }); + await transaction.get(docRef); + // Do a write outside of the transaction. Because the transaction + // will retry, set the document to a different value each time. + await setDoc(docRef, { count: 1234 + counter }); + // Get the docRef again in the transaction with the new + // version. + await transaction.get(docRef); + // Now try to update the docRef from within the transaction. + // This should fail, because we read 15 earlier. + await transaction.set(docRef, { count: 16 }); + }, + options + ); + expect.fail('transaction should fail'); + } catch (err) { + expect(err).to.exist; + expect(err.code).to.equal('aborted'); + } + const snapshot = await getDoc(docRef); + expect(snapshot.data()!['count']).to.equal(1234 + counter); + expect(counter).to.equal(options.maxAttempts); }); }); }