diff --git a/.changeset/lets-go-travel.md b/.changeset/lets-go-travel.md new file mode 100644 index 00000000000..1731f975ff3 --- /dev/null +++ b/.changeset/lets-go-travel.md @@ -0,0 +1,5 @@ +--- +"@firebase/util": major +--- + +Internal changes to validation APIs. diff --git a/packages/database/src/api/Reference.ts b/packages/database/src/api/Reference.ts index d5862154df3..e918b3020f9 100644 --- a/packages/database/src/api/Reference.ts +++ b/packages/database/src/api/Reference.ts @@ -123,30 +123,27 @@ export class DataSnapshot implements Compat { /** * Returns a DataSnapshot of the specified child node's contents. * - * @param childPathString Path to a child. + * @param path Path to a child. * @return DataSnapshot for child node. */ - child(childPathString: string): DataSnapshot { + child(path: string): DataSnapshot { validateArgCount('DataSnapshot.child', 0, 1, arguments.length); // Ensure the childPath is a string (can be a number) - childPathString = String(childPathString); - validatePathString('DataSnapshot.child', 1, childPathString, false); - return new DataSnapshot( - this._database, - this._delegate.child(childPathString) - ); + path = String(path); + validatePathString('DataSnapshot.child', 'path', path, false); + return new DataSnapshot(this._database, this._delegate.child(path)); } /** * Returns whether the snapshot contains a child at the specified path. * - * @param childPathString Path to a child. + * @param path Path to a child. * @return Whether the child exists. */ - hasChild(childPathString: string): boolean { + hasChild(path: string): boolean { validateArgCount('DataSnapshot.hasChild', 1, 1, arguments.length); - validatePathString('DataSnapshot.hasChild', 1, childPathString, false); - return this._delegate.hasChild(childPathString); + validatePathString('DataSnapshot.hasChild', 'path', path, false); + return this._delegate.hasChild(path); } /** @@ -169,7 +166,7 @@ export class DataSnapshot implements Compat { */ forEach(action: (snapshot: DataSnapshot) => boolean | void): boolean { validateArgCount('DataSnapshot.forEach', 1, 1, arguments.length); - validateCallback('DataSnapshot.forEach', 1, action, false); + validateCallback('DataSnapshot.forEach', 'action', action, false); return this._delegate.forEach(expDataSnapshot => action(new DataSnapshot(this._database, expDataSnapshot)) ); @@ -231,7 +228,7 @@ export class Query implements Compat { context?: object | null ): SnapshotCallback { validateArgCount('Query.on', 2, 4, arguments.length); - validateCallback('Query.on', 2, callback, false); + validateCallback('Query.on', 'callback', callback, false); const ret = Query.getCancelAndContextArgs_( 'Query.on', @@ -267,7 +264,7 @@ export class Query implements Compat { return callback; default: throw new Error( - errorPrefix('Query.on', 1, false) + + errorPrefix('Query.on', 'eventType') + 'must be a valid event type = "value", "child_added", "child_removed", ' + '"child_changed", or "child_moved".' ); @@ -280,9 +277,9 @@ export class Query implements Compat { context?: object | null ): void { validateArgCount('Query.off', 0, 3, arguments.length); - validateEventType('Query.off', 1, eventType, true); - validateCallback('Query.off', 2, callback, true); - validateContextObject('Query.off', 3, context, true); + validateEventType('Query.off', eventType, true); + validateCallback('Query.off', 'callback', callback, true); + validateContextObject('Query.off', 'context', context, true); if (callback) { const valueCallback: UserCallback = () => {}; valueCallback.userCallback = callback; @@ -307,12 +304,12 @@ export class Query implements Compat { */ once( eventType: string, - userCallback?: SnapshotCallback, + callback?: SnapshotCallback, failureCallbackOrContext?: ((a: Error) => void) | object | null, context?: object | null ): Promise { validateArgCount('Query.once', 1, 4, arguments.length); - validateCallback('Query.once', 2, userCallback, true); + validateCallback('Query.once', 'callback', callback, true); const ret = Query.getCancelAndContextArgs_( 'Query.on', @@ -322,12 +319,12 @@ export class Query implements Compat { const deferred = new Deferred(); const valueCallback: UserCallback = (expSnapshot, previousChildName?) => { const result = new DataSnapshot(this.database, expSnapshot); - if (userCallback) { - userCallback.call(ret.context, result, previousChildName); + if (callback) { + callback.call(ret.context, result, previousChildName); } deferred.resolve(result); }; - valueCallback.userCallback = userCallback; + valueCallback.userCallback = callback; valueCallback.context = ret.context; const cancelCallback = (error: Error) => { if (ret.cancel) { @@ -364,7 +361,7 @@ export class Query implements Compat { break; default: throw new Error( - errorPrefix('Query.once', 1, false) + + errorPrefix('Query.once', 'eventType') + 'must be a valid event type = "value", "child_added", "child_removed", ' + '"child_changed", or "child_moved".' ); @@ -519,10 +516,10 @@ export class Query implements Compat { } = { cancel: undefined, context: undefined }; if (cancelOrContext && context) { ret.cancel = cancelOrContext as (a: Error) => void; - validateCallback(fnName, 3, ret.cancel, true); + validateCallback(fnName, 'cancel', ret.cancel, true); ret.context = context; - validateContextObject(fnName, 4, ret.context, true); + validateContextObject(fnName, 'context', ret.context, true); } else if (cancelOrContext) { // we have either a cancel callback or a context. if (typeof cancelOrContext === 'object' && cancelOrContext !== null) { @@ -532,7 +529,7 @@ export class Query implements Compat { ret.cancel = cancelOrContext as (a: Error) => void; } else { throw new Error( - errorPrefix(fnName, 3, true) + + errorPrefix(fnName, 'cancelOrContext') + ' must either be a cancel callback or a context object.' ); } @@ -598,7 +595,7 @@ export class Reference extends Query implements Compat { onComplete?: (error: Error | null) => void ): Promise { validateArgCount('Reference.set', 1, 2, arguments.length); - validateCallback('Reference.set', 2, onComplete, true); + validateCallback('Reference.set', 'onComplete', onComplete, true); const result = set(this._delegate, newVal); if (onComplete) { result.then( @@ -610,17 +607,17 @@ export class Reference extends Query implements Compat { } update( - objectToMerge: object, + values: object, onComplete?: (a: Error | null) => void ): Promise { validateArgCount('Reference.update', 1, 2, arguments.length); - if (Array.isArray(objectToMerge)) { + if (Array.isArray(values)) { const newObjectToMerge: { [k: string]: unknown } = {}; - for (let i = 0; i < objectToMerge.length; ++i) { - newObjectToMerge['' + i] = objectToMerge[i]; + for (let i = 0; i < values.length; ++i) { + newObjectToMerge['' + i] = values[i]; } - objectToMerge = newObjectToMerge; + values = newObjectToMerge; warn( 'Passing an Array to Firebase.update() is deprecated. ' + 'Use set() if you want to overwrite the existing data, or ' + @@ -629,9 +626,9 @@ export class Reference extends Query implements Compat { ); } validateWritablePath('Reference.update', this._delegate._path); - validateCallback('Reference.update', 2, onComplete, true); + validateCallback('Reference.update', 'onComplete', onComplete, true); - const result = update(this._delegate, objectToMerge); + const result = update(this._delegate, values); if (onComplete) { result.then( () => onComplete(null), @@ -647,7 +644,12 @@ export class Reference extends Query implements Compat { onComplete?: (a: Error | null) => void ): Promise { validateArgCount('Reference.setWithPriority', 2, 3, arguments.length); - validateCallback('Reference.setWithPriority', 3, onComplete, true); + validateCallback( + 'Reference.setWithPriority', + 'onComplete', + onComplete, + true + ); const result = setWithPriority(this._delegate, newVal, newPriority); if (onComplete) { @@ -661,7 +663,7 @@ export class Reference extends Query implements Compat { remove(onComplete?: (a: Error | null) => void): Promise { validateArgCount('Reference.remove', 0, 1, arguments.length); - validateCallback('Reference.remove', 1, onComplete, true); + validateCallback('Reference.remove', 'onComplete', onComplete, true); const result = remove(this._delegate); if (onComplete) { @@ -683,9 +685,19 @@ export class Reference extends Query implements Compat { applyLocally?: boolean ): Promise { validateArgCount('Reference.transaction', 1, 3, arguments.length); - validateCallback('Reference.transaction', 1, transactionUpdate, false); - validateCallback('Reference.transaction', 2, onComplete, true); - validateBoolean('Reference.transaction', 3, applyLocally, true); + validateCallback( + 'Reference.transaction', + 'transactionUpdate', + transactionUpdate, + false + ); + validateCallback('Reference.transaction', 'onComplete', onComplete, true); + validateBoolean( + 'Reference.transaction', + 'applyLocally', + applyLocally, + true + ); const result = runTransaction(this._delegate, transactionUpdate, { applyLocally @@ -715,7 +727,7 @@ export class Reference extends Query implements Compat { onComplete?: (a: Error | null) => void ): Promise { validateArgCount('Reference.setPriority', 1, 2, arguments.length); - validateCallback('Reference.setPriority', 2, onComplete, true); + validateCallback('Reference.setPriority', 'onComplete', onComplete, true); const result = setPriority(this._delegate, priority); if (onComplete) { @@ -729,7 +741,7 @@ export class Reference extends Query implements Compat { push(value?: unknown, onComplete?: (a: Error | null) => void): Reference { validateArgCount('Reference.push', 0, 2, arguments.length); - validateCallback('Reference.push', 2, onComplete, true); + validateCallback('Reference.push', 'onComplete', onComplete, true); const expPromise = push(this._delegate, value); const promise = expPromise.then( diff --git a/packages/database/src/api/onDisconnect.ts b/packages/database/src/api/onDisconnect.ts index fbeb27674cc..82ea5858e43 100644 --- a/packages/database/src/api/onDisconnect.ts +++ b/packages/database/src/api/onDisconnect.ts @@ -26,7 +26,7 @@ export class OnDisconnect implements Compat { cancel(onComplete?: (a: Error | null) => void): Promise { validateArgCount('OnDisconnect.cancel', 0, 1, arguments.length); - validateCallback('OnDisconnect.cancel', 1, onComplete, true); + validateCallback('OnDisconnect.cancel', 'onComplete', onComplete, true); const result = this._delegate.cancel(); if (onComplete) { result.then( @@ -39,7 +39,7 @@ export class OnDisconnect implements Compat { remove(onComplete?: (a: Error | null) => void): Promise { validateArgCount('OnDisconnect.remove', 0, 1, arguments.length); - validateCallback('OnDisconnect.remove', 1, onComplete, true); + validateCallback('OnDisconnect.remove', 'onComplete', onComplete, true); const result = this._delegate.remove(); if (onComplete) { result.then( @@ -52,7 +52,7 @@ export class OnDisconnect implements Compat { set(value: unknown, onComplete?: (a: Error | null) => void): Promise { validateArgCount('OnDisconnect.set', 1, 2, arguments.length); - validateCallback('OnDisconnect.set', 2, onComplete, true); + validateCallback('OnDisconnect.set', 'onComplete', onComplete, true); const result = this._delegate.set(value); if (onComplete) { result.then( @@ -69,7 +69,12 @@ export class OnDisconnect implements Compat { onComplete?: (a: Error | null) => void ): Promise { validateArgCount('OnDisconnect.setWithPriority', 2, 3, arguments.length); - validateCallback('OnDisconnect.setWithPriority', 3, onComplete, true); + validateCallback( + 'OnDisconnect.setWithPriority', + 'onComplete', + onComplete, + true + ); const result = this._delegate.setWithPriority(value, priority); if (onComplete) { result.then( @@ -96,7 +101,7 @@ export class OnDisconnect implements Compat { 'existing data, or an Object with integer keys if you really do want to only update some of the children.' ); } - validateCallback('OnDisconnect.update', 2, onComplete, true); + validateCallback('OnDisconnect.update', 'onComplete', onComplete, true); const result = this._delegate.update(objectToMerge); if (onComplete) { result.then( diff --git a/packages/database/src/core/util/validation.ts b/packages/database/src/core/util/validation.ts index 2a50b13d0f7..356f03f3ac2 100644 --- a/packages/database/src/core/util/validation.ts +++ b/packages/database/src/core/util/validation.ts @@ -95,20 +95,15 @@ export const isValidPriority = function (priority: unknown): boolean { */ export const validateFirebaseDataArg = function ( fnName: string, - argumentNumber: number, - data: unknown, + value: unknown, path: Path, optional: boolean ) { - if (optional && data === undefined) { + if (optional && value === undefined) { return; } - validateFirebaseData( - errorPrefixFxn(fnName, argumentNumber, optional), - data, - path - ); + validateFirebaseData(errorPrefixFxn(fnName, 'value'), value, path); }; /** @@ -257,7 +252,6 @@ export const validateFirebaseMergePaths = function ( */ export const validateFirebaseMergeDataArg = function ( fnName: string, - argumentNumber: number, data: unknown, path: Path, optional: boolean @@ -266,7 +260,7 @@ export const validateFirebaseMergeDataArg = function ( return; } - const errorPrefix = errorPrefixFxn(fnName, argumentNumber, optional); + const errorPrefix = errorPrefixFxn(fnName, 'values'); if (!(data && typeof data === 'object') || Array.isArray(data)) { throw new Error( @@ -296,7 +290,6 @@ export const validateFirebaseMergeDataArg = function ( export const validatePriority = function ( fnName: string, - argumentNumber: number, priority: unknown, optional: boolean ) { @@ -305,7 +298,7 @@ export const validatePriority = function ( } if (isInvalidJSONNumber(priority)) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, 'priority') + 'is ' + priority.toString() + ', but must be a valid Firebase priority (a string, finite number, ' + @@ -315,7 +308,7 @@ export const validatePriority = function ( // Special case to allow importing data with a .sv. if (!isValidPriority(priority)) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, 'priority') + 'must be a valid Firebase priority ' + '(a string, finite number, server value, or null).' ); @@ -324,7 +317,6 @@ export const validatePriority = function ( export const validateEventType = function ( fnName: string, - argumentNumber: number, eventType: string, optional: boolean ) { @@ -341,7 +333,7 @@ export const validateEventType = function ( break; default: throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, 'eventType') + 'must be a valid event type = "value", "child_added", "child_removed", ' + '"child_changed", or "child_moved".' ); @@ -350,7 +342,7 @@ export const validateEventType = function ( export const validateKey = function ( fnName: string, - argumentNumber: number, + argumentName: string, key: string, optional: boolean ) { @@ -359,7 +351,7 @@ export const validateKey = function ( } if (!isValidKey(key)) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, argumentName) + 'was an invalid key = "' + key + '". Firebase keys must be non-empty strings and ' + @@ -370,7 +362,7 @@ export const validateKey = function ( export const validatePathString = function ( fnName: string, - argumentNumber: number, + argumentName: string, pathString: string, optional: boolean ) { @@ -380,7 +372,7 @@ export const validatePathString = function ( if (!isValidPathString(pathString)) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, argumentName) + 'was an invalid path = "' + pathString + '". Paths must be non-empty strings and ' + @@ -391,7 +383,7 @@ export const validatePathString = function ( export const validateRootPathString = function ( fnName: string, - argumentNumber: number, + argumentName: string, pathString: string, optional: boolean ) { @@ -400,7 +392,7 @@ export const validateRootPathString = function ( pathString = pathString.replace(/^\/*\.info(\/|$)/, '/'); } - validatePathString(fnName, argumentNumber, pathString, optional); + validatePathString(fnName, argumentName, pathString, optional); }; export const validateWritablePath = function (fnName: string, path: Path) { @@ -411,7 +403,6 @@ export const validateWritablePath = function (fnName: string, path: Path) { export const validateUrl = function ( fnName: string, - argumentNumber: number, parsedUrl: { repoInfo: RepoInfo; path: Path } ) { // TODO = Validate server better. @@ -424,33 +415,16 @@ export const validateUrl = function ( (pathString.length !== 0 && !isValidRootPathString(pathString)) ) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, false) + + errorPrefixFxn(fnName, 'url') + 'must be a valid firebase URL and ' + 'the path can\'t contain ".", "#", "$", "[", or "]".' ); } }; -export const validateCredential = function ( - fnName: string, - argumentNumber: number, - cred: unknown, - optional: boolean -) { - if (optional && cred === undefined) { - return; - } - if (!(typeof cred === 'string')) { - throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + - 'must be a valid credential (a string).' - ); - } -}; - export const validateBoolean = function ( fnName: string, - argumentNumber: number, + argumentName: string, bool: unknown, optional: boolean ) { @@ -459,14 +433,14 @@ export const validateBoolean = function ( } if (typeof bool !== 'boolean') { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + 'must be a boolean.' + errorPrefixFxn(fnName, argumentName) + 'must be a boolean.' ); } }; export const validateString = function ( fnName: string, - argumentNumber: number, + argumentName: string, string: unknown, optional: boolean ) { @@ -475,15 +449,14 @@ export const validateString = function ( } if (!(typeof string === 'string')) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + - 'must be a valid string.' + errorPrefixFxn(fnName, argumentName) + 'must be a valid string.' ); } }; export const validateObject = function ( fnName: string, - argumentNumber: number, + argumentName: string, obj: unknown, optional: boolean ) { @@ -492,15 +465,14 @@ export const validateObject = function ( } if (!(obj && typeof obj === 'object') || obj === null) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + - 'must be a valid object.' + errorPrefixFxn(fnName, argumentName) + 'must be a valid object.' ); } }; export const validateObjectContainsKey = function ( fnName: string, - argumentNumber: number, + argumentName: string, obj: unknown, key: string, optional: boolean, @@ -515,7 +487,7 @@ export const validateObjectContainsKey = function ( return; } else { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, argumentName) + 'must contain the key "' + key + '"' @@ -535,7 +507,7 @@ export const validateObjectContainsKey = function ( ) { if (optional) { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, argumentName) + 'contains invalid value for key "' + key + '" (must be of type "' + @@ -544,7 +516,7 @@ export const validateObjectContainsKey = function ( ); } else { throw new Error( - errorPrefixFxn(fnName, argumentNumber, optional) + + errorPrefixFxn(fnName, argumentName) + 'must contain the key "' + key + '" with type "' + diff --git a/packages/database/src/exp/Database.ts b/packages/database/src/exp/Database.ts index a8b86bdc183..db01fe5bbb3 100644 --- a/packages/database/src/exp/Database.ts +++ b/packages/database/src/exp/Database.ts @@ -132,7 +132,7 @@ export function repoManagerDatabaseFromApp( ? new EmulatorAdminTokenProvider() : new FirebaseAuthTokenProvider(app.name, app.options, authProvider); - validateUrl('Invalid Firebase Database URL', 1, parsedUrl); + validateUrl('Invalid Firebase Database URL', parsedUrl); if (!pathIsEmpty(parsedUrl.path)) { fatal( 'Database URL must point to the root of a Firebase Database ' + diff --git a/packages/database/src/exp/OnDisconnect.ts b/packages/database/src/exp/OnDisconnect.ts index 8441cae5aab..efb57f9eda9 100644 --- a/packages/database/src/exp/OnDisconnect.ts +++ b/packages/database/src/exp/OnDisconnect.ts @@ -60,7 +60,7 @@ export class OnDisconnect { set(value: unknown): Promise { validateWritablePath('OnDisconnect.set', this._path); - validateFirebaseDataArg('OnDisconnect.set', 1, value, this._path, false); + validateFirebaseDataArg('OnDisconnect.set', value, this._path, false); const deferred = new Deferred(); repoOnDisconnectSet( this._repo, @@ -78,12 +78,11 @@ export class OnDisconnect { validateWritablePath('OnDisconnect.setWithPriority', this._path); validateFirebaseDataArg( 'OnDisconnect.setWithPriority', - 1, value, this._path, false ); - validatePriority('OnDisconnect.setWithPriority', 2, priority, false); + validatePriority('OnDisconnect.setWithPriority', priority, false); const deferred = new Deferred(); repoOnDisconnectSetWithPriority( @@ -96,12 +95,11 @@ export class OnDisconnect { return deferred.promise; } - update(objectToMerge: Indexable): Promise { + update(values: Indexable): Promise { validateWritablePath('OnDisconnect.update', this._path); validateFirebaseMergeDataArg( 'OnDisconnect.update', - 1, - objectToMerge, + values, this._path, false ); @@ -109,7 +107,7 @@ export class OnDisconnect { repoOnDisconnectUpdate( this._repo, this._path, - objectToMerge, + values, deferred.wrapCallback(() => {}) ); return deferred.promise; diff --git a/packages/database/src/exp/Reference_impl.ts b/packages/database/src/exp/Reference_impl.ts index 35bec7fe164..948ad2e3cc7 100644 --- a/packages/database/src/exp/Reference_impl.ts +++ b/packages/database/src/exp/Reference_impl.ts @@ -357,7 +357,7 @@ export function refFromURL(db: FirebaseDatabase, url: string): ReferenceImpl { db = getModularInstance(db); db._checkNotDeleted('refFromURL'); const parsedURL = parseRepoInfo(url, db._repo.repoInfo_.nodeAdmin); - validateUrl('refFromURL', 1, parsedURL); + validateUrl('refFromURL', parsedURL); const repoInfo = parsedURL.repoInfo; if ( @@ -381,10 +381,9 @@ export function refFromURL(db: FirebaseDatabase, url: string): ReferenceImpl { export function child(ref: Reference, path: string): ReferenceImpl { ref = getModularInstance(ref); if (pathGetFront(ref._path) === null) { - // TODO(database-exp): Remove argument numbers from all error messages - validateRootPathString('child', 1, path, false); + validateRootPathString('child', 'path', path, false); } else { - validatePathString('child', 1, path, false); + validatePathString('child', 'path', path, false); } return new ReferenceImpl(ref._repo, pathChild(ref._path, path)); } @@ -401,7 +400,7 @@ export interface ThenableReferenceImpl export function push(ref: Reference, value?: unknown): ThenableReferenceImpl { ref = getModularInstance(ref); validateWritablePath('push', ref._path); - validateFirebaseDataArg('push', 1, value, ref._path, true); + validateFirebaseDataArg('push', value, ref._path, true); const now = repoServerTime(ref._repo); const name = nextPushId(now); @@ -434,7 +433,7 @@ export function remove(ref: Reference): Promise { export function set(ref: Reference, value: unknown): Promise { ref = getModularInstance(ref); validateWritablePath('set', ref._path); - validateFirebaseDataArg('set', 1, value, ref._path, false); + validateFirebaseDataArg('set', value, ref._path, false); const deferred = new Deferred(); repoSetWithPriority( ref._repo, @@ -452,7 +451,7 @@ export function setPriority( ): Promise { ref = getModularInstance(ref); validateWritablePath('setPriority', ref._path); - validatePriority('setPriority', 1, priority, false); + validatePriority('setPriority', priority, false); const deferred = new Deferred(); repoSetWithPriority( ref._repo, @@ -470,9 +469,8 @@ export function setWithPriority( priority: string | number | null ): Promise { validateWritablePath('setWithPriority', ref._path); - validateFirebaseDataArg('setWithPriority', 1, value, ref._path, false); - validatePriority('setWithPriority', 2, priority, false); - + validateFirebaseDataArg('setWithPriority', value, ref._path, false); + validatePriority('setWithPriority', priority, false); if (ref.key === '.length' || ref.key === '.keys') { throw 'setWithPriority failed: ' + ref.key + ' is a read-only object.'; } @@ -489,7 +487,7 @@ export function setWithPriority( } export function update(ref: Reference, values: object): Promise { - validateFirebaseMergeDataArg('update', 1, values, ref._path, false); + validateFirebaseMergeDataArg('update', values, ref._path, false); const deferred = new Deferred(); repoUpdate( ref._repo, @@ -1006,7 +1004,7 @@ class QueryEndAtConstraint extends QueryConstraint { } _apply(query: QueryImpl): QueryImpl { - validateFirebaseDataArg('endAt', 1, this._value, query._path, true); + validateFirebaseDataArg('endAt', this._value, query._path, true); const newParams = queryParamsEndAt( query._queryParams, this._value, @@ -1033,7 +1031,7 @@ export function endAt( value: number | string | boolean | null, key?: string ): QueryConstraint { - validateKey('endAt', 2, key, true); + validateKey('endAt', 'key', key, true); return new QueryEndAtConstraint(value, key); } @@ -1048,7 +1046,7 @@ class QueryEndBeforeConstraint extends QueryConstraint { } _apply(query: QueryImpl): QueryImpl { - validateFirebaseDataArg('endBefore', 1, this._value, query._path, false); + validateFirebaseDataArg('endBefore', this._value, query._path, false); const newParams = queryParamsEndBefore( query._queryParams, this._value, @@ -1075,7 +1073,7 @@ export function endBefore( value: number | string | boolean | null, key?: string ): QueryConstraint { - validateKey('endBefore', 2, key, true); + validateKey('endBefore', 'key', key, true); return new QueryEndBeforeConstraint(value, key); } @@ -1090,7 +1088,7 @@ class QueryStartAtConstraint extends QueryConstraint { } _apply(query: QueryImpl): QueryImpl { - validateFirebaseDataArg('startAt', 1, this._value, query._path, true); + validateFirebaseDataArg('startAt', this._value, query._path, true); const newParams = queryParamsStartAt( query._queryParams, this._value, @@ -1117,7 +1115,7 @@ export function startAt( value: number | string | boolean | null = null, key?: string ): QueryConstraint { - validateKey('startAt', 2, key, true); + validateKey('startAt', 'key', key, true); return new QueryStartAtConstraint(value, key); } @@ -1132,7 +1130,7 @@ class QueryStartAfterConstraint extends QueryConstraint { } _apply(query: QueryImpl): QueryImpl { - validateFirebaseDataArg('startAfter', 1, this._value, query._path, false); + validateFirebaseDataArg('startAfter', this._value, query._path, false); const newParams = queryParamsStartAfter( query._queryParams, this._value, @@ -1159,7 +1157,7 @@ export function startAfter( value: number | string | boolean | null, key?: string ): QueryConstraint { - validateKey('startAfter', 2, key, true); + validateKey('startAfter', 'key', key, true); return new QueryStartAfterConstraint(value, key); } @@ -1266,7 +1264,7 @@ export function orderByChild(path: string): QueryConstraint { 'orderByChild: "$value" is invalid. Use orderByValue() instead.' ); } - validatePathString('orderByChild', 1, path, false); + validatePathString('orderByChild', 'path', path, false); return new QueryOrderByChildConstraint(path); } @@ -1341,7 +1339,7 @@ class QueryEqualToValueConstraint extends QueryConstraint { } _apply(query: QueryImpl): QueryImpl { - validateFirebaseDataArg('equalTo', 1, this._value, query._path, false); + validateFirebaseDataArg('equalTo', this._value, query._path, false); if (query._queryParams.hasStart()) { throw new Error( 'equalTo: Starting point was already set (by another call to startAt/startAfter or ' + @@ -1364,7 +1362,7 @@ export function equalTo( value: number | string | boolean | null, key?: string ): QueryConstraint { - validateKey('equalTo', 2, key, true); + validateKey('equalTo', 'key', key, true); return new QueryEqualToValueConstraint(value, key); } diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 36f90e971d3..9790c88d24c 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -27,15 +27,15 @@ import { ref, refFromURL } from '../../exp/index'; -import { set } from '../../src/exp/Reference_impl'; +import { onValue, set } from '../../src/exp/Reference_impl'; +import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -// TODO(database-exp): Re-enable these tests -describe.skip('Database Tests', () => { +describe('Database@exp Tests', () => { let defaultApp; beforeEach(() => { @@ -65,7 +65,7 @@ describe.skip('Database Tests', () => { expect(db.app).to.equal(defaultApp); }); - it('Can set and ge tref', async () => { + it('Can set and get ref', async () => { const db = getDatabase(defaultApp); await set(ref(db, 'foo/bar'), 'foobar'); const snap = await get(ref(db, 'foo/bar')); @@ -77,6 +77,60 @@ describe.skip('Database Tests', () => { await get(refFromURL(db, `${DATABASE_ADDRESS}/foo/bar`)); }); + it('Can get updates', async () => { + const db = getDatabase(defaultApp); + const fooRef = ref(db, 'foo'); + + const ea = EventAccumulatorFactory.waitsForCount(2); + onValue(fooRef, snap => { + ea.addEvent(snap.val()); + }); + + await set(fooRef, 'a'); + await set(fooRef, 'b'); + + const [snap1, snap2] = await ea.promise; + expect(snap1).to.equal('a'); + expect(snap2).to.equal('b'); + }); + + it('Can use onlyOnce', async () => { + const db = getDatabase(defaultApp); + const fooRef = ref(db, 'foo'); + + const ea = EventAccumulatorFactory.waitsForCount(1); + onValue( + fooRef, + snap => { + ea.addEvent(snap.val()); + }, + { onlyOnce: true } + ); + + await set(fooRef, 'a'); + await set(fooRef, 'b'); + + const [snap1] = await ea.promise; + expect(snap1).to.equal('a'); + }); + + it('Can unsubscribe', async () => { + const db = getDatabase(defaultApp); + const fooRef = ref(db, 'foo'); + + const ea = EventAccumulatorFactory.waitsForCount(1); + const unsubscribe = onValue(fooRef, snap => { + ea.addEvent(snap.val()); + }); + + await set(fooRef, 'a'); + unsubscribe(); + await set(fooRef, 'b'); + + const [snap1] = await ea.promise; + expect(snap1).to.equal('a'); + }); + it('Can goOffline/goOnline', async () => { const db = getDatabase(defaultApp); goOffline(db); diff --git a/packages/util/src/validation.ts b/packages/util/src/validation.ts index a4751801ff2..372144b6645 100644 --- a/packages/util/src/validation.ts +++ b/packages/util/src/validation.ts @@ -53,39 +53,11 @@ export const validateArgCount = function ( * Generates a string to prefix an error message about failed argument validation * * @param fnName The function name - * @param argumentNumber The index of the argument - * @param optional Whether or not the argument is optional + * @param argName The name of the argument * @return The prefix to add to the error thrown for validation. */ -export function errorPrefix( - fnName: string, - argumentNumber: number, - optional: boolean -): string { - let argName = ''; - switch (argumentNumber) { - case 1: - argName = optional ? 'first' : 'First'; - break; - case 2: - argName = optional ? 'second' : 'Second'; - break; - case 3: - argName = optional ? 'third' : 'Third'; - break; - case 4: - argName = optional ? 'fourth' : 'Fourth'; - break; - default: - throw new Error( - 'errorPrefix called with argumentNumber > 4. Need to update it?' - ); - } - - let error = fnName + ' failed: '; - - error += argName + ' argument '; - return error; +export function errorPrefix(fnName: string, argName: string): string { + return `${fnName} failed: ${argName} argument `; } /** @@ -96,7 +68,6 @@ export function errorPrefix( */ export function validateNamespace( fnName: string, - argumentNumber: number, namespace: string, optional: boolean ): void { @@ -106,15 +77,14 @@ export function validateNamespace( if (typeof namespace !== 'string') { //TODO: I should do more validation here. We only allow certain chars in namespaces. throw new Error( - errorPrefix(fnName, argumentNumber, optional) + - 'must be a valid firebase namespace.' + errorPrefix(fnName, 'namespace') + 'must be a valid firebase namespace.' ); } } export function validateCallback( fnName: string, - argumentNumber: number, + argumentName: string, // eslint-disable-next-line @typescript-eslint/ban-types callback: Function, optional: boolean @@ -124,15 +94,14 @@ export function validateCallback( } if (typeof callback !== 'function') { throw new Error( - errorPrefix(fnName, argumentNumber, optional) + - 'must be a valid function.' + errorPrefix(fnName, argumentName) + 'must be a valid function.' ); } } export function validateContextObject( fnName: string, - argumentNumber: number, + argumentName: string, context: unknown, optional: boolean ): void { @@ -141,8 +110,7 @@ export function validateContextObject( } if (typeof context !== 'object' || context === null) { throw new Error( - errorPrefix(fnName, argumentNumber, optional) + - 'must be a valid context object.' + errorPrefix(fnName, argumentName) + 'must be a valid context object.' ); } }