From 655000b85cb438da1be7edfe6da29cd8b9141940 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 19 Oct 2018 19:22:06 +0100 Subject: [PATCH 01/15] [android][database] database listeners now correctly tearing down between RN reloads, additionally the latest react context is now used rather than old references to form instances (handler on a dead thread issue) Fixes #1498 #1611 #1609 --- .../firebase/database/RNFirebaseDatabase.java | 39 +++++++++++--- .../database/RNFirebaseDatabaseReference.java | 53 +++++++++++++------ 2 files changed, 70 insertions(+), 22 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 8d84bfb4ec..727c906e88 100644 --- a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java +++ b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabase.java @@ -25,6 +25,7 @@ import com.google.firebase.database.Transaction; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -36,18 +37,18 @@ public class RNFirebaseDatabase extends ReactContextBaseJavaModule { private static final String TAG = "RNFirebaseDatabase"; private static boolean enableLogging = false; + private static ReactApplicationContext reactApplicationContext = null; private static HashMap loggingLevelSet = new HashMap<>(); - private HashMap references = new HashMap<>(); - private SparseArray transactionHandlers = new SparseArray<>(); + private static HashMap references = new HashMap<>(); + private static SparseArray transactionHandlers = new SparseArray<>(); RNFirebaseDatabase(ReactApplicationContext reactContext) { super(reactContext); } - - /* - * REACT NATIVE METHODS - */ + static ReactApplicationContext getReactApplicationContextInstance() { + return reactApplicationContext; + } /** * Resolve null or reject with a js like error if databaseError exists @@ -68,6 +69,11 @@ static void handlePromise(Promise promise, DatabaseError databaseError) { } } + + /* + * REACT NATIVE METHODS + */ + /** * Get a database instance for a specific firebase app instance * @@ -253,6 +259,26 @@ static WritableMap getJSError(DatabaseError nativeError) { return errorMap; } + @Override + public void initialize() { + super.initialize(); + Log.d(TAG, "RNFirebaseDatabase:initialized"); + reactApplicationContext = getReactApplicationContext(); + } + + @Override + public void onCatalystInstanceDestroy() { + super.onCatalystInstanceDestroy(); + + Iterator refIterator = references.entrySet().iterator(); + while (refIterator.hasNext()) { + Map.Entry pair = (Map.Entry) refIterator.next(); + RNFirebaseDatabaseReference nativeRef = (RNFirebaseDatabaseReference) pair.getValue(); + nativeRef.removeAllEventListeners(); + refIterator.remove(); // avoids a ConcurrentModificationException + } + } + /** * @param appName */ @@ -792,7 +818,6 @@ private RNFirebaseDatabaseReference getInternalReferenceForApp( ReadableArray modifiers ) { return new RNFirebaseDatabaseReference( - getReactApplicationContext(), appName, dbURL, key, diff --git a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabaseReference.java b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabaseReference.java index 68ff1759ce..f3e660e162 100644 --- a/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabaseReference.java +++ b/android/src/main/java/io/invertase/firebase/database/RNFirebaseDatabaseReference.java @@ -19,6 +19,7 @@ import java.lang.ref.WeakReference; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -33,7 +34,6 @@ class RNFirebaseDatabaseReference { private Query query; private String appName; private String dbURL; - private ReactContext reactContext; private HashMap childEventListeners = new HashMap<>(); private HashMap valueEventListeners = new HashMap<>(); @@ -41,14 +41,12 @@ class RNFirebaseDatabaseReference { * RNFirebase wrapper around FirebaseDatabaseReference, * handles Query generation and event listeners. * - * @param context * @param app * @param refKey * @param refPath * @param modifiersArray */ RNFirebaseDatabaseReference( - ReactContext context, String app, String url, String refKey, @@ -59,10 +57,32 @@ class RNFirebaseDatabaseReference { query = null; appName = app; dbURL = url; - reactContext = context; buildDatabaseQueryAtPathAndModifiers(refPath, modifiersArray); } + void removeAllEventListeners() { + if (hasListeners()) { + Iterator valueIterator = valueEventListeners.entrySet().iterator(); + + while (valueIterator.hasNext()) { + Map.Entry pair = (Map.Entry) valueIterator.next(); + ValueEventListener valueEventListener = (ValueEventListener) pair.getValue(); + query.removeEventListener(valueEventListener); + valueIterator.remove(); + } + + Iterator childIterator = childEventListeners.entrySet().iterator(); + + while (childIterator.hasNext()) { + Map.Entry pair = (Map.Entry) childIterator.next(); + ChildEventListener childEventListener = (ChildEventListener) pair.getValue(); + query.removeEventListener(childEventListener); + childIterator.remove(); + } + } + } + + /** * Used outside of class for keepSynced etc. * @@ -141,7 +161,6 @@ private void addEventListener(String eventRegistrationKey, ChildEventListener li */ private void addOnceValueEventListener(final Promise promise) { @SuppressLint("StaticFieldLeak") final DataSnapshotToMapAsyncTask asyncTask = new DataSnapshotToMapAsyncTask( - reactContext, this ) { @Override @@ -338,7 +357,7 @@ private void handleDatabaseEvent( @Nullable String previousChildName ) { @SuppressLint("StaticFieldLeak") - DataSnapshotToMapAsyncTask asyncTask = new DataSnapshotToMapAsyncTask(reactContext, this) { + DataSnapshotToMapAsyncTask asyncTask = new DataSnapshotToMapAsyncTask(this) { @Override protected void onPostExecute(WritableMap data) { if (this.isAvailable()) { @@ -347,7 +366,11 @@ protected void onPostExecute(WritableMap data) { event.putString("key", key); event.putString("eventType", eventType); event.putMap("registration", Utils.readableMapToWritableMap(registration)); - Utils.sendEvent(reactContext, "database_sync_event", event); + Utils.sendEvent( + RNFirebaseDatabase.getReactApplicationContextInstance(), + "database_sync_event", + event + ); } } }; @@ -367,7 +390,11 @@ private void handleDatabaseError(ReadableMap registration, DatabaseError error) event.putMap("error", RNFirebaseDatabase.getJSError(error)); event.putMap("registration", Utils.readableMapToWritableMap(registration)); - Utils.sendEvent(reactContext, "database_sync_event", event); + Utils.sendEvent( + RNFirebaseDatabase.getReactApplicationContextInstance(), + "database_sync_event", + event + ); } /** @@ -554,13 +581,10 @@ private void applyStartAtFilter(String key, String valueType, Map modifier) { * Introduced due to https://github.com/invertase/react-native-firebase/issues/1284 */ private static class DataSnapshotToMapAsyncTask extends AsyncTask { - - private WeakReference reactContextWeakReference; private WeakReference referenceWeakReference; - DataSnapshotToMapAsyncTask(ReactContext context, RNFirebaseDatabaseReference reference) { + DataSnapshotToMapAsyncTask(RNFirebaseDatabaseReference reference) { referenceWeakReference = new WeakReference<>(reference); - reactContextWeakReference = new WeakReference<>(context); } @Override @@ -572,8 +596,7 @@ protected final WritableMap doInBackground(Object... params) { return RNFirebaseDatabaseUtils.snapshotToMap(dataSnapshot, previousChildName); } catch (RuntimeException e) { if (isAvailable()) { - reactContextWeakReference - .get() + RNFirebaseDatabase.getReactApplicationContextInstance() .handleException(e); } throw e; @@ -586,7 +609,7 @@ protected void onPostExecute(WritableMap writableMap) { } Boolean isAvailable() { - return reactContextWeakReference.get() != null && referenceWeakReference.get() != null; + return RNFirebaseDatabase.getReactApplicationContextInstance() != null && referenceWeakReference.get() != null; } } } From 25bdf6e2ecd5a37a40e1d1c038ddf7fef6ca1679 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 20:33:50 +0100 Subject: [PATCH 02/15] [tests][database] start migration of tests from old tests project --- tests/e2e/database/ref/child.e2e.js | 39 +++++++++++++++++++ tests/e2e/database/ref/factory.e2e.js | 24 ++++++++++++ tests/e2e/database/ref/isEqual.e2e.js | 29 ++++++++++++++ .../database/{ => ref}/transactions.e2e.js | 0 4 files changed, 92 insertions(+) create mode 100644 tests/e2e/database/ref/child.e2e.js create mode 100644 tests/e2e/database/ref/factory.e2e.js create mode 100644 tests/e2e/database/ref/isEqual.e2e.js rename tests/e2e/database/{ => ref}/transactions.e2e.js (100%) diff --git a/tests/e2e/database/ref/child.e2e.js b/tests/e2e/database/ref/child.e2e.js new file mode 100644 index 0000000000..4f180778f1 --- /dev/null +++ b/tests/e2e/database/ref/child.e2e.js @@ -0,0 +1,39 @@ +const { setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref().child', () => { + describe('when passed a shallow path', () => { + it('returns correct child ref', () => { + const ref = firebase.database().ref('tests'); + const childRef = ref.child('tests'); + childRef.key.should.eql('tests'); + }); + }); + + describe('when passed a nested path', () => { + it('returns correct child ref', () => { + const ref = firebase.database().ref('tests'); + const grandChildRef = ref.child('tests/number'); + grandChildRef.key.should.eql('number'); + }); + }); + + describe("when passed a path that doesn't exist", () => { + it('creates a reference, anyway', () => { + const ref = firebase.database().ref('tests'); + const grandChildRef = ref.child('doesnt/exist'); + grandChildRef.key.should.eql('exist'); + }); + }); + + describe('when passed an invalid path', () => { + it('creates a reference, anyway', () => { + const ref = firebase.database().ref('tests'); + const grandChildRef = ref.child('does$&nt/exist'); + grandChildRef.key.should.eql('exist'); + }); + }); + }); +}); diff --git a/tests/e2e/database/ref/factory.e2e.js b/tests/e2e/database/ref/factory.e2e.js new file mode 100644 index 0000000000..aa95e525ca --- /dev/null +++ b/tests/e2e/database/ref/factory.e2e.js @@ -0,0 +1,24 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref()', () => { + it('returns root reference when provided no path', () => { + const ref = firebase.database().ref(); + (ref.key === null).should.be.true(); + (ref.parent === null).should.be.true(); + }); + + it('returns reference to data at path', async () => { + const ref = firebase.database().ref('tests/types/number'); + + let valueAtRef; + await ref.once('value', snapshot => { + valueAtRef = snapshot.val(); + }); + + valueAtRef.should.eql(CONTENTS.DEFAULT.number); + }); + }); +}); diff --git a/tests/e2e/database/ref/isEqual.e2e.js b/tests/e2e/database/ref/isEqual.e2e.js new file mode 100644 index 0000000000..d1a6b0d938 --- /dev/null +++ b/tests/e2e/database/ref/isEqual.e2e.js @@ -0,0 +1,29 @@ +const { setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref().isEqual()', () => { + before(() => { + this.ref = firebase.database().ref('tests/types'); + }); + + it('returns true when the reference is for the same location', () => { + const ref2 = firebase.database().ref('tests/types'); + this.ref.isEqual(ref2).should.eql(true); + }); + + it('returns false when the reference is for a different location', () => { + const ref2 = firebase.database().ref('tests/types/number'); + this.ref.isEqual(ref2).should.eql(false); + }); + + it('returns false when the reference is null', () => { + this.ref.isEqual(null).should.eql(false); + }); + + it('returns false when the reference is not a Reference', () => { + this.ref.isEqual(1).should.eql(false); + }); + }); +}); diff --git a/tests/e2e/database/transactions.e2e.js b/tests/e2e/database/ref/transactions.e2e.js similarity index 100% rename from tests/e2e/database/transactions.e2e.js rename to tests/e2e/database/ref/transactions.e2e.js From b7e651ba022f9ea99fd810ed6b529f401c7f86c5 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 22:27:30 +0100 Subject: [PATCH 03/15] [js][database] fix issue where toString incorrectly contains `//` in the path --- src/modules/database/Reference.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/database/Reference.js b/src/modules/database/Reference.js index 3510d303ee..dfc4f73f25 100644 --- a/src/modules/database/Reference.js +++ b/src/modules/database/Reference.js @@ -500,7 +500,7 @@ export default class Reference extends ReferenceBase { * @returns {string} */ toString(): string { - return `${this._database.databaseUrl}/${this.path}`; + return `${this._database.databaseUrl}${this.path}`; } /** From eebb4d296c1614cd9f7e9bdafad3fa3e3ddc453e Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 22:28:08 +0100 Subject: [PATCH 04/15] [tests][database] add issue specific tests --- tests/e2e/database/issueSpecific.e2e.js | 279 ++++++++++++++++++++++++ tests/e2e/mocha.opts | 1 + tests/helpers/database/index.js | 1 + 3 files changed, 281 insertions(+) create mode 100644 tests/e2e/database/issueSpecific.e2e.js diff --git a/tests/e2e/database/issueSpecific.e2e.js b/tests/e2e/database/issueSpecific.e2e.js new file mode 100644 index 0000000000..58976de0e8 --- /dev/null +++ b/tests/e2e/database/issueSpecific.e2e.js @@ -0,0 +1,279 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + beforeEach(() => setDatabaseContents()); + + describe('issue_100', () => { + describe('array-like values should', () => { + it('return null in returned array at positions where a key is missing', async () => { + const ref = firebase.database().ref('tests/issues/100'); + + const snapshot = await ref.once('value'); + + snapshot + .val() + .should.eql( + jet.contextify([ + null, + jet.contextify(CONTENTS.ISSUES[100][1]), + jet.contextify(CONTENTS.ISSUES[100][2]), + jet.contextify(CONTENTS.ISSUES[100][3]), + ]) + ); + }); + }); + }); + + describe('issue_108', () => { + describe('filters using floats', () => { + it('return correct results', async () => { + const ref = firebase.database().ref('tests/issues/108'); + + const snapshot = await ref + .orderByChild('latitude') + .startAt(34.00867000999119) + .endAt(34.17462960866099) + .once('value'); + + const val = snapshot.val(); + + val.foobar.should.eql(jet.contextify(CONTENTS.ISSUES[108].foobar)); + should.equal(Object.keys(val).length, 1); + }); + + it('return correct results when not using float values', async () => { + const ref = firebase.database().ref('tests/issues/108'); + + const snapshot = await ref + .orderByChild('latitude') + .equalTo(37) + .once('value'); + + const val = snapshot.val(); + + val.notAFloat.should.eql( + jet.contextify(CONTENTS.ISSUES[108].notAFloat) + ); + + should.equal(Object.keys(val).length, 1); + }); + }); + }); + + xdescribe('issue_171', () => { + describe('non array-like values should', () => { + it('return as objects', async () => { + const ref = firebase.database().ref('tests/issues/171'); + const snapshot = await ref.once('value'); + + snapshot.val().should.eql(jet.contextify(CONTENTS.ISSUES[171])); + }); + }); + }); + + describe('issue_489', () => { + describe('long numbers should', () => { + it('return as longs', async () => { + const long1Ref = firebase.database().ref('tests/issues/489/long1'); + const long2Ref = firebase.database().ref('tests/issues/489/long2'); + const long2 = 1234567890123456; + + let snapshot = await long1Ref.once('value'); + snapshot.val().should.eql(CONTENTS.ISSUES[489].long1); + + await long2Ref.set(long2); + snapshot = await long2Ref.once('value'); + snapshot.val().should.eql(long2); + }); + }); + }); + + describe('issue_521', () => { + describe('orderByChild (numerical field) and limitToLast', () => { + it('once() returns correct results', async () => { + const ref = firebase.database().ref('tests/issues/521'); + + const snapshot = await ref + .orderByChild('number') + .limitToLast(1) + .once('value'); + + const val = snapshot.val(); + + val.key3.should.eql(jet.contextify(CONTENTS.ISSUES[521].key3)); + should.equal(Object.keys(val).length, 1); + }); + + it('on() returns correct initial results', async () => { + const ref = firebase + .database() + .ref('tests/issues/521') + .orderByChild('number') + .limitToLast(2); + + const callback = sinon.spy(); + + await new Promise(resolve => { + ref.on('value', snapshot => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: CONTENTS.ISSUES[521].key2, + key3: CONTENTS.ISSUES[521].key3, + }); + + callback.should.be.calledOnce(); + }); + + it('on() returns correct subsequent results', async () => { + const ref = firebase + .database() + .ref('tests/issues/521') + .orderByChild('number') + .limitToLast(2); + + const callback = sinon.spy(); + + await new Promise(resolve => { + ref.on('value', snapshot => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: CONTENTS.ISSUES[521].key2, + key3: CONTENTS.ISSUES[521].key3, + }); + + callback.should.be.calledOnce(); + + const newDataValue = { + name: 'Item 4', + number: 4, + string: 'item4', + }; + + const newRef = firebase.database().ref('tests/issues/521/key4'); + + await newRef.set(newDataValue); + await sleep(5); + + callback.should.be.calledWith({ + key3: CONTENTS.ISSUES[521].key3, + key4: newDataValue, + }); + + callback.should.be.calledTwice(); + }); + }); + + describe('orderByChild (string field) and limitToLast', () => { + it('once() returns correct results', async () => { + const ref = firebase.database().ref('tests/issues/521'); + + const snapshot = await ref + .orderByChild('string') + .limitToLast(1) + .once('value'); + + const val = snapshot.val(); + + val.key3.should.eql(jet.contextify(CONTENTS.ISSUES[521].key3)); + should.equal(Object.keys(val).length, 1); + }); + + it('on() returns correct initial results', async () => { + const ref = firebase + .database() + .ref('tests/issues/521') + .orderByChild('string') + .limitToLast(2); + + const callback = sinon.spy(); + + await new Promise(resolve => { + ref.on('value', snapshot => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: CONTENTS.ISSUES[521].key2, + key3: CONTENTS.ISSUES[521].key3, + }); + + callback.should.be.calledOnce(); + }); + + it('on() returns correct subsequent results', async () => { + const ref = firebase + .database() + .ref('tests/issues/521') + .orderByChild('string') + .limitToLast(2); + + const callback = sinon.spy(); + + await new Promise(resolve => { + ref.on('value', snapshot => { + callback(snapshot.val()); + resolve(); + }); + }); + + callback.should.be.calledWith({ + key2: CONTENTS.ISSUES[521].key2, + key3: CONTENTS.ISSUES[521].key3, + }); + + callback.should.be.calledOnce(); + + const newDataValue = { + name: 'Item 4', + number: 4, + string: 'item4', + }; + + const newRef = firebase.database().ref('tests/issues/521/key4'); + await newRef.set(newDataValue); + await sleep(5); + + callback.should.be.calledWith({ + key3: CONTENTS.ISSUES[521].key3, + key4: newDataValue, + }); + + callback.should.be.calledTwice(); + }); + }); + }); + + describe('issue_679', () => { + describe('path from snapshot reference', () => { + it('should match web SDK', async () => { + const nativeRef = firebase.database().ref('tests/issues/679'); + const webRef = firebaseAdmin.database().ref('tests/issues/679'); + const nativeRef2 = firebase.database().ref('tests/issues/679/'); + const webRef2 = firebaseAdmin.database().ref('tests/issues/679/'); + + webRef.toString().should.equal(nativeRef.toString()); + webRef2.toString().should.equal(nativeRef2.toString()); + }); + + it('should be correct when returned from native', async () => { + const nativeRef = firebase.database().ref('tests/issues/679/'); + const webRef = firebaseAdmin.database().ref('tests/issues/679/'); + + const nativeSnapshot = await nativeRef.once('value'); + const webSnapshot = await webRef.once('value'); + + webSnapshot.ref.toString().should.equal(nativeSnapshot.ref.toString()); + }); + }); + }); +}); diff --git a/tests/e2e/mocha.opts b/tests/e2e/mocha.opts index 0e9980c95a..bd89b31cc8 100755 --- a/tests/e2e/mocha.opts +++ b/tests/e2e/mocha.opts @@ -7,3 +7,4 @@ --exit --require jet/platform/node --require ./helpers +--inspect diff --git a/tests/helpers/database/index.js b/tests/helpers/database/index.js index f83fd9da91..13d37e05af 100644 --- a/tests/helpers/database/index.js +++ b/tests/helpers/database/index.js @@ -13,6 +13,7 @@ module.exports = { 666 ), database.ref('tests/query').set(CONTENTS.QUERY), + database.ref('tests/issues').set(CONTENTS.ISSUES), ]); }, }; From 9cebeea1a88ac35cfb222ccb026f35c22e5eaeda Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 22:37:52 +0100 Subject: [PATCH 05/15] [tests][database] add key tests --- tests/e2e/database/ref/key.e2e.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 tests/e2e/database/ref/key.e2e.js diff --git a/tests/e2e/database/ref/key.e2e.js b/tests/e2e/database/ref/key.e2e.js new file mode 100644 index 0000000000..99c8516b5b --- /dev/null +++ b/tests/e2e/database/ref/key.e2e.js @@ -0,0 +1,16 @@ +describe('database()', () => { + describe('ref().key', () => { + it('returns null for root ref', () => { + const ref = firebase.database().ref(); + (ref.key === null).should.be.true(); + }); + + it('returns correct key for path', () => { + const ref = firebase.database().ref('tests/types/number'); + const arrayItemRef = firebase.database().ref('tests/types/array/1'); + + ref.key.should.eql('number'); + arrayItemRef.key.should.eql('1'); + }); + }); +}); From 4926724f669a12931341bb558a2868466bfe7da4 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 22:50:08 +0100 Subject: [PATCH 06/15] [tests][database] add parent tests --- tests/e2e/database/ref/parent.e2e.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/e2e/database/ref/parent.e2e.js diff --git a/tests/e2e/database/ref/parent.e2e.js b/tests/e2e/database/ref/parent.e2e.js new file mode 100644 index 0000000000..1b567a0eb9 --- /dev/null +++ b/tests/e2e/database/ref/parent.e2e.js @@ -0,0 +1,19 @@ +describe('database()', () => { + describe('ref().parent', () => { + describe('on the root ref', () => { + it('returns null', () => { + const ref = firebase.database().ref(); + (ref.parent === null).should.be.true(); + }); + }); + + describe('on a non-root ref', () => { + it('returns correct parent', () => { + const ref = firebase.database().ref('tests/types/number'); + const parentRef = firebase.database().ref('tests/types'); + + ref.parent.key.should.eql(parentRef.key); + }); + }); + }); +}); From ed140d1060b7ac003818bd660624c1c058f59ace Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 26 Oct 2018 22:50:17 +0100 Subject: [PATCH 07/15] [tests][database] add priority tests --- tests/e2e/database/ref/priority.js | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/e2e/database/ref/priority.js diff --git a/tests/e2e/database/ref/priority.js b/tests/e2e/database/ref/priority.js new file mode 100644 index 0000000000..e84244474d --- /dev/null +++ b/tests/e2e/database/ref/priority.js @@ -0,0 +1,33 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref().priority', () => { + it('setPriority() should correctly set a priority for all non-null values', async () => { + await Promise.all( + Object.keys(CONTENTS.DEFAULT).map(async dataRef => { + const ref = firebase.database().ref(`tests/types/${dataRef}`); + + await ref.setPriority(1); + + await ref.once('value').then(snapshot => { + if (snapshot.val() !== null) { + snapshot.getPriority().should.eql(1); + } + }); + }) + ); + }); + + it('setWithPriority() should correctly set the priority', async () => { + const ref = firebase.database().ref('tests/types/number'); + + await ref.setWithPriority(CONTENTS.DEFAULT.number, '2'); + + await ref.once('value').then(snapshot => { + snapshot.getPriority().should.eql('2'); + }); + }); + }); +}); From 114314187737a8bb0dce625c66819511cbbd1aea Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 01:45:33 +0100 Subject: [PATCH 08/15] [database][js] Rework .push() behaviour to match websdk. Fixes #893 #1464 #1572 --- src/modules/database/Reference.js | 104 ++++++------------------------ 1 file changed, 19 insertions(+), 85 deletions(-) diff --git a/src/modules/database/Reference.js b/src/modules/database/Reference.js index dfc4f73f25..e0142686bf 100644 --- a/src/modules/database/Reference.js +++ b/src/modules/database/Reference.js @@ -77,19 +77,20 @@ type DatabaseListener = { export default class Reference extends ReferenceBase { _database: Database; - _promise: ?Promise<*>; - _query: Query; _refListeners: { [listenerId: number]: DatabaseListener }; + then: (a?: any) => Promise; + + catch: (a?: any) => Promise; + constructor( database: Database, path: string, existingModifiers?: Array ) { super(path); - this._promise = null; this._refListeners = {}; this._database = database; this._query = new Query(this, existingModifiers); @@ -303,34 +304,26 @@ export default class Reference extends ReferenceBase { * @returns {*} */ push(value: any, onComplete?: Function): Reference | Promise { - if (value === null || value === undefined) { - return new Reference( - this._database, - `${this.path}/${generatePushID(this._database._serverTimeOffset)}` - ); + const name = generatePushID(this._database._serverTimeOffset); + + const pushRef = this.child(name); + const thennablePushRef = this.child(name); + + let promise; + if (value != null) { + promise = thennablePushRef.set(value, onComplete).then(() => pushRef); + } else { + promise = Promise.resolve(pushRef); } - const newRef = new Reference( - this._database, - `${this.path}/${generatePushID(this._database._serverTimeOffset)}` - ); - const promise = newRef.set(value); + thennablePushRef.then = promise.then.bind(promise); + thennablePushRef.catch = promise.catch.bind(promise); - // if callback provided then internally call the set promise with value if (isFunction(onComplete)) { - return ( - promise - // $FlowExpectedError: Reports that onComplete can change to null despite the null check: https://github.com/facebook/flow/issues/1655 - .then(() => onComplete(null, newRef)) - // $FlowExpectedError: Reports that onComplete can change to null despite the null check: https://github.com/facebook/flow/issues/1655 - .catch(error => onComplete(error, null)) - ); + promise.catch(() => {}); } - // otherwise attach promise to 'thenable' reference and return the - // new reference - newRef._setThenable(promise); - return newRef; + return thennablePushRef; } /** @@ -566,47 +559,6 @@ export default class Reference extends ReferenceBase { return new Reference(this._database, '/'); } - /** - * Access then method of promise if set - * @return {*} - */ - then(fnResolve: any => any, fnReject: any => any) { - if (isFunction(fnResolve) && this._promise && this._promise.then) { - return this._promise.then.bind(this._promise)( - result => { - this._promise = null; - return fnResolve(result); - }, - possibleErr => { - this._promise = null; - - if (isFunction(fnReject)) { - return fnReject(possibleErr); - } - - throw possibleErr; - } - ); - } - - throw new Error("Cannot read property 'then' of undefined."); - } - - /** - * Access catch method of promise if set - * @return {*} - */ - catch(fnReject: any => any) { - if (isFunction(fnReject) && this._promise && this._promise.catch) { - return this._promise.catch.bind(this._promise)(possibleErr => { - this._promise = null; - return fnReject(possibleErr); - }); - } - - throw new Error("Cannot read property 'catch' of undefined."); - } - /** * INTERNALS */ @@ -635,15 +587,6 @@ export default class Reference extends ReferenceBase { }$${this._query.queryIdentifier()}`; } - /** - * Set the promise this 'thenable' reference relates to - * @param promise - * @private - */ - _setThenable(promise: Promise<*>) { - this._promise = promise; - } - /** * * @param obj @@ -812,7 +755,7 @@ export default class Reference extends ReferenceBase { }, }); - // increment number of listeners - just s short way of making + // increment number of listeners - just a short way of making // every registration unique per .on() call listeners += 1; @@ -903,12 +846,3 @@ export default class Reference extends ReferenceBase { return SyncTree.removeListenersForRegistrations(registrations); } } - -// eslint-disable-next-line no-unused-vars -// class ThenableReference<+R> extends Reference { -// then( -// onFulfill?: (value: R) => Promise | U, -// onReject?: (error: any) => Promise | U -// ): Promise; -// catch(onReject?: (error: any) => Promise | U): Promise; -// } From f707f8dcbb969a1af8a7a9bf1c71195e3e61b5f6 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 01:47:45 +0100 Subject: [PATCH 09/15] [tests][database] .push() tests --- tests/e2e/database/ref/push.js | 126 +++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 tests/e2e/database/ref/push.js diff --git a/tests/e2e/database/ref/push.js b/tests/e2e/database/ref/push.js new file mode 100644 index 0000000000..dae1599ce0 --- /dev/null +++ b/tests/e2e/database/ref/push.js @@ -0,0 +1,126 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref().push()', () => { + it('returns a ref that can be used to set value later', async () => { + const ref = firebase.database().ref('tests/types/array'); + + let originalListValue; + await ref.once('value', snapshot => { + originalListValue = snapshot.val(); + }); + await sleep(5); + + originalListValue.should.eql(jet.contextify(CONTENTS.DEFAULT.array)); + + const newItemRef = ref.push(); + const valueToAddToList = CONTENTS.NEW.number; + await newItemRef.set(valueToAddToList); + + let newItemValue; + await newItemRef.once('value', snapshot => { + newItemValue = snapshot.val(); + }); + await sleep(5); + + newItemValue.should.eql(valueToAddToList); + + let newListValue; + await ref.once('value', snapshot => { + newListValue = snapshot.val(); + }); + await sleep(5); + + const originalListAsObject = { + ...originalListValue, + [newItemRef.key]: valueToAddToList, + }; + + newListValue.should.eql(jet.contextify(originalListAsObject)); + }); + + it('allows setting value immediately', async () => { + let snapshot; + + const ref = firebase.database().ref('tests/types/array'); + const valueToAddToList = CONTENTS.NEW.number; + + snapshot = await ref.once('value'); + const originalListValue = snapshot.val(); + const newItemRef = ref.push(valueToAddToList); + + snapshot = await newItemRef.once('value'); + const newItemValue = snapshot.val(); + newItemValue.should.eql(valueToAddToList); + + snapshot = await firebase + .database() + .ref('tests/types/array') + .once('value'); + const newListValue = snapshot.val(); + + const originalListAsObject = { + ...originalListValue, + [newItemRef.key]: valueToAddToList, + }; + + newListValue.should.eql(jet.contextify(originalListAsObject)); + }); + + // https://github.com/invertase/react-native-firebase/issues/893 + it('correctly returns the reference', async () => { + let result; + const path = 'tests/types/array'; + const valueToAddToList = CONTENTS.NEW.number; + const Reference = jet.require('src/modules/database/Reference'); + + // 1 + const ref1 = firebase + .database() + .ref(path) + .push(); + + should.exist(ref1, 'ref1 did not return a Reference instance'); + ref1.key.should.be.a.String(); + ref1.should.be.instanceOf(Reference); + result = await ref1.set(valueToAddToList); + should.not.exist(result); + + // 2 + const ref2 = await firebase + .database() + .ref(path) + .push(valueToAddToList); + + should.exist(ref2, 'ref2 did not return a Reference instance'); + ref2.key.should.be.a.String(); + ref2.should.be.instanceOf(Reference); + + // 3 + const ref3 = await firebase + .database() + .ref(path) + .push(); + + should.exist(ref3, 'ref3 did not return a Reference instance'); + ref3.key.should.be.a.String(); + ref3.should.be.instanceOf(Reference); + + result = await ref3.set(valueToAddToList); + should.not.exist(result); + }); + + it('calls an onComplete callback', async () => { + const callback = sinon.spy(); + const ref = firebase.database().ref('tests/types/array'); + + const valueToAddToList = CONTENTS.NEW.number; + const newItemRef = await ref.push(valueToAddToList, callback); + + callback.should.be.calledWith(null); + newItemRef.parent.path.should.equal('tests/types/array'); + }); + }); +}); From b97ce444f3e16d89562fe9a28206d43205315e8e Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 02:16:55 +0100 Subject: [PATCH 10/15] [tests][database] query tests --- tests/e2e/database/ref/isEqual.e2e.js | 3 +++ .../ref/{priority.js => priority.e2e.js} | 0 .../e2e/database/ref/{push.js => push.e2e.js} | 0 tests/e2e/database/ref/query.e2e.js | 21 +++++++++++++++++++ 4 files changed, 24 insertions(+) rename tests/e2e/database/ref/{priority.js => priority.e2e.js} (100%) rename tests/e2e/database/ref/{push.js => push.e2e.js} (100%) create mode 100644 tests/e2e/database/ref/query.e2e.js diff --git a/tests/e2e/database/ref/isEqual.e2e.js b/tests/e2e/database/ref/isEqual.e2e.js index d1a6b0d938..1f1d975c3c 100644 --- a/tests/e2e/database/ref/isEqual.e2e.js +++ b/tests/e2e/database/ref/isEqual.e2e.js @@ -9,6 +9,9 @@ describe('database()', () => { }); it('returns true when the reference is for the same location', () => { + const ref = firebase.database().ref(); + ref.ref.should.eql(ref); + const ref2 = firebase.database().ref('tests/types'); this.ref.isEqual(ref2).should.eql(true); }); diff --git a/tests/e2e/database/ref/priority.js b/tests/e2e/database/ref/priority.e2e.js similarity index 100% rename from tests/e2e/database/ref/priority.js rename to tests/e2e/database/ref/priority.e2e.js diff --git a/tests/e2e/database/ref/push.js b/tests/e2e/database/ref/push.e2e.js similarity index 100% rename from tests/e2e/database/ref/push.js rename to tests/e2e/database/ref/push.e2e.js diff --git a/tests/e2e/database/ref/query.e2e.js b/tests/e2e/database/ref/query.e2e.js new file mode 100644 index 0000000000..d527926d9d --- /dev/null +++ b/tests/e2e/database/ref/query.e2e.js @@ -0,0 +1,21 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref() query', () => { + it('orderByChild().equalTo()', async () => { + const snapshot = await firebase + .database() + .ref('tests/query') + .orderByChild('search') + .equalTo('foo') + .once('value'); + + const val = snapshot.val(); + CONTENTS.QUERY[0].should.eql({ ...val[0] }); + }); + + // TODO more query tests + }); +}); From 8a7605c6559557b231534cff3611c2ad8d2fba6b Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 04:46:43 +0100 Subject: [PATCH 11/15] [tests] add require cycle to ignoreWarnings --- tests/app.js | 13 +++++++++++-- tests/helpers/index.js | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/app.js b/tests/app.js index def93ad681..c3c62b5f32 100755 --- a/tests/app.js +++ b/tests/app.js @@ -1,6 +1,15 @@ -/* eslint-disable import/extensions,import/no-unresolved */ +/* eslint-disable import/extensions,import/no-unresolved,import/first */ import React, { Component } from 'react'; -import { AppRegistry, Text, View, Image, StyleSheet } from 'react-native'; +import { + AppRegistry, + Text, + View, + Image, + StyleSheet, + YellowBox, +} from 'react-native'; + +YellowBox.ignoreWarnings(['Require cycle:']); import firebase from 'react-native-firebase'; import jet from 'jet/platform/react-native'; diff --git a/tests/helpers/index.js b/tests/helpers/index.js index 7b9629fcac..92bf8b284e 100644 --- a/tests/helpers/index.js +++ b/tests/helpers/index.js @@ -62,6 +62,7 @@ console.log = (...args) => { args[0] && typeof args[0] === 'string' && (args[0].toLowerCase().includes('deprecated') || + args[0].toLowerCase().includes('require cycle') || args[0].toLowerCase().includes('restrictions in the native sdk')) ) { return undefined; From fccb725808423a857f75e49deea1711c5978b983 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 04:47:50 +0100 Subject: [PATCH 12/15] [tests] update to jet 0.2.0 - required bug fixes --- src/modules/utils/database.js | 12 ++++++++++++ src/modules/utils/index.js | 5 +++++ tests/package.json | 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 src/modules/utils/database.js diff --git a/src/modules/utils/database.js b/src/modules/utils/database.js new file mode 100644 index 0000000000..12fda5432d --- /dev/null +++ b/src/modules/utils/database.js @@ -0,0 +1,12 @@ +import SyncTree from '../../utils/SyncTree'; + +export default { + /** + * Removes all database listeners (JS & Native) + */ + cleanup(): void { + SyncTree.removeListenersForRegistrations( + Object.keys(SyncTree._reverseLookup) + ); + }, +}; diff --git a/src/modules/utils/index.js b/src/modules/utils/index.js index b680075de9..463de928d4 100644 --- a/src/modules/utils/index.js +++ b/src/modules/utils/index.js @@ -4,6 +4,7 @@ import INTERNALS from '../../utils/internals'; import { isIOS } from '../../utils'; import ModuleBase from '../../utils/ModuleBase'; import type App from '../core/app'; +import DatabaseUtils from './database'; const FirebaseCoreModule = NativeModules.RNFirebase; @@ -28,6 +29,10 @@ export default class RNFirebaseUtils extends ModuleBase { }); } + get database(): DatabaseUtils { + return DatabaseUtils; + } + /** * */ diff --git a/tests/package.json b/tests/package.json index 9155230718..d2ead1af5b 100755 --- a/tests/package.json +++ b/tests/package.json @@ -23,7 +23,7 @@ "detox": "^9.0.4", "fbjs": "^0.8.16", "firebase-admin": "^5.12.0", - "jet": "^0.1.0", + "jet": "^0.2.0", "jsonwebtoken": "^8.2.1", "mocha": "^5.2.0", "prop-types": "^15.6.1", From 2cf778b76aad0080d65b1cc4247d53e8cb1c9001 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 04:49:18 +0100 Subject: [PATCH 13/15] [tests][database] add RN reload tests / app backgrounded tests for Reference on/once listeners --- tests/e2e/database/rnReload.e2e.js | 97 ++++++++++++++++++++++++++++++ tests/e2e/mocha.opts | 1 - 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/database/rnReload.e2e.js diff --git a/tests/e2e/database/rnReload.e2e.js b/tests/e2e/database/rnReload.e2e.js new file mode 100644 index 0000000000..1b9045ccae --- /dev/null +++ b/tests/e2e/database/rnReload.e2e.js @@ -0,0 +1,97 @@ +const { CONTENTS, setDatabaseContents } = TestHelpers.database; + +describe('database()', () => { + before(() => setDatabaseContents()); + + describe('ref().once()', () => { + it('same reference path works after React Native reload', async () => { + let ref; + let snapshot; + const path = 'tests/types/number'; + const dataTypeValue = CONTENTS.DEFAULT.number; + + // before reload + ref = firebase.database().ref(path); + snapshot = await ref.once('value'); + snapshot.val().should.eql(dataTypeValue); + + // RELOAD + await device.reloadReactNative(); + + // after reload + ref = firebase.database().ref(path); + snapshot = await ref.once('value'); + snapshot.val().should.eql(dataTypeValue); + }); + + it('same reference path works after app backgrounded', async () => { + let ref; + let snapshot; + const path = 'tests/types/number'; + const dataTypeValue = CONTENTS.DEFAULT.number; + + // before + ref = firebase.database().ref(path); + snapshot = await ref.once('value'); + snapshot.val().should.eql(dataTypeValue); + + await device.sendToHome(); + await sleep(250); + await device.launchApp({ newInstance: false }); + await sleep(250); + + // after + ref = firebase.database().ref(path); + snapshot = await ref.once('value'); + snapshot.val().should.eql(dataTypeValue); + }); + }); + + describe('ref().on()', () => { + it('same reference path works after React Native reload', async () => { + let ref; + let snapshot; + const path = 'tests/types/number'; + const dataTypeValue = CONTENTS.DEFAULT.number; + + // before reload + ref = firebase.database().ref(path); + snapshot = await new Promise(resolve => ref.on('value', resolve)); + snapshot.val().should.eql(dataTypeValue); + + // RELOAD + await device.reloadReactNative(); + + // after reload + ref = firebase.database().ref(path); + snapshot = await new Promise(resolve => ref.on('value', resolve)); + snapshot.val().should.eql(dataTypeValue); + + firebase.utils().database.cleanup(); + }); + + it('same reference path works after app backgrounded', async () => { + let ref; + let snapshot; + const path = 'tests/types/number'; + const dataTypeValue = CONTENTS.DEFAULT.number; + + // before background + ref = firebase.database().ref(path); + snapshot = await new Promise(resolve => ref.on('value', resolve)); + snapshot.val().should.eql(dataTypeValue); + + await device.sendToHome(); + await sleep(250); + await device.launchApp({ newInstance: false }); + await sleep(250); + + // after background + ref = firebase.database().ref(path); + snapshot = await new Promise(resolve => ref.on('value', resolve)); + snapshot.val().should.eql(dataTypeValue); + + firebase.utils().database.cleanup(); + }); + }); +}); diff --git a/tests/e2e/mocha.opts b/tests/e2e/mocha.opts index bd89b31cc8..0e9980c95a 100755 --- a/tests/e2e/mocha.opts +++ b/tests/e2e/mocha.opts @@ -7,4 +7,3 @@ --exit --require jet/platform/node --require ./helpers ---inspect From 4aac3aaf5e6eabcae214e1efd849a506368b2e96 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 04:50:20 +0100 Subject: [PATCH 14/15] [tests] update Podfile.lock --- tests/ios/Podfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ios/Podfile.lock b/tests/ios/Podfile.lock index 3c3c274ef4..e0f84317a3 100644 --- a/tests/ios/Podfile.lock +++ b/tests/ios/Podfile.lock @@ -254,7 +254,7 @@ PODS: - React/Core - React/fishhook - React/RCTBlob - - RNFirebase (5.0.0): + - RNFirebase (5.1.0-rc1): - Firebase/Core - React - yoga (0.57.1.React) @@ -325,7 +325,7 @@ EXTERNAL SOURCES: :path: "../node_modules/react-native" RNFirebase: :path: "../../ios/RNFirebase.podspec" - :version: "~> 5.0.0" + :version: "~> 5.1.0-rc1" yoga: :path: "../node_modules/react-native/ReactCommon/yoga" From 3d54d8e147da75c7c32c532ac2a683174cbde5d3 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 27 Oct 2018 05:19:33 +0100 Subject: [PATCH 15/15] [tests][database] add test timeouts and make background tests android specific --- tests/e2e/database/rnReload.e2e.js | 12 ++++++------ tests/e2e/database/snapshot.e2e.js | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/e2e/database/rnReload.e2e.js b/tests/e2e/database/rnReload.e2e.js index 1b9045ccae..a7aeb6508a 100644 --- a/tests/e2e/database/rnReload.e2e.js +++ b/tests/e2e/database/rnReload.e2e.js @@ -22,9 +22,9 @@ describe('database()', () => { ref = firebase.database().ref(path); snapshot = await ref.once('value'); snapshot.val().should.eql(dataTypeValue); - }); + }).timeout(15000); - it('same reference path works after app backgrounded', async () => { + it(':android: same reference path works after app backgrounded', async () => { let ref; let snapshot; const path = 'tests/types/number'; @@ -44,7 +44,7 @@ describe('database()', () => { ref = firebase.database().ref(path); snapshot = await ref.once('value'); snapshot.val().should.eql(dataTypeValue); - }); + }).timeout(15000); }); describe('ref().on()', () => { @@ -68,9 +68,9 @@ describe('database()', () => { snapshot.val().should.eql(dataTypeValue); firebase.utils().database.cleanup(); - }); + }).timeout(15000); - it('same reference path works after app backgrounded', async () => { + it(':android: same reference path works after app backgrounded', async () => { let ref; let snapshot; const path = 'tests/types/number'; @@ -92,6 +92,6 @@ describe('database()', () => { snapshot.val().should.eql(dataTypeValue); firebase.utils().database.cleanup(); - }); + }).timeout(15000); }); }); diff --git a/tests/e2e/database/snapshot.e2e.js b/tests/e2e/database/snapshot.e2e.js index d91187caf8..cd4d087f71 100644 --- a/tests/e2e/database/snapshot.e2e.js +++ b/tests/e2e/database/snapshot.e2e.js @@ -1,6 +1,5 @@ const { setDatabaseContents } = TestHelpers.database; -// TODO use testRunId in refs to prevent multiple test instances interfering with each other describe('database()', () => { describe('Snapshot', () => { before(() => setDatabaseContents());