From a42194a32acc3f8e3a68ac5fe7f6f78cf64a54ce Mon Sep 17 00:00:00 2001 From: Chris Bianca Date: Mon, 23 Oct 2017 12:23:00 +0100 Subject: [PATCH 01/23] [database][tests] Add tests for issue 521 --- .../tests/database/ref/issueSpecificTests.js | 186 ++++++++++++++++++ tests/src/tests/support/DatabaseContents.js | 19 ++ 2 files changed, 205 insertions(+) diff --git a/tests/src/tests/database/ref/issueSpecificTests.js b/tests/src/tests/database/ref/issueSpecificTests.js index 9abbb868..a1afbd78 100644 --- a/tests/src/tests/database/ref/issueSpecificTests.js +++ b/tests/src/tests/database/ref/issueSpecificTests.js @@ -1,4 +1,6 @@ import should from 'should'; +import sinon from 'sinon'; +import 'should-sinon'; import DatabaseContents from '../../support/DatabaseContents'; function issueTests({ describe, it, context, firebase }) { @@ -81,6 +83,190 @@ function issueTests({ describe, it, context, firebase }) { }); }); }); + + describe('issue_521', () => { + context('orderByChild (numerical field) and limitToLast', () => { + it('once() returns correct results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521'); + // Test + + return ref + .orderByChild('number') + .limitToLast(2) + .once('value') + .then((snapshot) => { + const val = snapshot.val(); + // Assertion + val.key2.should.eql(DatabaseContents.ISSUES[521].key2); + val.key3.should.eql(DatabaseContents.ISSUES[521].key3); + should.equal(Object.keys(val).length, 2); + + return Promise.resolve(); + }); + }); + + it('on() returns correct initial results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521').orderByChild('number').limitToLast(2); + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: DatabaseContents.ISSUES[521].key2, + key3: DatabaseContents.ISSUES[521].key3, + }); + callback.should.be.calledOnce(); + + return Promise.resolve(); + }); + + it('on() returns correct subsequent results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521').orderByChild('number').limitToLast(2); + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: DatabaseContents.ISSUES[521].key2, + key3: DatabaseContents.ISSUES[521].key3, + }); + callback.should.be.calledOnce(); + + const newDataValue = { + name: 'Item 4', + number: 4, + string: 'item4', + }; + const newRef = firebase.native.database().ref('tests/issues/521/key4'); + await newRef.set(newDataValue); + + await new Promise((resolve) => { + setTimeout(() => resolve(), 5); + }); + + // Assertions + + callback.should.be.calledWith({ + key3: DatabaseContents.ISSUES[521].key3, + key4: newDataValue, + }); + callback.should.be.calledTwice(); + + return Promise.resolve(); + }); + }); + + context('orderByChild (string field) and limitToLast', () => { + it('once() returns correct results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521'); + // Test + + return ref + .orderByChild('string') + .limitToLast(2) + .once('value') + .then((snapshot) => { + const val = snapshot.val(); + // Assertion + val.key2.should.eql(DatabaseContents.ISSUES[521].key2); + val.key3.should.eql(DatabaseContents.ISSUES[521].key3); + should.equal(Object.keys(val).length, 2); + + return Promise.resolve(); + }); + }); + + it('on() returns correct initial results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521').orderByChild('string').limitToLast(2); + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: DatabaseContents.ISSUES[521].key2, + key3: DatabaseContents.ISSUES[521].key3, + }); + callback.should.be.calledOnce(); + + return Promise.resolve(); + }); + + it('on() returns correct subsequent results', async () => { + // Setup + + const ref = firebase.native.database().ref('tests/issues/521').orderByChild('string').limitToLast(2); + const callback = sinon.spy(); + + // Test + + await new Promise((resolve) => { + ref.on('value', (snapshot) => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: DatabaseContents.ISSUES[521].key2, + key3: DatabaseContents.ISSUES[521].key3, + }); + callback.should.be.calledOnce(); + + const newDataValue = { + name: 'Item 4', + number: 4, + string: 'item4', + }; + const newRef = firebase.native.database().ref('tests/issues/521/key4'); + await newRef.set(newDataValue); + + await new Promise((resolve) => { + setTimeout(() => resolve(), 5); + }); + + // Assertions + + callback.should.be.calledWith({ + key3: DatabaseContents.ISSUES[521].key3, + key4: newDataValue, + }); + callback.should.be.calledTwice(); + + return Promise.resolve(); + }); + }); + }); } export default issueTests; diff --git a/tests/src/tests/support/DatabaseContents.js b/tests/src/tests/support/DatabaseContents.js index a4a913c9..bace7f99 100644 --- a/tests/src/tests/support/DatabaseContents.js +++ b/tests/src/tests/support/DatabaseContents.js @@ -91,5 +91,24 @@ export default { uid: 'aNYxLexOb2WsXGOPiEAu47q5bxH3', }, }, + + // https://github.com/invertase/react-native-firebase/issues/521 + 521: { + key1: { + name: 'Item 1', + number: 1, + string: 'item1', + }, + key3: { + name: 'Item 3', + number: 3, + string: 'item3', + }, + key2: { + name: 'Item 2', + number: 2, + string: 'item2', + }, + }, }, }; From 9822cb34d46308edc3abce9126013aef8d7b62b9 Mon Sep 17 00:00:00 2001 From: Chris Bianca Date: Mon, 23 Oct 2017 17:03:49 +0100 Subject: [PATCH 02/23] [database] Correctly differentiate limitToLast and other similar clauses --- .../firebase/database/RNFirebaseDatabase.java | 33 ++++++------------- ios/RNFirebase/database/RNFirebaseDatabase.m | 23 ++++++++----- lib/modules/database/query.js | 22 +++++++++---- .../tests/database/ref/issueSpecificTests.js | 10 +++--- 4 files changed, 44 insertions(+), 44 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 93ce73f4..da448ba8 100644 --- a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java +++ b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java @@ -399,7 +399,7 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { */ @ReactMethod public void on(String appName, ReadableMap props) { - getInternalReferenceForApp(appName, props) + getCachedInternalReferenceForApp(appName, props) .on( props.getString("eventType"), props.getMap("registration") @@ -481,19 +481,13 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { * @return */ private RNFirebaseDatabaseReference getInternalReferenceForApp(String appName, String key, String path, ReadableArray modifiers) { - RNFirebaseDatabaseReference existingRef = references.get(key); - - if (existingRef == null) { - existingRef = new RNFirebaseDatabaseReference( - getReactApplicationContext(), - appName, - key, - path, - modifiers - ); - } - - return existingRef; + return new RNFirebaseDatabaseReference( + getReactApplicationContext(), + appName, + key, + path, + modifiers + ); } /** @@ -503,7 +497,7 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { * @param props * @return */ - private RNFirebaseDatabaseReference getInternalReferenceForApp(String appName, ReadableMap props) { + private RNFirebaseDatabaseReference getCachedInternalReferenceForApp(String appName, ReadableMap props) { String key = props.getString("key"); String path = props.getString("path"); ReadableArray modifiers = props.getArray("modifiers"); @@ -511,14 +505,7 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { RNFirebaseDatabaseReference existingRef = references.get(key); if (existingRef == null) { - existingRef = new RNFirebaseDatabaseReference( - getReactApplicationContext(), - appName, - key, - path, - modifiers - ); - + existingRef = getInternalReferenceForApp(appName, key, path, modifiers); references.put(key, existingRef); } diff --git a/ios/RNFirebase/database/RNFirebaseDatabase.m b/ios/RNFirebase/database/RNFirebaseDatabase.m index 70efdf8d..3851ab34 100644 --- a/ios/RNFirebase/database/RNFirebaseDatabase.m +++ b/ios/RNFirebase/database/RNFirebaseDatabase.m @@ -39,7 +39,7 @@ RCT_EXPORT_METHOD(keepSynced:(NSString *) appName path:(NSString *) path modifiers:(NSArray *) modifiers state:(BOOL) state) { - FIRDatabaseQuery *query = [self getInternalReferenceForApp:appName key:key path:path modifiers:modifiers keep:false].query; + FIRDatabaseQuery *query = [self getInternalReferenceForApp:appName key:key path:path modifiers:modifiers].query; [query keepSynced:state]; } @@ -233,13 +233,13 @@ RCT_EXPORT_METHOD(once:(NSString *) appName eventName:(NSString *) eventName resolver:(RCTPromiseResolveBlock) resolve rejecter:(RCTPromiseRejectBlock) reject) { - RNFirebaseDatabaseReference *ref = [self getInternalReferenceForApp:appName key:key path:path modifiers:modifiers keep:false]; + RNFirebaseDatabaseReference *ref = [self getInternalReferenceForApp:appName key:key path:path modifiers:modifiers]; [ref once:eventName resolver:resolve rejecter:reject]; } RCT_EXPORT_METHOD(on:(NSString *) appName props:(NSDictionary *) props) { - RNFirebaseDatabaseReference *ref = [self getInternalReferenceForApp:appName key:props[@"key"] path:props[@"path"] modifiers:props[@"modifiers"] keep:false]; + RNFirebaseDatabaseReference *ref = [self getCachedInternalReferenceForApp:appName props:props]; [ref on:props[@"eventType"] registration:props[@"registration"]]; } @@ -278,15 +278,20 @@ RCT_EXPORT_METHOD(off:(NSString *) key return [[RNFirebaseDatabase getDatabaseForApp:appName] referenceWithPath:path]; } -- (RNFirebaseDatabaseReference *)getInternalReferenceForApp:(NSString *)appName key:(NSString *)key path:(NSString *)path modifiers:(NSArray *)modifiers keep:(BOOL)keep { - RNFirebaseDatabaseReference *ref = _dbReferences[key]; +- (RNFirebaseDatabaseReference *)getInternalReferenceForApp:(NSString *)appName key:(NSString *)key path:(NSString *)path modifiers:(NSArray *)modifiers { + return [[RNFirebaseDatabaseReference alloc] initWithPathAndModifiers:self app:appName key:key refPath:path modifiers:modifiers]; +} +- (RNFirebaseDatabaseReference *)getCachedInternalReferenceForApp:(NSString *)appName props:(NSDictionary *)props { + NSString *key = props[@"key"]; + NSString *path = props[@"path"]; + NSDictionary *modifiers = props[@"modifiers"]; + + RNFirebaseDatabaseReference *ref = _dbReferences[key]; + if (ref == nil) { ref = [[RNFirebaseDatabaseReference alloc] initWithPathAndModifiers:self app:appName key:key refPath:path modifiers:modifiers]; - - if (keep) { - _dbReferences[key] = ref; - } + _dbReferences[key] = ref; } return ref; } diff --git a/lib/modules/database/query.js b/lib/modules/database/query.js index 4b29b1c0..ce1e41b0 100644 --- a/lib/modules/database/query.js +++ b/lib/modules/database/query.js @@ -26,6 +26,7 @@ export default class Query { */ orderBy(name: string, key?: string) { this.modifiers.push({ + id: `orderBy-${name}:${key}`, type: 'orderBy', name, key, @@ -42,6 +43,7 @@ export default class Query { */ limit(name: string, limit: number) { this.modifiers.push({ + id: `limit-${name}:${limit}`, type: 'limit', name, limit, @@ -59,6 +61,7 @@ export default class Query { */ filter(name: string, value: any, key?: string) { this.modifiers.push({ + id: `filter-${name}:${objectToUniqueId(value)}:${key}`, type: 'filter', name, value, @@ -82,14 +85,21 @@ export default class Query { * @return {*} */ queryIdentifier() { - // convert query modifiers array into an object for generating a unique key - const object = {}; + // sort modifiers to enforce ordering + const sortedModifiers = this.getModifiers().sort((a, b) => { + if (a.id < b.id) return -1; + if (a.id > b.id) return 1; + return 0; + }); - for (let i = 0, len = this.modifiers.length; i < len; i++) { - const { name, type, value } = this.modifiers[i]; - object[`${type}-${name}`] = value; + // Convert modifiers to unique key + let key = '{'; + for (let i = 0; i < sortedModifiers.length; i++) { + if (i !== 0) key += ','; + key += sortedModifiers[i].id; } + key += '}'; - return objectToUniqueId(object); + return key; } } diff --git a/tests/src/tests/database/ref/issueSpecificTests.js b/tests/src/tests/database/ref/issueSpecificTests.js index a1afbd78..816774de 100644 --- a/tests/src/tests/database/ref/issueSpecificTests.js +++ b/tests/src/tests/database/ref/issueSpecificTests.js @@ -94,14 +94,13 @@ function issueTests({ describe, it, context, firebase }) { return ref .orderByChild('number') - .limitToLast(2) + .limitToLast(1) .once('value') .then((snapshot) => { const val = snapshot.val(); // Assertion - val.key2.should.eql(DatabaseContents.ISSUES[521].key2); val.key3.should.eql(DatabaseContents.ISSUES[521].key3); - should.equal(Object.keys(val).length, 2); + should.equal(Object.keys(val).length, 1); return Promise.resolve(); }); @@ -185,14 +184,13 @@ function issueTests({ describe, it, context, firebase }) { return ref .orderByChild('string') - .limitToLast(2) + .limitToLast(1) .once('value') .then((snapshot) => { const val = snapshot.val(); // Assertion - val.key2.should.eql(DatabaseContents.ISSUES[521].key2); val.key3.should.eql(DatabaseContents.ISSUES[521].key3); - should.equal(Object.keys(val).length, 2); + should.equal(Object.keys(val).length, 1); return Promise.resolve(); }); From dd5230f42f8fe5d20b7efc37052e074caea320d7 Mon Sep 17 00:00:00 2001 From: Chris Bianca Date: Mon, 23 Oct 2017 17:17:03 +0100 Subject: [PATCH 03/23] [database] Tests add test for Issue 532 --- tests/src/tests/database/ref/offTests.js | 161 +++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/tests/src/tests/database/ref/offTests.js b/tests/src/tests/database/ref/offTests.js index df6ebe08..a1627aea 100644 --- a/tests/src/tests/database/ref/offTests.js +++ b/tests/src/tests/database/ref/offTests.js @@ -297,6 +297,167 @@ function offTests({ describe, it, xcontext, context, firebase }) { }); }); + context('when 2 different child_added callbacks on the same path', () => { + context('that has been added and removed in the same order', () => { + it('must be completely removed', async () => { + // Setup + + const spyA = sinon.spy(); + let callbackA; + + const spyB = sinon.spy(); + let callbackB; + + const ref = firebase.native.database().ref('tests/types/array'); + const arrayLength = DatabaseContents.DEFAULT.array.length; + // Attach callbackA + await new Promise((resolve) => { + callbackA = () => { + spyA(); + resolve(); + }; + ref.on('child_added', callbackA); + }); + + // Attach callbackB + await new Promise((resolve) => { + callbackB = () => { + spyB(); + resolve(); + }; + ref.on('child_added', callbackB); + }); + + // Add a delay to ensure that the .on() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + spyA.should.have.callCount(arrayLength); + spyB.should.have.callCount(arrayLength); + + // Undo the first callback + const resp = await ref.off('child_added', callbackA); + should(resp, undefined); + + // Trigger the event the callback is listening to + await ref.push(DatabaseContents.DEFAULT.number); + + // Add a delay to ensure that the .set() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + // CallbackA should have been called zero more times its attachment + // has been removed, and callBackB only one more time becuase it's still attached + spyA.should.have.callCount(arrayLength); + spyB.should.have.callCount(arrayLength + 1); + + // Undo the second attachment + const resp2 = await ref.off('child_added', callbackB); + should(resp2, undefined); + + // Trigger the event the callback is listening to + await ref.push(DatabaseContents.DEFAULT.number); + + // Add a delay to ensure that the .set() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + // Both Callbacks should not have been called any more times + spyA.should.have.callCount(arrayLength); + spyB.should.have.callCount(arrayLength + 1); + }); + }); + + // ******This test is failed******* + context('that has been added and removed in reverse order', () => { + it('must be completely removed', async () => { + // Setup + + const spyA = sinon.spy(); + let callbackA; + + const spyB = sinon.spy(); + let callbackB; + + const ref = firebase.native.database().ref('tests/types/array'); + const arrayLength = DatabaseContents.DEFAULT.array.length; + // Attach callbackA + await new Promise((resolve) => { + callbackA = () => { + spyA(); + resolve(); + }; + ref.on('child_added', callbackA); + }); + + // Attach callbackB + await new Promise((resolve) => { + callbackB = () => { + spyB(); + resolve(); + }; + ref.on('child_added', callbackB); + }); + + // Add a delay to ensure that the .on() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + spyA.should.have.callCount(arrayLength); + spyB.should.have.callCount(arrayLength); + + // Undo the second callback + const resp = await ref.off('child_added', callbackB); + should(resp, undefined); + + // Trigger the event the callback is listening to + await ref.push(DatabaseContents.DEFAULT.number); + + // Add a delay to ensure that the .set() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + // CallbackB should have been called zero more times its attachment + // has been removed, and callBackA only one more time becuase it's still attached + spyA.should.have.callCount(arrayLength + 1); + spyB.should.have.callCount(arrayLength); + + // Undo the second attachment + const resp2 = await ref.off('child_added', callbackA); + should(resp2, undefined); + + // Trigger the event the callback is listening to + await ref.push(DatabaseContents.DEFAULT.number); + + // Add a delay to ensure that the .set() has had time to be registered + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 15); + }); + + // Both Callbacks should not have been called any more times + spyA.should.have.callCount(arrayLength + 1); + spyB.should.have.callCount(arrayLength); + }); + }); + }); + xcontext('when a context', () => { /** * @todo Add tests for when a context is passed. Not sure what the intended From 665e753a2c4571706fdbd122de486a24c4dd73de Mon Sep 17 00:00:00 2001 From: Chris Bianca Date: Mon, 23 Oct 2017 17:22:14 +0100 Subject: [PATCH 04/23] 3.0.5 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index d699d662..5e3654b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "react-native-firebase", - "version": "3.0.4", + "version": "3.0.5", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index c3a38996..def297a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-native-firebase", - "version": "3.0.4", + "version": "3.0.5", "author": "Invertase (http://invertase.io)", "description": "A well tested, feature rich Firebase implementation for React Native, supporting iOS & Android. Individual module support for Admob, Analytics, Auth, Crash Reporting, Cloud Firestore, Database, Messaging (FCM), Remote Config, Storage and Performance.", "main": "index", From f414796d1624fabf4803facc24ec12a57ef59c18 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Mon, 23 Oct 2017 17:38:33 +0100 Subject: [PATCH 05/23] Delete scratch.js --- scratch.js | 240 ----------------------------------------------------- 1 file changed, 240 deletions(-) delete mode 100644 scratch.js diff --git a/scratch.js b/scratch.js deleted file mode 100644 index 32177a2c..00000000 --- a/scratch.js +++ /dev/null @@ -1,240 +0,0 @@ -// global.fps = 0; -// global.fpsTarget = 60; -// global.delay = 0; -// global.fillSize = 100; -// -// setInterval(() => { -// const _fps = fps; -// fps = 0; -// console.log(`${_fps} - ${delay}`); -// }, 1000); -// -// function someWork() {w -// new Array(Math.floor(Math.random() * 1000000) + 100).fill(1); -// } -// -// global.tick = function tick() { -// fps++; -// const start = Date.now(); -// someWork(); -// delay = Math.floor((1000 / (fpsTarget + 5)) - (Date.now() - start)); -// -// if (delay < 1) { -// process.nextTick(() => tick()); -// } else { -// setTimeout(() => tick(), delay); -// } -// }; -// -// tick(); - -// // --------------------- -// // SAMPLE DATA -// // --------------------- -// -// const loadedData = [ -// { -// 0: { -// Ticket: '123', -// Mid: '987654321', -// }, -// }, -// { -// 0: { -// Ticket: '567', -// Mid: '12345678', -// }, -// }, -// ]; -// -// const newData = [ -// { -// Ticket: '123', -// Mid: '987654321', -// }, -// { -// Ticket: '345', -// Mid: '54568656', -// }, -// ]; -// -// -// // ----------- -// // UTILS -// // ------------ -// /** -// * -// * @param obj -// * @param keys -// * @return {string} -// */ -// function objectToMapKey(obj, keys) { -// let mapKey = ''; -// -// for (let j = 0, klen = keys.length; j < klen; j++) { -// const key = keys[j]; -// if (obj[key]) { -// if (j > 0) mapKey += '-' + obj[key]; -// else mapKey += obj[key]; -// } -// } -// -// return mapKey; -// } -// -// /** -// * -// * @param array -// * @param keys -// * @return {{}} -// */ -// function arrayToLookupMap(array, keys) { -// const map = {}; -// -// for (let i = 0, len = array.length; i < len; i++) { -// const item = array[i]; -// const mapKey = objectToMapKey(item, keys); -// map[mapKey] = Object.assign({}, item); -// } -// -// return map; -// } -// -// // ----------------- -// // actual code -// // ----------------- -// -// const lookupFields = ['Ticket', 'Mid']; -// const lookupMap = arrayToLookupMap(loadedData, lookupFields); -// -// for (let i = 0, len = newData.length; i < len; i++) { -// const newObj = newData[i]; -// const lookupKey = objectToMapKey(newObj, lookupFields); -// const exists = lookupMap[lookupKey]; -// -// if (exists) { -// // exists in loaded and loaded data is: console.log(exists); -// console.log('Item ' + lookupKey + ' exists!'); -// // TODO ignore as exists? -// } else { -// // doesn't exist in loaded data -// console.log('Item ' + lookupKey + ' does NOT exist!'); -// // TODO do something as doesn't exist? -// } -// } - -// // top of file - in your imports -// const vm = require('vm'); // part of node api - don't npm install it -// // also top of your file -// const start = "$(document).find('#flot-chart'),"; // where the js code to extract starts after -// const end = '$.plot('; // where the js code to extract end -// -// // your actual code: - -// const html = "
\n
\n
\n
\n