From 484e90a1d8f63e04268ff5bce4e3e0873c56c8e1 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 9 Nov 2020 14:35:40 -0800 Subject: [PATCH] Compat class for DocumentReference (#4043) --- .changeset/short-mangos-beg.md | 5 + packages/firestore/exp/src/api/reference.ts | 10 +- packages/firestore/exp/src/api/snapshot.ts | 9 +- packages/firestore/exp/test/shim.ts | 197 +-------- packages/firestore/lite/src/api/reference.ts | 30 +- packages/firestore/lite/src/api/snapshot.ts | 11 +- packages/firestore/src/api/database.ts | 375 ++++++++++-------- packages/firestore/src/api/observer.ts | 2 +- .../firestore/src/api/user_data_reader.ts | 31 +- .../firestore/src/api/user_data_writer.ts | 7 +- .../firestore/src/util/input_validation.ts | 5 + packages/firestore/test/util/api_helpers.ts | 2 +- packages/firestore/test/util/helpers.ts | 6 +- 13 files changed, 266 insertions(+), 424 deletions(-) create mode 100644 .changeset/short-mangos-beg.md diff --git a/.changeset/short-mangos-beg.md b/.changeset/short-mangos-beg.md new file mode 100644 index 00000000000..2b140b68814 --- /dev/null +++ b/.changeset/short-mangos-beg.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Internal changes to support upcoming modular API. diff --git a/packages/firestore/exp/src/api/reference.ts b/packages/firestore/exp/src/api/reference.ts index 06cc7a5094c..906b860fd91 100644 --- a/packages/firestore/exp/src/api/reference.ts +++ b/packages/firestore/exp/src/api/reference.ts @@ -17,7 +17,6 @@ import { FirebaseFirestore } from './database'; import { - _DocumentKeyReference, ParsedUpdateData, parseSetData, parseUpdateData, @@ -80,6 +79,7 @@ import { removeSnapshotsInSyncListener } from '../../../src/core/event_manager'; import { FirestoreError } from '../../../src/util/error'; +import { Compat } from '../../../src/compat/compat'; /** * An options object that can be passed to {@link onSnapshot()} and {@link @@ -374,6 +374,12 @@ export function updateDoc( const dataReader = newUserDataReader(firestore); + // For Compat types, we have to "extract" the underlying types before + // performing validation. + if (fieldOrUpdateData instanceof Compat) { + fieldOrUpdateData = fieldOrUpdateData._delegate; + } + let parsed: ParsedUpdateData; if ( typeof fieldOrUpdateData === 'string' || @@ -821,7 +827,7 @@ export function executeWrite( */ function convertToDocSnapshot( firestore: FirebaseFirestore, - ref: _DocumentKeyReference, + ref: DocumentReference, snapshot: ViewSnapshot ): DocumentSnapshot { debugAssert( diff --git a/packages/firestore/exp/src/api/snapshot.ts b/packages/firestore/exp/src/api/snapshot.ts index 7002d725b39..cd064784b6a 100644 --- a/packages/firestore/exp/src/api/snapshot.ts +++ b/packages/firestore/exp/src/api/snapshot.ts @@ -238,11 +238,7 @@ export class DocumentSnapshot extends LiteDocumentSnapshot< this._firestoreImpl._databaseId, options?.serverTimestamps || DEFAULT_SERVER_TIMESTAMP_BEHAVIOR, key => - new DocumentReference( - this._firestore, - /* converter= */ null, - key.path - ), + new DocumentReference(this._firestore, /* converter= */ null, key), bytes => new Bytes(bytes) ); return userDataWriter.convertValue(this._document.toProto()) as T; @@ -276,8 +272,7 @@ export class DocumentSnapshot extends LiteDocumentSnapshot< const userDataWriter = new UserDataWriter( this._firestoreImpl._databaseId, options.serverTimestamps || DEFAULT_SERVER_TIMESTAMP_BEHAVIOR, - key => - new DocumentReference(this._firestore, this._converter, key.path), + key => new DocumentReference(this._firestore, this._converter, key), bytes => new Bytes(bytes) ); return userDataWriter.convertValue(value); diff --git a/packages/firestore/exp/test/shim.ts b/packages/firestore/exp/test/shim.ts index e7d4bb2491c..5c18197d47e 100644 --- a/packages/firestore/exp/test/shim.ts +++ b/packages/firestore/exp/test/shim.ts @@ -20,14 +20,8 @@ import * as exp from '../index'; import { addDoc, - collection, - deleteDoc, doc, - DocumentReference as DocumentReferenceExp, FieldPath as FieldPathExp, - getDoc, - getDocFromCache, - getDocFromServer, getDocs, getDocsFromCache, getDocsFromServer, @@ -35,9 +29,7 @@ import { query, queryEqual, refEqual, - setDoc, snapshotEqual, - updateDoc, endAt, endBefore, startAfter, @@ -49,13 +41,17 @@ import { Bytes as BytesExp } from '../../exp/index'; import { UntypedFirestoreDataConverter } from '../../src/api/user_data_reader'; -import { isPartialObserver, PartialObserver } from '../../src/api/observer'; import { isPlainObject, validateSetOptions } from '../../src/util/input_validation'; import { Compat } from '../../src/compat/compat'; -import { Firestore } from '../../src/api/database'; +import { + Firestore, + DocumentReference, + wrapObserver, + extractSnapshotOptions +} from '../../src/api/database'; export { GeoPoint, Timestamp } from '../index'; @@ -188,129 +184,6 @@ export class WriteBatch } } -export class DocumentReference - extends Compat> - implements legacy.DocumentReference { - constructor( - readonly firestore: Firestore, - delegate: exp.DocumentReference - ) { - super(delegate); - } - - readonly id = this._delegate.id; - readonly path = this._delegate.path; - - get parent(): legacy.CollectionReference { - return new CollectionReference(this.firestore, this._delegate.parent); - } - - collection( - collectionPath: string - ): legacy.CollectionReference { - return new CollectionReference( - this.firestore, - collection(this._delegate, collectionPath) - ); - } - - isEqual(other: DocumentReference): boolean { - return refEqual(this._delegate, other._delegate); - } - - set(data: Partial, options?: legacy.SetOptions): Promise { - if (options) { - validateSetOptions('DocumentReference.set', options); - return setDoc(this._delegate, unwrap(data), options); - } else { - return setDoc(this._delegate, unwrap(data)); - } - } - - update(data: legacy.UpdateData): Promise; - update( - field: string | FieldPath, - value: any, - ...moreFieldsAndValues: any[] - ): Promise; - update( - dataOrField: any, - value?: any, - ...moreFieldsAndValues: any[] - ): Promise { - if (arguments.length === 1) { - return updateDoc(this._delegate, unwrap(dataOrField)); - } else { - return updateDoc( - this._delegate, - unwrap(dataOrField), - unwrap(value), - ...unwrap(moreFieldsAndValues) - ); - } - } - - delete(): Promise { - return deleteDoc(this._delegate); - } - - get(options?: legacy.GetOptions): Promise> { - let snap: Promise>; - if (options?.source === 'cache') { - snap = getDocFromCache(this._delegate); - } else if (options?.source === 'server') { - snap = getDocFromServer(this._delegate); - } else { - snap = getDoc(this._delegate); - } - return snap.then(result => new DocumentSnapshot(this.firestore, result)); - } - - onSnapshot(observer: { - next?: (snapshot: DocumentSnapshot) => void; - error?: (error: legacy.FirestoreError) => void; - complete?: () => void; - }): () => void; - onSnapshot( - options: legacy.SnapshotListenOptions, - observer: { - next?: (snapshot: DocumentSnapshot) => void; - error?: (error: legacy.FirestoreError) => void; - complete?: () => void; - } - ): () => void; - onSnapshot( - onNext: (snapshot: DocumentSnapshot) => void, - onError?: (error: legacy.FirestoreError) => void, - onCompletion?: () => void - ): () => void; - onSnapshot( - options: legacy.SnapshotListenOptions, - onNext: (snapshot: DocumentSnapshot) => void, - onError?: (error: legacy.FirestoreError) => void, - onCompletion?: () => void - ): () => void; - onSnapshot(...args: any): () => void { - const options = extractSnapshotOptions(args); - const observer = wrapObserver, exp.DocumentSnapshot>( - args, - snap => new DocumentSnapshot(this.firestore, snap) - ); - return onSnapshot(this._delegate, options, observer); - } - - withConverter( - converter: legacy.FirestoreDataConverter - ): DocumentReference { - return new DocumentReference( - this.firestore, - this._delegate.withConverter( - converter as UntypedFirestoreDataConverter - ) - ); - } -} - export class DocumentSnapshot extends Compat> implements legacy.DocumentSnapshot { @@ -634,8 +507,6 @@ function wrap(firestore: Firestore, value: any): any { return new FieldPath(...value._internalPath.toArray()); } else if (value instanceof BytesExp) { return new Blob(value); - } else if (value instanceof DocumentReferenceExp) { - return new DocumentReference(firestore, value); } else if (isPlainObject(value)) { const obj: any = {}; for (const key in value) { @@ -672,59 +543,3 @@ function unwrap(value: any): any { return value; } } - -/** - * Creates an observer that can be passed to the firestore-exp SDK. The - * observer converts all observed values into the format expected by the shim. - * - * @param args The list of arguments from an `onSnapshot` call. - * @param wrapper The function that converts the firestore-exp type into the - * type used by this shim. - */ -function wrapObserver( - args: any, - wrapper: (val: ExpType) => ShimType -): PartialObserver { - let userObserver: PartialObserver; - if (isPartialObserver(args[0])) { - userObserver = args[0] as PartialObserver; - } else if (isPartialObserver(args[1])) { - userObserver = args[1]; - } else if (typeof args[0] === 'function') { - userObserver = { - next: args[0], - error: args[1], - complete: args[2] - }; - } else { - userObserver = { - next: args[1], - error: args[2], - complete: args[3] - }; - } - - return { - next: val => { - if (userObserver!.next) { - userObserver!.next(wrapper(val)); - } - }, - error: userObserver.error?.bind(userObserver), - complete: userObserver.complete?.bind(userObserver) - }; -} - -/** - * Iterates the list of arguments from an `onSnapshot` call and returns the - * first argument that may be an `SnapshotListenOptions` object. Returns an - * empty object if none is found. - */ -function extractSnapshotOptions(args: any): exp.SnapshotListenOptions { - for (const arg of args) { - if (typeof arg === 'object' && !isPartialObserver(arg)) { - return arg as exp.SnapshotListenOptions; - } - } - return {}; -} diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts index bd899a4edb7..df2a62db817 100644 --- a/packages/firestore/lite/src/api/reference.ts +++ b/packages/firestore/lite/src/api/reference.ts @@ -19,7 +19,6 @@ import { Document } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; import { FirebaseFirestore } from './database'; import { - _DocumentKeyReference, ParsedUpdateData, parseSetData, parseUpdateData, @@ -125,9 +124,7 @@ export type SetOptions = * and can be used to write, read, or listen to the location. The document at * the referenced location may or may not exist. */ -export class DocumentReference extends _DocumentKeyReference< - T -> { +export class DocumentReference { /** The type of this Firestore reference. */ readonly type = 'document'; @@ -139,18 +136,21 @@ export class DocumentReference extends _DocumentKeyReference< constructor( firestore: FirebaseFirestore, - _converter: FirestoreDataConverter | null, - readonly _path: ResourcePath + readonly _converter: FirestoreDataConverter | null, + readonly _key: DocumentKey ) { - super(firestore._databaseId, new DocumentKey(_path), _converter); this.firestore = firestore; } + get _path(): ResourcePath { + return this._key.path; + } + /** * The document's identifier within its collection. */ get id(): string { - return this._path.lastSegment(); + return this._key.path.lastSegment(); } /** @@ -158,7 +158,7 @@ export class DocumentReference extends _DocumentKeyReference< * to the root of the database). */ get path(): string { - return this._path.canonicalString(); + return this._key.path.canonicalString(); } /** @@ -183,7 +183,7 @@ export class DocumentReference extends _DocumentKeyReference< * @return A `DocumentReference` that uses the provided converter. */ withConverter(converter: FirestoreDataConverter): DocumentReference { - return new DocumentReference(this.firestore, converter, this._path); + return new DocumentReference(this.firestore, converter, this._key); } } @@ -653,7 +653,7 @@ export class CollectionReference extends Query { return new DocumentReference( this.firestore, /* converter= */ null, - parentPath + new DocumentKey(parentPath) ); } } @@ -868,7 +868,11 @@ export function doc( if (parent instanceof FirebaseFirestore) { const absolutePath = ResourcePath.fromString(path, ...pathSegments); validateDocumentPath(absolutePath); - return new DocumentReference(parent, /* converter= */ null, absolutePath); + return new DocumentReference( + parent, + /* converter= */ null, + new DocumentKey(absolutePath) + ); } else { if ( !(parent instanceof DocumentReference) && @@ -887,7 +891,7 @@ export function doc( return new DocumentReference( parent.firestore, parent instanceof CollectionReference ? parent._converter : null, - absolutePath + new DocumentKey(absolutePath) ); } } diff --git a/packages/firestore/lite/src/api/snapshot.ts b/packages/firestore/lite/src/api/snapshot.ts index cff40b44c0b..d4c01415075 100644 --- a/packages/firestore/lite/src/api/snapshot.ts +++ b/packages/firestore/lite/src/api/snapshot.ts @@ -135,7 +135,7 @@ export class DocumentSnapshot { return new DocumentReference( this._firestore, this._converter, - this._key.path + this._key ); } @@ -173,11 +173,7 @@ export class DocumentSnapshot { this._firestore._databaseId, /* serverTimestampBehavior=*/ 'none', key => - new DocumentReference( - this._firestore, - /* converter= */ null, - key.path - ), + new DocumentReference(this._firestore, /* converter= */ null, key), bytes => new Bytes(bytes) ); return userDataWriter.convertValue(this._document.toProto()) as T; @@ -204,8 +200,7 @@ export class DocumentSnapshot { const userDataWriter = new UserDataWriter( this._firestore._databaseId, /* serverTimestampBehavior=*/ 'none', - key => - new DocumentReference(this._firestore, this._converter, key.path), + key => new DocumentReference(this._firestore, this._converter, key), bytes => new Bytes(bytes) ); return userDataWriter.convertValue(value); diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 73905859ada..02161b9f87d 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -24,10 +24,8 @@ import { DatabaseId } from '../core/database_info'; import { ListenOptions } from '../core/event_manager'; import { FirestoreClient, - firestoreClientGetDocumentFromLocalCache, firestoreClientGetDocumentsFromLocalCache, firestoreClientGetDocumentsViaSnapshotListener, - firestoreClientGetDocumentViaSnapshotListener, firestoreClientListen, firestoreClientTransaction, firestoreClientWrite @@ -87,7 +85,6 @@ import { Unsubscribe } from './observer'; import { - _DocumentKeyReference, fieldPathFromArgument, parseQueryValue, parseSetData, @@ -108,7 +105,22 @@ import { waitForPendingWrites, FirebaseFirestore as ExpFirebaseFirestore } from '../../exp/src/api/database'; -import { onSnapshotsInSync } from '../../exp/src/api/reference'; +import { + DocumentReference as ExpDocumentReference, + refEqual, + newUserDataReader +} from '../../lite/src/api/reference'; +import { + onSnapshotsInSync, + setDoc, + updateDoc, + deleteDoc, + getDocFromCache, + getDocFromServer, + getDoc, + onSnapshot +} from '../../exp/src/api/reference'; +import { DocumentSnapshot as ExpDocumentSnapshot } from '../../exp/src/api/snapshot'; import { LRU_COLLECTION_DISABLED } from '../local/lru_garbage_collector'; import { Compat } from '../compat/compat'; @@ -139,7 +151,7 @@ import { WhereFilterOp as PublicWhereFilterOp, WriteBatch as PublicWriteBatch } from '@firebase/firestore-types'; -import { newUserDataReader } from '../../lite/src/api/reference'; + import { makeDatabaseInfo } from '../../lite/src/api/database'; import { DEFAULT_HOST } from '../../lite/src/api/components'; @@ -720,25 +732,19 @@ export class WriteBatch implements PublicWriteBatch { * A reference to a particular document in a collection in the database. */ export class DocumentReference - extends _DocumentKeyReference + extends Compat> implements PublicDocumentReference { - private _firestoreClient: FirestoreClient; - private _dataReader: UserDataReader; - constructor( - public _key: DocumentKey, readonly firestore: Firestore, - readonly _converter: PublicFirestoreDataConverter | null + delegate: ExpDocumentReference ) { - super(firestore._databaseId, _key, _converter); - this._firestoreClient = ensureFirestoreConfigured(firestore._delegate); - this._dataReader = newUserDataReader(firestore._delegate); + super(delegate); } static forPath( path: ResourcePath, firestore: Firestore, - converter: PublicFirestoreDataConverter | null + converter: UntypedFirestoreDataConverter | null ): DocumentReference { if (path.length % 2 !== 0) { throw new FirestoreError( @@ -748,23 +754,41 @@ export class DocumentReference `${path.canonicalString()} has ${path.length}` ); } - return new DocumentReference(new DocumentKey(path), firestore, converter); + return new DocumentReference( + firestore, + new ExpDocumentReference( + firestore._delegate, + converter, + new DocumentKey(path) + ) + ); + } + + static forKey( + key: DocumentKey, + firestore: Firestore, + converter: UntypedFirestoreDataConverter | null + ): DocumentReference { + return new DocumentReference( + firestore, + new ExpDocumentReference(firestore._delegate, converter, key) + ); } get id(): string { - return this._key.path.lastSegment(); + return this._delegate.id; } get parent(): PublicCollectionReference { return new CollectionReference( - this._key.path.popLast(), + this._delegate._path.popLast(), this.firestore, - this._converter + this._delegate._converter ); } get path(): string { - return this._key.path.canonicalString(); + return this._delegate.path; } collection( @@ -783,44 +807,31 @@ export class DocumentReference } const path = ResourcePath.fromString(pathString); return new CollectionReference( - this._key.path.child(path), + this._delegate._path.child(path), this.firestore, /* converter= */ null ); } isEqual(other: PublicDocumentReference): boolean { - if (!(other instanceof DocumentReference)) { + if (other instanceof Compat) { + other = other._delegate; + } + if (!(other instanceof ExpDocumentReference)) { return false; } - return ( - this.firestore === other.firestore && - this._key.isEqual(other._key) && - this._converter === other._converter - ); + return refEqual(this._delegate, other); } set(value: Partial, options: PublicSetOptions): Promise; set(value: T): Promise; set(value: T | Partial, options?: PublicSetOptions): Promise { options = validateSetOptions('DocumentReference.set', options); - const convertedValue = applyFirestoreDataConverter( - this._converter, - value, - options - ); - const parsed = parseSetData( - this._dataReader, - 'DocumentReference.set', - this._key, - convertedValue, - this._converter !== null, - options - ); - return firestoreClientWrite( - this._firestoreClient, - parsed.toMutations(this._key, Precondition.none()) - ); + try { + return setDoc(this._delegate, value, options); + } catch (e) { + throw replaceFunctionName(e, 'setDoc', 'DocumentReference.set'); + } } update(value: PublicUpdateData): Promise; @@ -834,44 +845,24 @@ export class DocumentReference value?: unknown, ...moreFieldsAndValues: unknown[] ): Promise { - // 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, - 'DocumentReference.update', - this._key, - fieldOrUpdateData, - value, - moreFieldsAndValues - ); - } else { - parsed = parseUpdateData( - this._dataReader, - 'DocumentReference.update', - this._key, - fieldOrUpdateData - ); + try { + if (arguments.length === 1) { + return updateDoc(this._delegate, fieldOrUpdateData as PublicUpdateData); + } else { + return updateDoc( + this._delegate, + fieldOrUpdateData as string | ExpFieldPath, + value, + ...moreFieldsAndValues + ); + } + } catch (e) { + throw replaceFunctionName(e, 'updateDoc', 'DocumentReference.update'); } - - return firestoreClientWrite( - this._firestoreClient, - parsed.toMutations(this._key, Precondition.exists(true)) - ); } delete(): Promise { - return firestoreClientWrite(this._firestoreClient, [ - new DeleteMutation(this._key, Precondition.none()) - ]); + return deleteDoc(this._delegate); } onSnapshot(observer: PartialObserver>): Unsubscribe; @@ -892,102 +883,130 @@ export class DocumentReference ): Unsubscribe; onSnapshot(...args: unknown[]): Unsubscribe { - let options: ListenOptions = { - includeMetadataChanges: false - }; - let currArg = 0; - if ( - typeof args[currArg] === 'object' && - !isPartialObserver(args[currArg]) - ) { - options = args[currArg] as PublicSnapshotListenOptions; - currArg++; - } - - const internalOptions = { - includeMetadataChanges: options.includeMetadataChanges - }; - - if (isPartialObserver(args[currArg])) { - const userObserver = args[currArg] as PartialObserver< - PublicDocumentSnapshot - >; - args[currArg] = userObserver.next?.bind(userObserver); - args[currArg + 1] = userObserver.error?.bind(userObserver); - args[currArg + 2] = userObserver.complete?.bind(userObserver); - } - - const observer: PartialObserver = { - next: snapshot => { - if (args[currArg]) { - (args[currArg] as NextFn>)( - this._convertToDocSnapshot(snapshot) - ); - } - }, - error: args[currArg + 1] as ErrorFn, - complete: args[currArg + 2] as CompleteFn - }; - - return firestoreClientListen( - this._firestoreClient, - newQueryForPath(this._key.path), - internalOptions, - observer + const options = extractSnapshotOptions(args); + const observer = wrapObserver, ExpDocumentSnapshot>( + args, + result => + new DocumentSnapshot( + this.firestore, + result._key, + result._document, + result.metadata.fromCache, + result.metadata.hasPendingWrites, + this._delegate._converter as UntypedFirestoreDataConverter + ) ); + return onSnapshot(this._delegate, options, observer); } get(options?: PublicGetOptions): Promise> { - if (options && options.source === 'cache') { - return firestoreClientGetDocumentFromLocalCache( - this._firestoreClient, - this._key - ).then( - doc => - new DocumentSnapshot( - this.firestore, - this._key, - doc, - /*fromCache=*/ true, - doc instanceof Document ? doc.hasLocalMutations : false, - this._converter - ) - ); + let snap: Promise>; + if (options?.source === 'cache') { + snap = getDocFromCache(this._delegate); + } else if (options?.source === 'server') { + snap = getDocFromServer(this._delegate); } else { - return firestoreClientGetDocumentViaSnapshotListener( - this._firestoreClient, - this._key, - options - ).then(snapshot => this._convertToDocSnapshot(snapshot)); + snap = getDoc(this._delegate); } + + return snap.then( + result => + new DocumentSnapshot( + this.firestore, + result._key, + result._document, + result.metadata.fromCache, + result.metadata.hasPendingWrites, + this._delegate._converter as UntypedFirestoreDataConverter + ) + ); } withConverter( converter: PublicFirestoreDataConverter ): PublicDocumentReference { - return new DocumentReference(this._key, this.firestore, converter); + return new DocumentReference( + this.firestore, + this._delegate.withConverter( + converter as UntypedFirestoreDataConverter + ) + ); } +} - /** - * Converts a ViewSnapshot that contains the current document to a - * DocumentSnapshot. - */ - private _convertToDocSnapshot(snapshot: ViewSnapshot): DocumentSnapshot { - debugAssert( - snapshot.docs.size <= 1, - 'Too many documents returned on a document query' - ); - const doc = snapshot.docs.get(this._key); +/** + * Replaces the function name in an error thrown by the firestore-exp API + * with the function names used in the classic API. + */ +function replaceFunctionName( + e: Error, + originalFunctionName: string, + updatedFunctionName: string +): Error { + e.message = e.message.replace( + `${originalFunctionName}()`, + `${updatedFunctionName}()` + ); + return e; +} - return new DocumentSnapshot( - this.firestore, - this._key, - doc, - snapshot.fromCache, - snapshot.hasPendingWrites, - this._converter - ); +/** + * Iterates the list of arguments from an `onSnapshot` call and returns the + * first argument that may be an `SnapshotListenOptions` object. Returns an + * empty object if none is found. + */ +export function extractSnapshotOptions( + args: unknown[] +): PublicSnapshotListenOptions { + for (const arg of args) { + if (typeof arg === 'object' && !isPartialObserver(arg)) { + return arg as PublicSnapshotListenOptions; + } + } + return {}; +} + +/** + * Creates an observer that can be passed to the firestore-exp SDK. The + * observer converts all observed values into the format expected by the classic + * SDK. + * + * @param args The list of arguments from an `onSnapshot` call. + * @param wrapper The function that converts the firestore-exp type into the + * type used by this shim. + */ +export function wrapObserver( + args: unknown[], + wrapper: (val: ExpType) => CompatType +): PartialObserver { + let userObserver: PartialObserver; + if (isPartialObserver(args[0])) { + userObserver = args[0] as PartialObserver; + } else if (isPartialObserver(args[1])) { + userObserver = args[1]; + } else if (typeof args[0] === 'function') { + userObserver = { + next: args[0] as NextFn | undefined, + error: args[1] as ErrorFn | undefined, + complete: args[2] as CompleteFn | undefined + }; + } else { + userObserver = { + next: args[1] as NextFn | undefined, + error: args[2] as ErrorFn | undefined, + complete: args[3] as CompleteFn | undefined + }; } + + return { + next: val => { + if (userObserver!.next) { + userObserver!.next(wrapper(val)); + } + }, + error: userObserver.error?.bind(userObserver), + complete: userObserver.complete?.bind(userObserver) + }; } /** @@ -1046,7 +1065,7 @@ export class DocumentSnapshot public _document: Document | null, private _fromCache: boolean, private _hasPendingWrites: boolean, - private readonly _converter: PublicFirestoreDataConverter | null + private readonly _converter: UntypedFirestoreDataConverter | null ) {} data(options: PublicSnapshotOptions = {}): T | undefined { @@ -1070,7 +1089,11 @@ export class DocumentSnapshot this._firestore._databaseId, options.serverTimestamps || 'none', key => - new DocumentReference(key, this._firestore, /* converter= */ null), + DocumentReference.forKey( + key, + this._firestore, + /* converter= */ null + ), bytes => new Blob(bytes) ); return userDataWriter.convertValue(this._document.toProto()) as T; @@ -1092,7 +1115,8 @@ export class DocumentSnapshot const userDataWriter = new UserDataWriter( this._firestore._databaseId, options.serverTimestamps || 'none', - key => new DocumentReference(key, this._firestore, this._converter), + key => + DocumentReference.forKey(key, this._firestore, this._converter), bytes => new Blob(bytes) ); return userDataWriter.convertValue(value); @@ -1106,7 +1130,7 @@ export class DocumentSnapshot } get ref(): PublicDocumentReference { - return new DocumentReference( + return DocumentReference.forKey( this._key, this._firestore, this._converter @@ -1359,6 +1383,10 @@ function parseDocumentIdValue( query: InternalQuery, documentIdValue: unknown ): ProtoValue { + if (documentIdValue instanceof Compat) { + documentIdValue = documentIdValue._delegate; + } + if (typeof documentIdValue === 'string') { if (documentIdValue === '') { throw new FirestoreError( @@ -1385,7 +1413,7 @@ function parseDocumentIdValue( ); } return refValue(databaseId, new DocumentKey(path)); - } else if (documentIdValue instanceof _DocumentKeyReference) { + } else if (documentIdValue instanceof ExpDocumentReference) { return refValue(databaseId, documentIdValue._key); } else { throw new FirestoreError( @@ -1930,7 +1958,7 @@ export class CollectionReference constructor( readonly _path: ResourcePath, firestore: Firestore, - _converter: PublicFirestoreDataConverter | null + _converter: UntypedFirestoreDataConverter | null ) { super(newQueryForPath(_path), firestore, _converter); if (_path.length % 2 !== 1) { @@ -1952,8 +1980,8 @@ export class CollectionReference if (parentPath.isEmpty()) { return null; } else { - return new DocumentReference( - new DocumentKey(parentPath), + return DocumentReference.forPath( + parentPath, this.firestore, /* converter= */ null ); @@ -1986,8 +2014,8 @@ export class CollectionReference const docRef = this.doc(); // Call set() with the converted value directly to avoid calling toFirestore() a second time. - return new DocumentReference( - (docRef as DocumentReference)._key, + return DocumentReference.forKey( + (docRef as DocumentReference)._delegate._key, this.firestore, null ) @@ -2006,9 +2034,12 @@ function validateReference( methodName: string, documentRef: PublicDocumentReference, firestore: Firestore -): _DocumentKeyReference { - const reference = cast>(documentRef, DocumentReference); - if (reference.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.' diff --git a/packages/firestore/src/api/observer.ts b/packages/firestore/src/api/observer.ts index 69264fd20f0..a724f1e96e1 100644 --- a/packages/firestore/src/api/observer.ts +++ b/packages/firestore/src/api/observer.ts @@ -36,7 +36,7 @@ export interface Unsubscribe { (): void; } -export function isPartialObserver(obj: unknown): boolean { +export function isPartialObserver(obj: unknown): obj is PartialObserver { return implementsAnyMethods(obj, ['next', 'error', 'complete']); } diff --git a/packages/firestore/src/api/user_data_reader.ts b/packages/firestore/src/api/user_data_reader.ts index bf63ab6cfa9..d799222c344 100644 --- a/packages/firestore/src/api/user_data_reader.ts +++ b/packages/firestore/src/api/user_data_reader.ts @@ -56,13 +56,14 @@ import { newSerializer } from '../platform/serializer'; import { Bytes } from '../../lite/src/api/bytes'; import { Compat } from '../compat/compat'; import { FieldValue } from '../../lite/src/api/field_value'; +import { DocumentReference } from '../../lite/src/api/reference'; import { FieldPath } from '../../lite/src/api/field_path'; const RESERVED_FIELD_REGEX = /^__.*__$/; /** * An untyped Firestore Data Converter interface that is shared between the - * lite, full and legacy SDK. + * lite, firestore-exp and classic SDK. */ export interface UntypedFirestoreDataConverter { toFirestore(modelObject: T): DocumentData; @@ -70,22 +71,6 @@ export interface UntypedFirestoreDataConverter { fromFirestore(snapshot: unknown, options?: unknown): T; } -/** - * A reference to a document in a Firebase project. - * - * This class serves as a common base class for the public DocumentReferences - * exposed in the lite, full and legacy SDK. - */ -// Use underscore prefix to hide this class from our Public API. -// eslint-disable-next-line @typescript-eslint/naming-convention -export class _DocumentKeyReference { - constructor( - readonly _databaseId: DatabaseId, - readonly _key: DocumentKey, - readonly _converter: UntypedFirestoreDataConverter | null - ) {} -} - /** The result of parsing document data (e.g. for a setData call). */ export class ParsedSetData { constructor( @@ -692,6 +677,10 @@ function parseScalarValue( value: unknown, context: ParseContext ): ProtoValue | null { + if (value instanceof Compat) { + value = value._delegate; + } + if (value === null) { return { nullValue: 'NULL_VALUE' }; } else if (typeof value === 'number') { @@ -725,9 +714,9 @@ function parseScalarValue( }; } else if (value instanceof Bytes) { return { bytesValue: toBytes(context.serializer, value._byteString) }; - } else if (value instanceof _DocumentKeyReference) { + } else if (value instanceof DocumentReference) { const thisDb = context.databaseId; - const otherDb = value._databaseId; + const otherDb = value.firestore._databaseId; if (!otherDb.isEqual(thisDb)) { throw context.createError( 'Document reference is for database ' + @@ -737,7 +726,7 @@ function parseScalarValue( } return { referenceValue: toResourceName( - value._databaseId || context.databaseId, + value.firestore._databaseId || context.databaseId, value._key.path ) }; @@ -766,7 +755,7 @@ function looksLikeJsonObject(input: unknown): boolean { !(input instanceof Timestamp) && !(input instanceof GeoPoint) && !(input instanceof Bytes) && - !(input instanceof _DocumentKeyReference) && + !(input instanceof DocumentReference) && !(input instanceof FieldValue) ); } diff --git a/packages/firestore/src/api/user_data_writer.ts b/packages/firestore/src/api/user_data_writer.ts index 84888fe6fd2..183bcf8baea 100644 --- a/packages/firestore/src/api/user_data_writer.ts +++ b/packages/firestore/src/api/user_data_writer.ts @@ -24,7 +24,6 @@ import { Timestamp as ProtoTimestamp, Value as ProtoValue } from '../protos/firestore_proto_api'; -import { _DocumentKeyReference } from './user_data_reader'; import { GeoPoint } from './geo_point'; import { Timestamp } from './timestamp'; import { DatabaseId } from '../core/database_info'; @@ -58,9 +57,7 @@ export class UserDataWriter { constructor( private readonly databaseId: DatabaseId, private readonly serverTimestampBehavior: ServerTimestampBehavior, - private readonly referenceFactory: ( - key: DocumentKey - ) => _DocumentKeyReference, + private readonly referenceFactory: (key: DocumentKey) => unknown, private readonly bytesFactory: (bytes: ByteString) => Bytes ) {} @@ -132,7 +129,7 @@ export class UserDataWriter { return new Timestamp(normalizedValue.seconds, normalizedValue.nanos); } - private convertReference(name: string): _DocumentKeyReference { + private convertReference(name: string): unknown { const resourcePath = ResourcePath.fromString(name); hardAssert( isValidResourceName(resourcePath), diff --git a/packages/firestore/src/util/input_validation.ts b/packages/firestore/src/util/input_validation.ts index 4d3402904d9..443fd411d93 100644 --- a/packages/firestore/src/util/input_validation.ts +++ b/packages/firestore/src/util/input_validation.ts @@ -20,6 +20,7 @@ import { fail } from './assert'; import { Code, FirestoreError } from './error'; import { DocumentKey } from '../model/document_key'; import { ResourcePath } from '../model/path'; +import { Compat } from '../compat/compat'; /** Types accepted by validateType() and related methods for validation. */ export type ValidationType = @@ -175,6 +176,10 @@ export function cast( // eslint-disable-next-line @typescript-eslint/no-explicit-any constructor: { new (...args: any[]): T } ): T | never { + if (obj instanceof Compat) { + obj = obj._delegate; + } + if (!(obj instanceof constructor)) { if (constructor.name === obj.constructor.name) { throw new FirestoreError( diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index b55113849a7..48cf0488aa8 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -73,7 +73,7 @@ export function collectionReference(path: string): CollectionReference { export function documentReference(path: string): DocumentReference { const db = firestore(); ensureFirestoreConfigured(db._delegate); - return new DocumentReference(key(path), db, /* converter= */ null); + return DocumentReference.forKey(key(path), db, /* converter= */ null); } export function documentSnapshot( diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 855fe5afb76..3f294ef9eee 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -111,7 +111,7 @@ export function testUserDataWriter(): UserDataWriter { return new UserDataWriter( TEST_DATABASE_ID, 'none', - key => new DocumentReference(key, FIRESTORE, /* converter= */ null), + key => DocumentReference.forKey(key, FIRESTORE, /* converter= */ null), bytes => new Blob(bytes) ); } @@ -133,8 +133,8 @@ export function version(v: TestSnapshotVersion): SnapshotVersion { } export function ref(key: string, offset?: number): DocumentReference { - return new DocumentReference( - new DocumentKey(path(key, offset)), + return DocumentReference.forPath( + path(key, offset), FIRESTORE, /* converter= */ null );