[android][database] database improvements

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<String, Boolean> loggingLevelSet = new HashMap<>();
-  private HashMap<String, RNFirebaseDatabaseReference> references = new HashMap<>();
-  private SparseArray<RNFirebaseTransactionHandler> transactionHandlers = new SparseArray<>();
+  private static HashMap<String, RNFirebaseDatabaseReference> references = new HashMap<>();
+  private static SparseArray<RNFirebaseTransactionHandler> 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,

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,22 +34,19 @@ class RNFirebaseDatabaseReference {
   private Query query;
   private String appName;
   private String dbURL;
-  private ReactContext reactContext;
   private HashMap<String, ChildEventListener> childEventListeners = new HashMap<>();
   private HashMap<String, ValueEventListener> valueEventListeners = new HashMap<>();
 
   /**
    * 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<Object, Void, WritableMap> {
-
-    private WeakReference<ReactContext> reactContextWeakReference;
     private WeakReference<RNFirebaseDatabaseReference> 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;
     }
   }
 }

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<any>;
+
+  catch: (a?: any) => Promise<any>;
+
   constructor(
     database: Database,
     path: string,
     existingModifiers?: Array<DatabaseModifier>
   ) {
     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<void> {
-    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;
   }
 
   /**
@@ -500,7 +493,7 @@ export default class Reference extends ReferenceBase {
    * @returns {string}
    */
   toString(): string {
-    return `${this._database.databaseUrl}/${this.path}`;
+    return `${this._database.databaseUrl}${this.path}`;
   }
 
   /**
@@ -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<U>(
-//     onFulfill?: (value: R) => Promise<U> | U,
-//     onReject?: (error: any) => Promise<U> | U
-//   ): Promise<U>;
-//   catch<U>(onReject?: (error: any) => Promise<U> | U): Promise<R | U>;
-// }