From c7d4b977af5b72a2adc24ebd1c5fe82daec48358 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 12 Nov 2020 15:43:36 -0800 Subject: [PATCH 1/3] Compat class for Query --- packages/firestore/exp/src/api/reference.ts | 15 +- packages/firestore/exp/test/shim.ts | 204 +------ packages/firestore/lite/src/api/reference.ts | 5 + packages/firestore/src/api/database.ts | 554 ++++++------------ .../test/integration/api/validation.test.ts | 8 +- packages/firestore/test/util/api_helpers.ts | 22 +- 6 files changed, 226 insertions(+), 582 deletions(-) diff --git a/packages/firestore/exp/src/api/reference.ts b/packages/firestore/exp/src/api/reference.ts index 921bcb29576..807e9106f57 100644 --- a/packages/firestore/exp/src/api/reference.ts +++ b/packages/firestore/exp/src/api/reference.ts @@ -87,7 +87,20 @@ import { AbstractUserDataWriter } from '../../../src/api/user_data_writer'; export { DocumentReference, CollectionReference, - Query + Query, + collection, + collectionGroup, + doc, + query, + where, + limit, + limitToLast, + orderBy, + startAt, + startAfter, + endAt, + endBefore, + queryEqual } from '../../../lite/src/api/reference'; /** diff --git a/packages/firestore/exp/test/shim.ts b/packages/firestore/exp/test/shim.ts index b2b9b5ba896..1fac8866b91 100644 --- a/packages/firestore/exp/test/shim.ts +++ b/packages/firestore/exp/test/shim.ts @@ -18,28 +18,7 @@ import * as legacy from '@firebase/firestore-types'; import * as exp from '../index'; -import { - addDoc, - doc, - FieldPath as FieldPathExp, - getDocs, - getDocsFromCache, - getDocsFromServer, - onSnapshot, - query, - queryEqual, - refEqual, - endAt, - endBefore, - startAfter, - startAt, - limitToLast, - limit, - orderBy, - where, - Bytes as BytesExp -} from '../../exp/index'; -import { UntypedFirestoreDataConverter } from '../../src/api/user_data_reader'; +import { FieldPath as FieldPathExp, Bytes as BytesExp } from '../../exp/index'; import { isPlainObject, validateSetOptions @@ -48,10 +27,7 @@ import { Compat } from '../../src/compat/compat'; import { Firestore, DocumentReference, - DocumentSnapshot, - QuerySnapshot, - wrapObserver, - extractSnapshotOptions + DocumentSnapshot } from '../../src/api/database'; export { GeoPoint, Timestamp } from '../index'; @@ -185,182 +161,6 @@ export class WriteBatch } } -export class Query - extends Compat> - implements legacy.Query { - constructor(readonly firestore: Firestore, delegate: exp.Query) { - super(delegate); - } - - where( - fieldPath: string | FieldPath, - opStr: legacy.WhereFilterOp, - value: any - ): Query { - return new Query( - this.firestore, - query(this._delegate, where(unwrap(fieldPath), opStr, unwrap(value))) - ); - } - - orderBy( - fieldPath: string | FieldPath, - directionStr?: legacy.OrderByDirection - ): Query { - return new Query( - this.firestore, - query(this._delegate, orderBy(unwrap(fieldPath), directionStr)) - ); - } - - limit(n: number): Query { - return new Query(this.firestore, query(this._delegate, limit(n))); - } - - limitToLast(n: number): Query { - return new Query(this.firestore, query(this._delegate, limitToLast(n))); - } - - startAt(...args: any[]): Query { - return new Query( - this.firestore, - query(this._delegate, startAt(...unwrap(args))) - ); - } - - startAfter(...args: any[]): Query { - return new Query( - this.firestore, - query(this._delegate, startAfter(...unwrap(args))) - ); - } - - endBefore(...args: any[]): Query { - return new Query( - this.firestore, - query(this._delegate, endBefore(...unwrap(args))) - ); - } - - endAt(...args: any[]): Query { - return new Query( - this.firestore, - query(this._delegate, endAt(...unwrap(args))) - ); - } - - isEqual(other: legacy.Query): boolean { - return queryEqual(this._delegate, (other as Query)._delegate); - } - - get(options?: legacy.GetOptions): Promise> { - let query: Promise>; - if (options?.source === 'cache') { - query = getDocsFromCache(this._delegate); - } else if (options?.source === 'server') { - query = getDocsFromServer(this._delegate); - } else { - query = getDocs(this._delegate); - } - return query.then(result => new QuerySnapshot(this.firestore, result)); - } - - onSnapshot(observer: { - next?: (snapshot: QuerySnapshot) => void; - error?: (error: legacy.FirestoreError) => void; - complete?: () => void; - }): () => void; - onSnapshot( - options: legacy.SnapshotListenOptions, - observer: { - next?: (snapshot: QuerySnapshot) => void; - error?: (error: legacy.FirestoreError) => void; - complete?: () => void; - } - ): () => void; - onSnapshot( - onNext: (snapshot: QuerySnapshot) => void, - onError?: (error: legacy.FirestoreError) => void, - onCompletion?: () => void - ): () => void; - onSnapshot( - options: legacy.SnapshotListenOptions, - onNext: (snapshot: QuerySnapshot) => void, - onError?: (error: legacy.FirestoreError) => void, - onCompletion?: () => void - ): () => void; - onSnapshot(...args: any): () => void { - const options = extractSnapshotOptions(args); - const observer = wrapObserver, exp.QuerySnapshot>( - args, - snap => new QuerySnapshot(this.firestore, snap) - ); - return onSnapshot(this._delegate, options, observer); - } - - withConverter(converter: legacy.FirestoreDataConverter): Query { - return new Query( - this.firestore, - this._delegate.withConverter( - converter as UntypedFirestoreDataConverter - ) - ); - } -} - -export class CollectionReference - extends Query - implements legacy.CollectionReference { - constructor( - readonly firestore: Firestore, - readonly _delegate: exp.CollectionReference - ) { - super(firestore, _delegate); - } - - readonly id = this._delegate.id; - readonly path = this._delegate.path; - - get parent(): DocumentReference | null { - const docRef = this._delegate.parent; - return docRef - ? new DocumentReference(this.firestore, docRef) - : null; - } - - doc(documentPath?: string): DocumentReference { - if (documentPath !== undefined) { - return new DocumentReference( - this.firestore, - doc(this._delegate, documentPath) - ); - } else { - return new DocumentReference(this.firestore, doc(this._delegate)); - } - } - - add(data: T): Promise> { - return addDoc(this._delegate, unwrap(data)).then( - docRef => new DocumentReference(this.firestore, docRef) - ); - } - - isEqual(other: CollectionReference): boolean { - return refEqual(this._delegate, other._delegate); - } - - withConverter( - converter: legacy.FirestoreDataConverter - ): CollectionReference { - return new CollectionReference( - this.firestore, - this._delegate.withConverter( - converter as UntypedFirestoreDataConverter - ) - ); - } -} - export class FieldPath extends Compat implements legacy.FieldPath { diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts index a0e915d9c10..e4283e8c6c5 100644 --- a/packages/firestore/lite/src/api/reference.ts +++ b/packages/firestore/lite/src/api/reference.ts @@ -81,6 +81,7 @@ import { getDatastore } from './components'; import { ByteString } from '../../../src/util/byte_string'; import { Bytes } from './bytes'; import { AbstractUserDataWriter } from '../../../src/api/user_data_writer'; +import { Compat } from '../../../src/compat/compat'; /** * Document data (for use with {@link setDoc()}) consists of fields mapped to @@ -595,6 +596,10 @@ function newQueryBoundFromDocOrFields( docOrFields: Array>, before: boolean ): Bound { + if (docOrFields[0] instanceof Compat) { + docOrFields[0] = docOrFields[0]._delegate; + } + if (docOrFields[0] instanceof DocumentSnapshot) { return newQueryBoundFromDocument( query._query, diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 6874fd9b9f1..5a607c5fb77 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -20,13 +20,9 @@ import { Value as ProtoValue } from '../protos/firestore_proto_api'; import { FirebaseApp } from '@firebase/app-types'; import { _FirebaseApp, FirebaseService } from '@firebase/app-types/private'; import { DatabaseId } from '../core/database_info'; -import { ListenOptions } from '../core/event_manager'; import { FirestoreClient, - firestoreClientGetDocumentsFromLocalCache, - firestoreClientGetDocumentsViaSnapshotListener, firestoreClientGetNamedQuery, - firestoreClientListen, firestoreClientLoadBundle, firestoreClientTransaction, firestoreClientWrite @@ -41,22 +37,12 @@ import { getInequalityFilterField, hasLimitToLast, isCollectionGroupQuery, - LimitType, - newQueryForCollectionGroup, - newQueryForPath, Operator, OrderBy, Query as InternalQuery, - queryEquals, - queryOrderBy, - queryWithAddedFilter, - queryWithAddedOrderBy, - queryWithEndAt, - queryWithLimit, - queryWithStartAt + queryOrderBy } from '../core/query'; import { Transaction as InternalTransaction } from '../core/transaction'; -import { ViewSnapshot } from '../core/view_snapshot'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { DeleteMutation, Mutation, Precondition } from '../model/mutation'; @@ -68,13 +54,10 @@ import { Code, FirestoreError } from '../util/error'; import { cast, validateIsNotUsedTogether, - validateNonEmptyArgument, - validatePositiveNumber, validateSetOptions, valueDescription } from '../util/input_validation'; import { logWarn, setLogLevel as setClientLogLevel } from '../util/log'; -import { AutoId } from '../util/misc'; import { FieldPath as ExpFieldPath } from '../../lite/src/api/field_path'; import { CompleteFn, @@ -85,7 +68,6 @@ import { Unsubscribe } from './observer'; import { - fieldPathFromArgument, parseQueryValue, parseSetData, parseUpdateData, @@ -120,11 +102,30 @@ import { getDocFromServer, getDoc, onSnapshot, + collection, + doc, + collectionGroup, + query, + where, + limit, + limitToLast, + orderBy, + startAt, + startAfter, + endAt, + endBefore, + queryEqual, + getDocsFromCache, + getDocsFromServer, + getDocs, + addDoc, DocumentReference as ExpDocumentReference, - Query as ExpQuery + Query as ExpQuery, + CollectionReference as ExpCollectionReference } from '../../exp/src/api/reference'; import { LRU_COLLECTION_DISABLED } from '../local/lru_garbage_collector'; import { Compat } from '../compat/compat'; +import { ApiLoadBundleTask, LoadBundleTask } from './bundle'; import { CollectionReference as PublicCollectionReference, @@ -156,7 +157,6 @@ import { import { makeDatabaseInfo } from '../../lite/src/api/database'; import { DEFAULT_HOST } from '../../lite/src/api/components'; -import { ApiLoadBundleTask, LoadBundleTask } from './bundle'; /** * Constant used to indicate the LRU garbage collection should be disabled. @@ -349,44 +349,34 @@ export class Firestore }; collection(pathString: string): PublicCollectionReference { - validateNonEmptyArgument('Firestore.collection', 'path', pathString); - ensureFirestoreConfigured(this._delegate); - return new CollectionReference( - ResourcePath.fromString(pathString), - this, - /* converter= */ null - ); + try { + return new CollectionReference( + this, + collection(this._delegate, pathString) + ); + } catch (e) { + throw replaceFunctionName(e, 'collection()', 'Firestore.collection()'); + } } doc(pathString: string): PublicDocumentReference { - validateNonEmptyArgument('Firestore.doc', 'path', pathString); - ensureFirestoreConfigured(this._delegate); - return DocumentReference.forPath( - ResourcePath.fromString(pathString), - this, - /* converter= */ null - ); + try { + return new DocumentReference(this, doc(this._delegate, pathString)); + } catch (e) { + throw replaceFunctionName(e, 'doc()', 'Firestore.doc()'); + } } collectionGroup(collectionId: string): PublicQuery { - validateNonEmptyArgument( - 'Firestore.collectionGroup', - 'collectionId', - collectionId - ); - if (collectionId.indexOf('/') >= 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid collection ID '${collectionId}' passed to function ` + - `Firestore.collectionGroup(). Collection IDs must not contain '/'.` + try { + return new Query(this, collectionGroup(this._delegate, collectionId)); + } catch (e) { + throw replaceFunctionName( + e, + 'collectionGroup()', + 'Firestore.collectionGroup()' ); } - ensureFirestoreConfigured(this._delegate); - return new Query( - newQueryForCollectionGroup(collectionId), - this, - /* converter= */ null - ); } runTransaction( @@ -466,8 +456,7 @@ export function namedQuery( if (!namedQuery) { return null; } - - return new Query(namedQuery.query, db, null); + return new Query(db, new ExpQuery(db._delegate, null, namedQuery.query)); }); } @@ -830,11 +819,7 @@ export class DocumentReference } get parent(): PublicCollectionReference { - return new CollectionReference( - this._delegate._path.popLast(), - this.firestore, - this._delegate._converter - ); + return new CollectionReference(this.firestore, this._delegate.parent); } get path(): string { @@ -844,23 +829,18 @@ export class DocumentReference collection( pathString: string ): PublicCollectionReference { - validateNonEmptyArgument( - 'DocumentReference.collection', - 'path', - pathString - ); - if (!pathString) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Must provide a non-empty collection name to collection()' + try { + return new CollectionReference( + this.firestore, + collection(this._delegate, pathString) + ); + } catch (e) { + throw replaceFunctionName( + e, + 'collection()', + 'DocumentReference.collection()' ); } - const path = ResourcePath.fromString(pathString); - return new CollectionReference( - this._delegate._path.child(path), - this.firestore, - /* converter= */ null - ); } isEqual(other: PublicDocumentReference): boolean { @@ -880,7 +860,7 @@ export class DocumentReference try { return setDoc(this._delegate, value, options); } catch (e) { - throw replaceFunctionName(e, 'setDoc', 'DocumentReference.set'); + throw replaceFunctionName(e, 'setDoc()', 'DocumentReference.set()'); } } @@ -907,7 +887,7 @@ export class DocumentReference ); } } catch (e) { - throw replaceFunctionName(e, 'updateDoc', 'DocumentReference.update'); + throw replaceFunctionName(e, 'updateDoc()', 'DocumentReference.update()'); } } @@ -996,13 +976,10 @@ export class DocumentReference */ function replaceFunctionName( e: Error, - originalFunctionName: string, - updatedFunctionName: string + original: string | RegExp, + updated: string ): Error { - e.message = e.message.replace( - `${originalFunctionName}()`, - `${updatedFunctionName}()` - ); + e.message = e.message.replace(original, updated); return e; } @@ -1550,7 +1527,7 @@ function validateOrderByAndInequalityMatch( `Invalid query. You have a where filter with an inequality ` + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + `and so you must also use '${inequality.toString()}' ` + - `as your first orderBy(), but your first orderBy() ` + + `as your first argument to orderBy(), but your first orderBy() ` + `is on field '${orderBy.toString()}' instead.` ); } @@ -1567,193 +1544,113 @@ export function validateHasExplicitOrderByForLimitToLast( } } -export class Query implements PublicQuery { - private _userDataReader: UserDataReader; - private _userDataWriter: UserDataWriter; - - constructor( - public _query: InternalQuery, - readonly firestore: Firestore, - protected readonly _converter: UntypedFirestoreDataConverter | null - ) { - this._userDataReader = newUserDataReader(firestore._delegate); - this._userDataWriter = new UserDataWriter(firestore); +export class Query + extends Compat> + implements PublicQuery { + constructor(readonly firestore: Firestore, delegate: ExpQuery) { + super(delegate); } where( - field: string | PublicFieldPath, + fieldPath: string | FieldPath, opStr: PublicWhereFilterOp, value: unknown - ): PublicQuery { - const fieldPath = fieldPathFromArgument('Query.where', field); - const filter = newQueryFilter( - this._query, - 'Query.where', - this._userDataReader, - this.firestore._databaseId, - fieldPath, - opStr as Operator, - value - ); - return new Query( - queryWithAddedFilter(this._query, filter), - this.firestore, - this._converter - ); + ): Query { + try { + return new Query( + this.firestore, + query(this._delegate, where(fieldPath as string, opStr, value)) + ); + } catch (e) { + throw replaceFunctionName(e, /(orderBy|where)\(\)/, 'Query.$1()'); + } } orderBy( - field: string | PublicFieldPath, + fieldPath: string | FieldPath, directionStr?: PublicOrderByDirection - ): PublicQuery { - let direction: Direction; - if (directionStr === undefined || directionStr === 'asc') { - direction = Direction.ASCENDING; - } else if (directionStr === 'desc') { - direction = Direction.DESCENDING; - } else { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function Query.orderBy() has unknown direction '${directionStr}', ` + - `expected 'asc' or 'desc'.` + ): Query { + try { + return new Query( + this.firestore, + query(this._delegate, orderBy(fieldPath as string, directionStr)) ); + } catch (e) { + throw replaceFunctionName(e, /(orderBy|where)\(\)/, 'Query.$1()'); } - const fieldPath = fieldPathFromArgument('Query.orderBy', field); - const orderBy = newQueryOrderBy(this._query, fieldPath, direction); - return new Query( - queryWithAddedOrderBy(this._query, orderBy), - this.firestore, - this._converter - ); } - limit(n: number): PublicQuery { - validatePositiveNumber('Query.limit', n); - return new Query( - queryWithLimit(this._query, n, LimitType.First), - this.firestore, - this._converter - ); + limit(n: number): Query { + try { + return new Query(this.firestore, query(this._delegate, limit(n))); + } catch (e) { + throw replaceFunctionName(e, 'limit()', 'Query.limit()'); + } } - limitToLast(n: number): PublicQuery { - validatePositiveNumber('Query.limitToLast', n); - return new Query( - queryWithLimit(this._query, n, LimitType.Last), - this.firestore, - this._converter - ); + limitToLast(n: number): Query { + try { + return new Query( + this.firestore, + query(this._delegate, limitToLast(n)) + ); + } catch (e) { + throw replaceFunctionName(e, 'limitToLast()', 'Query.limitToLast()'); + } } - startAt( - docOrField: unknown | PublicDocumentSnapshot, - ...fields: unknown[] - ): PublicQuery { - const bound = this.boundFromDocOrFields( - 'Query.startAt', - docOrField, - fields, - /*before=*/ true - ); - return new Query( - queryWithStartAt(this._query, bound), - this.firestore, - this._converter - ); + startAt(...args: any[]): Query { + try { + return new Query(this.firestore, query(this._delegate, startAt(...args))); + } catch (e) { + throw replaceFunctionName(e, 'startAt()', 'Query.startAt()'); + } } - startAfter( - docOrField: unknown | PublicDocumentSnapshot, - ...fields: unknown[] - ): PublicQuery { - const bound = this.boundFromDocOrFields( - 'Query.startAfter', - docOrField, - fields, - /*before=*/ false - ); - return new Query( - queryWithStartAt(this._query, bound), - this.firestore, - this._converter - ); + startAfter(...args: any[]): Query { + try { + return new Query( + this.firestore, + query(this._delegate, startAfter(...args)) + ); + } catch (e) { + throw replaceFunctionName(e, 'startAfter()', 'Query.startAfter()'); + } } - endBefore( - docOrField: unknown | PublicDocumentSnapshot, - ...fields: unknown[] - ): PublicQuery { - const bound = this.boundFromDocOrFields( - 'Query.endBefore', - docOrField, - fields, - /*before=*/ true - ); - return new Query( - queryWithEndAt(this._query, bound), - this.firestore, - this._converter - ); + endBefore(...args: any[]): Query { + try { + return new Query( + this.firestore, + query(this._delegate, endBefore(...args)) + ); + } catch (e) { + throw replaceFunctionName(e, 'endBefore()', 'Query.endBefore()'); + } } - endAt( - docOrField: unknown | PublicDocumentSnapshot, - ...fields: unknown[] - ): PublicQuery { - const bound = this.boundFromDocOrFields( - 'Query.endAt', - docOrField, - fields, - /*before=*/ false - ); - return new Query( - queryWithEndAt(this._query, bound), - this.firestore, - this._converter - ); + endAt(...args: any[]): Query { + try { + return new Query(this.firestore, query(this._delegate, endAt(...args))); + } catch (e) { + throw replaceFunctionName(e, 'endAt()', 'Query.endAt()'); + } } isEqual(other: PublicQuery): boolean { - if (!(other instanceof Query)) { - return false; - } - return ( - this.firestore === other.firestore && - queryEquals(this._query, other._query) && - this._converter === other._converter - ); + return queryEqual(this._delegate, (other as Query)._delegate); } - withConverter(converter: PublicFirestoreDataConverter): PublicQuery { - return new Query(this._query, this.firestore, converter); - } - - /** Helper function to create a bound from a document or fields */ - private boundFromDocOrFields( - methodName: string, - docOrField: unknown | PublicDocumentSnapshot, - fields: unknown[], - before: boolean - ): Bound { - if (docOrField instanceof DocumentSnapshot) { - return newQueryBoundFromDocument( - this._query, - this.firestore._databaseId, - methodName, - docOrField._delegate._document, - before - ); + get(options?: PublicGetOptions): Promise> { + let query: Promise>; + if (options?.source === 'cache') { + query = getDocsFromCache(this._delegate); + } else if (options?.source === 'server') { + query = getDocsFromServer(this._delegate); } else { - const allFields = [docOrField].concat(fields); - return newQueryBoundFromFields( - this._query, - this.firestore._databaseId, - this._userDataReader, - methodName, - allFields, - before - ); + query = getDocs(this._delegate); } + return query.then(result => new QuerySnapshot(this.firestore, result)); } onSnapshot(observer: PartialObserver>): Unsubscribe; @@ -1774,81 +1671,20 @@ export class Query implements PublicQuery { ): Unsubscribe; onSnapshot(...args: unknown[]): Unsubscribe { - let options: ListenOptions = {}; - let currArg = 0; - if ( - typeof args[currArg] === 'object' && - !isPartialObserver(args[currArg]) - ) { - options = args[currArg] as PublicSnapshotListenOptions; - currArg++; - } - - if (isPartialObserver(args[currArg])) { - const userObserver = args[currArg] as PartialObserver< - PublicQuerySnapshot - >; - args[currArg] = userObserver.next?.bind(userObserver); - args[currArg + 1] = userObserver.error?.bind(userObserver); - args[currArg + 2] = userObserver.complete?.bind(userObserver); - } else { - } - - const observer: PartialObserver = { - next: snapshot => { - if (args[currArg]) { - (args[currArg] as NextFn>)( - new QuerySnapshot( - this.firestore, - new ExpQuerySnapshot( - this.firestore._delegate, - this._userDataWriter, - new ExpQuery( - this.firestore._delegate, - this._converter, - this._query - ), - snapshot - ) - ) - ); - } - }, - error: args[currArg + 1] as ErrorFn, - complete: args[currArg + 2] as CompleteFn - }; - - validateHasExplicitOrderByForLimitToLast(this._query); - const client = ensureFirestoreConfigured(this.firestore._delegate); - return firestoreClientListen(client, this._query, options, observer); + const options = extractSnapshotOptions(args); + const observer = wrapObserver, ExpQuerySnapshot>( + args, + snap => new QuerySnapshot(this.firestore, snap) + ); + return onSnapshot(this._delegate, options, observer); } - get(options?: PublicGetOptions): Promise> { - validateHasExplicitOrderByForLimitToLast(this._query); - - const client = ensureFirestoreConfigured(this.firestore._delegate); - return (options && options.source === 'cache' - ? firestoreClientGetDocumentsFromLocalCache(client, this._query) - : firestoreClientGetDocumentsViaSnapshotListener( - client, - this._query, - options - ) - ).then( - snap => - new QuerySnapshot( - this.firestore, - new ExpQuerySnapshot( - this.firestore._delegate, - this._userDataWriter, - new ExpQuery( - this.firestore._delegate, - this._converter, - this._query - ), - snap - ) - ) + withConverter(converter: PublicFirestoreDataConverter): Query { + return new Query( + this.firestore, + this._delegate.withConverter( + converter as UntypedFirestoreDataConverter + ) ); } } @@ -1888,11 +1724,7 @@ export class QuerySnapshot } get query(): Query { - return new Query( - this._delegate.query._query, - this._firestore, - this._delegate.query._converter - ); + return new Query(this._firestore, this._delegate.query); } get metadata(): SnapshotMetadata { @@ -1942,77 +1774,59 @@ export class CollectionReference extends Query implements PublicCollectionReference { constructor( - readonly _path: ResourcePath, - firestore: Firestore, - _converter: UntypedFirestoreDataConverter | null + readonly firestore: Firestore, + readonly _delegate: ExpCollectionReference ) { - super(newQueryForPath(_path), firestore, _converter); - if (_path.length % 2 !== 1) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid collection reference. Collection ' + - 'references must have an odd number of segments, but ' + - `${_path.canonicalString()} has ${_path.length}` - ); - } + super(firestore, _delegate); } get id(): string { - return this._query.path.lastSegment(); + return this._delegate.id; } - get parent(): PublicDocumentReference | null { - const parentPath = this._query.path.popLast(); - if (parentPath.isEmpty()) { - return null; - } else { - return DocumentReference.forPath( - parentPath, - this.firestore, - /* converter= */ null - ); - } + get path(): string { + return this._delegate.path; } - get path(): string { - return this._query.path.canonicalString(); + get parent(): DocumentReference | null { + const docRef = this._delegate.parent; + return docRef ? new DocumentReference(this.firestore, docRef) : null; } - doc(pathString?: string): PublicDocumentReference { - // We allow omission of 'pathString' but explicitly prohibit passing in both - // 'undefined' and 'null'. - if (arguments.length === 0) { - pathString = AutoId.newId(); + doc(documentPath?: string): DocumentReference { + try { + if (documentPath !== undefined) { + return new DocumentReference( + this.firestore, + doc(this._delegate, documentPath) + ); + } else { + return new DocumentReference(this.firestore, doc(this._delegate)); + } + } catch (e) { + throw replaceFunctionName(e, 'doc()', 'CollectionReference.doc()'); } - validateNonEmptyArgument('CollectionReference.doc', 'path', pathString); - const path = ResourcePath.fromString(pathString!); - return DocumentReference.forPath( - this._query.path.child(path), - this.firestore, - this._converter - ); } - add(value: T): Promise> { - const convertedValue = this._converter - ? this._converter.toFirestore(value) - : value; - const docRef = this.doc(); + add(data: T): Promise> { + return addDoc(this._delegate, data).then( + docRef => new DocumentReference(this.firestore, docRef) + ); + } - // Call set() with the converted value directly to avoid calling toFirestore() a second time. - return DocumentReference.forKey( - (docRef as DocumentReference)._delegate._key, - this.firestore, - null - ) - .set(convertedValue) - .then(() => docRef); + isEqual(other: CollectionReference): boolean { + return refEqual(this._delegate, other._delegate); } withConverter( converter: PublicFirestoreDataConverter - ): PublicCollectionReference { - return new CollectionReference(this._path, this.firestore, converter); + ): CollectionReference { + return new CollectionReference( + this.firestore, + this._delegate.withConverter( + converter as UntypedFirestoreDataConverter + ) + ); } } diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index 1449b0603ca..dc1102432e4 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -847,8 +847,8 @@ apiDescribe('Validation:', (persistence: boolean) => { const reason = `Invalid query. You have a where filter with an ` + `inequality (<, <=, >, or >=) on field 'x' and so you must also ` + - `use 'x' as your first orderBy(), but your first orderBy() is on ` + - `field 'y' instead.`; + `use 'x' as your first argument to Query.orderBy(), but your first ` + + `orderBy() is on field 'y' instead.`; expect(() => collection.where('x', '>', 32).orderBy('y')).to.throw( reason ); @@ -1163,12 +1163,12 @@ apiDescribe('Validation:', (persistence: boolean) => { const collection = db.collection('collection'); const query = collection.orderBy('foo'); let reason = - 'Invalid query. You must not call startAt() or startAfter() before calling orderBy().'; + 'Invalid query. You must not call startAt() or startAfter() before calling Query.orderBy().'; expect(() => query.startAt(1).orderBy('bar')).to.throw(reason); expect(() => query.startAfter(1).orderBy('bar')).to.throw(reason); reason = - 'Invalid query. You must not call endAt() or endBefore() before calling orderBy().'; + 'Invalid query. You must not call endAt() or endBefore() before calling Query.orderBy().'; expect(() => query.endAt(1).orderBy('bar')).to.throw(reason); expect(() => query.endBefore(1).orderBy('bar')).to.throw(reason); } diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index 3fd0d053a68..95775e0bba7 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -51,7 +51,8 @@ import { import { UserDataWriter } from '../../src/api/user_data_writer'; import { ExpUserDataWriter, - Query as ExpQuery + Query as ExpQuery, + CollectionReference as ExpCollectionReference } from '../../exp/src/api/reference'; /** @@ -77,7 +78,14 @@ export function newTestFirestore(projectId = 'new-project'): Firestore { export function collectionReference(path: string): CollectionReference { const db = firestore(); ensureFirestoreConfigured(db._delegate); - return new CollectionReference(pathFrom(path), db, /* converter= */ null); + return new CollectionReference( + db, + new ExpCollectionReference( + db._delegate, + /* converter= */ null, + pathFrom(path) + ) + ); } export function documentReference(path: string): DocumentReference { @@ -121,10 +129,14 @@ export function documentSnapshot( } export function query(path: string): Query { + const db = firestore(); return new Query( - newQueryForPath(pathFrom(path)), - firestore(), - /* converter= */ null + db, + new ExpQuery( + db._delegate, + /* converter= */ null, + newQueryForPath(pathFrom(path)) + ) ); } From a67e4cef8d66b03f55f23efef3825513a97dfa03 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 12 Nov 2020 16:09:00 -0800 Subject: [PATCH 2/3] Move methods --- packages/firestore/exp/src/api/reference.ts | 6 +- packages/firestore/lite/src/api/reference.ts | 426 ++++++++++++++++- packages/firestore/src/api/database.ts | 475 ++----------------- 3 files changed, 447 insertions(+), 460 deletions(-) diff --git a/packages/firestore/exp/src/api/reference.ts b/packages/firestore/exp/src/api/reference.ts index 807e9106f57..8bef4978eab 100644 --- a/packages/firestore/exp/src/api/reference.ts +++ b/packages/firestore/exp/src/api/reference.ts @@ -28,8 +28,7 @@ import { DocumentSnapshot, QuerySnapshot } from './snapshot'; import { applyFirestoreDataConverter, ensureFirestoreConfigured, - SnapshotMetadata, - validateHasExplicitOrderByForLimitToLast + SnapshotMetadata } from '../../../src/api/database'; import { ViewSnapshot } from '../../../src/core/view_snapshot'; import { @@ -39,7 +38,8 @@ import { newUserDataReader, Query, SetOptions, - UpdateData + UpdateData, + validateHasExplicitOrderByForLimitToLast } from '../../../lite/src/api/reference'; import { Document } from '../../../src/model/document'; import { diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts index e4283e8c6c5..ae9e6ba155b 100644 --- a/packages/firestore/lite/src/api/reference.ts +++ b/packages/firestore/lite/src/api/reference.ts @@ -15,11 +15,14 @@ * limitations under the License. */ +import { Value as ProtoValue } from '../../../src/protos/firestore_proto_api'; + import { Document } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; import { FirebaseFirestore } from './database'; import { ParsedUpdateData, + parseQueryValue, parseSetData, parseUpdateData, parseUpdateVarargs, @@ -28,13 +31,21 @@ import { import { Bound, Direction, + FieldFilter, + Filter, + findFilterOperator, + getFirstOrderByField, + getInequalityFilterField, hasLimitToLast, + isCollectionGroupQuery, LimitType, newQueryForCollectionGroup, newQueryForPath, Operator, + OrderBy, Query as InternalQuery, queryEquals, + queryOrderBy, queryWithAddedFilter, queryWithAddedOrderBy, queryWithEndAt, @@ -58,22 +69,16 @@ import { invokeCommitRpc, invokeRunQueryRpc } from '../../../src/remote/datastore'; -import { hardAssert } from '../../../src/util/assert'; +import { debugAssert, hardAssert } from '../../../src/util/assert'; import { DeleteMutation, Precondition } from '../../../src/model/mutation'; -import { - applyFirestoreDataConverter, - newQueryBoundFromDocument, - newQueryBoundFromFields, - newQueryFilter, - newQueryOrderBy, - validateHasExplicitOrderByForLimitToLast -} from '../../../src/api/database'; +import { applyFirestoreDataConverter } from '../../../src/api/database'; import { FieldPath } from './field_path'; import { validateCollectionPath, validateDocumentPath, validateNonEmptyArgument, - validatePositiveNumber + validatePositiveNumber, + valueDescription } from '../../../src/util/input_validation'; import { newSerializer } from '../../../src/platform/serializer'; import { Code, FirestoreError } from '../../../src/util/error'; @@ -82,6 +87,10 @@ import { ByteString } from '../../../src/util/byte_string'; import { Bytes } from './bytes'; import { AbstractUserDataWriter } from '../../../src/api/user_data_writer'; import { Compat } from '../../../src/compat/compat'; +import { DatabaseId } from '../../../src/core/database_info'; +import { refValue } from '../../../src/model/values'; +import { DocumentReference as ExpDocumentReference } from '../../../exp/src/api/reference'; +import { isServerTimestamp } from '../../../src/model/server_timestamps'; /** * Document data (for use with {@link setDoc()}) consists of fields mapped to @@ -621,6 +630,403 @@ function newQueryBoundFromDocOrFields( } } +export function newQueryFilter( + query: InternalQuery, + methodName: string, + dataReader: UserDataReader, + databaseId: DatabaseId, + fieldPath: InternalFieldPath, + op: Operator, + value: unknown +): FieldFilter { + let fieldValue: ProtoValue; + if (fieldPath.isKeyField()) { + if (op === Operator.ARRAY_CONTAINS || op === Operator.ARRAY_CONTAINS_ANY) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid Query. You can't perform '${op}' ` + + 'queries on FieldPath.documentId().' + ); + } else if (op === Operator.IN || op === Operator.NOT_IN) { + validateDisjunctiveFilterElements(value, op); + const referenceList: ProtoValue[] = []; + for (const arrayValue of value as ProtoValue[]) { + referenceList.push(parseDocumentIdValue(databaseId, query, arrayValue)); + } + fieldValue = { arrayValue: { values: referenceList } }; + } else { + fieldValue = parseDocumentIdValue(databaseId, query, value); + } + } else { + if ( + op === Operator.IN || + op === Operator.NOT_IN || + op === Operator.ARRAY_CONTAINS_ANY + ) { + validateDisjunctiveFilterElements(value, op); + } + fieldValue = parseQueryValue( + dataReader, + methodName, + value, + /* allowArrays= */ op === Operator.IN || op === Operator.NOT_IN + ); + } + const filter = FieldFilter.create(fieldPath, op, fieldValue); + validateNewFilter(query, filter); + return filter; +} + +export function newQueryOrderBy( + query: InternalQuery, + fieldPath: InternalFieldPath, + direction: Direction +): OrderBy { + if (query.startAt !== null) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You must not call startAt() or startAfter() before ' + + 'calling orderBy().' + ); + } + if (query.endAt !== null) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You must not call endAt() or endBefore() before ' + + 'calling orderBy().' + ); + } + const orderBy = new OrderBy(fieldPath, direction); + validateNewOrderBy(query, orderBy); + return orderBy; +} + +/** + * Create a Bound from a query and a document. + * + * Note that the Bound will always include the key of the document + * and so only the provided document will compare equal to the returned + * position. + * + * Will throw if the document does not contain all fields of the order by + * of the query or if any of the fields in the order by are an uncommitted + * server timestamp. + */ +export function newQueryBoundFromDocument( + query: InternalQuery, + databaseId: DatabaseId, + methodName: string, + doc: Document | null, + before: boolean +): Bound { + if (!doc) { + throw new FirestoreError( + Code.NOT_FOUND, + `Can't use a DocumentSnapshot that doesn't exist for ` + + `${methodName}().` + ); + } + + const components: ProtoValue[] = []; + + // Because people expect to continue/end a query at the exact document + // provided, we need to use the implicit sort order rather than the explicit + // sort order, because it's guaranteed to contain the document key. That way + // the position becomes unambiguous and the query continues/ends exactly at + // the provided document. Without the key (by using the explicit sort + // orders), multiple documents could match the position, yielding duplicate + // results. + for (const orderBy of queryOrderBy(query)) { + if (orderBy.field.isKeyField()) { + components.push(refValue(databaseId, doc.key)); + } else { + const value = doc.field(orderBy.field); + if (isServerTimestamp(value)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You are trying to start or end a query using a ' + + 'document for which the field "' + + orderBy.field + + '" is an uncommitted server timestamp. (Since the value of ' + + 'this field is unknown, you cannot start/end a query with it.)' + ); + } else if (value !== null) { + components.push(value); + } else { + const field = orderBy.field.canonicalString(); + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. You are trying to start or end a query using a ` + + `document for which the field '${field}' (used as the ` + + `orderBy) does not exist.` + ); + } + } + } + return new Bound(components, before); +} + +/** + * Converts a list of field values to a Bound for the given query. + */ +export function newQueryBoundFromFields( + query: InternalQuery, + databaseId: DatabaseId, + dataReader: UserDataReader, + methodName: string, + values: unknown[], + before: boolean +): Bound { + // Use explicit order by's because it has to match the query the user made + const orderBy = query.explicitOrderBy; + if (values.length > orderBy.length) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Too many arguments provided to ${methodName}(). ` + + `The number of arguments must be less than or equal to the ` + + `number of orderBy() clauses` + ); + } + + const components: ProtoValue[] = []; + for (let i = 0; i < values.length; i++) { + const rawValue = values[i]; + const orderByComponent = orderBy[i]; + if (orderByComponent.field.isKeyField()) { + if (typeof rawValue !== 'string') { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. Expected a string for document ID in ` + + `${methodName}(), but got a ${typeof rawValue}` + ); + } + if (!isCollectionGroupQuery(query) && rawValue.indexOf('/') !== -1) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + + `the value passed to ${methodName}() must be a plain document ID, but ` + + `'${rawValue}' contains a slash.` + ); + } + const path = query.path.child(ResourcePath.fromString(rawValue)); + if (!DocumentKey.isDocumentKey(path)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. When querying a collection group and ordering by ` + + `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + + `valid document path, but '${path}' is not because it contains an odd number ` + + `of segments.` + ); + } + const key = new DocumentKey(path); + components.push(refValue(databaseId, key)); + } else { + const wrapped = parseQueryValue(dataReader, methodName, rawValue); + components.push(wrapped); + } + } + + return new Bound(components, before); +} + +/** + * Parses the given documentIdValue into a ReferenceValue, throwing + * appropriate errors if the value is anything other than a DocumentReference + * or String, or if the string is malformed. + */ +function parseDocumentIdValue( + databaseId: DatabaseId, + query: InternalQuery, + documentIdValue: unknown +): ProtoValue { + if (documentIdValue instanceof Compat) { + documentIdValue = documentIdValue._delegate; + } + + if (typeof documentIdValue === 'string') { + if (documentIdValue === '') { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. When querying with FieldPath.documentId(), you ' + + 'must provide a valid document ID, but it was an empty string.' + ); + } + if (!isCollectionGroupQuery(query) && documentIdValue.indexOf('/') !== -1) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. When querying a collection by ` + + `FieldPath.documentId(), you must provide a plain document ID, but ` + + `'${documentIdValue}' contains a '/' character.` + ); + } + const path = query.path.child(ResourcePath.fromString(documentIdValue)); + if (!DocumentKey.isDocumentKey(path)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. When querying a collection group by ` + + `FieldPath.documentId(), the value provided must result in a valid document path, ` + + `but '${path}' is not because it has an odd number of segments (${path.length}).` + ); + } + return refValue(databaseId, new DocumentKey(path)); + } else if (documentIdValue instanceof ExpDocumentReference) { + return refValue(databaseId, documentIdValue._key); + } else { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + + `string or a DocumentReference, but it was: ` + + `${valueDescription(documentIdValue)}.` + ); + } +} + +/** + * Validates that the value passed into a disjunctive filter satisfies all + * array requirements. + */ +function validateDisjunctiveFilterElements( + value: unknown, + operator: Operator +): void { + if (!Array.isArray(value) || value.length === 0) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid Query. A non-empty array is required for ' + + `'${operator.toString()}' filters.` + ); + } + if (value.length > 10) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid Query. '${operator.toString()}' filters support a ` + + 'maximum of 10 elements in the value array.' + ); + } +} + +/** + * Given an operator, returns the set of operators that cannot be used with it. + * + * Operators in a query must adhere to the following set of rules: + * 1. Only one array operator is allowed. + * 2. Only one disjunctive operator is allowed. + * 3. NOT_EQUAL cannot be used with another NOT_EQUAL operator. + * 4. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators. + * + * Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY + * Disjunctive operators: IN, ARRAY_CONTAINS_ANY, NOT_IN + */ +function conflictingOps(op: Operator): Operator[] { + switch (op) { + case Operator.NOT_EQUAL: + return [Operator.NOT_EQUAL, Operator.NOT_IN]; + case Operator.ARRAY_CONTAINS: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.NOT_IN + ]; + case Operator.IN: + return [Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN]; + case Operator.ARRAY_CONTAINS_ANY: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.IN, + Operator.NOT_IN + ]; + case Operator.NOT_IN: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.IN, + Operator.NOT_IN, + Operator.NOT_EQUAL + ]; + default: + return []; + } +} + +function validateNewFilter(query: InternalQuery, filter: Filter): void { + debugAssert(filter instanceof FieldFilter, 'Only FieldFilters are supported'); + + if (filter.isInequality()) { + const existingField = getInequalityFilterField(query); + if (existingField !== null && !existingField.isEqual(filter.field)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. All where filters with an inequality' + + ' (<, <=, >, or >=) must be on the same field. But you have' + + ` inequality filters on '${existingField.toString()}'` + + ` and '${filter.field.toString()}'` + ); + } + + const firstOrderByField = getFirstOrderByField(query); + if (firstOrderByField !== null) { + validateOrderByAndInequalityMatch(query, filter.field, firstOrderByField); + } + } + + const conflictingOp = findFilterOperator(query, conflictingOps(filter.op)); + if (conflictingOp !== null) { + // Special case when it's a duplicate op to give a slightly clearer error message. + if (conflictingOp === filter.op) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You cannot use more than one ' + + `'${filter.op.toString()}' filter.` + ); + } else { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. You cannot use '${filter.op.toString()}' filters ` + + `with '${conflictingOp.toString()}' filters.` + ); + } + } +} + +function validateNewOrderBy(query: InternalQuery, orderBy: OrderBy): void { + if (getFirstOrderByField(query) === null) { + // This is the first order by. It must match any inequality. + const inequalityField = getInequalityFilterField(query); + if (inequalityField !== null) { + validateOrderByAndInequalityMatch(query, inequalityField, orderBy.field); + } + } +} + +function validateOrderByAndInequalityMatch( + baseQuery: InternalQuery, + inequality: InternalFieldPath, + orderBy: InternalFieldPath +): void { + if (!orderBy.isEqual(inequality)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. You have a where filter with an inequality ` + + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + + `and so you must also use '${inequality.toString()}' ` + + `as your first argument to orderBy(), but your first orderBy() ` + + `is on field '${orderBy.toString()}' instead.` + ); + } +} + +export function validateHasExplicitOrderByForLimitToLast( + query: InternalQuery +): void { + if (hasLimitToLast(query) && query.explicitOrderBy.length === 0) { + throw new FirestoreError( + Code.UNIMPLEMENTED, + 'limitToLast() queries require specifying at least one orderBy() clause' + ); + } +} + /** * A `CollectionReference` object can be used for adding documents, getting * document references, and querying for documents (using {@link query()}`). diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 5a607c5fb77..ba1074b29d4 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -15,8 +15,6 @@ * limitations under the License. */ -import { Value as ProtoValue } from '../protos/firestore_proto_api'; - import { FirebaseApp } from '@firebase/app-types'; import { _FirebaseApp, FirebaseService } from '@firebase/app-types/private'; import { DatabaseId } from '../core/database_info'; @@ -27,35 +25,17 @@ import { firestoreClientTransaction, firestoreClientWrite } from '../core/firestore_client'; -import { - Bound, - Direction, - FieldFilter, - Filter, - findFilterOperator, - getFirstOrderByField, - getInequalityFilterField, - hasLimitToLast, - isCollectionGroupQuery, - Operator, - OrderBy, - Query as InternalQuery, - queryOrderBy -} from '../core/query'; import { Transaction as InternalTransaction } from '../core/transaction'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { DeleteMutation, Mutation, Precondition } from '../model/mutation'; import { FieldPath, ResourcePath } from '../model/path'; -import { isServerTimestamp } from '../model/server_timestamps'; -import { refValue } from '../model/values'; import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { cast, validateIsNotUsedTogether, - validateSetOptions, - valueDescription + validateSetOptions } from '../util/input_validation'; import { logWarn, setLogLevel as setClientLogLevel } from '../util/log'; import { FieldPath as ExpFieldPath } from '../../lite/src/api/field_path'; @@ -68,7 +48,6 @@ import { Unsubscribe } from './observer'; import { - parseQueryValue, parseSetData, parseUpdateData, parseUpdateVarargs, @@ -83,49 +62,51 @@ import { enableMultiTabIndexedDbPersistence, enableNetwork, FirebaseFirestore, - waitForPendingWrites, - FirebaseFirestore as ExpFirebaseFirestore + FirebaseFirestore as ExpFirebaseFirestore, + waitForPendingWrites } from '../../exp/src/api/database'; import { + DocumentChange as ExpDocumentChange, DocumentSnapshot as ExpDocumentSnapshot, QuerySnapshot as ExpQuerySnapshot, - DocumentChange as ExpDocumentChange, snapshotEqual } from '../../exp/src/api/snapshot'; -import { refEqual, newUserDataReader } from '../../lite/src/api/reference'; +import { newUserDataReader, refEqual } from '../../lite/src/api/reference'; import { - onSnapshotsInSync, - setDoc, - updateDoc, - deleteDoc, - getDocFromCache, - getDocFromServer, - getDoc, - onSnapshot, + addDoc, collection, - doc, collectionGroup, - query, - where, - limit, - limitToLast, - orderBy, - startAt, - startAfter, + CollectionReference as ExpCollectionReference, + deleteDoc, + doc, + DocumentReference as ExpDocumentReference, endAt, endBefore, - queryEqual, + getDoc, + getDocFromCache, + getDocFromServer, + getDocs, getDocsFromCache, getDocsFromServer, - getDocs, - addDoc, - DocumentReference as ExpDocumentReference, + limit, + limitToLast, + onSnapshot, + onSnapshotsInSync, + orderBy, + query, Query as ExpQuery, - CollectionReference as ExpCollectionReference + queryEqual, + setDoc, + startAfter, + startAt, + updateDoc, + where } from '../../exp/src/api/reference'; import { LRU_COLLECTION_DISABLED } from '../local/lru_garbage_collector'; import { Compat } from '../compat/compat'; import { ApiLoadBundleTask, LoadBundleTask } from './bundle'; +import { makeDatabaseInfo } from '../../lite/src/api/database'; +import { DEFAULT_HOST } from '../../lite/src/api/components'; import { CollectionReference as PublicCollectionReference, @@ -155,9 +136,6 @@ import { WriteBatch as PublicWriteBatch } from '@firebase/firestore-types'; -import { makeDatabaseInfo } from '../../lite/src/api/database'; -import { DEFAULT_HOST } from '../../lite/src/api/components'; - /** * Constant used to indicate the LRU garbage collection should be disabled. * Set this value as the `cacheSizeBytes` on the settings passed to the @@ -1147,403 +1125,6 @@ export class QueryDocumentSnapshot } } -export function newQueryFilter( - query: InternalQuery, - methodName: string, - dataReader: UserDataReader, - databaseId: DatabaseId, - fieldPath: FieldPath, - op: Operator, - value: unknown -): FieldFilter { - let fieldValue: ProtoValue; - if (fieldPath.isKeyField()) { - if (op === Operator.ARRAY_CONTAINS || op === Operator.ARRAY_CONTAINS_ANY) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid Query. You can't perform '${op}' ` + - 'queries on FieldPath.documentId().' - ); - } else if (op === Operator.IN || op === Operator.NOT_IN) { - validateDisjunctiveFilterElements(value, op); - const referenceList: ProtoValue[] = []; - for (const arrayValue of value as ProtoValue[]) { - referenceList.push(parseDocumentIdValue(databaseId, query, arrayValue)); - } - fieldValue = { arrayValue: { values: referenceList } }; - } else { - fieldValue = parseDocumentIdValue(databaseId, query, value); - } - } else { - if ( - op === Operator.IN || - op === Operator.NOT_IN || - op === Operator.ARRAY_CONTAINS_ANY - ) { - validateDisjunctiveFilterElements(value, op); - } - fieldValue = parseQueryValue( - dataReader, - methodName, - value, - /* allowArrays= */ op === Operator.IN || op === Operator.NOT_IN - ); - } - const filter = FieldFilter.create(fieldPath, op, fieldValue); - validateNewFilter(query, filter); - return filter; -} - -export function newQueryOrderBy( - query: InternalQuery, - fieldPath: FieldPath, - direction: Direction -): OrderBy { - if (query.startAt !== null) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You must not call startAt() or startAfter() before ' + - 'calling orderBy().' - ); - } - if (query.endAt !== null) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You must not call endAt() or endBefore() before ' + - 'calling orderBy().' - ); - } - const orderBy = new OrderBy(fieldPath, direction); - validateNewOrderBy(query, orderBy); - return orderBy; -} - -/** - * Create a Bound from a query and a document. - * - * Note that the Bound will always include the key of the document - * and so only the provided document will compare equal to the returned - * position. - * - * Will throw if the document does not contain all fields of the order by - * of the query or if any of the fields in the order by are an uncommitted - * server timestamp. - */ -export function newQueryBoundFromDocument( - query: InternalQuery, - databaseId: DatabaseId, - methodName: string, - doc: Document | null, - before: boolean -): Bound { - if (!doc) { - throw new FirestoreError( - Code.NOT_FOUND, - `Can't use a DocumentSnapshot that doesn't exist for ` + - `${methodName}().` - ); - } - - const components: ProtoValue[] = []; - - // Because people expect to continue/end a query at the exact document - // provided, we need to use the implicit sort order rather than the explicit - // sort order, because it's guaranteed to contain the document key. That way - // the position becomes unambiguous and the query continues/ends exactly at - // the provided document. Without the key (by using the explicit sort - // orders), multiple documents could match the position, yielding duplicate - // results. - for (const orderBy of queryOrderBy(query)) { - if (orderBy.field.isKeyField()) { - components.push(refValue(databaseId, doc.key)); - } else { - const value = doc.field(orderBy.field); - if (isServerTimestamp(value)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You are trying to start or end a query using a ' + - 'document for which the field "' + - orderBy.field + - '" is an uncommitted server timestamp. (Since the value of ' + - 'this field is unknown, you cannot start/end a query with it.)' - ); - } else if (value !== null) { - components.push(value); - } else { - const field = orderBy.field.canonicalString(); - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. You are trying to start or end a query using a ` + - `document for which the field '${field}' (used as the ` + - `orderBy) does not exist.` - ); - } - } - } - return new Bound(components, before); -} - -/** - * Converts a list of field values to a Bound for the given query. - */ -export function newQueryBoundFromFields( - query: InternalQuery, - databaseId: DatabaseId, - dataReader: UserDataReader, - methodName: string, - values: unknown[], - before: boolean -): Bound { - // Use explicit order by's because it has to match the query the user made - const orderBy = query.explicitOrderBy; - if (values.length > orderBy.length) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Too many arguments provided to ${methodName}(). ` + - `The number of arguments must be less than or equal to the ` + - `number of orderBy() clauses` - ); - } - - const components: ProtoValue[] = []; - for (let i = 0; i < values.length; i++) { - const rawValue = values[i]; - const orderByComponent = orderBy[i]; - if (orderByComponent.field.isKeyField()) { - if (typeof rawValue !== 'string') { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. Expected a string for document ID in ` + - `${methodName}(), but got a ${typeof rawValue}` - ); - } - if (!isCollectionGroupQuery(query) && rawValue.indexOf('/') !== -1) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + - `the value passed to ${methodName}() must be a plain document ID, but ` + - `'${rawValue}' contains a slash.` - ); - } - const path = query.path.child(ResourcePath.fromString(rawValue)); - if (!DocumentKey.isDocumentKey(path)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. When querying a collection group and ordering by ` + - `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + - `valid document path, but '${path}' is not because it contains an odd number ` + - `of segments.` - ); - } - const key = new DocumentKey(path); - components.push(refValue(databaseId, key)); - } else { - const wrapped = parseQueryValue(dataReader, methodName, rawValue); - components.push(wrapped); - } - } - - return new Bound(components, before); -} - -/** - * Parses the given documentIdValue into a ReferenceValue, throwing - * appropriate errors if the value is anything other than a DocumentReference - * or String, or if the string is malformed. - */ -function parseDocumentIdValue( - databaseId: DatabaseId, - query: InternalQuery, - documentIdValue: unknown -): ProtoValue { - if (documentIdValue instanceof Compat) { - documentIdValue = documentIdValue._delegate; - } - - if (typeof documentIdValue === 'string') { - if (documentIdValue === '') { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. When querying with FieldPath.documentId(), you ' + - 'must provide a valid document ID, but it was an empty string.' - ); - } - if (!isCollectionGroupQuery(query) && documentIdValue.indexOf('/') !== -1) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. When querying a collection by ` + - `FieldPath.documentId(), you must provide a plain document ID, but ` + - `'${documentIdValue}' contains a '/' character.` - ); - } - const path = query.path.child(ResourcePath.fromString(documentIdValue)); - if (!DocumentKey.isDocumentKey(path)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. When querying a collection group by ` + - `FieldPath.documentId(), the value provided must result in a valid document path, ` + - `but '${path}' is not because it has an odd number of segments (${path.length}).` - ); - } - return refValue(databaseId, new DocumentKey(path)); - } else if (documentIdValue instanceof ExpDocumentReference) { - return refValue(databaseId, documentIdValue._key); - } else { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + - `string or a DocumentReference, but it was: ` + - `${valueDescription(documentIdValue)}.` - ); - } -} - -/** - * Validates that the value passed into a disjunctive filter satisfies all - * array requirements. - */ -function validateDisjunctiveFilterElements( - value: unknown, - operator: Operator -): void { - if (!Array.isArray(value) || value.length === 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid Query. A non-empty array is required for ' + - `'${operator.toString()}' filters.` - ); - } - if (value.length > 10) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid Query. '${operator.toString()}' filters support a ` + - 'maximum of 10 elements in the value array.' - ); - } -} - -/** - * Given an operator, returns the set of operators that cannot be used with it. - * - * Operators in a query must adhere to the following set of rules: - * 1. Only one array operator is allowed. - * 2. Only one disjunctive operator is allowed. - * 3. NOT_EQUAL cannot be used with another NOT_EQUAL operator. - * 4. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators. - * - * Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY - * Disjunctive operators: IN, ARRAY_CONTAINS_ANY, NOT_IN - */ -function conflictingOps(op: Operator): Operator[] { - switch (op) { - case Operator.NOT_EQUAL: - return [Operator.NOT_EQUAL, Operator.NOT_IN]; - case Operator.ARRAY_CONTAINS: - return [ - Operator.ARRAY_CONTAINS, - Operator.ARRAY_CONTAINS_ANY, - Operator.NOT_IN - ]; - case Operator.IN: - return [Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN]; - case Operator.ARRAY_CONTAINS_ANY: - return [ - Operator.ARRAY_CONTAINS, - Operator.ARRAY_CONTAINS_ANY, - Operator.IN, - Operator.NOT_IN - ]; - case Operator.NOT_IN: - return [ - Operator.ARRAY_CONTAINS, - Operator.ARRAY_CONTAINS_ANY, - Operator.IN, - Operator.NOT_IN, - Operator.NOT_EQUAL - ]; - default: - return []; - } -} - -function validateNewFilter(query: InternalQuery, filter: Filter): void { - debugAssert(filter instanceof FieldFilter, 'Only FieldFilters are supported'); - - if (filter.isInequality()) { - const existingField = getInequalityFilterField(query); - if (existingField !== null && !existingField.isEqual(filter.field)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. All where filters with an inequality' + - ' (<, <=, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` - ); - } - - const firstOrderByField = getFirstOrderByField(query); - if (firstOrderByField !== null) { - validateOrderByAndInequalityMatch(query, filter.field, firstOrderByField); - } - } - - const conflictingOp = findFilterOperator(query, conflictingOps(filter.op)); - if (conflictingOp !== null) { - // Special case when it's a duplicate op to give a slightly clearer error message. - if (conflictingOp === filter.op) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` - ); - } else { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` - ); - } - } -} - -function validateNewOrderBy(query: InternalQuery, orderBy: OrderBy): void { - if (getFirstOrderByField(query) === null) { - // This is the first order by. It must match any inequality. - const inequalityField = getInequalityFilterField(query); - if (inequalityField !== null) { - validateOrderByAndInequalityMatch(query, inequalityField, orderBy.field); - } - } -} - -function validateOrderByAndInequalityMatch( - baseQuery: InternalQuery, - inequality: FieldPath, - orderBy: FieldPath -): void { - if (!orderBy.isEqual(inequality)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. You have a where filter with an inequality ` + - `(<, <=, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first argument to orderBy(), but your first orderBy() ` + - `is on field '${orderBy.toString()}' instead.` - ); - } -} - -export function validateHasExplicitOrderByForLimitToLast( - query: InternalQuery -): void { - if (hasLimitToLast(query) && query.explicitOrderBy.length === 0) { - throw new FirestoreError( - Code.UNIMPLEMENTED, - 'limitToLast() queries require specifying at least one orderBy() clause' - ); - } -} - export class Query extends Compat> implements PublicQuery { From 163f9549f4829823ddf030f2675c997d1d772824 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 13 Nov 2020 09:29:16 -0800 Subject: [PATCH 3/3] Add comments --- packages/firestore/src/api/database.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index ba1074b29d4..34e6fa15f6e 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1138,6 +1138,9 @@ export class Query value: unknown ): Query { try { + // The "as string" cast is a little bit of a hack. `where` accepts the + // FieldPath Compat type as input, but is not typed as such in order to + // not expose this via our public typings file. return new Query( this.firestore, query(this._delegate, where(fieldPath as string, opStr, value)) @@ -1152,6 +1155,9 @@ export class Query directionStr?: PublicOrderByDirection ): Query { try { + // The "as string" cast is a little bit of a hack. `orderBy` accepts the + // FieldPath Compat type as input, but is not typed as such in order to + // not expose this via our public typings file. return new Query( this.firestore, query(this._delegate, orderBy(fieldPath as string, directionStr))