diff --git a/packages/firestore/exp/src/api/transaction.ts b/packages/firestore/exp/src/api/transaction.ts index bad3afeda51..3a3a2bf6a2a 100644 --- a/packages/firestore/exp/src/api/transaction.ts +++ b/packages/firestore/exp/src/api/transaction.ts @@ -17,19 +17,16 @@ import { Transaction as LiteTransaction } from '../../../lite/src/api/transaction'; import { DocumentSnapshot } from './snapshot'; -import { TransactionRunner } from '../../../src/core/transaction_runner'; -import { AsyncQueue } from '../../../src/util/async_queue'; import { FirebaseFirestore } from './database'; -import { Deferred } from '../../../src/util/promise'; import { ensureFirestoreConfigured, SnapshotMetadata } from '../../../src/api/database'; import { Transaction as InternalTransaction } from '../../../src/core/transaction'; import { validateReference } from '../../../lite/src/api/write_batch'; -import { getDatastore } from '../../../lite/src/api/components'; import { DocumentReference } from '../../../lite/src/api/reference'; import { ExpUserDataWriter } from './reference'; +import { firestoreClientTransaction } from '../../../src/core/firestore_client'; /** * A reference to a transaction. @@ -97,18 +94,8 @@ export function runTransaction( firestore: FirebaseFirestore, updateFunction: (transaction: Transaction) => Promise ): Promise { - ensureFirestoreConfigured(firestore); - - const deferred = new Deferred(); - firestore._queue.enqueueAndForget(async () => { - const datastore = await getDatastore(firestore); - new TransactionRunner( - new AsyncQueue(), - datastore, - internalTransaction => - updateFunction(new Transaction(firestore, internalTransaction)), - deferred - ).run(); - }); - return deferred.promise; + const client = ensureFirestoreConfigured(firestore); + return firestoreClientTransaction(client, internalTransaction => + updateFunction(new Transaction(firestore, internalTransaction)) + ); } diff --git a/packages/firestore/exp/test/shim.ts b/packages/firestore/exp/test/shim.ts index 3fed8132aa7..deb6543ac11 100644 --- a/packages/firestore/exp/test/shim.ts +++ b/packages/firestore/exp/test/shim.ts @@ -16,19 +16,9 @@ */ import * as legacy from '@firebase/firestore-types'; -import * as exp from '../index'; import { FieldPath as FieldPathExp, Bytes as BytesExp } from '../../exp/index'; -import { - isPlainObject, - validateSetOptions -} from '../../src/util/input_validation'; import { Compat } from '../../src/compat/compat'; -import { - Firestore, - DocumentReference, - DocumentSnapshot -} from '../../src/api/database'; export { GeoPoint, Timestamp } from '../index'; @@ -38,72 +28,6 @@ export { GeoPoint, Timestamp } from '../index'; // of the experimental SDK. This shim is used to run integration tests against // both SDK versions. -export class Transaction - extends Compat - implements legacy.Transaction { - constructor( - private readonly _firestore: Firestore, - delegate: exp.Transaction - ) { - super(delegate); - } - - get(documentRef: DocumentReference): Promise> { - return this._delegate - .get(documentRef._delegate) - .then(result => new DocumentSnapshot(this._firestore, result)); - } - - set( - documentRef: DocumentReference, - data: T, - options?: legacy.SetOptions - ): Transaction { - if (options) { - validateSetOptions('Transaction.set', options); - this._delegate.set(documentRef._delegate, unwrap(data), options); - } else { - this._delegate.set(documentRef._delegate, unwrap(data)); - } - return this; - } - - update( - documentRef: DocumentReference, - data: legacy.UpdateData - ): Transaction; - update( - documentRef: DocumentReference, - field: string | FieldPath, - value: any, - ...moreFieldsAndValues: any[] - ): Transaction; - update( - documentRef: DocumentReference, - dataOrField: any, - value?: any, - ...moreFieldsAndValues: any[] - ): Transaction { - if (arguments.length === 2) { - this._delegate.update(documentRef._delegate, unwrap(dataOrField)); - } else { - this._delegate.update( - documentRef._delegate, - unwrap(dataOrField), - unwrap(value), - ...unwrap(moreFieldsAndValues) - ); - } - - return this; - } - - delete(documentRef: DocumentReference): Transaction { - this._delegate.delete(documentRef._delegate); - return this; - } -} - export class FieldPath extends Compat implements legacy.FieldPath { @@ -141,27 +65,3 @@ export class Blob extends Compat implements legacy.Blob { return this._delegate.isEqual(other._delegate); } } - -/** - * Takes user data that uses API types from this shim and replaces them - * with the the firestore-exp API types. - */ -function unwrap(value: any): any { - if (Array.isArray(value)) { - return value.map(v => unwrap(v)); - } else if (value instanceof Compat) { - return value._delegate; - } else if (value instanceof FieldPath) { - return value._delegate; - } else if (isPlainObject(value)) { - const obj: any = {}; - for (const key in value) { - if (value.hasOwnProperty(key)) { - obj[key] = unwrap(value[key]); - } - } - return obj; - } else { - return value; - } -} diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index f913e2143e8..dd420f55ae1 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -21,14 +21,11 @@ import { DatabaseId } from '../core/database_info'; import { FirestoreClient, firestoreClientGetNamedQuery, - firestoreClientLoadBundle, - firestoreClientTransaction + firestoreClientLoadBundle } from '../core/firestore_client'; -import { Transaction as InternalTransaction } from '../core/transaction'; -import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { FieldPath, ResourcePath } from '../model/path'; -import { debugAssert, fail } from '../util/assert'; +import { debugAssert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { cast, @@ -45,13 +42,7 @@ import { PartialObserver, Unsubscribe } from './observer'; -import { - parseSetData, - parseUpdateData, - parseUpdateVarargs, - UntypedFirestoreDataConverter, - UserDataReader -} from './user_data_reader'; +import { UntypedFirestoreDataConverter } from './user_data_reader'; import { UserDataWriter } from './user_data_writer'; import { clearIndexedDbPersistence, @@ -69,7 +60,7 @@ import { QuerySnapshot as ExpQuerySnapshot, snapshotEqual } from '../../exp/src/api/snapshot'; -import { newUserDataReader, refEqual } from '../../lite/src/api/reference'; +import { refEqual } from '../../lite/src/api/reference'; import { addDoc, collection, @@ -107,6 +98,10 @@ import { ApiLoadBundleTask, LoadBundleTask } from './bundle'; import { makeDatabaseInfo } from '../../lite/src/api/database'; import { DEFAULT_HOST } from '../../lite/src/api/components'; import { WriteBatch as ExpWriteBatch } from '../../exp/src/api/write_batch'; +import { + runTransaction, + Transaction as ExpTransaction +} from '../../exp/src/api/transaction'; import { CollectionReference as PublicCollectionReference, @@ -360,12 +355,8 @@ export class Firestore runTransaction( updateFunction: (transaction: PublicTransaction) => Promise ): Promise { - const client = ensureFirestoreConfigured(this._delegate); - return firestoreClientTransaction( - client, - (transaction: InternalTransaction) => { - return updateFunction(new Transaction(this, transaction)); - } + return runTransaction(this._delegate, transaction => + updateFunction(new Transaction(this, transaction)) ); } @@ -445,68 +436,23 @@ export function namedQuery( /** * A reference to a transaction. */ -export class Transaction implements PublicTransaction { - private _dataReader: UserDataReader; - +export class Transaction + extends Compat + implements PublicTransaction { constructor( - private _firestore: Firestore, - private _transaction: InternalTransaction + private readonly _firestore: Firestore, + delegate: ExpTransaction ) { - this._dataReader = newUserDataReader(this._firestore._delegate); + super(delegate); } get( documentRef: PublicDocumentReference ): Promise> { - const ref = validateReference( - 'Transaction.get', - documentRef, - this._firestore - ); - const userDataWriter = new UserDataWriter(this._firestore); - return this._transaction - .lookup([ref._key]) - .then((docs: MaybeDocument[]) => { - if (!docs || docs.length !== 1) { - return fail('Mismatch in docs returned from document lookup.'); - } - const doc = docs[0]; - if (doc instanceof NoDocument) { - return new DocumentSnapshot( - this._firestore, - new ExpDocumentSnapshot( - this._firestore._delegate, - userDataWriter, - ref._key, - null, - new SnapshotMetadata( - /*hasPendingWrites= */ false, - /* fromCache= */ false - ), - ref._converter - ) - ); - } else if (doc instanceof Document) { - return new DocumentSnapshot( - this._firestore, - new ExpDocumentSnapshot( - this._firestore._delegate, - userDataWriter, - ref._key, - doc, - new SnapshotMetadata( - /*hasPendingWrites= */ false, - /* fromCache= */ false - ), - ref._converter - ) - ); - } else { - throw fail( - `BatchGetDocumentsRequest returned unexpected document type: ${doc.constructor.name}` - ); - } - }); + const ref = castReference(documentRef); + return this._delegate + .get(ref) + .then(result => new DocumentSnapshot(this._firestore, result)); } set( @@ -517,35 +463,22 @@ export class Transaction implements PublicTransaction { set(documentRef: DocumentReference, data: T): Transaction; set( documentRef: PublicDocumentReference, - value: T | Partial, + data: T | Partial, options?: PublicSetOptions ): Transaction { - const ref = validateReference( - 'Transaction.set', - documentRef, - this._firestore - ); - options = validateSetOptions('Transaction.set', options); - const convertedValue = applyFirestoreDataConverter( - ref._converter, - value, - options - ); - const parsed = parseSetData( - this._dataReader, - 'Transaction.set', - ref._key, - convertedValue, - ref._converter !== null, - options - ); - this._transaction.set(ref._key, parsed); + const ref = castReference(documentRef); + if (options) { + validateSetOptions('Transaction.set', options); + this._delegate.set(ref, data, options); + } else { + this._delegate.set(ref, data); + } return this; } update( documentRef: PublicDocumentReference, - value: PublicUpdateData + data: PublicUpdateData ): Transaction; update( documentRef: PublicDocumentReference, @@ -555,55 +488,28 @@ export class Transaction implements PublicTransaction { ): Transaction; update( documentRef: PublicDocumentReference, - fieldOrUpdateData: string | PublicFieldPath | PublicUpdateData, + dataOrField: unknown, value?: unknown, ...moreFieldsAndValues: unknown[] ): Transaction { - const ref = validateReference( - 'Transaction.update', - documentRef, - this._firestore - ); - - // For Compat types, we have to "extract" the underlying types before - // performing validation. - if (fieldOrUpdateData instanceof Compat) { - fieldOrUpdateData = (fieldOrUpdateData as Compat)._delegate; - } - - let parsed; - if ( - typeof fieldOrUpdateData === 'string' || - fieldOrUpdateData instanceof ExpFieldPath - ) { - parsed = parseUpdateVarargs( - this._dataReader, - 'Transaction.update', - ref._key, - fieldOrUpdateData, - value, - moreFieldsAndValues - ); + const ref = castReference(documentRef); + if (arguments.length === 2) { + this._delegate.update(ref, dataOrField as PublicUpdateData); } else { - parsed = parseUpdateData( - this._dataReader, - 'Transaction.update', - ref._key, - fieldOrUpdateData + this._delegate.update( + ref, + dataOrField as string | ExpFieldPath, + value, + ...moreFieldsAndValues ); } - this._transaction.update(ref._key, parsed); return this; } delete(documentRef: PublicDocumentReference): Transaction { - const ref = validateReference( - 'Transaction.delete', - documentRef, - this._firestore - ); - this._transaction.delete(ref._key); + const ref = castReference(documentRef); + this._delegate.delete(ref); return this; } } @@ -1360,25 +1266,6 @@ function castReference( return cast>(documentRef, ExpDocumentReference); } -function validateReference( - methodName: string, - documentRef: PublicDocumentReference, - firestore: Firestore -): ExpDocumentReference { - const reference = cast>( - documentRef, - ExpDocumentReference - ); - if (reference.firestore !== firestore._delegate) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Provided document reference is from a different Firestore instance.' - ); - } else { - return reference; - } -} - /** * Converts custom model object of type T into DocumentData by applying the * converter if it exists.