From ff78c4838591d38f6ceceb7591bd30c4a1a93661 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 13 Oct 2020 16:55:29 -0700 Subject: [PATCH] Remove JS Input Validation --- .../firestore/lite/src/api/field_value.ts | 3 - packages/firestore/lite/src/api/reference.ts | 26 +- packages/firestore/src/api/blob.ts | 11 +- packages/firestore/src/api/database.ts | 277 +------------- packages/firestore/src/api/field_path.ts | 14 +- packages/firestore/src/api/geo_point.ts | 7 - packages/firestore/src/compat/field_value.ts | 12 - .../firestore/src/util/input_validation.ts | 350 +----------------- .../test/integration/api/validation.test.ts | 304 ++------------- 9 files changed, 60 insertions(+), 944 deletions(-) diff --git a/packages/firestore/lite/src/api/field_value.ts b/packages/firestore/lite/src/api/field_value.ts index 505d3d3d9a1..63a02c909ad 100644 --- a/packages/firestore/lite/src/api/field_value.ts +++ b/packages/firestore/lite/src/api/field_value.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { validateAtLeastNumberOfArgs } from '../../../src/util/input_validation'; import { ArrayRemoveFieldValueImpl, ArrayUnionFieldValueImpl, @@ -69,7 +68,6 @@ export function serverTimestamp(): FieldValue { * `updateDoc()`. */ export function arrayUnion(...elements: unknown[]): FieldValue { - validateAtLeastNumberOfArgs('arrayUnion()', arguments, 1); // NOTE: We don't actually parse the data until it's used in set() or // update() since we'd need the Firestore instance to do this. return new ArrayUnionFieldValueImpl('arrayUnion', elements); @@ -87,7 +85,6 @@ export function arrayUnion(...elements: unknown[]): FieldValue { * `updateDoc()` */ export function arrayRemove(...elements: unknown[]): FieldValue { - validateAtLeastNumberOfArgs('arrayRemove()', arguments, 1); // NOTE: We don't actually parse the data until it's used in set() or // update() since we'd need the Firestore instance to do this. return new ArrayRemoveFieldValueImpl('arrayRemove', elements); diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts index a19d46654cd..e4ed5bf3a6b 100644 --- a/packages/firestore/lite/src/api/reference.ts +++ b/packages/firestore/lite/src/api/reference.ts @@ -73,7 +73,7 @@ import { FieldPath } from './field_path'; import { validateCollectionPath, validateDocumentPath, - validateExactNumberOfArgs, + validateNonEmptyString, validatePositiveNumber } from '../../../src/util/input_validation'; import { newSerializer } from '../../../src/platform/serializer'; @@ -335,8 +335,6 @@ export function where( opStr: WhereFilterOp, value: unknown ): QueryConstraint { - // TODO(firestorelite): Consider validating the enum strings (note that - // TypeScript does not support passing invalid values). const op = opStr as Operator; const field = fieldPathFromArgument('where', fieldPath); return new QueryFilterConstraint(field, op, value); @@ -381,8 +379,6 @@ export function orderBy( fieldPath: string | FieldPath, directionStr: OrderByDirection = 'asc' ): QueryConstraint { - // TODO(firestorelite): Consider validating the enum strings (note that - // TypeScript does not support passing invalid values). const direction = directionStr as Direction; const path = fieldPathFromArgument('orderBy', fieldPath); return new QueryOrderByConstraint(path, direction); @@ -597,7 +593,6 @@ function newQueryBoundFromDocOrFields( before: boolean ): Bound { if (docOrFields[0] instanceof DocumentSnapshot) { - validateExactNumberOfArgs(methodName, docOrFields, 1); return newQueryBoundFromDocument( query._query, query.firestore._databaseId, @@ -738,7 +733,7 @@ export function collection( path: string, ...pathSegments: string[] ): CollectionReference { - validateNonEmptyArgument('collection', 'path', path); + validateNonEmptyString('collection', 'path', path); if (parent instanceof FirebaseFirestore) { const absolutePath = ResourcePath.fromString(path, ...pathSegments); validateCollectionPath(absolutePath); @@ -785,7 +780,7 @@ export function collectionGroup( firestore: FirebaseFirestore, collectionId: string ): Query { - validateNonEmptyArgument('collectionGroup', 'collection id', collectionId); + validateNonEmptyString('collectionGroup', 'collection id', collectionId); if (collectionId.indexOf('/') >= 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -868,7 +863,7 @@ export function doc( if (arguments.length === 1) { path = AutoId.newId(); } - validateNonEmptyArgument('doc', 'path', path); + validateNonEmptyString('doc', 'path', path); if (parent instanceof FirebaseFirestore) { const absolutePath = ResourcePath.fromString(path, ...pathSegments); @@ -1234,16 +1229,3 @@ export function newUserDataReader( serializer ); } - -function validateNonEmptyArgument( - functionName: string, - argumentName: string, - argument?: string -): asserts argument is string { - if (!argument) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() cannot be called with an empty ${argumentName}.` - ); - } -} diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index 119a07217e6..c2404026cdd 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -17,11 +17,7 @@ import { isBase64Available } from '../platform/base64'; import { Code, FirestoreError } from '../util/error'; -import { - invalidClassError, - validateArgType, - validateExactNumberOfArgs -} from '../util/input_validation'; +import { invalidClassError } from '../util/input_validation'; import { ByteString } from '../util/byte_string'; import { Bytes } from '../../lite/src/api/bytes'; @@ -57,8 +53,6 @@ function assertBase64Available(): void { */ export class Blob extends Bytes { static fromBase64String(base64: string): Blob { - validateExactNumberOfArgs('Blob.fromBase64String', arguments, 1); - validateArgType('Blob.fromBase64String', 'string', 1, base64); assertBase64Available(); try { return new Blob(ByteString.fromBase64String(base64)); @@ -71,7 +65,6 @@ export class Blob extends Bytes { } static fromUint8Array(array: Uint8Array): Blob { - validateExactNumberOfArgs('Blob.fromUint8Array', arguments, 1); assertUint8ArrayAvailable(); if (!(array instanceof Uint8Array)) { throw invalidClassError('Blob.fromUint8Array', 'Uint8Array', 1, array); @@ -80,13 +73,11 @@ export class Blob extends Bytes { } toBase64(): string { - validateExactNumberOfArgs('Blob.toBase64', arguments, 0); assertBase64Available(); return super.toBase64(); } toUint8Array(): Uint8Array { - validateExactNumberOfArgs('Blob.toUint8Array', arguments, 0); assertUint8ArrayAvailable(); return super.toUint8Array(); } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index c9912e81a9f..4ae402bfe0c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -93,21 +93,10 @@ import { AsyncQueue } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { invalidClassError, - validateArgType, - validateAtLeastNumberOfArgs, - validateBetweenNumberOfArgs, - validateDefined, - validateExactNumberOfArgs, - validateNamedOptionalPropertyEquals, - validateNamedOptionalType, - validateNamedType, - validateOptionalArgType, - validateOptionalArrayElements, - validateOptionNames, validatePositiveNumber, - validateStringEnum, valueDescription, - validateIsNotUsedTogether + validateIsNotUsedTogether, + validateNonEmptyString } from '../util/input_validation'; import { getLogLevel, logError, LogLevel, setLogLevel } from '../util/log'; import { AutoId } from '../util/misc'; @@ -212,45 +201,13 @@ class FirestoreSettings { this.host = DEFAULT_HOST; this.ssl = DEFAULT_SSL; } else { - validateNamedType('settings', 'non-empty string', 'host', settings.host); this.host = settings.host; - validateNamedOptionalType('settings', 'boolean', 'ssl', settings.ssl); this.ssl = settings.ssl ?? DEFAULT_SSL; } - validateOptionNames('settings', settings, [ - 'host', - 'ssl', - 'credentials', - 'timestampsInSnapshots', - 'cacheSizeBytes', - 'experimentalForceLongPolling', - 'experimentalAutoDetectLongPolling', - 'ignoreUndefinedProperties' - ]); - validateNamedOptionalType( - 'settings', - 'object', - 'credentials', - settings.credentials - ); this.credentials = settings.credentials; - validateNamedOptionalType( - 'settings', - 'boolean', - 'timestampsInSnapshots', - settings.timestampsInSnapshots - ); - - validateNamedOptionalType( - 'settings', - 'boolean', - 'ignoreUndefinedProperties', - settings.ignoreUndefinedProperties - ); - // Nobody should set timestampsInSnapshots anymore, but the error depends on // whether they set it to true or false... if (settings.timestampsInSnapshots === true) { @@ -269,12 +226,6 @@ class FirestoreSettings { this.ignoreUndefinedProperties = settings.ignoreUndefinedProperties ?? DEFAULT_IGNORE_UNDEFINED_PROPERTIES; - validateNamedOptionalType( - 'settings', - 'number', - 'cacheSizeBytes', - settings.cacheSizeBytes - ); if (settings.cacheSizeBytes === undefined) { this.cacheSizeBytes = LruParams.DEFAULT_CACHE_SIZE_BYTES; } else { @@ -291,21 +242,9 @@ class FirestoreSettings { } } - validateNamedOptionalType( - 'settings', - 'boolean', - 'experimentalForceLongPolling', - settings.experimentalForceLongPolling - ); this.experimentalForceLongPolling = settings.experimentalForceLongPolling ?? DEFAULT_FORCE_LONG_POLLING; - validateNamedOptionalType( - 'settings', - 'boolean', - 'experimentalAutoDetectLongPolling', - settings.experimentalAutoDetectLongPolling - ); this.experimentalAutoDetectLongPolling = settings.experimentalAutoDetectLongPolling ?? DEFAULT_FORCE_AUTO_DETECT_LONG_POLLING; @@ -412,9 +351,6 @@ export class Firestore implements PublicFirestore, FirebaseService { } settings(settingsLiteral: PublicSettings): void { - validateExactNumberOfArgs('Firestore.settings', arguments, 1); - validateArgType('Firestore.settings', 'object', 1, settingsLiteral); - if (settingsLiteral.merge) { settingsLiteral = { ...this._settings, ...settingsLiteral }; // Remove the property from the settings once the merge is completed @@ -547,7 +483,6 @@ export class Firestore implements PublicFirestore, FirebaseService { arg as PartialObserver ); } else { - validateArgType('Firestore.onSnapshotsInSync', 'function', 1, arg); const observer: PartialObserver = { next: arg as () => void }; @@ -644,8 +579,7 @@ export class Firestore implements PublicFirestore, FirebaseService { }; collection(pathString: string): PublicCollectionReference { - validateExactNumberOfArgs('Firestore.collection', arguments, 1); - validateArgType('Firestore.collection', 'non-empty string', 1, pathString); + validateNonEmptyString('Firestore.collection', 'path', pathString); this.ensureClientConfigured(); return new CollectionReference( ResourcePath.fromString(pathString), @@ -655,8 +589,7 @@ export class Firestore implements PublicFirestore, FirebaseService { } doc(pathString: string): PublicDocumentReference { - validateExactNumberOfArgs('Firestore.doc', arguments, 1); - validateArgType('Firestore.doc', 'non-empty string', 1, pathString); + validateNonEmptyString('Firestore.doc', 'path', pathString); this.ensureClientConfigured(); return DocumentReference.forPath( ResourcePath.fromString(pathString), @@ -666,11 +599,9 @@ export class Firestore implements PublicFirestore, FirebaseService { } collectionGroup(collectionId: string): PublicQuery { - validateExactNumberOfArgs('Firestore.collectionGroup', arguments, 1); - validateArgType( + validateNonEmptyString( 'Firestore.collectionGroup', - 'non-empty string', - 1, + 'collectionId', collectionId ); if (collectionId.indexOf('/') >= 0) { @@ -691,8 +622,6 @@ export class Firestore implements PublicFirestore, FirebaseService { runTransaction( updateFunction: (transaction: PublicTransaction) => Promise ): Promise { - validateExactNumberOfArgs('Firestore.runTransaction', arguments, 1); - validateArgType('Firestore.runTransaction', 'function', 1, updateFunction); return this.ensureClientConfigured().transaction( (transaction: InternalTransaction) => { return updateFunction(new Transaction(this, transaction)); @@ -727,13 +656,6 @@ export class Firestore implements PublicFirestore, FirebaseService { } static setLogLevel(level: PublicLogLevel): void { - validateExactNumberOfArgs('Firestore.setLogLevel', arguments, 1); - validateStringEnum( - 'setLogLevel', - ['debug', 'error', 'silent', 'warn', 'info', 'verbose'], - 1, - level - ); setLogLevel(level); } @@ -761,7 +683,6 @@ export class Transaction implements PublicTransaction { get( documentRef: PublicDocumentReference ): Promise> { - validateExactNumberOfArgs('Transaction.get', arguments, 1); const ref = validateReference( 'Transaction.get', documentRef, @@ -811,7 +732,6 @@ export class Transaction implements PublicTransaction { value: T | Partial, options?: SetOptions ): Transaction { - validateBetweenNumberOfArgs('Transaction.set', arguments, 2, 3); const ref = validateReference( 'Transaction.set', documentRef, @@ -858,7 +778,6 @@ export class Transaction implements PublicTransaction { typeof fieldOrUpdateData === 'string' || fieldOrUpdateData instanceof ExternalFieldPath ) { - validateAtLeastNumberOfArgs('Transaction.update', arguments, 3); ref = validateReference( 'Transaction.update', documentRef, @@ -873,7 +792,6 @@ export class Transaction implements PublicTransaction { moreFieldsAndValues ); } else { - validateExactNumberOfArgs('Transaction.update', arguments, 2); ref = validateReference( 'Transaction.update', documentRef, @@ -892,7 +810,6 @@ export class Transaction implements PublicTransaction { } delete(documentRef: PublicDocumentReference): Transaction { - validateExactNumberOfArgs('Transaction.delete', arguments, 1); const ref = validateReference( 'Transaction.delete', documentRef, @@ -920,7 +837,6 @@ export class WriteBatch implements PublicWriteBatch { value: T | Partial, options?: SetOptions ): WriteBatch { - validateBetweenNumberOfArgs('WriteBatch.set', arguments, 2, 3); this.verifyNotCommitted(); const ref = validateReference( 'WriteBatch.set', @@ -972,7 +888,6 @@ export class WriteBatch implements PublicWriteBatch { typeof fieldOrUpdateData === 'string' || fieldOrUpdateData instanceof ExternalFieldPath ) { - validateAtLeastNumberOfArgs('WriteBatch.update', arguments, 3); ref = validateReference( 'WriteBatch.update', documentRef, @@ -987,7 +902,6 @@ export class WriteBatch implements PublicWriteBatch { moreFieldsAndValues ); } else { - validateExactNumberOfArgs('WriteBatch.update', arguments, 2); ref = validateReference( 'WriteBatch.update', documentRef, @@ -1008,7 +922,6 @@ export class WriteBatch implements PublicWriteBatch { } delete(documentRef: PublicDocumentReference): WriteBatch { - validateExactNumberOfArgs('WriteBatch.delete', arguments, 1); this.verifyNotCommitted(); const ref = validateReference( 'WriteBatch.delete', @@ -1092,13 +1005,7 @@ export class DocumentReference } collection(pathString: string): PublicCollectionReference { - validateExactNumberOfArgs('DocumentReference.collection', arguments, 1); - validateArgType( - 'DocumentReference.collection', - 'non-empty string', - 1, - pathString - ); + validateNonEmptyString('DocumentReference.collection', 'path', pathString); if (!pathString) { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -1127,7 +1034,6 @@ export class DocumentReference set(value: Partial, options: SetOptions): Promise; set(value: T): Promise; set(value: T | Partial, options?: SetOptions): Promise { - validateBetweenNumberOfArgs('DocumentReference.set', arguments, 1, 2); options = validateSetOptions('DocumentReference.set', options); const convertedValue = applyFirestoreDataConverter( this._converter, @@ -1164,7 +1070,6 @@ export class DocumentReference typeof fieldOrUpdateData === 'string' || fieldOrUpdateData instanceof ExternalFieldPath ) { - validateAtLeastNumberOfArgs('DocumentReference.update', arguments, 2); parsed = parseUpdateVarargs( this.firestore._dataReader, 'DocumentReference.update', @@ -1174,7 +1079,6 @@ export class DocumentReference moreFieldsAndValues ); } else { - validateExactNumberOfArgs('DocumentReference.update', arguments, 1); parsed = parseUpdateData( this.firestore._dataReader, 'DocumentReference.update', @@ -1189,7 +1093,6 @@ export class DocumentReference } delete(): Promise { - validateExactNumberOfArgs('DocumentReference.delete', arguments, 0); return this._firestoreClient.write([ new DeleteMutation(this._key, Precondition.none()) ]); @@ -1213,12 +1116,6 @@ export class DocumentReference ): Unsubscribe; onSnapshot(...args: unknown[]): Unsubscribe { - validateBetweenNumberOfArgs( - 'DocumentReference.onSnapshot', - arguments, - 1, - 4 - ); let options: ListenOptions = { includeMetadataChanges: false }; @@ -1228,15 +1125,6 @@ export class DocumentReference !isPartialObserver(args[currArg]) ) { options = args[currArg] as SnapshotListenOptions; - validateOptionNames('DocumentReference.onSnapshot', options, [ - 'includeMetadataChanges' - ]); - validateNamedOptionalType( - 'DocumentReference.onSnapshot', - 'boolean', - 'includeMetadataChanges', - options.includeMetadataChanges - ); currArg++; } @@ -1251,25 +1139,6 @@ export class DocumentReference args[currArg] = userObserver.next?.bind(userObserver); args[currArg + 1] = userObserver.error?.bind(userObserver); args[currArg + 2] = userObserver.complete?.bind(userObserver); - } else { - validateArgType( - 'DocumentReference.onSnapshot', - 'function', - currArg, - args[currArg] - ); - validateOptionalArgType( - 'DocumentReference.onSnapshot', - 'function', - currArg + 1, - args[currArg + 1] - ); - validateOptionalArgType( - 'DocumentReference.onSnapshot', - 'function', - currArg + 2, - args[currArg + 2] - ); } const observer: PartialObserver = { @@ -1292,9 +1161,6 @@ export class DocumentReference } get(options?: GetOptions): Promise> { - validateBetweenNumberOfArgs('DocumentReference.get', arguments, 0, 1); - validateGetOptions('DocumentReference.get', options); - const firestoreClient = this.firestore.ensureClientConfigured(); if (options && options.source === 'cache') { return firestoreClient @@ -1404,9 +1270,7 @@ export class DocumentSnapshot private readonly _converter: FirestoreDataConverter | null ) {} - data(options?: PublicSnapshotOptions): T | undefined { - validateBetweenNumberOfArgs('DocumentSnapshot.data', arguments, 0, 1); - options = validateSnapshotOptions('DocumentSnapshot.data', options); + data(options: PublicSnapshotOptions = {}): T | undefined { if (!this._document) { return undefined; } else { @@ -1438,10 +1302,8 @@ export class DocumentSnapshot get( fieldPath: string | ExternalFieldPath, - options?: PublicSnapshotOptions + options: PublicSnapshotOptions = {} ): unknown { - validateBetweenNumberOfArgs('DocumentSnapshot.get', arguments, 1, 2); - options = validateSnapshotOptions('DocumentSnapshot.get', options); if (this._document) { const value = this._document .data() @@ -1932,24 +1794,6 @@ export class Query implements PublicQuery { opStr: WhereFilterOp, value: unknown ): PublicQuery { - validateExactNumberOfArgs('Query.where', arguments, 3); - validateDefined('Query.where', 3, value); - - // Enumerated from the WhereFilterOp type in index.d.ts. - const whereFilterOpEnums = [ - Operator.LESS_THAN, - Operator.LESS_THAN_OR_EQUAL, - Operator.EQUAL, - Operator.NOT_EQUAL, - Operator.GREATER_THAN_OR_EQUAL, - Operator.GREATER_THAN, - Operator.ARRAY_CONTAINS, - Operator.IN, - Operator.ARRAY_CONTAINS_ANY, - Operator.NOT_IN - ]; - const op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr); - const fieldPath = fieldPathFromArgument('Query.where', field); const filter = newQueryFilter( this._query, @@ -1957,7 +1801,7 @@ export class Query implements PublicQuery { this.firestore._dataReader, this.firestore._databaseId, fieldPath, - op, + opStr as Operator, value ); return new Query( @@ -1971,13 +1815,6 @@ export class Query implements PublicQuery { field: string | ExternalFieldPath, directionStr?: OrderByDirection ): PublicQuery { - validateBetweenNumberOfArgs('Query.orderBy', arguments, 1, 2); - validateOptionalArgType( - 'Query.orderBy', - 'non-empty string', - 2, - directionStr - ); let direction: Direction; if (directionStr === undefined || directionStr === 'asc') { direction = Direction.ASCENDING; @@ -2000,8 +1837,6 @@ export class Query implements PublicQuery { } limit(n: number): PublicQuery { - validateExactNumberOfArgs('Query.limit', arguments, 1); - validateArgType('Query.limit', 'number', 1, n); validatePositiveNumber('Query.limit', 1, n); return new Query( queryWithLimit(this._query, n, LimitType.First), @@ -2011,8 +1846,6 @@ export class Query implements PublicQuery { } limitToLast(n: number): PublicQuery { - validateExactNumberOfArgs('Query.limitToLast', arguments, 1); - validateArgType('Query.limitToLast', 'number', 1, n); validatePositiveNumber('Query.limitToLast', 1, n); return new Query( queryWithLimit(this._query, n, LimitType.Last), @@ -2025,7 +1858,6 @@ export class Query implements PublicQuery { docOrField: unknown | PublicDocumentSnapshot, ...fields: unknown[] ): PublicQuery { - validateAtLeastNumberOfArgs('Query.startAt', arguments, 1); const bound = this.boundFromDocOrFields( 'Query.startAt', docOrField, @@ -2043,7 +1875,6 @@ export class Query implements PublicQuery { docOrField: unknown | PublicDocumentSnapshot, ...fields: unknown[] ): PublicQuery { - validateAtLeastNumberOfArgs('Query.startAfter', arguments, 1); const bound = this.boundFromDocOrFields( 'Query.startAfter', docOrField, @@ -2061,7 +1892,6 @@ export class Query implements PublicQuery { docOrField: unknown | PublicDocumentSnapshot, ...fields: unknown[] ): PublicQuery { - validateAtLeastNumberOfArgs('Query.endBefore', arguments, 1); const bound = this.boundFromDocOrFields( 'Query.endBefore', docOrField, @@ -2079,7 +1909,6 @@ export class Query implements PublicQuery { docOrField: unknown | PublicDocumentSnapshot, ...fields: unknown[] ): PublicQuery { - validateAtLeastNumberOfArgs('Query.endAt', arguments, 1); const bound = this.boundFromDocOrFields( 'Query.endAt', docOrField, @@ -2115,9 +1944,7 @@ export class Query implements PublicQuery { fields: unknown[], before: boolean ): Bound { - validateDefined(methodName, 1, docOrField); if (docOrField instanceof DocumentSnapshot) { - validateExactNumberOfArgs(methodName, [docOrField, ...fields], 1); return newQueryBoundFromDocument( this._query, this.firestore._databaseId, @@ -2156,7 +1983,6 @@ export class Query implements PublicQuery { ): Unsubscribe; onSnapshot(...args: unknown[]): Unsubscribe { - validateBetweenNumberOfArgs('Query.onSnapshot', arguments, 1, 4); let options: ListenOptions = {}; let currArg = 0; if ( @@ -2164,15 +1990,6 @@ export class Query implements PublicQuery { !isPartialObserver(args[currArg]) ) { options = args[currArg] as SnapshotListenOptions; - validateOptionNames('Query.onSnapshot', options, [ - 'includeMetadataChanges' - ]); - validateNamedOptionalType( - 'Query.onSnapshot', - 'boolean', - 'includeMetadataChanges', - options.includeMetadataChanges - ); currArg++; } @@ -2184,19 +2001,6 @@ export class Query implements PublicQuery { args[currArg + 1] = userObserver.error?.bind(userObserver); args[currArg + 2] = userObserver.complete?.bind(userObserver); } else { - validateArgType('Query.onSnapshot', 'function', currArg, args[currArg]); - validateOptionalArgType( - 'Query.onSnapshot', - 'function', - currArg + 1, - args[currArg + 1] - ); - validateOptionalArgType( - 'Query.onSnapshot', - 'function', - currArg + 2, - args[currArg + 2] - ); } const observer: PartialObserver = { @@ -2222,7 +2026,6 @@ export class Query implements PublicQuery { } get(options?: GetOptions): Promise> { - validateBetweenNumberOfArgs('Query.get', arguments, 0, 1); validateGetOptions('Query.get', options); validateHasExplicitOrderByForLimitToLast(this._query); @@ -2273,8 +2076,6 @@ export class QuerySnapshot implements PublicQuerySnapshot { callback: (result: PublicQueryDocumentSnapshot) => void, thisArg?: unknown ): void { - validateBetweenNumberOfArgs('QuerySnapshot.forEach', arguments, 1, 2); - validateArgType('QuerySnapshot.forEach', 'function', 1, callback); this._snapshot.docs.forEach(doc => { callback.call( thisArg, @@ -2293,15 +2094,6 @@ export class QuerySnapshot implements PublicQuerySnapshot { docChanges(options?: SnapshotListenOptions): Array> { if (options) { - validateOptionNames('QuerySnapshot.docChanges', options, [ - 'includeMetadataChanges' - ]); - validateNamedOptionalType( - 'QuerySnapshot.docChanges', - 'boolean', - 'includeMetadataChanges', - options.includeMetadataChanges - ); } const includeMetadataChanges = !!( @@ -2402,18 +2194,11 @@ export class CollectionReference } doc(pathString?: string): PublicDocumentReference { - validateBetweenNumberOfArgs('CollectionReference.doc', arguments, 0, 1); // We allow omission of 'pathString' but explicitly prohibit passing in both // 'undefined' and 'null'. if (arguments.length === 0) { pathString = AutoId.newId(); } - validateArgType( - 'CollectionReference.doc', - 'non-empty string', - 1, - pathString - ); const path = ResourcePath.fromString(pathString!); return DocumentReference.forPath( this._query.path.child(path), @@ -2423,12 +2208,9 @@ export class CollectionReference } add(value: T): Promise> { - validateExactNumberOfArgs('CollectionReference.add', arguments, 1); const convertedValue = this._converter ? this._converter.toFirestore(value) : value; - validateArgType('CollectionReference.add', 'object', 1, convertedValue); - const docRef = this.doc(); // Call set() with the converted value directly to avoid calling toFirestore() a second time. @@ -2458,17 +2240,6 @@ function validateSetOptions( }; } - validateOptionNames(methodName, options, ['merge', 'mergeFields']); - validateNamedOptionalType(methodName, 'boolean', 'merge', options.merge); - validateOptionalArrayElements( - methodName, - 'mergeFields', - 'a string or a FieldPath', - options.mergeFields, - element => - typeof element === 'string' || element instanceof ExternalFieldPath - ); - if (options.mergeFields !== undefined && options.merge !== undefined) { throw new FirestoreError( Code.INVALID_ARGUMENT, @@ -2480,39 +2251,11 @@ function validateSetOptions( return options; } -function validateSnapshotOptions( - methodName: string, - options: PublicSnapshotOptions | undefined -): PublicSnapshotOptions { - if (options === undefined) { - return {}; - } - - validateOptionNames(methodName, options, ['serverTimestamps']); - validateNamedOptionalPropertyEquals( - methodName, - 'options', - 'serverTimestamps', - options.serverTimestamps, - ['estimate', 'previous', 'none'] - ); - return options; -} - function validateGetOptions( methodName: string, options: GetOptions | undefined ): void { - validateOptionalArgType(methodName, 'object', 1, options); if (options) { - validateOptionNames(methodName, options, ['source']); - validateNamedOptionalPropertyEquals( - methodName, - 'options', - 'source', - options.source, - ['default', 'server', 'cache'] - ); } } diff --git a/packages/firestore/src/api/field_path.ts b/packages/firestore/src/api/field_path.ts index b0017a9d7e4..b504c977bdd 100644 --- a/packages/firestore/src/api/field_path.ts +++ b/packages/firestore/src/api/field_path.ts @@ -19,11 +19,7 @@ import { FieldPath as PublicFieldPath } from '@firebase/firestore-types'; import { FieldPath as InternalFieldPath } from '../model/path'; import { Code, FirestoreError } from '../util/error'; -import { - invalidClassError, - validateArgType, - validateNamedArrayAtLeastNumberOfElements -} from '../util/input_validation'; +import { invalidClassError } from '../util/input_validation'; // 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 @@ -40,15 +36,7 @@ export abstract class _BaseFieldPath { readonly _internalPath: InternalFieldPath; constructor(fieldNames: string[]) { - validateNamedArrayAtLeastNumberOfElements( - 'FieldPath', - fieldNames, - 'fieldNames', - 1 - ); - for (let i = 0; i < fieldNames.length; ++i) { - validateArgType('FieldPath', 'string', i, fieldNames[i]); if (fieldNames[i].length === 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, diff --git a/packages/firestore/src/api/geo_point.ts b/packages/firestore/src/api/geo_point.ts index 18b3ff08c01..fd60b454490 100644 --- a/packages/firestore/src/api/geo_point.ts +++ b/packages/firestore/src/api/geo_point.ts @@ -16,10 +16,6 @@ */ import { Code, FirestoreError } from '../util/error'; -import { - validateArgType, - validateExactNumberOfArgs -} from '../util/input_validation'; import { primitiveComparator } from '../util/misc'; /** @@ -42,9 +38,6 @@ export class GeoPoint { * @param longitude The longitude as number between -180 and 180. */ constructor(latitude: number, longitude: number) { - validateExactNumberOfArgs('GeoPoint', arguments, 2); - validateArgType('GeoPoint', 'number', 1, latitude); - validateArgType('GeoPoint', 'number', 2, longitude); if (!isFinite(latitude) || latitude < -90 || latitude > 90) { throw new FirestoreError( Code.INVALID_ARGUMENT, diff --git a/packages/firestore/src/compat/field_value.ts b/packages/firestore/src/compat/field_value.ts index 94951b20fad..54ed982743e 100644 --- a/packages/firestore/src/compat/field_value.ts +++ b/packages/firestore/src/compat/field_value.ts @@ -24,48 +24,36 @@ import { increment } from '../../exp/index'; import * as legacy from '@firebase/firestore-types'; -import { - validateArgType, - validateAtLeastNumberOfArgs, - validateExactNumberOfArgs, - validateNoArgs -} from '../util/input_validation'; import { Compat } from './compat'; export class FieldValue extends Compat implements legacy.FieldValue { static serverTimestamp(): FieldValue { - validateNoArgs('FieldValue.serverTimestamp', arguments); const delegate = serverTimestamp(); delegate._methodName = 'FieldValue.serverTimestamp'; return new FieldValue(delegate); } static delete(): FieldValue { - validateNoArgs('FieldValue.delete', arguments); const delegate = deleteField(); delegate._methodName = 'FieldValue.delete'; return new FieldValue(delegate); } static arrayUnion(...elements: unknown[]): FieldValue { - validateAtLeastNumberOfArgs('FieldValue.arrayUnion', arguments, 1); const delegate = arrayUnion(...elements); delegate._methodName = 'FieldValue.arrayUnion'; return new FieldValue(delegate); } static arrayRemove(...elements: unknown[]): FieldValue { - validateAtLeastNumberOfArgs('FieldValue.arrayRemove', arguments, 1); const delegate = arrayRemove(...elements); delegate._methodName = 'FieldValue.arrayRemove'; return new FieldValue(delegate); } static increment(n: number): FieldValue { - validateArgType('FieldValue.increment', 'number', 1, n); - validateExactNumberOfArgs('FieldValue.increment', arguments, 1); const delegate = increment(n); delegate._methodName = 'FieldValue.increment'; return new FieldValue(delegate); diff --git a/packages/firestore/src/util/input_validation.ts b/packages/firestore/src/util/input_validation.ts index 4f97a13f8e7..d7e04343329 100644 --- a/packages/firestore/src/util/input_validation.ts +++ b/packages/firestore/src/util/input_validation.ts @@ -30,175 +30,19 @@ export type ValidationType = | 'string' | 'non-empty string'; -/** - * Validates that no arguments were passed in the invocation of functionName. - * - * Forward the magic "arguments" variable as second parameter on which the - * parameter validation is performed: - * validateNoArgs('myFunction', arguments); - */ -export function validateNoArgs(functionName: string, args: IArguments): void { - if (args.length !== 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() does not support arguments, ` + - 'but was called with ' + - formatPlural(args.length, 'argument') + - '.' - ); - } -} - -/** - * Validates the invocation of functionName has the exact number of arguments. - * - * Forward the magic "arguments" variable as second parameter on which the - * parameter validation is performed: - * validateExactNumberOfArgs('myFunction', arguments, 2); - */ -export function validateExactNumberOfArgs( +export function validateNonEmptyString( functionName: string, - args: ArrayLike, - numberOfArgs: number -): void { - if (args.length !== numberOfArgs) { + argumentName: string, + argument?: string +): asserts argument is string { + if (!argument) { throw new FirestoreError( Code.INVALID_ARGUMENT, - `Function ${functionName}() requires ` + - formatPlural(numberOfArgs, 'argument') + - ', but was called with ' + - formatPlural(args.length, 'argument') + - '.' + `Function ${functionName}() cannot be called with an empty ${argumentName}.` ); } } -/** - * Validates the invocation of functionName has at least the provided number of - * arguments (but can have many more). - * - * Forward the magic "arguments" variable as second parameter on which the - * parameter validation is performed: - * validateAtLeastNumberOfArgs('myFunction', arguments, 2); - */ -export function validateAtLeastNumberOfArgs( - functionName: string, - args: IArguments, - minNumberOfArgs: number -): void { - if (args.length < minNumberOfArgs) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires at least ` + - formatPlural(minNumberOfArgs, 'argument') + - ', but was called with ' + - formatPlural(args.length, 'argument') + - '.' - ); - } -} - -/** - * Validates the invocation of functionName has number of arguments between - * the values provided. - * - * Forward the magic "arguments" variable as second parameter on which the - * parameter validation is performed: - * validateBetweenNumberOfArgs('myFunction', arguments, 2, 3); - */ -export function validateBetweenNumberOfArgs( - functionName: string, - args: IArguments, - minNumberOfArgs: number, - maxNumberOfArgs: number -): void { - if (args.length < minNumberOfArgs || args.length > maxNumberOfArgs) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires between ${minNumberOfArgs} and ` + - `${maxNumberOfArgs} arguments, but was called with ` + - formatPlural(args.length, 'argument') + - '.' - ); - } -} - -/** - * Validates the provided argument is an array and has as least the expected - * number of elements. - */ -export function validateNamedArrayAtLeastNumberOfElements( - functionName: string, - value: T[], - name: string, - minNumberOfElements: number -): void { - if (!(value instanceof Array) || value.length < minNumberOfElements) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires its ${name} argument to be an ` + - 'array with at least ' + - `${formatPlural(minNumberOfElements, 'element')}.` - ); - } -} - -/** - * Validates the provided positional argument has the native JavaScript type - * using typeof checks. - */ -export function validateArgType( - functionName: string, - type: ValidationType, - position: number, - argument: unknown -): void { - validateType(functionName, type, `${ordinal(position)} argument`, argument); -} - -/** - * Validates the provided argument has the native JavaScript type using - * typeof checks or is undefined. - */ -export function validateOptionalArgType( - functionName: string, - type: ValidationType, - position: number, - argument: unknown -): void { - if (argument !== undefined) { - validateArgType(functionName, type, position, argument); - } -} - -/** - * Validates the provided named option has the native JavaScript type using - * typeof checks. - */ -export function validateNamedType( - functionName: string, - type: ValidationType, - optionName: string, - argument: unknown -): void { - validateType(functionName, type, `${optionName} option`, argument); -} - -/** - * Validates the provided named option has the native JavaScript type using - * typeof checks or is undefined. - */ -export function validateNamedOptionalType( - functionName: string, - type: ValidationType, - optionName: string, - argument: unknown -): void { - if (argument !== undefined) { - validateNamedType(functionName, type, optionName, argument); - } -} - /** * Validates that two boolean options are not set at the same time. */ @@ -216,126 +60,6 @@ export function validateIsNotUsedTogether( } } -export function validateArrayElements( - functionName: string, - optionName: string, - typeDescription: string, - argument: T[], - validator: (arg0: T) => boolean -): void { - if (!(argument instanceof Array)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires its ${optionName} ` + - `option to be an array, but it was: ${valueDescription(argument)}` - ); - } - - for (let i = 0; i < argument.length; ++i) { - if (!validator(argument[i])) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires all ${optionName} ` + - `elements to be ${typeDescription}, but the value at index ${i} ` + - `was: ${valueDescription(argument[i])}` - ); - } - } -} - -export function validateOptionalArrayElements( - functionName: string, - optionName: string, - typeDescription: string, - argument: T[] | undefined, - validator: (arg0: T) => boolean -): void { - if (argument !== undefined) { - validateArrayElements( - functionName, - optionName, - typeDescription, - argument, - validator - ); - } -} - -/** - * Validates that the provided named option equals one of the expected values. - */ -export function validateNamedPropertyEquals( - functionName: string, - inputName: string, - optionName: string, - input: T, - expected: T[] -): void { - const expectedDescription: string[] = []; - - for (const val of expected) { - if (val === input) { - return; - } - expectedDescription.push(valueDescription(val)); - } - - const actualDescription = valueDescription(input); - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid value ${actualDescription} provided to function ${functionName}() for option ` + - `"${optionName}". Acceptable values: ${expectedDescription.join(', ')}` - ); -} - -/** - * Validates that the provided named option equals one of the expected values or - * is undefined. - */ -export function validateNamedOptionalPropertyEquals( - functionName: string, - inputName: string, - optionName: string, - input: T, - expected: T[] -): void { - if (input !== undefined) { - validateNamedPropertyEquals( - functionName, - inputName, - optionName, - input, - expected - ); - } -} - -/** - * Validates that the provided argument is a valid enum. - * - * @param functionName Function making the validation call. - * @param enums Array containing all possible values for the enum. - * @param position Position of the argument in `functionName`. - * @param argument Argument to validate. - * @return The value as T if the argument can be converted. - */ -export function validateStringEnum( - functionName: string, - enums: T[], - position: number, - argument: unknown -): T { - if (!enums.some(element => element === argument)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid value ${valueDescription(argument)} provided to function ` + - `${functionName}() for its ${ordinal(position)} argument. Acceptable ` + - `values: ${enums.join(', ')}` - ); - } - return argument as T; -} - /** * Validates that `path` refers to a document (indicated by the fact it contains * an even numbers of segments). @@ -362,32 +86,6 @@ export function validateCollectionPath(path: ResourcePath): void { } } -/** Helper to validate the type of a provided input. */ -function validateType( - functionName: string, - type: ValidationType, - inputName: string, - input: unknown -): void { - let valid = false; - if (type === 'object') { - valid = isPlainObject(input); - } else if (type === 'non-empty string') { - valid = typeof input === 'string' && input !== ''; - } else { - valid = typeof input === type; - } - - if (!valid) { - const description = valueDescription(input); - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires its ${inputName} ` + - `to be of type ${type}, but it was: ${description}` - ); - } -} - /** * Returns true if it's a non-null object without a custom prototype * (i.e. excludes Array, Date, etc.). @@ -444,42 +142,6 @@ export function tryGetCustomObjectType(input: object): string | null { return null; } -/** Validates the provided argument is defined. */ -export function validateDefined( - functionName: string, - position: number, - argument: unknown -): void { - if (argument === undefined) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Function ${functionName}() requires a valid ${ordinal(position)} ` + - `argument, but it was undefined.` - ); - } -} - -/** - * Validates the provided positional argument is an object, and its keys and - * values match the expected keys and types provided in optionTypes. - */ -export function validateOptionNames( - functionName: string, - options: object, - optionNames: string[] -): void { - forEach(options as Dict, (key, _) => { - if (optionNames.indexOf(key) < 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Unknown option '${key}' passed to function ${functionName}(). ` + - 'Available options: ' + - optionNames.join(', ') - ); - } - }); -} - /** * Helper method to throw an error that the provided argument did not pass * an instanceof check. diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index ca5238686d9..f91cbcb3d41 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -103,13 +103,6 @@ class TestClass { constructor(readonly property: string) {} } -// NOTE: The JS SDK does extensive validation of argument counts, types, etc. -// since it is an untyped language. These tests are not exhaustive as that would -// be extremely tedious, but we do try to hit every error template at least -// once. Where applicable, some tests are ignored for the modular API, as we -// assume that most users will use TypeScript to catch invalid input. -const validatesJsInput = !usesFunctionalApi(); - apiDescribe('Validation:', (persistence: boolean) => { describe('FirestoreSettings', () => { // Enabling persistence counts as a use of the firestore instance, meaning @@ -120,31 +113,6 @@ apiDescribe('Validation:', (persistence: boolean) => { return; } - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'validates options', - db => { - // NOTE: 'credentials' is an undocumented API so ideally we wouldn't - // show it in the error, but I don't think it's worth the trouble of - // hiding it. - expect(() => db.settings({ invalid: true } as any)).to.throw( - "Unknown option 'invalid' passed to function settings(). " + - 'Available options: host, ssl, credentials' - ); - - expect(() => - db.settings({ - ssl: true - }) - ).to.throw("Can't provide ssl option if host option is not set"); - - expect(() => db.settings({ host: null as any })).to.throw( - 'Function settings() requires its host option to be of type ' + - 'non-empty string, but it was: null' - ); - } - ); - validationIt( persistence, 'disallows changing settings after use', @@ -206,17 +174,6 @@ apiDescribe('Validation:', (persistence: boolean) => { } ); - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'throws for invalid transaction functions.', - db => { - expect(() => db.runTransaction(null as any)).to.throw( - 'Function Firestore.runTransaction() requires its first ' + - 'argument to be of type function, but it was: null' - ); - } - ); - it("fails transaction if function doesn't return a Promise.", () => { return withTestDb(persistence, db => { return db @@ -246,24 +203,11 @@ apiDescribe('Validation:', (persistence: boolean) => { ); } else { expect(() => db.collection('')).to.throw( - 'Function Firestore.collection() requires its first ' + - 'argument to be of type non-empty string, but it was: ""' + 'Function Firestore.collection() cannot be called with an empty path.' ); expect(() => baseDocRef.collection('')).to.throw( - 'Function DocumentReference.collection() requires its first ' + - 'argument to be of type non-empty string, but it was: ""' - ); - expect(() => db.collection(null as any)).to.throw( - 'Function Firestore.collection() requires its first argument ' + - 'to be of type non-empty string, but it was: null' - ); - expect(() => baseDocRef.collection(null as any)).to.throw( - 'Function DocumentReference.collection() requires its first ' + - 'argument to be of type non-empty string, but it was: null' - ); - expect(() => (baseDocRef.collection as any)('foo', 'bar')).to.throw( - 'Function DocumentReference.collection() requires 1 argument, ' + - 'but was called with 2 arguments.' + 'Function DocumentReference.collection() cannot be called with an ' + + 'empty path.' ); } }); @@ -318,28 +262,11 @@ apiDescribe('Validation:', (persistence: boolean) => { ); } else { expect(() => db.doc('')).to.throw( - 'Function Firestore.doc() requires its first ' + - 'argument to be of type non-empty string, but it was: ""' + 'Function Firestore.doc() cannot be called with an empty path.' ); expect(() => baseCollectionRef.doc('')).to.throw( - 'Function CollectionReference.doc() requires its first ' + - 'argument to be of type non-empty string, but it was: ""' - ); - expect(() => db.doc(null as any)).to.throw( - 'Function Firestore.doc() requires its first argument to be ' + - 'of type non-empty string, but it was: null' - ); - expect(() => baseCollectionRef.doc(null as any)).to.throw( - 'Function CollectionReference.doc() requires its first ' + - 'argument to be of type non-empty string, but it was: null' - ); - expect(() => baseCollectionRef.doc(undefined as any)).to.throw( - 'Function CollectionReference.doc() requires its first ' + - 'argument to be of type non-empty string, but it was: undefined' - ); - expect(() => (baseCollectionRef.doc as any)('foo', 'bar')).to.throw( - 'Function CollectionReference.doc() requires between 0 and ' + - '1 arguments, but was called with 2 arguments.' + 'Function CollectionReference.doc() cannot be called with an empty ' + + 'path.' ); } }); @@ -363,115 +290,18 @@ apiDescribe('Validation:', (persistence: boolean) => { }); }); - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'Listen options are validated', - db => { - const collection = db.collection('test'); - const fn = (): void => {}; - - const doc = collection.doc(); - expect(() => doc.onSnapshot({ bad: true } as any, fn)).to.throw( - `Unknown option 'bad' passed to function ` + - `DocumentReference.onSnapshot(). Available options: ` + - `includeMetadataChanges` - ); - - expect(() => collection.onSnapshot({ bad: true } as any, fn)).to.throw( - `Unknown option 'bad' passed to function ` + - `Query.onSnapshot(). Available options: includeMetadataChanges` - ); - - expect(() => db.onSnapshotsInSync('bad' as any)).to.throw( - `Function Firestore.onSnapshotsInSync() requires its first ` + - `argument to be of type function, but it was: "bad"` - ); - } - ); - - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'get options are validated', - db => { - const collection = db.collection('test'); - const doc = collection.doc(); - const fn = (): void => {}; - - expect(() => doc.get(fn as any)).to.throw( - 'Function DocumentReference.get() requires its first argument to be of type object, ' + - 'but it was: a function' - ); - expect(() => doc.get({ abc: 'cache' } as any)).to.throw( - `Unknown option 'abc' passed to function DocumentReference.get(). Available options: source` - ); - - expect(() => collection.get(fn as any)).to.throw( - 'Function Query.get() requires its first argument to be of type object, but it was: ' + - 'a function' - ); - expect(() => collection.get({ abc: 'cache' } as any)).to.throw( - `Unknown option 'abc' passed to function Query.get(). Available options: source` - ); - } - ); - - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'Snapshot options are validated', - db => { - const docRef = db.collection('test').doc(); - - return docRef - .set({ test: 1 }) - .then(() => { - return docRef.get(); - }) - .then(snapshot => { - expect(() => snapshot.get('test', { bad: true } as any)).to.throw( - `Unknown option 'bad' passed to function ` + - `DocumentSnapshot.get(). Available options: ` + - `serverTimestamps` - ); - expect(() => - snapshot.data({ serverTimestamps: 'foo' } as any) - ).to.throw( - `Invalid value "foo" provided to function DocumentSnapshot.data() for option ` + - `"serverTimestamps". Acceptable values: "estimate", "previous", "none"` - ); - }); - } - ); - - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'Merge options are validated', - db => { - const docRef = db.collection('test').doc(); + validationIt(persistence, 'Merge options are validated', db => { + const docRef = db.collection('test').doc(); - expect(() => docRef.set({}, { merge: true, mergeFields: [] })).to.throw( - 'Invalid options passed to function DocumentReference.set(): You cannot specify both ' + - '"merge" and "mergeFields".' - ); - expect(() => docRef.set({}, { merge: false, mergeFields: [] })).to.throw( - 'Invalid options passed to function DocumentReference.set(): You cannot specify both ' + - '"merge" and "mergeFields".' - ); - expect(() => docRef.set({}, { merge: 'foo' as any })).to.throw( - 'Function DocumentReference.set() requires its merge option to be of type boolean, but it ' + - 'was: "foo"' - ); - expect(() => docRef.set({}, { mergeFields: 'foo' as any })).to.throw( - 'Function DocumentReference.set() requires its mergeFields option to be an array, but it ' + - 'was: "foo"' - ); - expect(() => - docRef.set({}, { mergeFields: ['foo', false as any] }) - ).to.throw( - 'Function DocumentReference.set() requires all mergeFields elements to be a string or a ' + - 'FieldPath, but the value at index 1 was: false' - ); - } - ); + expect(() => docRef.set({}, { merge: true, mergeFields: [] })).to.throw( + 'Invalid options passed to function DocumentReference.set(): You cannot specify both ' + + '"merge" and "mergeFields".' + ); + expect(() => docRef.set({}, { merge: false, mergeFields: [] })).to.throw( + 'Invalid options passed to function DocumentReference.set(): You cannot specify both ' + + '"merge" and "mergeFields".' + ); + }); describe('Writes', () => { validationIt(persistence, 'must be objects.', db => { @@ -834,22 +664,6 @@ apiDescribe('Validation:', (persistence: boolean) => { }); }); - describe('Server timestamps transforms', () => { - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'reject any arguments', - db => { - const doc = db.collection('test').doc(); - expect(() => - doc.set({ x: (FieldValue as any).serverTimestamp('foo') }) - ).to.throw( - 'Function FieldValue.serverTimestamp() does not support ' + - 'arguments, but was called with 1 argument.' - ); - } - ); - }); - describe('Numeric transforms', () => { validationIt(persistence, 'fail in queries', db => { const collection = db.collection('test'); @@ -862,40 +676,6 @@ apiDescribe('Validation:', (persistence: boolean) => { 'used with update() and set() (found in field test)' ); }); - - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'reject invalid operands', - db => { - const doc = db.collection('test').doc(); - expect(() => - doc.set({ x: FieldValue.increment('foo' as any) }) - ).to.throw( - 'Function FieldValue.increment() requires its first argument to ' + - 'be of type number, but it was: "foo"' - ); - expect(() => - doc.set({ x: FieldValue.increment(undefined as any) }) - ).to.throw( - 'Function FieldValue.increment() requires its first argument to ' + - 'be of type number, but it was: undefined' - ); - } - ); - - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'reject more than one argument', - db => { - const doc = db.collection('test').doc(); - expect(() => - doc.set({ x: (FieldValue as any).increment(1337, 'leet') }) - ).to.throw( - 'Function FieldValue.increment() requires 1 argument, but was ' + - 'called with 2 arguments.' - ); - } - ); }); describe('Queries', () => { @@ -913,18 +693,6 @@ apiDescribe('Validation:', (persistence: boolean) => { ); }); - (validatesJsInput ? validationIt : validationIt.skip)( - persistence, - 'enum', - db => { - const collection = db.collection('test') as any; - expect(() => collection.where('a', 'foo' as any, 'b')).to.throw( - 'Invalid value "foo" provided to function Query.where() for its second argument. ' + - 'Acceptable values: <, <=, ==, !=, >=, >, array-contains, in, array-contains-any, not-in' - ); - } - ); - validationIt( persistence, 'with null or NaN non-equality filters fail', @@ -1599,24 +1367,28 @@ apiDescribe('Validation:', (persistence: boolean) => { } ); - validationIt(persistence, 'cannot pass undefined as a field value', db => { - const collection = db.collection('test'); - if (usesFunctionalApi()) { - expect(() => collection.where('foo', '==', undefined)).to.throw( - 'Function where() called with invalid data. Unsupported field value: undefined' - ); - expect(() => collection.orderBy('foo').startAt(undefined)).to.throw( - 'Function startAt() called with invalid data. Unsupported field value: undefined' - ); - } else { - expect(() => collection.where('foo', '==', undefined)).to.throw( - 'Function Query.where() requires a valid third argument, but it was undefined.' - ); - expect(() => collection.orderBy('foo').startAt(undefined)).to.throw( - 'Function Query.startAt() requires a valid first argument, but it was undefined.' - ); + validationIt.only( + persistence, + 'cannot pass undefined as a field value', + db => { + const collection = db.collection('test'); + if (usesFunctionalApi()) { + expect(() => collection.where('foo', '==', undefined)).to.throw( + 'Function where() called with invalid data. Unsupported field value: undefined' + ); + expect(() => collection.orderBy('foo').startAt(undefined)).to.throw( + 'Function startAt() called with invalid data. Unsupported field value: undefined' + ); + } else { + expect(() => collection.where('foo', '==', undefined)).to.throw( + 'Function Query.where() called with invalid data. Unsupported field value: undefined' + ); + expect(() => collection.orderBy('foo').startAt(undefined)).to.throw( + 'Function Query.startAt() called with invalid data. Unsupported field value: undefined' + ); + } } - }); + ); }); });