From 10b85563d6885b10a94b465676308f75fbada7db Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 4 Nov 2020 21:55:00 -0800 Subject: [PATCH 1/6] FieldPath Compat class --- packages/firestore/lite/src/api/field_path.ts | 28 +++-- packages/firestore/src/api/database.ts | 77 +++++++------ packages/firestore/src/api/field_path.ts | 67 ++---------- .../firestore/src/api/user_data_reader.ts | 101 ++++++++++-------- .../test/integration/util/firebase_export.ts | 4 +- packages/firestore/test/util/helpers.ts | 3 +- 6 files changed, 122 insertions(+), 158 deletions(-) diff --git a/packages/firestore/lite/src/api/field_path.ts b/packages/firestore/lite/src/api/field_path.ts index 64e435dec95..021ed57f36a 100644 --- a/packages/firestore/lite/src/api/field_path.ts +++ b/packages/firestore/lite/src/api/field_path.ts @@ -15,8 +15,11 @@ * limitations under the License. */ -import { _BaseFieldPath } from '../../../src/api/field_path'; -import { DOCUMENT_KEY_NAME } from '../../../src/model/path'; +import { + DOCUMENT_KEY_NAME, + FieldPath as InternalFieldPath +} from '../../../src/model/path'; +import { Code, FirestoreError } from '../../../src/util/error'; /** * A `FieldPath` refers to a field in a document. The path may consist of a @@ -26,12 +29,9 @@ import { DOCUMENT_KEY_NAME } from '../../../src/model/path'; * Create a `FieldPath` by providing field names. If more than one field * name is provided, the path will point to a nested field in a document. */ -export class FieldPath extends _BaseFieldPath { - // Note: This class is stripped down a copy of the FieldPath class in the - // legacy SDK. The changes are: - // - The `documentId()` static method has been removed - // - Input validation is limited to errors that cannot be caught by the - // TypeScript transpiler. +export class FieldPath { + /** Internal representation of a Firestore field path. */ + readonly _internalPath: InternalFieldPath; /** * Creates a FieldPath from the provided field names. If more than one field @@ -40,7 +40,17 @@ export class FieldPath extends _BaseFieldPath { * @param fieldNames A list of field names. */ constructor(...fieldNames: string[]) { - super(fieldNames); + for (let i = 0; i < fieldNames.length; ++i) { + if (fieldNames[i].length === 0) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid field name at argument $(i + 1). ` + + 'Field names must not be empty.' + ); + } + } + + this._internalPath = new InternalFieldPath(fieldNames); } /** diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 834388c17d5..73905859ada 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -77,7 +77,7 @@ import { } from '../util/input_validation'; import { logWarn, setLogLevel as setClientLogLevel } from '../util/log'; import { AutoId } from '../util/misc'; -import { _BaseFieldPath, FieldPath as ExternalFieldPath } from './field_path'; +import { FieldPath as ExpFieldPath } from '../../lite/src/api/field_path'; import { CompleteFn, ErrorFn, @@ -119,6 +119,7 @@ import { DocumentData as PublicDocumentData, DocumentReference as PublicDocumentReference, DocumentSnapshot as PublicDocumentSnapshot, + FieldPath as PublicFieldPath, FirebaseFirestore as PublicFirestore, FirestoreDataConverter as PublicFirestoreDataConverter, GetOptions as PublicGetOptions, @@ -518,28 +519,33 @@ export class Transaction implements PublicTransaction { ): Transaction; update( documentRef: PublicDocumentReference, - field: string | ExternalFieldPath, + field: string | PublicFieldPath, value: unknown, ...moreFieldsAndValues: unknown[] ): Transaction; update( documentRef: PublicDocumentReference, - fieldOrUpdateData: string | ExternalFieldPath | PublicUpdateData, + fieldOrUpdateData: string | PublicFieldPath | PublicUpdateData, value?: unknown, ...moreFieldsAndValues: unknown[] ): Transaction { - let ref; - let parsed; + 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 ExternalFieldPath + fieldOrUpdateData instanceof ExpFieldPath ) { - ref = validateReference( - 'Transaction.update', - documentRef, - this._firestore - ); parsed = parseUpdateVarargs( this._dataReader, 'Transaction.update', @@ -549,11 +555,6 @@ export class Transaction implements PublicTransaction { moreFieldsAndValues ); } else { - ref = validateReference( - 'Transaction.update', - documentRef, - this._firestore - ); parsed = parseUpdateData( this._dataReader, 'Transaction.update', @@ -629,30 +630,34 @@ export class WriteBatch implements PublicWriteBatch { ): WriteBatch; update( documentRef: PublicDocumentReference, - field: string | ExternalFieldPath, + field: string | PublicFieldPath, value: unknown, ...moreFieldsAndValues: unknown[] ): WriteBatch; update( documentRef: PublicDocumentReference, - fieldOrUpdateData: string | ExternalFieldPath | PublicUpdateData, + fieldOrUpdateData: string | PublicFieldPath | PublicUpdateData, value?: unknown, ...moreFieldsAndValues: unknown[] ): WriteBatch { this.verifyNotCommitted(); + const ref = validateReference( + 'WriteBatch.update', + documentRef, + this._firestore + ); - let ref; - let parsed; + // 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 ExternalFieldPath + fieldOrUpdateData instanceof ExpFieldPath ) { - ref = validateReference( - 'WriteBatch.update', - documentRef, - this._firestore - ); parsed = parseUpdateVarargs( this._dataReader, 'WriteBatch.update', @@ -662,11 +667,6 @@ export class WriteBatch implements PublicWriteBatch { moreFieldsAndValues ); } else { - ref = validateReference( - 'WriteBatch.update', - documentRef, - this._firestore - ); parsed = parseUpdateData( this._dataReader, 'WriteBatch.update', @@ -825,26 +825,25 @@ export class DocumentReference update(value: PublicUpdateData): Promise; update( - field: string | ExternalFieldPath, + field: string | PublicFieldPath, value: unknown, ...moreFieldsAndValues: unknown[] ): Promise; update( - fieldOrUpdateData: string | ExternalFieldPath | PublicUpdateData, + fieldOrUpdateData: string | PublicFieldPath | PublicUpdateData, 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<_BaseFieldPath>) - ._delegate; + fieldOrUpdateData = (fieldOrUpdateData as Compat)._delegate; } let parsed; if ( typeof fieldOrUpdateData === 'string' || - fieldOrUpdateData instanceof _BaseFieldPath + fieldOrUpdateData instanceof ExpFieldPath ) { parsed = parseUpdateVarargs( this._dataReader, @@ -1080,7 +1079,7 @@ export class DocumentSnapshot } get( - fieldPath: string | ExternalFieldPath, + fieldPath: string | PublicFieldPath, options: PublicSnapshotOptions = {} ): unknown { if (this._document) { @@ -1556,7 +1555,7 @@ export class Query implements PublicQuery { } where( - field: string | ExternalFieldPath, + field: string | PublicFieldPath, opStr: PublicWhereFilterOp, value: unknown ): PublicQuery { @@ -1578,7 +1577,7 @@ export class Query implements PublicQuery { } orderBy( - field: string | ExternalFieldPath, + field: string | PublicFieldPath, directionStr?: PublicOrderByDirection ): PublicQuery { let direction: Direction; diff --git a/packages/firestore/src/api/field_path.ts b/packages/firestore/src/api/field_path.ts index 4008a9936c1..bf4e24491d2 100644 --- a/packages/firestore/src/api/field_path.ts +++ b/packages/firestore/src/api/field_path.ts @@ -17,44 +17,20 @@ import { FieldPath as PublicFieldPath } from '@firebase/firestore-types'; +import { FieldPath as ExpFieldPath } from '../../lite/src/api/field_path'; import { FieldPath as InternalFieldPath } from '../model/path'; -import { Code, FirestoreError } from '../util/error'; +import { Compat } from '../compat/compat'; // The objects that are a part of this API are exposed to third-parties as // compiled javascript so we want to flag our private members with a leading // underscore to discourage their use. -/** - * A field class base class that is shared by the lite, full and legacy SDK, - * which supports shared code that deals with FieldPaths. - */ -// Use underscore prefix to hide this class from our Public API. -// eslint-disable-next-line @typescript-eslint/naming-convention -export abstract class _BaseFieldPath { - /** Internal representation of a Firestore field path. */ - readonly _internalPath: InternalFieldPath; - - constructor(fieldNames: string[]) { - for (let i = 0; i < fieldNames.length; ++i) { - if (fieldNames[i].length === 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid field name at argument $(i + 1). ` + - 'Field names must not be empty.' - ); - } - } - - this._internalPath = new InternalFieldPath(fieldNames); - } -} - /** * A `FieldPath` refers to a field in a document. The path may consist of a * single field name (referring to a top-level field in the document), or a list * of field names (referring to a nested field in the document). */ -export class FieldPath extends _BaseFieldPath implements PublicFieldPath { +export class FieldPath extends Compat implements PublicFieldPath { /** * Creates a FieldPath from the provided field names. If more than one field * name is provided, the path will point to a nested field in a document. @@ -62,7 +38,7 @@ export class FieldPath extends _BaseFieldPath implements PublicFieldPath { * @param fieldNames A list of field names. */ constructor(...fieldNames: string[]) { - super(fieldNames); + super(new ExpFieldPath(...fieldNames)); } static documentId(): FieldPath { @@ -76,37 +52,12 @@ export class FieldPath extends _BaseFieldPath implements PublicFieldPath { } isEqual(other: PublicFieldPath): boolean { - if (!(other instanceof FieldPath)) { + if (other instanceof Compat) { + other = other._delegate; + } + if (!(other instanceof ExpFieldPath)) { return false; } - return this._internalPath.isEqual(other._internalPath); - } -} - -/** - * Matches any characters in a field path string that are reserved. - */ -const RESERVED = new RegExp('[~\\*/\\[\\]]'); - -/** - * Parses a field path string into a FieldPath, treating dots as separators. - */ -export function fromDotSeparatedString(path: string): FieldPath { - const found = path.search(RESERVED); - if (found >= 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid field path (${path}). Paths must not contain ` + - `'~', '*', '/', '[', or ']'` - ); - } - try { - return new FieldPath(...path.split('.')); - } catch (e) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid field path (${path}). Paths must not be empty, ` + - `begin with '.', end with '.', or contain '..'` - ); + return this._delegate._internalPath.isEqual(other._internalPath); } } diff --git a/packages/firestore/src/api/user_data_reader.ts b/packages/firestore/src/api/user_data_reader.ts index a574191d365..bf63ab6cfa9 100644 --- a/packages/firestore/src/api/user_data_reader.ts +++ b/packages/firestore/src/api/user_data_reader.ts @@ -15,7 +15,11 @@ * limitations under the License. */ -import { DocumentData, SetOptions } from '@firebase/firestore-types'; +import { + DocumentData, + SetOptions, + FieldPath as PublicFieldPath +} from '@firebase/firestore-types'; import { Value as ProtoValue, @@ -33,7 +37,7 @@ import { SetMutation, TransformMutation } from '../model/mutation'; -import { FieldPath } from '../model/path'; +import { FieldPath as InternalFieldPath } from '../model/path'; import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { isPlainObject, valueDescription } from '../util/input_validation'; @@ -46,13 +50,13 @@ import { toResourceName, toTimestamp } from '../remote/serializer'; -import { _BaseFieldPath, fromDotSeparatedString } from './field_path'; import { DeleteFieldValueImpl } from './field_value'; import { GeoPoint } from './geo_point'; 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 { FieldPath } from '../../lite/src/api/field_path'; const RESERVED_FIELD_REGEX = /^__.*__$/; @@ -174,7 +178,7 @@ interface ContextSettings { * nonempty path (indicating the context represents a nested location within * the data). */ - readonly path?: FieldPath; + readonly path?: InternalFieldPath; /** * Whether or not this context corresponds to an element of an array. * If not set, elements are treated as if they were outside of arrays. @@ -190,7 +194,7 @@ interface ContextSettings { /** A "context" object passed around while parsing user data. */ export class ParseContext { readonly fieldTransforms: FieldTransform[]; - readonly fieldMask: FieldPath[]; + readonly fieldMask: InternalFieldPath[]; /** * Initializes a ParseContext with the given source and path. * @@ -215,7 +219,7 @@ export class ParseContext { readonly serializer: JsonProtoSerializer, readonly ignoreUndefinedProperties: boolean, fieldTransforms?: FieldTransform[], - fieldMask?: FieldPath[] + fieldMask?: InternalFieldPath[] ) { // Minor hack: If fieldTransforms is undefined, we assume this is an // external call and we need to validate the entire path. @@ -226,7 +230,7 @@ export class ParseContext { this.fieldMask = fieldMask || []; } - get path(): FieldPath | undefined { + get path(): InternalFieldPath | undefined { return this.settings.path; } @@ -253,7 +257,7 @@ export class ParseContext { return context; } - childContextForFieldPath(field: FieldPath): ParseContext { + childContextForFieldPath(field: InternalFieldPath): ParseContext { const childPath = this.path?.child(field); const context = this.contextWith({ path: childPath, arrayElement: false }); context.validatePath(); @@ -277,7 +281,7 @@ export class ParseContext { } /** Returns 'true' if 'fieldPath' was traversed when creating this context. */ - contains(fieldPath: FieldPath): boolean { + contains(fieldPath: InternalFieldPath): boolean { return ( this.fieldMask.find(field => fieldPath.isPrefixOf(field)) !== undefined || this.fieldTransforms.find(transform => @@ -334,7 +338,7 @@ export class UserDataReader { dataSource, methodName, targetDoc, - path: FieldPath.emptyPath(), + path: InternalFieldPath.emptyPath(), arrayElement: false, hasConverter }, @@ -372,23 +376,14 @@ export function parseSetData( fieldMask = new FieldMask(context.fieldMask); fieldTransforms = context.fieldTransforms; } else if (options.mergeFields) { - const validatedFieldPaths: FieldPath[] = []; + const validatedFieldPaths: InternalFieldPath[] = []; for (const stringOrFieldPath of options.mergeFields) { - let fieldPath: FieldPath; - - if (stringOrFieldPath instanceof _BaseFieldPath) { - fieldPath = stringOrFieldPath._internalPath; - } else if (typeof stringOrFieldPath === 'string') { - fieldPath = fieldPathFromDotSeparatedString( - methodName, - stringOrFieldPath, - targetDoc - ); - } else { - throw fail('Expected stringOrFieldPath to be a string or a FieldPath'); - } - + const fieldPath = fieldPathFromArgument( + methodName, + stringOrFieldPath, + targetDoc + ); if (!context.contains(fieldPath)) { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -431,7 +426,7 @@ export function parseUpdateData( ); validatePlainObject('Data must be an object, but it was:', context, input); - const fieldMaskPaths: FieldPath[] = []; + const fieldMaskPaths: InternalFieldPath[] = []; const updateData = new ObjectValueBuilder(); forEach(input as Dict, (key, value) => { const path = fieldPathFromDotSeparatedString(methodName, key, targetDoc); @@ -468,7 +463,7 @@ export function parseUpdateVarargs( userDataReader: UserDataReader, methodName: string, targetDoc: DocumentKey, - field: string | _BaseFieldPath, + field: string | PublicFieldPath | Compat, value: unknown, moreFieldsAndValues: unknown[] ): ParsedUpdateData { @@ -492,13 +487,13 @@ export function parseUpdateVarargs( keys.push( fieldPathFromArgument( methodName, - moreFieldsAndValues[i] as string | _BaseFieldPath + moreFieldsAndValues[i] as string | PublicFieldPath ) ); values.push(moreFieldsAndValues[i + 1]); } - const fieldMaskPaths: FieldPath[] = []; + const fieldMaskPaths: InternalFieldPath[] = []; const updateData = new ObjectValueBuilder(); // We iterate in reverse order to pick the last value for a field if the @@ -797,16 +792,16 @@ function validatePlainObject( */ export function fieldPathFromArgument( methodName: string, - path: string | _BaseFieldPath | Compat<_BaseFieldPath>, + path: string | PublicFieldPath | Compat, targetDoc?: DocumentKey -): FieldPath { +): InternalFieldPath { // If required, replace the FieldPath Compat class with with the firestore-exp // FieldPath. if (path instanceof Compat) { - path = (path as Compat<_BaseFieldPath>)._delegate; + path = (path as Compat)._delegate; } - if (path instanceof _BaseFieldPath) { + if (path instanceof FieldPath) { return path._internalPath; } else if (typeof path === 'string') { return fieldPathFromDotSeparatedString(methodName, path); @@ -822,6 +817,11 @@ export function fieldPathFromArgument( } } +/** + * Matches any characters in a field path string that are reserved. + */ +const FIELD_PATH_RESERVED = new RegExp('[~\\*/\\[\\]]'); + /** * Wraps fromDotSeparatedString with an error message about the method that * was thrown. @@ -834,13 +834,25 @@ export function fieldPathFromDotSeparatedString( methodName: string, path: string, targetDoc?: DocumentKey -): FieldPath { +): InternalFieldPath { + const found = path.search(FIELD_PATH_RESERVED); + if (found >= 0) { + throw createError( + `Invalid field path (${path}). Paths must not contain ` + + `'~', '*', '/', '[', or ']'`, + methodName, + /* hasConverter= */ false, + /* path= */ undefined, + targetDoc + ); + } + try { - return fromDotSeparatedString(path)._internalPath; + return new FieldPath(...path.split('.'))._internalPath; } catch (e) { - const message = errorMessage(e); throw createError( - message, + `Invalid field path (${path}). Paths must not be empty, ` + + `begin with '.', end with '.', or contain '..'`, methodName, /* hasConverter= */ false, /* path= */ undefined, @@ -853,7 +865,7 @@ function createError( reason: string, methodName: string, hasConverter: boolean, - path?: FieldPath, + path?: InternalFieldPath, targetDoc?: DocumentKey ): FirestoreError { const hasPath = path && !path.isEmpty(); @@ -883,15 +895,10 @@ function createError( ); } -/** - * Extracts the message from a caught exception, which should be an Error object - * though JS doesn't guarantee that. - */ -function errorMessage(error: Error | object): string { - return error instanceof Error ? error.message : error.toString(); -} - /** Checks `haystack` if FieldPath `needle` is present. Runs in O(n). */ -function fieldMaskContains(haystack: FieldPath[], needle: FieldPath): boolean { +function fieldMaskContains( + haystack: InternalFieldPath[], + needle: InternalFieldPath +): boolean { return haystack.some(v => v.isEqual(needle)); } diff --git a/packages/firestore/test/integration/util/firebase_export.ts b/packages/firestore/test/integration/util/firebase_export.ts index 3dfd156c8eb..6024db21309 100644 --- a/packages/firestore/test/integration/util/firebase_export.ts +++ b/packages/firestore/test/integration/util/firebase_export.ts @@ -34,6 +34,7 @@ import firebaseAppCompat from '@firebase/app-compat'; import * as exp from '../../../exp/test/shim'; import { FieldValue } from '../../../src/compat/field_value'; +import { FieldPath } from '../../../src/api/field_path'; import { FirebaseApp } from '@firebase/app-types'; import { Firestore, @@ -115,9 +116,6 @@ export function newTestFirestore( // eslint-disable-next-line @typescript-eslint/no-explicit-any const legacyNamespace = (firebase as any).firestore; -const FieldPath = usesFunctionalApi() - ? exp.FieldPath - : legacyNamespace.FieldPath; const Timestamp = usesFunctionalApi() ? exp.Timestamp : legacyNamespace.Timestamp; diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 67c63479b9d..855fe5afb76 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -22,7 +22,6 @@ import * as api from '../../src/protos/firestore_proto_api'; import { expect } from 'chai'; import { Blob } from '../../src/api/blob'; -import { fromDotSeparatedString } from '../../src/api/field_path'; import { UserDataWriter } from '../../src/api/user_data_writer'; import { parseQueryValue, @@ -203,7 +202,7 @@ export function path(path: string, offset?: number): ResourcePath { } export function field(path: string): FieldPath { - return fromDotSeparatedString(path)._internalPath; + return new FieldPath(path.split('.')); } export function mask(...paths: string[]): FieldMask { From 0a0df24a64ea8fb80152ae1cbb4079f348022400 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 5 Nov 2020 15:26:43 -0800 Subject: [PATCH 2/6] Compat class for DocumentReference --- packages-exp/auth-exp/index.webworker.ts | 5 +- packages/database/src/api/Database.ts | 7 +- packages/firestore/exp/src/api/reference.ts | 13 +- 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 | 381 ++++++++++-------- 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 +- 14 files changed, 271 insertions(+), 435 deletions(-) diff --git a/packages-exp/auth-exp/index.webworker.ts b/packages-exp/auth-exp/index.webworker.ts index 886105720b8..e9b29a71912 100644 --- a/packages-exp/auth-exp/index.webworker.ts +++ b/packages-exp/auth-exp/index.webworker.ts @@ -37,7 +37,10 @@ registerAuth(ClientPlatform.WORKER); export function getAuth(app = getApp()): Auth { // Unlike the other environments, we need to explicitly check if indexedDb is // available. That means doing the whole rigamarole - const auth = _getProvider(app, _ComponentName.AUTH).getImmediate() as AuthImpl; + const auth = _getProvider( + app, + _ComponentName.AUTH + ).getImmediate() as AuthImpl; // This promise is intended to float; auth initialization happens in the // background, meanwhile the auth object may be used by the app. diff --git a/packages/database/src/api/Database.ts b/packages/database/src/api/Database.ts index b75e09cdced..94f321cc297 100644 --- a/packages/database/src/api/Database.ts +++ b/packages/database/src/api/Database.ts @@ -157,14 +157,17 @@ export class Database implements FirebaseService { validateUrl(apiName, 1, parsedURL); const repoInfo = parsedURL.repoInfo; - if (!repoInfo.isCustomHost() && repoInfo.host !== this.repo_.repoInfo_.host) { + if ( + !repoInfo.isCustomHost() && + repoInfo.host !== this.repo_.repoInfo_.host + ) { fatal( apiName + ': Host name does not match the current database: ' + '(found ' + repoInfo.host + ' but expected ' + - this.repo_.repoInfo_.host+ + this.repo_.repoInfo_.host + ')' ); } diff --git a/packages/firestore/exp/src/api/reference.ts b/packages/firestore/exp/src/api/reference.ts index 06cc7a5094c..4e8b9a7eee4 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,8 @@ import { removeSnapshotsInSyncListener } from '../../../src/core/event_manager'; import { FirestoreError } from '../../../src/util/error'; +import { Compat } from '../../../src/compat/compat'; +import { _BaseFieldPath } from '../../../src/api/field_path'; /** * An options object that can be passed to {@link onSnapshot()} and {@link @@ -374,10 +375,16 @@ 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' || - fieldOrUpdateData instanceof FieldPath + fieldOrUpdateData instanceof _BaseFieldPath ) { parsed = parseUpdateVarargs( dataReader, @@ -821,7 +828,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 834388c17d5..53d9a483bdf 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,21 @@ 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 { LRU_COLLECTION_DISABLED } from '../local/lru_garbage_collector'; import { Compat } from '../compat/compat'; @@ -138,9 +149,10 @@ 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'; +import * as exp from '../../exp/index'; /** * Constant used to indicate the LRU garbage collection should be disabled. @@ -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( @@ -775,52 +799,33 @@ export class DocumentReference 'path', pathString ); - if (!pathString) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Must provide a non-empty collection name to collection()' - ); - } 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 rewriteError(e, 'setDoc', 'DocumentReference.set'); + } } update(value: PublicUpdateData): Promise; @@ -834,45 +839,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<_BaseFieldPath>) - ._delegate; - } - - let parsed; - if ( - typeof fieldOrUpdateData === 'string' || - fieldOrUpdateData instanceof _BaseFieldPath - ) { - 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 | ExternalFieldPath, + value, + ...moreFieldsAndValues + ); + } + } catch (e) { + throw rewriteError(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; @@ -893,102 +877,129 @@ 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, exp.DocumentSnapshot>( + 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 rewriteError( + 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[] +): exp.SnapshotListenOptions { + for (const arg of args) { + if (typeof arg === 'object' && !isPartialObserver(arg)) { + return arg as exp.SnapshotListenOptions; + } + } + 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 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. + */ +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) + }; } /** @@ -1047,7 +1058,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 { @@ -1071,7 +1082,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; @@ -1093,7 +1108,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); @@ -1107,7 +1123,7 @@ export class DocumentSnapshot } get ref(): PublicDocumentReference { - return new DocumentReference( + return DocumentReference.forKey( this._key, this._firestore, this._converter @@ -1360,6 +1376,10 @@ function parseDocumentIdValue( query: InternalQuery, documentIdValue: unknown ): ProtoValue { + if (documentIdValue instanceof Compat) { + documentIdValue = documentIdValue._delegate; + } + if (typeof documentIdValue === 'string') { if (documentIdValue === '') { throw new FirestoreError( @@ -1386,7 +1406,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( @@ -1931,7 +1951,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) { @@ -1953,8 +1973,8 @@ export class CollectionReference if (parentPath.isEmpty()) { return null; } else { - return new DocumentReference( - new DocumentKey(parentPath), + return DocumentReference.forPath( + parentPath, this.firestore, /* converter= */ null ); @@ -1987,8 +2007,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 ) @@ -2007,9 +2027,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 a574191d365..91e07fabf00 100644 --- a/packages/firestore/src/api/user_data_reader.ts +++ b/packages/firestore/src/api/user_data_reader.ts @@ -53,12 +53,13 @@ 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'; 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; @@ -66,22 +67,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( @@ -697,6 +682,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') { @@ -730,9 +719,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 ' + @@ -742,7 +731,7 @@ function parseScalarValue( } return { referenceValue: toResourceName( - value._databaseId || context.databaseId, + value.firestore._databaseId || context.databaseId, value._key.path ) }; @@ -771,7 +760,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 67c63479b9d..8278c7d1e6f 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -112,7 +112,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) ); } @@ -134,8 +134,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 ); From c060c28d7402911667edd0bb6d30e163400d6284 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 5 Nov 2020 16:22:06 -0800 Subject: [PATCH 3/6] Create short-mangos-beg.md --- .changeset/short-mangos-beg.md | 5 +++++ 1 file changed, 5 insertions(+) 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. From 9fc4c3732cd25c6facb29c24f13a1eaed6d83eaf Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 5 Nov 2020 15:26:43 -0800 Subject: [PATCH 4/6] Compat class for DocumentReference --- packages/firestore/exp/src/api/reference.ts | 13 +- 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 | 381 ++++++++++-------- 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 +- 12 files changed, 262 insertions(+), 432 deletions(-) diff --git a/packages/firestore/exp/src/api/reference.ts b/packages/firestore/exp/src/api/reference.ts index 06cc7a5094c..4e8b9a7eee4 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,8 @@ import { removeSnapshotsInSyncListener } from '../../../src/core/event_manager'; import { FirestoreError } from '../../../src/util/error'; +import { Compat } from '../../../src/compat/compat'; +import { _BaseFieldPath } from '../../../src/api/field_path'; /** * An options object that can be passed to {@link onSnapshot()} and {@link @@ -374,10 +375,16 @@ 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' || - fieldOrUpdateData instanceof FieldPath + fieldOrUpdateData instanceof _BaseFieldPath ) { parsed = parseUpdateVarargs( dataReader, @@ -821,7 +828,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 834388c17d5..53d9a483bdf 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,21 @@ 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 { LRU_COLLECTION_DISABLED } from '../local/lru_garbage_collector'; import { Compat } from '../compat/compat'; @@ -138,9 +149,10 @@ 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'; +import * as exp from '../../exp/index'; /** * Constant used to indicate the LRU garbage collection should be disabled. @@ -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( @@ -775,52 +799,33 @@ export class DocumentReference 'path', pathString ); - if (!pathString) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Must provide a non-empty collection name to collection()' - ); - } 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 rewriteError(e, 'setDoc', 'DocumentReference.set'); + } } update(value: PublicUpdateData): Promise; @@ -834,45 +839,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<_BaseFieldPath>) - ._delegate; - } - - let parsed; - if ( - typeof fieldOrUpdateData === 'string' || - fieldOrUpdateData instanceof _BaseFieldPath - ) { - 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 | ExternalFieldPath, + value, + ...moreFieldsAndValues + ); + } + } catch (e) { + throw rewriteError(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; @@ -893,102 +877,129 @@ 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, exp.DocumentSnapshot>( + 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 rewriteError( + 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[] +): exp.SnapshotListenOptions { + for (const arg of args) { + if (typeof arg === 'object' && !isPartialObserver(arg)) { + return arg as exp.SnapshotListenOptions; + } + } + 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 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. + */ +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) + }; } /** @@ -1047,7 +1058,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 { @@ -1071,7 +1082,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; @@ -1093,7 +1108,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); @@ -1107,7 +1123,7 @@ export class DocumentSnapshot } get ref(): PublicDocumentReference { - return new DocumentReference( + return DocumentReference.forKey( this._key, this._firestore, this._converter @@ -1360,6 +1376,10 @@ function parseDocumentIdValue( query: InternalQuery, documentIdValue: unknown ): ProtoValue { + if (documentIdValue instanceof Compat) { + documentIdValue = documentIdValue._delegate; + } + if (typeof documentIdValue === 'string') { if (documentIdValue === '') { throw new FirestoreError( @@ -1386,7 +1406,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( @@ -1931,7 +1951,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) { @@ -1953,8 +1973,8 @@ export class CollectionReference if (parentPath.isEmpty()) { return null; } else { - return new DocumentReference( - new DocumentKey(parentPath), + return DocumentReference.forPath( + parentPath, this.firestore, /* converter= */ null ); @@ -1987,8 +2007,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 ) @@ -2007,9 +2027,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 a574191d365..91e07fabf00 100644 --- a/packages/firestore/src/api/user_data_reader.ts +++ b/packages/firestore/src/api/user_data_reader.ts @@ -53,12 +53,13 @@ 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'; 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; @@ -66,22 +67,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( @@ -697,6 +682,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') { @@ -730,9 +719,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 ' + @@ -742,7 +731,7 @@ function parseScalarValue( } return { referenceValue: toResourceName( - value._databaseId || context.databaseId, + value.firestore._databaseId || context.databaseId, value._key.path ) }; @@ -771,7 +760,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 67c63479b9d..8278c7d1e6f 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -112,7 +112,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) ); } @@ -134,8 +134,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 ); From 930be11f3283cf01699c26063ba27a5e7ea8aa05 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 9 Nov 2020 13:42:56 -0800 Subject: [PATCH 5/6] Udpate comment --- packages/firestore/src/api/database.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index f99c183f5f9..02161b9f87d 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -830,7 +830,7 @@ export class DocumentReference try { return setDoc(this._delegate, value, options); } catch (e) { - throw rewriteError(e, 'setDoc', 'DocumentReference.set'); + throw replaceFunctionName(e, 'setDoc', 'DocumentReference.set'); } } @@ -857,7 +857,7 @@ export class DocumentReference ); } } catch (e) { - throw rewriteError(e, 'updateDoc', 'DocumentReference.update'); + throw replaceFunctionName(e, 'updateDoc', 'DocumentReference.update'); } } @@ -938,7 +938,7 @@ export class DocumentReference * Replaces the function name in an error thrown by the firestore-exp API * with the function names used in the classic API. */ -function rewriteError( +function replaceFunctionName( e: Error, originalFunctionName: string, updatedFunctionName: string @@ -968,7 +968,8 @@ export function extractSnapshotOptions( /** * 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. + * 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 From 0862fdfb640e2972594ea82515595416e5e3d4e3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 9 Nov 2020 14:13:54 -0800 Subject: [PATCH 6/6] Undo formatting --- packages-exp/auth-exp/index.webworker.ts | 5 +---- packages/database/src/api/Database.ts | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages-exp/auth-exp/index.webworker.ts b/packages-exp/auth-exp/index.webworker.ts index e9b29a71912..886105720b8 100644 --- a/packages-exp/auth-exp/index.webworker.ts +++ b/packages-exp/auth-exp/index.webworker.ts @@ -37,10 +37,7 @@ registerAuth(ClientPlatform.WORKER); export function getAuth(app = getApp()): Auth { // Unlike the other environments, we need to explicitly check if indexedDb is // available. That means doing the whole rigamarole - const auth = _getProvider( - app, - _ComponentName.AUTH - ).getImmediate() as AuthImpl; + const auth = _getProvider(app, _ComponentName.AUTH).getImmediate() as AuthImpl; // This promise is intended to float; auth initialization happens in the // background, meanwhile the auth object may be used by the app. diff --git a/packages/database/src/api/Database.ts b/packages/database/src/api/Database.ts index 94f321cc297..b75e09cdced 100644 --- a/packages/database/src/api/Database.ts +++ b/packages/database/src/api/Database.ts @@ -157,17 +157,14 @@ export class Database implements FirebaseService { validateUrl(apiName, 1, parsedURL); const repoInfo = parsedURL.repoInfo; - if ( - !repoInfo.isCustomHost() && - repoInfo.host !== this.repo_.repoInfo_.host - ) { + if (!repoInfo.isCustomHost() && repoInfo.host !== this.repo_.repoInfo_.host) { fatal( apiName + ': Host name does not match the current database: ' + '(found ' + repoInfo.host + ' but expected ' + - this.repo_.repoInfo_.host + + this.repo_.repoInfo_.host+ ')' ); }