From 619661e8f7cc583d82551d936d66cf3397f07293 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 09:57:18 +0100 Subject: [PATCH 01/11] =?UTF-8?q?[Test=20Suite]=20Don=E2=80=99t=20run=20fo?= =?UTF-8?q?cused=20pending=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/.gitignore | 1 + tests/__tests__/src/tests/pendingTestTests.js | 78 +++++++++++++++++++ tests/lib/TestSuite.js | 20 ++--- 3 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 tests/.gitignore diff --git a/tests/.gitignore b/tests/.gitignore new file mode 100644 index 00000000..5afd82db --- /dev/null +++ b/tests/.gitignore @@ -0,0 +1 @@ +__tests__/build/ diff --git a/tests/__tests__/src/tests/pendingTestTests.js b/tests/__tests__/src/tests/pendingTestTests.js index e7c128a9..1dafed1c 100644 --- a/tests/__tests__/src/tests/pendingTestTests.js +++ b/tests/__tests__/src/tests/pendingTestTests.js @@ -103,6 +103,84 @@ function pendingTestTests({ it: _it, describe: _describe }) { otherTest.should.be.called(); }); }); + + _describe('when an outer context is focused', () => { + _it('a pending test will still not run', async () => { + const pendingTest = sinon.spy(); + const otherTest = sinon.spy(); + const unfocusedTest = sinon.spy(); + + const testSuite = new TestSuite('', '', {}); + + testSuite.addTests(({ fdescribe, it, xit }) => { + fdescribe('', () => { + xit('', pendingTest); + + it('', otherTest); + }); + + it('', unfocusedTest); + }); + + testSuite.setStore({ + getState: () => { return {}; }, + }); + + const testIdsToRun = Object.keys(testSuite.testDefinitions.focusedTestIds).reduce((memo, testId) => { + if (!testSuite.testDefinitions.pendingTestIds[testId]) { + memo.push(testId); + } + + return memo; + }, []); + + await testSuite.run(testIdsToRun); + + pendingTest.should.not.be.called(); + otherTest.should.be.called(); + unfocusedTest.should.not.be.called(); + }); + }); + + _describe('when an outer context is focused', () => { + _it('a pending context will still not run', async () => { + const pendingTest = sinon.spy(); + const otherTest = sinon.spy(); + const unfocusedTest = sinon.spy(); + + const testSuite = new TestSuite('', '', {}); + + testSuite.addTests(({ fdescribe, it, xdescribe }) => { + fdescribe('', () => { + xdescribe('', () => { + it('', pendingTest); + }); + + it('', otherTest); + }); + + it('', unfocusedTest); + }); + + testSuite.setStore({ + getState: () => { return {}; }, + }); + + const testIdsToRun = Object.keys(testSuite.testDefinitions.focusedTestIds).reduce((memo, testId) => { + if (!testSuite.testDefinitions.pendingTestIds[testId]) { + memo.push(testId); + } + + return memo; + }, []); + + await testSuite.run(testIdsToRun); + + pendingTest.should.not.be.called(); + otherTest.should.be.called(); + unfocusedTest.should.not.be.called(); + }); + }); } export default pendingTestTests; diff --git a/tests/lib/TestSuite.js b/tests/lib/TestSuite.js index 3924bb8b..aaa6735c 100644 --- a/tests/lib/TestSuite.js +++ b/tests/lib/TestSuite.js @@ -111,19 +111,19 @@ class TestSuite { */ async run(testIds = undefined) { const testsToRun = (() => { - if (testIds) { - return testIds.map((id) => { - const test = this.testDefinitions.tests[id]; + return (testIds || Object.keys(this.testDefinitions.tests)).reduce((memo, id) => { + const test = this.testDefinitions.tests[id]; - if (!test) { - throw new RangeError(`ReactNativeFirebaseTests.TestRunError: Test with id ${id} not found in test suite ${this.name}`); - } + if (!test) { + throw new RangeError(`ReactNativeFirebaseTests.TestRunError: Test with id ${id} not found in test suite ${this.name}`); + } - return test; - }); - } + if (!this.testDefinitions.pendingTestIds[id]) { + memo.push(test); + } - return Object.values(this.testDefinitions.tests); + return memo; + }, []); })(); const testRun = new TestRun(this, testsToRun.reverse(), this.testDefinitions); From 591cf735d645556ce3fbffac1ab50b82a72a5fff Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 11:49:44 +0100 Subject: [PATCH 02/11] [TestSuite] Show full test description on Test screen --- tests/src/screens/Test.js | 69 +++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index e716ece1..5ba50f27 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -51,33 +51,25 @@ class Test extends React.Component { setParams({ test }); } - renderError() { - const { test: { message } } = this.props; - - if (message) { - return ( - - Test Error - - {message} - - - ); - } - - return null; - } - - render() { - const { test: { func, status, time } } = this.props; + const { test: { message, description, func, status, time }, testContextName } = this.props; return ( {Test.renderBanner({ status, time })} - - {this.renderError()} - Test Code Preview + + + {testContextName}:{description} + + + Test Error + + {message || 'None'} + + + + Test Code Preview + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -95,8 +87,11 @@ Test.propTypes = { time: PropTypes.number, message: PropTypes.string, func: PropTypes.function, + description: PropTypes.string, }).isRequired, + testContextName: PropTypes.string, + navigation: PropTypes.shape({ setParams: PropTypes.func.isRequired, }).isRequired, @@ -107,27 +102,39 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - content: {}, - code: { - backgroundColor: '#3F373A', - color: '#c3c3c3', - padding: 5, - fontSize: 12, - }, - codeHeader: { + header: { fontWeight: '600', fontSize: 18, backgroundColor: '#000', color: '#fff', padding: 5, }, + code: { + backgroundColor: '#3F373A', + color: '#c3c3c3', + padding: 5, + fontSize: 12, + }, + description: { + padding: 5, + fontSize: 14, + }, + testLabel: { + fontWeight: '600', + fontSize: 16 + } }); -function select({ tests }, { navigation: { state: { params: { testId } } } }) { +function select({ tests, testContexts }, { navigation: { state: { params: { testId } } } }) { const test = tests[testId]; + let testContext = testContexts[test.testContextId]; + while(testContext.parentContextId && testContexts[testContext.parentContextId].parentContextId) { + testContext = testContexts[testContext.parentContextId]; + } return { test, + testContextName: testContext.name, }; } From 6874c83efd1987035241b01a9de43cc8014b96a6 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 12:47:31 +0100 Subject: [PATCH 03/11] [TestSuite] Clean up TestSuite code further --- tests/lib/TestRun.js | 38 ++++++++++++++++++++++++-------------- tests/src/screens/Test.js | 28 +++++++++------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/tests/lib/TestRun.js b/tests/lib/TestRun.js index c927949d..593d6d3d 100644 --- a/tests/lib/TestRun.js +++ b/tests/lib/TestRun.js @@ -306,7 +306,7 @@ class TestRun { } async _safelyRunFunction(func, timeOutDuration, description) { - const syncResultOrPromise = tryCatcher(func); + const syncResultOrPromise = captureThrownErrors(func); if (syncResultOrPromise.error) { // Synchronous Error @@ -314,49 +314,59 @@ class TestRun { } // Asynchronous Error - return promiseToCallBack(syncResultOrPromise.value, timeOutDuration, description); + return capturePromiseErrors(syncResultOrPromise.result, timeOutDuration, description); } } /** - * Try catch to object - * @returns {{}} + * Call a function and capture any errors that are immediately thrown. + * @returns {Object} Object containing result of executing the function, or the error + * message that was captured * @private */ -function tryCatcher(func) { +function captureThrownErrors(func) { const result = {}; try { - result.value = func(); - } catch (e) { - result.error = e; + result.result = func(); + } catch (error) { + result.error = error; } return result; } /** - * Make a promise callback-able to trap errors - * @param promise + * Wraps a promise so that if it's rejected or an error is thrown while it's being + * evaluated, it's captured and thrown no further + * @param {*} target - Target to wrap. If a thenable object, it's wrapped so if it's + * rejected or an error is thrown, it will be captured. If a non-thenable object, + * wrapped in resolved promise and returned. + * @param {Number} timeoutDuration - Number of milliseconds the promise is allowed + * to pend before it's considered timed out + * @param {String} description - Description of the context the promises is defined + * in, used for reporting where a timeout occurred in the resulting error message. * @private */ -function promiseToCallBack(promise, timeoutDuration, description) { +function capturePromiseErrors(target, timeoutDuration, description) { let returnValue = null; try { - returnValue = Promise.resolve(promise) + returnValue = Promise.resolve(target) .then(() => { return null; }, (error) => { return Promise.resolve(error); }) - .timeout(timeoutDuration, `${description} took longer than ${timeoutDuration}ms. This can be extended with the timeout option.`) .catch((error) => { return Promise.resolve(error); - }); + }) + .timeout(timeoutDuration, + `${description} took longer than ${timeoutDuration}ms. This can be extended with the timeout option.` + ); } catch (error) { returnValue = Promise.resolve(error); } diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index 5ba50f27..b4499656 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -62,16 +62,16 @@ class Test extends React.Component { {testContextName}:{description} - Test Error - + Test Error + {message || 'None'} - + Test Code Preview - + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -102,27 +102,17 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - header: { + testLabel: { + padding: 5, + backgroundColor: '#0288d1', fontWeight: '600', - fontSize: 18, - backgroundColor: '#000', - color: '#fff', - padding: 5, - }, - code: { - backgroundColor: '#3F373A', - color: '#c3c3c3', - padding: 5, - fontSize: 12, + color: '#ffffff', + fontSize: 16 }, description: { padding: 5, fontSize: 14, }, - testLabel: { - fontWeight: '600', - fontSize: 16 - } }); function select({ tests, testContexts }, { navigation: { state: { params: { testId } } } }) { From 86d771ba1bf4ac1f41fb8ce9d2b938082b30eff4 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 13:10:09 +0100 Subject: [PATCH 04/11] [TestSuite] Add stack traces to failed tests --- tests/lib/TestRun.js | 3 +-- tests/src/actions/TestActions.js | 3 ++- tests/src/reducers/testsReducers.js | 1 + tests/src/screens/Test.js | 26 +++++++++++++++----------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/lib/TestRun.js b/tests/lib/TestRun.js index 593d6d3d..82bd4484 100644 --- a/tests/lib/TestRun.js +++ b/tests/lib/TestRun.js @@ -286,8 +286,7 @@ class TestRun { suiteId: this.testSuite.id, status: RunStatus.ERR, time: Date.now() - this.runStartTime, - message: `Test suite failed: ${error.message}`, - stackTrace: error.stack, + message: `Test suite failed: ${error.message}` }); }); } diff --git a/tests/src/actions/TestActions.js b/tests/src/actions/TestActions.js index a2ed5fe3..71487b0a 100644 --- a/tests/src/actions/TestActions.js +++ b/tests/src/actions/TestActions.js @@ -14,13 +14,14 @@ export function setSuiteStatus({ suiteId, status, time, message, progress }) { }; } -export function setTestStatus({ testId, status, time = 0, message = null }) { +export function setTestStatus({ testId, status, stackTrace, time = 0, message = null }) { return { type: TEST_SET_STATUS, testId, status, message, + stackTrace, time, }; } diff --git a/tests/src/reducers/testsReducers.js b/tests/src/reducers/testsReducers.js index 204e600f..cfd6fb07 100644 --- a/tests/src/reducers/testsReducers.js +++ b/tests/src/reducers/testsReducers.js @@ -12,6 +12,7 @@ function testsReducers(state = initState.tests, action: Object): State { flattened[`${action.testId}.status`] = action.status; flattened[`${action.testId}.message`] = action.message; flattened[`${action.testId}.time`] = action.time; + flattened[`${action.testId}.stackTrace`] = action.stackTrace; return unflatten(flattened); } diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index b4499656..d69cb412 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -52,25 +52,26 @@ class Test extends React.Component { } render() { - const { test: { message, description, func, status, time }, testContextName } = this.props; + const { test: { stackTrace, description, func, status, time }, testContextName } = this.props; return ( {Test.renderBanner({ status, time })} - - {testContextName}:{description} + + {testContextName} + {description} - - Test Error + + Test Error - {message || 'None'} + {stackTrace || 'None.'} - + Test Code Preview - + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -85,8 +86,8 @@ Test.propTypes = { test: PropTypes.shape({ status: PropTypes.string, time: PropTypes.number, - message: PropTypes.string, func: PropTypes.function, + stackTrace: PropTypes.function, description: PropTypes.string, }).isRequired, @@ -102,12 +103,15 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - testLabel: { + sectionContainer: { + minHeight: 100, + }, + heading: { padding: 5, backgroundColor: '#0288d1', fontWeight: '600', color: '#ffffff', - fontSize: 16 + fontSize: 16, }, description: { padding: 5, From 8d665542cc8d780784e145333c6dff9b58a9bb12 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 14:33:55 +0100 Subject: [PATCH 05/11] [Database] Standardise error messages and add context support for Reference.on() - Make error messages raised by Reference.on() same as Web API - Add support for context argument to Reference.on() - Add tests for Reference.on() - Add JSDoc comments for Reference.on() and some other minor methods --- lib/modules/base.js | 6 + lib/modules/database/reference.js | 254 ++++++++++-- lib/modules/database/snapshot.js | 1 + tests/src/tests/database/ref/index.js | 7 +- tests/src/tests/database/ref/on/onTests.js | 52 +++ .../src/tests/database/ref/on/onValueTests.js | 362 ++++++++++++++++++ tests/src/tests/database/ref/onTests.js | 125 ------ 7 files changed, 649 insertions(+), 158 deletions(-) create mode 100644 tests/src/tests/database/ref/on/onTests.js create mode 100644 tests/src/tests/database/ref/on/onValueTests.js delete mode 100644 tests/src/tests/database/ref/onTests.js diff --git a/lib/modules/base.js b/lib/modules/base.js index 5021bf01..8a44b896 100644 --- a/lib/modules/base.js +++ b/lib/modules/base.js @@ -76,6 +76,12 @@ export class ReferenceBase extends Base { this.path = path || '/'; } + /** + * The last part of a Reference's path (after the last '/') + * The key of a root Reference is null. + * @type {String} + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#key} + */ get key(): string|null { return this.path === '/' ? null : this.path.substring(this.path.lastIndexOf('/') + 1); } diff --git a/lib/modules/database/reference.js b/lib/modules/database/reference.js index 67f88c2d..1d8662e9 100644 --- a/lib/modules/database/reference.js +++ b/lib/modules/database/reference.js @@ -14,8 +14,44 @@ const FirebaseDatabase = NativeModules.RNFirebaseDatabase; let refId = 1; /** + * Enum for event types + * @readonly + * @enum {String} + */ +const ReferenceEventTypes = { + value: 'value', + child_added: 'child_added', + child_removed: 'child_removed', + child_changed: 'child_changed', + child_moved: 'child_moved', +}; + +/** + * @typedef {String} ReferenceLocation - Path to location in the database, relative + * to the root reference. Consists of a path where segments are separated by a + * forward slash (/) and ends in a ReferenceKey - except the root location, which + * has no ReferenceKey. + * + * @example + * // root reference location: '/' + * // non-root reference: '/path/to/referenceKey' + */ + +/** + * @typedef {String} ReferenceKey - Identifier for each location that is unique to that + * location, within the scope of its parent. The last part of a ReferenceLocation. + */ + +/** + * Represents a specific location in your Database that can be used for + * reading or writing data. + * + * You can reference the root using firebase.database().ref() or a child location + * by calling firebase.database().ref("child/path"). + * * @link https://firebase.google.com/docs/reference/js/firebase.database.Reference * @class Reference + * @extends ReferenceBase */ export default class Reference extends ReferenceBase { @@ -99,23 +135,152 @@ export default class Reference extends ReferenceBase { } /** + * Called once with the initial data at the specified location and then once each + * time the data changes. * - * @param eventName - * @param successCallback - * @param failureCallback - * @param context TODO - * @returns {*} + * @callback onValueCallback + * @param {!DataSnapshot} dataSnapshot - Snapshot representing data at the location + * specified by the current ref. It won't trigger (.val() won't return a value) + * until the entire contents have been synchronized. If location has no data, .val() + * will return null. */ - on(eventName: string, successCallback: () => any, failureCallback: () => any) { - if (!isFunction(successCallback)) throw new Error('The specified callback must be a function'); - if (failureCallback && !isFunction(failureCallback)) throw new Error('The specified error callback must be a function'); - this.log.debug('adding reference.on', this.refId, eventName); + + /** + * Called once for each initial child at the specified location and then again + * every time a new child is added. + * + * @callback onChildAddedCallback + * @param {!DataSnapshot} dataSnapshot - Snapshot reflecting the data for the + * relevant child. + * @param {?ReferenceKey} previousChildKey - For ordering purposes, the key + * of the previous sibling child by sort order, or null if it is the first child. + */ + + /** + * Called once every time a child is removed. + * + * A child will get removed when either: + * - remove() is explicitly called on a child or one of its ancestors + * - set(null) is called on that child or one of its ancestors + * - a child has all of its children removed + * - there is a query in effect which now filters out the child (because it's sort + * order changed or the max limit was hit) + * + * @callback onChildRemovedCallback + * @param {!DataSnapshot} dataSnapshot - Snapshot reflecting the old data for + * the child that was removed. + */ + + /** + * Called when a child (or any of its descendants) changes. + * + * A single child_changed event may represent multiple changes to the child. + * + * @callback onChildChangedCallback + * @param {!DataSnapshot} dataSnapshot - Snapshot reflecting new child contents. + * @param {?ReferenceKey} previousChildKey - For ordering purposes, the key + * of the previous sibling child by sort order, or null if it is the first child. + */ + + /** + * Called when a child's sort order changes, i.e. its position relative to its + * siblings changes. + * + * @callback onChildMovedCallback + * @param {!DataSnapshot} dataSnapshot - Snapshot reflecting the data of the moved + * child. + * @param {?ReferenceKey} previousChildKey - For ordering purposes, the key + * of the previous sibling child by sort order, or null if it is the first child. + */ + + /** + * @typedef (onValueCallback|onChildAddedCallback|onChildRemovedCallback|onChildChangedCallback|onChildMovedCallback) ReferenceEventCallback + */ + + /** + * Called if the event subscription is cancelled because the client does + * not have permission to read this data (or has lost the permission to do so). + * + * @callback onFailureCallback + * @param {Error} error - Object indicating why the failure occurred + */ + + /** + * Binds callback handlers to when data changes at the current ref's location. + * The primary method of reading data from a Database. + * + * Callbacks can be unbound using {@link off}. + * + * Event Types: + * + * - value: {@link onValueCallback}. + * - child_added: {@link onChildAddedCallback} + * - child_removed: {@link onChildRemovedCallback} + * - child_changed: {@link onChildChangedCallback} + * - child_moved: {@link onChildMovedCallback} + * + * @param {ReferenceEventType} eventType - Type of event to attach a callback for. + * @param {ReferenceEventCallback} successCallback - Function that will be called + * when the event occurs with the new data. + * @param {onFailureCallback=} failureCallbackOrContext - Optional callback that is called + * if the event subscription fails. {@link onFailureCallback} + * @param {*=} context - Optional object to bind the callbacks to when calling them. + * @returns {ReferenceEventCallback} callback function, unmodified (unbound), for + * convenience if you want to pass an inline function to on() and store it later for + * removing using off(). + * + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#on} + */ + on(eventType: string, successCallback: () => any, failureCallbackOrContext: () => any, context: any) { + if (!eventType) throw new Error('Error: Query on failed: Was called with 0 arguments. Expects at least 2'); + if (!ReferenceEventTypes[eventType]) throw new Error('Query.on failed: First argument must be a valid event type: "value", "child_added", "child_removed", "child_changed", or "child_moved".'); + if (!successCallback) throw new Error('Query.on failed: Was called with 1 argument. Expects at least 2.'); + if (!isFunction(successCallback)) throw new Error('Query.on failed: Second argument must be a valid function.'); + if (arguments.length > 2 && !failureCallbackOrContext) throw new Error('Query.on failed: third argument must either be a cancel callback or a context object.'); + + this.log.debug('adding reference.on', this.refId, eventType); + + let _failureCallback; + let _context; + + if (context) { + _context = context; + _failureCallback = failureCallbackOrContext; + } else { + if (isFunction(failureCallbackOrContext)) { + _failureCallback = failureCallbackOrContext; + } else { + _context = failureCallbackOrContext; + } + } + + if (_failureCallback) { + _failureCallback = (error) => { + if (error.message.startsWith('FirebaseError: permission_denied')) { + + error.message = `permission_denied at /${this.path}: Client doesn\'t have permission to access the desired data.` + } + + failureCallbackOrContext(error); + + } + } + + let _successCallback; + + if (_context) { + _successCallback = successCallback.bind(_context); + } else { + _successCallback = successCallback; + } + const listener = { listenerId: Object.keys(this.listeners).length + 1, - eventName, - successCallback, - failureCallback, + eventName: eventType, + successCallback: _successCallback, + failureCallback: _failureCallback, }; + this.listeners[listener.listenerId] = listener; this.database.on(this, listener); return successCallback; @@ -144,29 +309,49 @@ export default class Reference extends ReferenceBase { } /** + * Detaches a callback attached with on(). * - * @param eventName - * @param origCB - * @returns {*} + * Calling off() on a parent listener will not automatically remove listeners + * registered on child nodes. + * + * If on() was called multiple times with the same eventType off() must be + * called multiple times to completely remove it. + * + * If a callback is not specified, all callbacks for the specified eventType + * will be removed. If no eventType or callback is specified, all callbacks + * for the Reference will be removed. + * + * If a context is specified, it too is used as a filter parameter: a callback + * will only be detached if, when it was attached with on(), the same event type, + * callback function and context were provided. + * + * If no callbacks matching the parameters provided are found, no callbacks are + * detached. + * + * @param {('value'|'child_added'|'child_changed'|'child_removed'|'child_moved')=} eventType - Type of event to detach callback for. + * @param {Function=} originalCallback - Original callback passed to on() + * @param {*=} context - The context passed to on() when the callback was bound + * + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#off} */ - off(eventName?: string = '', origCB?: () => any) { - this.log.debug('ref.off(): ', this.refId, eventName); + off(eventType?: string = '', originalCallback?: () => any) { + this.log.debug('ref.off(): ', this.refId, eventType); // $FlowFixMe const listeners: Array = Object.values(this.listeners); let listenersToRemove; - if (eventName && origCB) { + if (eventType && originalCallback) { listenersToRemove = listeners.filter((listener) => { - return listener.eventName === eventName && listener.successCallback === origCB; + return listener.eventName === eventType && listener.successCallback === originalCallback; }); // Only remove a single listener as per the web spec if (listenersToRemove.length > 1) listenersToRemove = [listenersToRemove[0]]; - } else if (eventName) { + } else if (eventType) { listenersToRemove = listeners.filter((listener) => { - return listener.eventName === eventName; + return listener.eventName === eventType; }); - } else if (origCB) { + } else if (originalCallback) { listenersToRemove = listeners.filter((listener) => { - return listener.successCallback === origCB; + return listener.successCallback === originalCallback; }); } else { listenersToRemove = listeners; @@ -349,9 +534,12 @@ export default class Reference extends ReferenceBase { } /** - * Get a specified child - * @param path - * @returns {Reference} + * Creates a Reference to a child of the current Reference, using a relative path. + * No validation is performed on the path to ensure it has a valid format. + * @param {String} path relative to current ref's location + * @returns {!Reference} A new Reference to the path provided, relative to the current + * Reference + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#child} */ child(path: string) { return new Reference(this.database, `${this.path}/${path}`); @@ -370,6 +558,7 @@ export default class Reference extends ReferenceBase { * same instance of firebase.app.App - multiple firebase apps not currently supported. * @param {Reference} otherRef - Other reference to compare to this one * @return {Boolean} Whether otherReference is equal to this one + * * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#isEqual} */ isEqual(otherRef: Reference): boolean { @@ -381,8 +570,10 @@ export default class Reference extends ReferenceBase { */ /** - * Returns the parent ref of the current ref i.e. a ref of /foo/bar would return a new ref to '/foo' - * @returns {*} + * The parent location of a Reference, or null for the root Reference. + * @type {Reference} + * + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#parent} */ get parent(): Reference|null { if (this.path === '/') return null; @@ -392,6 +583,7 @@ export default class Reference extends ReferenceBase { /** * A reference to itself * @type {!Reference} + * * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#ref} */ get ref(): Reference { @@ -399,8 +591,10 @@ export default class Reference extends ReferenceBase { } /** - * Returns a ref to the root of db - '/' - * @returns {Reference} + * Reference to the root of the database: '/' + * @type {!Reference} + * + * {@link https://firebase.google.com/docs/reference/js/firebase.database.Reference#root} */ get root(): Reference { return new Reference(this.database, '/'); diff --git a/lib/modules/database/snapshot.js b/lib/modules/database/snapshot.js index 97c28546..9abb6b01 100644 --- a/lib/modules/database/snapshot.js +++ b/lib/modules/database/snapshot.js @@ -5,6 +5,7 @@ import Reference from './reference.js'; import { isObject, deepGet, deepExists } from './../../utils'; /** + * @class DataSnapshot * @link https://firebase.google.com/docs/reference/js/firebase.database.DataSnapshot */ export default class Snapshot { diff --git a/tests/src/tests/database/ref/index.js b/tests/src/tests/database/ref/index.js index 15b7424d..3e4707db 100644 --- a/tests/src/tests/database/ref/index.js +++ b/tests/src/tests/database/ref/index.js @@ -1,4 +1,5 @@ -import onTests from './onTests'; +import onTests from './on/onTests'; +import onValueTests from './on/onValueTests'; import offTests from './offTests'; import onceTests from './onceTests'; import setTests from './setTests'; @@ -19,8 +20,8 @@ import DatabaseContents from '../../support/DatabaseContents'; const testGroups = [ factoryTests, keyTests, parentTests, childTests, rootTests, - pushTests, onTests, offTests, onceTests, updateTests, removeTests, setTests, - transactionTests, queryTests, refTests, isEqualTests, + pushTests, onTests, onValueTests, offTests, onceTests, updateTests, + removeTests, setTests, transactionTests, queryTests, refTests, isEqualTests, ]; function registerTestSuite(testSuite) { diff --git a/tests/src/tests/database/ref/on/onTests.js b/tests/src/tests/database/ref/on/onTests.js new file mode 100644 index 00000000..32aeb754 --- /dev/null +++ b/tests/src/tests/database/ref/on/onTests.js @@ -0,0 +1,52 @@ +import 'should-sinon'; + +function onTests({ describe, it, firebase, context }) { + describe('ref().on()', () => { + // Observed Web API Behaviour + context('when no eventName is provided', () => { + it('then raises an error', () => { + const ref = firebase.native.database().ref('tests/types/number'); + + (() => { ref.on(); }).should.throw('Error: Query on failed: Was called with 0 arguments. Expects at least 2'); + }); + }); + + // Observed Web API Behaviour + context('when no callback function is provided', () => { + it('then raises an error', () => { + const ref = firebase.native.database().ref('tests/types/number'); + + (() => { ref.on('value'); }).should.throw('Query.on failed: Was called with 1 argument. Expects at least 2.'); + }); + }); + + // Observed Web API Behaviour + context('when an invalid eventName is provided', () => { + it('then raises an error', () => { + const ref = firebase.native.database().ref('tests/types/number'); + + (() => { ref.on('invalid', () => {}); }).should.throw('Query.on failed: First argument must be a valid event type: "value", "child_added", "child_removed", "child_changed", or "child_moved".'); + }); + }); + + // Observed Web API Behaviour + context('when an invalid success callback function is provided', () => { + it('then raises an error', () => { + const ref = firebase.native.database().ref('tests/types/number'); + + (() => { ref.on('value', 1); }).should.throw('Query.on failed: Second argument must be a valid function.'); + }); + }); + + // Observed Web API Behaviour + context('when an invalid failure callback function is provided', () => { + it('then raises an error', () => { + const ref = firebase.native.database().ref('tests/types/number'); + + (() => { ref.on('value', () => {}, null); }).should.throw('Query.on failed: third argument must either be a cancel callback or a context object.'); + }); + }); + }); +} + +export default onTests; diff --git a/tests/src/tests/database/ref/on/onValueTests.js b/tests/src/tests/database/ref/on/onValueTests.js new file mode 100644 index 00000000..af0f8ca2 --- /dev/null +++ b/tests/src/tests/database/ref/on/onValueTests.js @@ -0,0 +1,362 @@ +import sinon from 'sinon'; +import 'should-sinon'; +import Promise from 'bluebird'; + +import DatabaseContents from '../../../support/DatabaseContents'; + +function onTests({ describe, context, it, xit, firebase, tryCatch }) { + describe('ref().on(\'value\')', () => { + // Documented Web API Behaviour + it('returns the success callback', () => { + // Setup + + const successCallback = sinon.spy(); + const ref = firebase.native.database().ref('tests/types/array'); + + // Assertions + + ref.on('value', successCallback).should.eql(successCallback); + + // Tear down + + ref.off(); + }); + + // Documented Web API Behaviour + it('calls callback with null if there is no data at ref', async () => { + // Setup + const ref = firebase.native.database().ref('tests/types/invalid'); + + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + // Assertions + + callback.should.be.calledWith(null); + + await ref.set(1); + + callback.should.be.calledWith(1); + + // Teardown + ref.off(); + await ref.set(null); + }); + + // Documented Web API Behaviour + it('calls callback with the initial data and then when value changes', () => { + return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { + // Setup + + const ref = firebase.native.database().ref(`tests/types/${dataRef}`); + const currentDataValue = DatabaseContents.DEFAULT[dataRef]; + + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith(currentDataValue); + + const newDataValue = DatabaseContents.NEW[dataRef]; + await ref.set(newDataValue); + + // Assertions + + callback.should.be.calledWith(newDataValue); + callback.should.be.calledTwice(); + + // Tear down + + ref.off(); + await ref.set(currentDataValue); + }); + }); + + it('calls callback when children of the ref change', async () => { + const ref = firebase.native.database().ref('tests/types/object'); + const currentDataValue = DatabaseContents.DEFAULT.object; + + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith(currentDataValue); + + const newDataValue = DatabaseContents.NEW.string; + const childRef = firebase.native.database().ref('tests/types/object/foo2'); + await childRef.set(newDataValue); + + // Assertions + + callback.should.be.calledWith({ + ...currentDataValue, + foo2: newDataValue, + }); + + callback.should.be.calledTwice(); + + // Tear down + + ref.off(); + await ref.set(currentDataValue); + }); + + /** + * Pending until behaviour of push is possibly fixed. Currently, causes + * an array to be converted to an object - don't think this is the intended + * behaviour + */ + xit('calls callback when child of the ref is added', async () => { + const ref = firebase.native.database().ref('tests/types/array'); + const currentDataValue = DatabaseContents.DEFAULT.array; + + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith(currentDataValue); + + await ref.push(37); + + // Assertions + callback.should.be.calledWith([ + ...currentDataValue, + 37, + ]); + + callback.should.be.calledTwice(); + + // Tear down + + ref.off(); + await ref.set(currentDataValue); + }); + + it('doesn\'t call callback when the ref is updated with the same value', async () => { + const ref = firebase.native.database().ref('tests/types/object'); + const currentDataValue = DatabaseContents.DEFAULT.object; + + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith(currentDataValue); + + await ref.set(currentDataValue); + + // Assertions + + callback.should.be.calledOnce(); // Callback is not called again + + // Tear down + + ref.off(); + }); + + // Documented Web API Behaviour + it('allows binding multiple callbacks to the same ref', () => { + return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { + // Setup + + const ref = firebase.native.database().ref(`tests/types/${dataRef}`); + const currentDataValue = DatabaseContents.DEFAULT[dataRef]; + + const callbackA = sinon.spy(); + const callbackB = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callbackA(snapshot.val()); + resolve(); + }); + }); + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callbackB(snapshot.val()); + resolve(); + }); + }); + + callbackA.should.be.calledWith(currentDataValue); + callbackA.should.be.calledOnce(); + + callbackB.should.be.calledWith(currentDataValue); + callbackB.should.be.calledOnce(); + + // Tear down + + ref.off(); + }); + }); + + context('when no failure callback is provided', () => { + it('then does not call the callback for a ref to un-permitted location', () => { + const invalidRef = firebase.native.database().ref('nope'); + + const callback = sinon.spy(); + + invalidRef.on('value', callback); + + /** + * As we are testing that a callback is "never" called, we just wait for + * a reasonable time before giving up. + */ + return new Promise((resolve) => { + setTimeout(() => { + callback.should.not.be.called(); + invalidRef.off(); + resolve(); + }, 1000); + }); + }); + + // Documented Web API Behaviour + it('then calls callback bound to the specified context with the initial data and then when value changes', () => { + return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { + // Setup + + const ref = firebase.native.database().ref(`tests/types/${dataRef}`); + const currentDataValue = DatabaseContents.DEFAULT[dataRef]; + + const context = { + callCount: 0, + }; + + // Test + + await new Promise((resolve) => { + ref.on('value', function(snapshot) { + this.value = snapshot.val(); + this.callCount += 1; + resolve(); + }, context); + }); + + context.value.should.eql(currentDataValue); + context.callCount.should.eql(1); + + const newDataValue = DatabaseContents.NEW[dataRef]; + await ref.set(newDataValue); + + // Assertions + + context.value.should.eql(newDataValue); + context.callCount.should.eql(2); + + // Tear down + + ref.off(); + await ref.set(currentDataValue); + }); + }); + + }); + + // Observed Web API Behaviour + context('when a failure callback is provided', () => { + it('then calls only the failure callback for a ref to un-permitted location', () => { + const invalidRef = firebase.native.database().ref('nope'); + + const callback = sinon.spy(); + + return new Promise((resolve, reject) => { + invalidRef.on('value', callback, tryCatch((error) => { + error.message.should.eql( + 'permission_denied at /nope: Client doesn\'t have permission to access the desired data.' + ); + error.name.should.eql('Error'); + + callback.should.not.be.called(); + + invalidRef.off(); + resolve(); + }, reject)); + }); + }); + + // Documented Web API Behaviour + it('then calls callback bound to the specified context with the initial data and then when value changes', () => { + return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { + // Setup + + const ref = firebase.native.database().ref(`tests/types/${dataRef}`); + const currentDataValue = DatabaseContents.DEFAULT[dataRef]; + + const context = { + callCount: 0, + }; + + const failureCallback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', function(snapshot) { + this.value = snapshot.val(); + this.callCount += 1; + resolve(); + }, failureCallback, context); + }); + + failureCallback.should.not.be.called(); + context.value.should.eql(currentDataValue); + context.callCount.should.eql(1); + + const newDataValue = DatabaseContents.NEW[dataRef]; + await ref.set(newDataValue); + + // Assertions + + context.value.should.eql(newDataValue); + context.callCount.should.eql(2); + + // Tear down + + ref.off(); + await ref.set(currentDataValue); + }); + }) + }); + }); +} + +export default onTests; diff --git a/tests/src/tests/database/ref/onTests.js b/tests/src/tests/database/ref/onTests.js deleted file mode 100644 index 8f01708b..00000000 --- a/tests/src/tests/database/ref/onTests.js +++ /dev/null @@ -1,125 +0,0 @@ -import sinon from 'sinon'; -import 'should-sinon'; -import Promise from 'bluebird'; - -import DatabaseContents from '../../support/DatabaseContents'; - -function onTests({ describe, it, firebase, tryCatch }) { - describe('ref().on()', () => { - it('calls callback when value changes', () => { - return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { - // Setup - - const ref = firebase.native.database().ref(`tests/types/${dataRef}`); - const currentDataValue = DatabaseContents.DEFAULT[dataRef]; - - const callback = sinon.spy(); - - // Test - - await new Promise((resolve) => { - ref.on('value', (snapshot) => { - callback(snapshot.val()); - resolve(); - }); - }); - - callback.should.be.calledWith(currentDataValue); - - const newDataValue = DatabaseContents.NEW[dataRef]; - await ref.set(newDataValue); - - // Assertions - - callback.should.be.calledWith(newDataValue); - - // Tear down - - ref.off(); - }); - }); - - it('allows binding multiple callbacks to the same ref', () => { - return Promise.each(Object.keys(DatabaseContents.DEFAULT), async (dataRef) => { - // Setup - - const ref = firebase.native.database().ref(`tests/types/${dataRef}`); - const currentDataValue = DatabaseContents.DEFAULT[dataRef]; - - const callbackA = sinon.spy(); - const callbackB = sinon.spy(); - - // Test - - await new Promise((resolve) => { - ref.on('value', (snapshot) => { - callbackA(snapshot.val()); - resolve(); - }); - }); - - await new Promise((resolve) => { - ref.on('value', (snapshot) => { - callbackB(snapshot.val()); - resolve(); - }); - }); - - callbackA.should.be.calledWith(currentDataValue); - callbackB.should.be.calledWith(currentDataValue); - - // Tear down - - ref.off(); - }); - }); - - it('calls callback with current values', () => { - return Promise.each(Object.keys(DatabaseContents.DEFAULT), (dataRef) => { - // Setup - - const dataTypeValue = DatabaseContents.DEFAULT[dataRef]; - const ref = firebase.native.database().ref(`tests/types/${dataRef}`); - - // Test - - return ref.on('value', (snapshot) => { - // Assertion - - snapshot.val().should.eql(dataTypeValue); - - // Tear down - - ref.off(); - }); - }); - }); - - it('errors if permission denied', () => { - return new Promise((resolve, reject) => { - const successCb = tryCatch(() => { - // Assertion - - reject(new Error('No permission denied error')); - }, reject); - - const failureCb = tryCatch((error) => { - // Assertion - - error.message.includes('permission_denied').should.be.true(); - resolve(); - }, reject); - - // Setup - - const invalidRef = firebase.native.database().ref('nope'); - - // Test - - invalidRef.on('value', successCb, failureCb); - }); - }); - }); -} - -export default onTests; From 61495c5bd3363583659f8dfe0aeab6ea3db0033b Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Mon, 8 May 2017 18:18:34 +0100 Subject: [PATCH 06/11] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5cbd3754..fb9c22dc 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ [![Gitter](https://badges.gitter.im/invertase/react-native-firebase.svg)](https://gitter.im/invertase/react-native-firebase?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) [![npm version](https://img.shields.io/npm/v/react-native-firebase.svg)](https://www.npmjs.com/package/react-native-firebase) [![License](https://img.shields.io/npm/l/react-native-firebase.svg)](/LICENSE) +[![Donate](https://img.shields.io/badge/Donate-Patreon-green.svg)](https://www.patreon.com/invertase) **RNFirebase** makes using [Firebase](http://firebase.com) with React Native simple. From 6220d9d7dd6d7ba2c637901ecaaf8d862d4cd4d6 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Tue, 9 May 2017 06:59:16 +0100 Subject: [PATCH 07/11] [Android][Database] Fix #97 Stop logging com.google.firebase.database.DatabaseException --- .../io/invertase/firebase/database/RNFirebaseDatabase.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java index c06e7ee2..70ffb308 100644 --- a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java +++ b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java @@ -28,6 +28,7 @@ import com.google.firebase.database.OnDisconnect; import com.google.firebase.database.DatabaseError; import com.google.firebase.database.DatabaseReference; import com.google.firebase.database.FirebaseDatabase; +import com.google.firebase.database.DatabaseException; import com.google.firebase.database.Transaction; import io.invertase.firebase.Utils; @@ -55,8 +56,8 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { final Callback callback) { try { mFirebaseDatabase.setPersistenceEnabled(enable); - } catch (Throwable t) { - Log.e(TAG, "FirebaseDatabase setPersistenceEnabled exception", t); + } catch (DatabaseException t) { + } WritableMap res = Arguments.createMap(); From febbca2e47fa97db0abbab28721dfb1ecf8f9f61 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Tue, 9 May 2017 07:50:09 +0100 Subject: [PATCH 08/11] [TestSuite] Update Podfile.lock --- tests/.gitignore | 1 + tests/ios/Podfile.lock | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/.gitignore b/tests/.gitignore index 5afd82db..397259db 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -1 +1,2 @@ __tests__/build/ +ios/build/ diff --git a/tests/ios/Podfile.lock b/tests/ios/Podfile.lock index c3bb0e75..73542cdb 100644 --- a/tests/ios/Podfile.lock +++ b/tests/ios/Podfile.lock @@ -110,7 +110,7 @@ PODS: - React/RCTWebSocket (0.40.0): - React/Core - React/yoga (0.40.0) - - RNFirebase (1.0.0-alpha13) + - RNFirebase (1.0.2) DEPENDENCIES: - Firebase/Analytics @@ -140,14 +140,14 @@ DEPENDENCIES: EXTERNAL SOURCES: React: - :path: ../node_modules/react-native + :path: "../node_modules/react-native" RNFirebase: - :path: ./../../ + :path: "./../../" SPEC CHECKSUMS: Firebase: 85a581fb04e44f63ae9f4fbc8d6dabf4a4c18653 FirebaseAnalytics: 0d1b7d81d5021155be37702a94ba1ec16d45365d - FirebaseAppIndexing: d0fa52ce0ad13f4b5b2f09e4b47fb0dc2213f4e9 + FirebaseAppIndexing: adf2f3b14b6828f1d4c539fc72434962cf29ea2c FirebaseAuth: cc8a1824170adbd351edb7f994490a3fb5c18be6 FirebaseCore: 225d40532489835a034b8f4e2c9c87fbf4f615a2 FirebaseCrash: db4c05d9c75baa050744d31b36357c8f1efba481 @@ -161,7 +161,7 @@ SPEC CHECKSUMS: GTMSessionFetcher: 6f8d8b28b7e345549ac471071608170b31cb4977 Protobuf: 745f59e122e5de98d4d7ef291e264a0eef80f58e React: 6dfb2f72edb1d74a800127ae157af038646673ce - RNFirebase: 46bfe1099349ac6fac8c5e57cf4f0b0f4b7938ac + RNFirebase: 9b9ab212d14243db38bee3637029f1752c2a349f PODFILE CHECKSUM: f8bc5de55afd159ec2faf523f1b8e0d861d0832b From 4177e3e6bb8399c271de29c094a6208e2b5575cd Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Tue, 9 May 2017 07:51:54 +0100 Subject: [PATCH 09/11] [TestSuite][Android] Fix #92 Adjust tests to reflect Reference.on() expected behaviour --- .../src/tests/database/ref/on/onValueTests.js | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/tests/src/tests/database/ref/on/onValueTests.js b/tests/src/tests/database/ref/on/onValueTests.js index af0f8ca2..daf87b07 100644 --- a/tests/src/tests/database/ref/on/onValueTests.js +++ b/tests/src/tests/database/ref/on/onValueTests.js @@ -1,9 +1,23 @@ +import { Platform } from 'react-native'; + import sinon from 'sinon'; import 'should-sinon'; import Promise from 'bluebird'; import DatabaseContents from '../../../support/DatabaseContents'; +/** + * On Android, some data types result in callbacks that get called twice every time + * they are updated. This appears to be behaviour coming from the Android Firebase + * library itself. + * + * See https://github.com/invertase/react-native-firebase/issues/92 for details + */ +const DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS = [ + 'array', + 'number' +]; + function onTests({ describe, context, it, xit, firebase, tryCatch }) { describe('ref().on(\'value\')', () => { // Documented Web API Behaviour @@ -78,7 +92,12 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { // Assertions callback.should.be.calledWith(newDataValue); - callback.should.be.calledTwice(); + + if (Platform.OS === 'android' && DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS.includes(dataRef)) { + callback.should.be.calledThrice(); + } else { + callback.should.be.calledTwice(); + } // Tear down @@ -222,6 +241,20 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { callbackB.should.be.calledWith(currentDataValue); callbackB.should.be.calledOnce(); + const newDataValue = DatabaseContents.NEW[dataRef]; + await ref.set(newDataValue); + + callbackA.should.be.calledWith(newDataValue); + callbackB.should.be.calledWith(newDataValue); + + if (Platform.OS === 'android' && DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS.includes(dataRef)) { + callbackA.should.be.calledThrice(); + callbackB.should.be.calledThrice(); + } else { + callbackA.should.be.calledTwice(); + callbackB.should.be.calledTwice(); + } + // Tear down ref.off(); @@ -280,7 +313,12 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { // Assertions context.value.should.eql(newDataValue); - context.callCount.should.eql(2); + + if (Platform.OS === 'android' && DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS.includes(dataRef)) { + context.callCount.should.eql(3); + } else { + context.callCount.should.eql(2); + } // Tear down @@ -347,7 +385,12 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { // Assertions context.value.should.eql(newDataValue); - context.callCount.should.eql(2); + + if (Platform.OS === 'android' && DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS.includes(dataRef)) { + context.callCount.should.eql(3); + } else { + context.callCount.should.eql(2); + } // Tear down From 9d5cc856c77e7b1e9641087c399f56ce9fe626ca Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Tue, 9 May 2017 08:09:03 +0100 Subject: [PATCH 10/11] [Database] Add more accurate comments of expected behaviour of Reference.on() --- lib/modules/database/reference.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/modules/database/reference.js b/lib/modules/database/reference.js index 1d8662e9..50fff25d 100644 --- a/lib/modules/database/reference.js +++ b/lib/modules/database/reference.js @@ -135,14 +135,19 @@ export default class Reference extends ReferenceBase { } /** - * Called once with the initial data at the specified location and then once each - * time the data changes. + * iOS: Called once with the initial data at the specified location and then once each + * time the data changes. It won't trigger until the entire contents have been + * synchronized. + * + * Android: (@link https://github.com/invertase/react-native-firebase/issues/92) + * - Array & number values: Called once with the initial data at the specified + * location and then twice each time the value changes. + * - Other data types: Called once with the initial data at the specified location + * and once each time the data type changes. * * @callback onValueCallback * @param {!DataSnapshot} dataSnapshot - Snapshot representing data at the location - * specified by the current ref. It won't trigger (.val() won't return a value) - * until the entire contents have been synchronized. If location has no data, .val() - * will return null. + * specified by the current ref. If location has no data, .val() will return null. */ /** From a81392c229ab6fc102002d196d83c20750aa1186 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Tue, 9 May 2017 08:09:36 +0100 Subject: [PATCH 11/11] [TestSuite] Update pending test in onValueTests --- .../src/tests/database/ref/on/onValueTests.js | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tests/src/tests/database/ref/on/onValueTests.js b/tests/src/tests/database/ref/on/onValueTests.js index daf87b07..2545442b 100644 --- a/tests/src/tests/database/ref/on/onValueTests.js +++ b/tests/src/tests/database/ref/on/onValueTests.js @@ -18,8 +18,8 @@ const DATATYPES_WITH_DUPLICATE_CALLBACK_CALLS = [ 'number' ]; -function onTests({ describe, context, it, xit, firebase, tryCatch }) { - describe('ref().on(\'value\')', () => { +function onTests({ fdescribe, context, it, firebase, tryCatch }) { + fdescribe('ref().on(\'value\')', () => { // Documented Web API Behaviour it('returns the success callback', () => { // Setup @@ -142,12 +142,7 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { await ref.set(currentDataValue); }); - /** - * Pending until behaviour of push is possibly fixed. Currently, causes - * an array to be converted to an object - don't think this is the intended - * behaviour - */ - xit('calls callback when child of the ref is added', async () => { + it('calls callback when child of the ref is added', async () => { const ref = firebase.native.database().ref('tests/types/array'); const currentDataValue = DatabaseContents.DEFAULT.array; @@ -164,15 +159,24 @@ function onTests({ describe, context, it, xit, firebase, tryCatch }) { callback.should.be.calledWith(currentDataValue); - await ref.push(37); + const newElementRef = await ref.push(37); + + const arrayAsObject = currentDataValue.reduce((memo, element, index) => { + memo[index] = element; + return memo; + }, {}); // Assertions - callback.should.be.calledWith([ - ...currentDataValue, - 37, - ]); + callback.should.be.calledWith({ + ...arrayAsObject, + [newElementRef.key]: 37, + }); - callback.should.be.calledTwice(); + if (Platform.OS === 'android') { + callback.should.be.calledThrice(); + } else { + callback.should.be.calledTwice(); + } // Tear down