From a108d43e95317ec551a20f395b33aa2949902d0f Mon Sep 17 00:00:00 2001 From: Salakar Date: Tue, 13 Feb 2018 13:58:23 +0000 Subject: [PATCH 1/5] [firestore] remove transaction listener instance from class --- lib/modules/firestore/TransactionHandler.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/modules/firestore/TransactionHandler.js b/lib/modules/firestore/TransactionHandler.js index e51c1fcb..b0b5d7c1 100644 --- a/lib/modules/firestore/TransactionHandler.js +++ b/lib/modules/firestore/TransactionHandler.js @@ -37,13 +37,12 @@ type TransactionEvent = { */ export default class TransactionHandler { _firestore: Firestore; - _transactionListener: Function; _pending: { [number]: TransactionMeta }; constructor(firestore: Firestore) { this._pending = {}; this._firestore = firestore; - this._transactionListener = SharedEventEmitter.addListener( + SharedEventEmitter.addListener( getAppEventName(this._firestore, 'firestore_transaction_event'), this._handleTransactionEvent.bind(this) ); From 609324e2929e211ffd7526e46b835475fe610f3a Mon Sep 17 00:00:00 2001 From: Salakar Date: Tue, 13 Feb 2018 13:58:52 +0000 Subject: [PATCH 2/5] [database] remove transaction listener instance from class --- lib/modules/database/transaction.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/modules/database/transaction.js b/lib/modules/database/transaction.js index cbde0edc..5ac6dd4e 100644 --- a/lib/modules/database/transaction.js +++ b/lib/modules/database/transaction.js @@ -21,14 +21,13 @@ const generateTransactionId = (): number => transactionId++; */ export default class TransactionHandler { _database: Database; - _transactionListener: Function; _transactions: { [number]: Object }; constructor(database: Database) { this._transactions = {}; this._database = database; - this._transactionListener = SharedEventEmitter.addListener( + SharedEventEmitter.addListener( getAppEventName(this._database, 'database_transaction_event'), this._handleTransactionEvent.bind(this) ); From 5fb1ad6781a74a330921da5fda5f86683133c8ba Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 23 Feb 2018 02:59:39 +0000 Subject: [PATCH 3/5] [firestore] fix issue with NaN serialization --- lib/modules/firestore/utils/serialize.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/firestore/utils/serialize.js b/lib/modules/firestore/utils/serialize.js index 24b8f0a5..2aacbe7b 100644 --- a/lib/modules/firestore/utils/serialize.js +++ b/lib/modules/firestore/utils/serialize.js @@ -51,7 +51,7 @@ export const buildNativeArray = (array: Object[]): FirestoreTypeMap[] => { export const buildTypeMap = (value: any): FirestoreTypeMap | null => { const type = typeOf(value); - if (value === null || value === undefined) { + if (value === null || value === undefined || Number.isNaN(value)) { return { type: 'null', value: null, From dd1bbc87f50da8fc22544731368a07bb1543a85a Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 23 Feb 2018 03:00:36 +0000 Subject: [PATCH 4/5] [firestore][android] transactions working --- .../firestore/RNFirebaseFirestore.java | 207 +++++++++++++++++- .../RNFirebaseFirestoreDocumentReference.java | 4 + ...RNFirebaseFirestoreTransactionHandler.java | 166 ++++++++++++++ 3 files changed, 373 insertions(+), 4 deletions(-) create mode 100644 android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreTransactionHandler.java diff --git a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestore.java b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestore.java index 0d0ba858..3bb250f5 100644 --- a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestore.java +++ b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestore.java @@ -1,7 +1,9 @@ package io.invertase.firebase.firestore; +import android.os.AsyncTask; import android.support.annotation.NonNull; import android.util.Log; +import android.util.SparseArray; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Promise; @@ -12,8 +14,11 @@ import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.WritableMap; import com.google.android.gms.tasks.OnCompleteListener; +import com.google.android.gms.tasks.OnFailureListener; +import com.google.android.gms.tasks.OnSuccessListener; import com.google.android.gms.tasks.Task; import com.google.firebase.FirebaseApp; +import com.google.firebase.firestore.Transaction; import com.google.firebase.firestore.DocumentReference; import com.google.firebase.firestore.FieldValue; import com.google.firebase.firestore.FirebaseFirestore; @@ -26,11 +31,12 @@ import java.util.List; import java.util.Map; import io.invertase.firebase.ErrorUtils; +import io.invertase.firebase.Utils; public class RNFirebaseFirestore extends ReactContextBaseJavaModule { private static final String TAG = "RNFirebaseFirestore"; - // private SparseArray transactionHandlers = new SparseArray<>(); + private SparseArray transactionHandlers = new SparseArray<>(); RNFirebaseFirestore(ReactApplicationContext reactContext) { super(reactContext); @@ -92,7 +98,7 @@ public class RNFirebaseFirestore extends ReactContextBaseJavaModule { break; case "SET": Map options = (Map) write.get("options"); - if (options != null && options.containsKey("merge") && (boolean)options.get("merge")) { + if (options != null && options.containsKey("merge") && (boolean) options.get("merge")) { batch = batch.set(ref, data, SetOptions.merge()); } else { batch = batch.set(ref, data); @@ -113,7 +119,7 @@ public class RNFirebaseFirestore extends ReactContextBaseJavaModule { promise.resolve(null); } else { Log.e(TAG, "documentBatch:onComplete:failure", task.getException()); - RNFirebaseFirestore.promiseRejectException(promise, (FirebaseFirestoreException)task.getException()); + RNFirebaseFirestore.promiseRejectException(promise, (FirebaseFirestoreException) task.getException()); } } }); @@ -160,6 +166,199 @@ public class RNFirebaseFirestore extends ReactContextBaseJavaModule { ref.update(data, promise); } + + /** + * Try clean up previous transactions on reload + * + */ + @Override + public void onCatalystInstanceDestroy() { + for (int i = 0, size = transactionHandlers.size(); i < size; i++) { + RNFirebaseFirestoreTransactionHandler transactionHandler = transactionHandlers.get(i); + if (transactionHandler != null) { + transactionHandler.abort(); + } + } + + transactionHandlers.clear(); + } + + + /* + * Transaction Methods + */ + + + /** + * Calls the internal Firestore Transaction classes instance .get(ref) method and resolves with + * the DocumentSnapshot. + * + * @param appName + * @param transactionId + * @param path + * @param promise + */ + @ReactMethod + public void transactionGetDocument(String appName, int transactionId, String path, final Promise promise) { + RNFirebaseFirestoreTransactionHandler handler = transactionHandlers.get(transactionId); + + if (handler == null) { + promise.reject( + "internal-error", + "An internal error occurred whilst attempting to find a native transaction by id." + ); + } else { + DocumentReference ref = getDocumentForAppPath(appName, path).getRef(); + handler.getDocument(ref, promise); + } + } + + /** + * Aborts any pending signals and deletes the transaction handler. + * + * @param appName + * @param transactionId + */ + @ReactMethod + public void transactionDispose(String appName, int transactionId) { + RNFirebaseFirestoreTransactionHandler handler = transactionHandlers.get(transactionId); + + if (handler != null) { + handler.abort(); + transactionHandlers.delete(transactionId); + } + } + + /** + * Signals to transactionHandler that the command buffer is ready. + * + * @param appName + * @param transactionId + * @param commandBuffer + */ + @ReactMethod + public void transactionApplyBuffer(String appName, int transactionId, ReadableArray commandBuffer) { + RNFirebaseFirestoreTransactionHandler handler = transactionHandlers.get(transactionId); + + if (handler != null) { + handler.signalBufferReceived(commandBuffer); + } + } + + /** + * Begin a new transaction via AsyncTask 's + * + * @param appName + * @param transactionId + */ + @ReactMethod + public void transactionBegin(final String appName, int transactionId) { + final RNFirebaseFirestoreTransactionHandler transactionHandler = new RNFirebaseFirestoreTransactionHandler(appName, transactionId); + transactionHandlers.put(transactionId, transactionHandler); + + AsyncTask.execute(new Runnable() { + @Override + public void run() { + getFirestoreForApp(appName) + .runTransaction(new Transaction.Function() { + @Override + public Void apply(@NonNull Transaction transaction) throws FirebaseFirestoreException { + transactionHandler.setFirestoreTransaction(transaction); + + // emit the update cycle to JS land using an async task + // otherwise it gets blocked by the pending lock await + AsyncTask.execute(new Runnable() { + @Override + public void run() { + WritableMap eventMap = transactionHandler.createEventMap(null, "update"); + Utils.sendEvent(getReactApplicationContext(), "firestore_transaction_event", eventMap); + } + }); + + // wait for a signal to be received from JS land code + transactionHandler.await(); + + // exit early if aborted - has to throw an exception otherwise will just keep trying ... + if (transactionHandler.aborted) { + throw new FirebaseFirestoreException("abort", FirebaseFirestoreException.Code.ABORTED); + } + + // exit early if timeout from bridge - has to throw an exception otherwise will just keep trying ... + if (transactionHandler.timeout) { + throw new FirebaseFirestoreException("timeout", FirebaseFirestoreException.Code.DEADLINE_EXCEEDED); + } + + // process any buffered commands from JS land + ReadableArray buffer = transactionHandler.getCommandBuffer(); + + // exit early if no commands + if (buffer == null) { + return null; + } + + for (int i = 0, size = buffer.size(); i < size; i++) { + ReadableMap data; + ReadableMap command = buffer.getMap(i); + String path = command.getString("path"); + String type = command.getString("type"); + RNFirebaseFirestoreDocumentReference documentReference = getDocumentForAppPath(appName, path); + + + switch (type) { + case "set": + data = command.getMap("data"); + + ReadableMap options = command.getMap("options"); + Map setData = FirestoreSerialize.parseReadableMap(RNFirebaseFirestore.getFirestoreForApp(appName), data); + + if (options != null && options.hasKey("merge") && options.getBoolean("merge")) { + transaction.set(documentReference.getRef(), setData, SetOptions.merge()); + } else { + transaction.set(documentReference.getRef(), setData); + } + break; + case "update": + data = command.getMap("data"); + + Map updateData = FirestoreSerialize.parseReadableMap(RNFirebaseFirestore.getFirestoreForApp(appName), data); + transaction.update(documentReference.getRef(), updateData); + break; + case "delete": + transaction.delete(documentReference.getRef()); + break; + default: + throw new IllegalArgumentException("Unknown command type at index " + i + "."); + } + } + + return null; + } + }) + .addOnSuccessListener(new OnSuccessListener() { + @Override + public void onSuccess(Void aVoid) { + if (!transactionHandler.aborted) { + Log.d(TAG, "Transaction onSuccess!"); + WritableMap eventMap = transactionHandler.createEventMap(null, "complete"); + Utils.sendEvent(getReactApplicationContext(), "firestore_transaction_event", eventMap); + } + } + }) + .addOnFailureListener(new OnFailureListener() { + @Override + public void onFailure(@NonNull Exception e) { + if (!transactionHandler.aborted) { + Log.w(TAG, "Transaction onFailure.", e); + WritableMap eventMap = transactionHandler.createEventMap((FirebaseFirestoreException) e, "error"); + Utils.sendEvent(getReactApplicationContext(), "firestore_transaction_event", eventMap); + } + } + }); + } + }); + } + + /* * INTERNALS/UTILS */ @@ -197,7 +396,7 @@ public class RNFirebaseFirestore extends ReactContextBaseJavaModule { * @param filters * @param orders * @param options - * @param path @return + * @param path @return */ private RNFirebaseFirestoreCollectionReference getCollectionForAppPath(String appName, String path, ReadableArray filters, diff --git a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreDocumentReference.java b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreDocumentReference.java index 93862c99..08eb6269 100644 --- a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreDocumentReference.java +++ b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreDocumentReference.java @@ -145,6 +145,10 @@ public class RNFirebaseFirestoreDocumentReference { * INTERNALS/UTILS */ + public DocumentReference getRef() { + return ref; + } + boolean hasListeners() { return !documentSnapshotListeners.isEmpty(); } diff --git a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreTransactionHandler.java b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreTransactionHandler.java new file mode 100644 index 00000000..cf84ca64 --- /dev/null +++ b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreTransactionHandler.java @@ -0,0 +1,166 @@ +package io.invertase.firebase.firestore; + +import com.facebook.react.bridge.Arguments; +import com.facebook.react.bridge.Promise; +import com.facebook.react.bridge.ReadableArray; +import com.facebook.react.bridge.WritableMap; +import com.google.firebase.firestore.DocumentReference; +import com.google.firebase.firestore.DocumentSnapshot; +import com.google.firebase.firestore.FirebaseFirestoreException; +import com.google.firebase.firestore.Transaction; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +import javax.annotation.Nullable; + + +class RNFirebaseFirestoreTransactionHandler { + private String appName; + private long timeoutAt; + private int transactionId; + private final ReentrantLock lock; + private final Condition condition; + private ReadableArray commandBuffer; + private Transaction firestoreTransaction; + + boolean aborted = false; + boolean timeout = false; + + RNFirebaseFirestoreTransactionHandler(String app, int id) { + appName = app; + transactionId = id; + updateInternalTimeout(); + lock = new ReentrantLock(); + condition = lock.newCondition(); + } + + /* + * ------------- + * PACKAGE API + * ------------- + */ + + /** + * Abort the currently in progress transaction if any. + */ + void abort() { + aborted = true; + safeUnlock(); + } + + /** + * Keep a ref to the Transaction instance. + * + * @param firestoreTransaction + */ + void setFirestoreTransaction(Transaction firestoreTransaction) { + this.firestoreTransaction = firestoreTransaction; + } + + /** + * Signal that the transaction buffer has been received and needs to be processed. + * + * @param buffer + */ + void signalBufferReceived(ReadableArray buffer) { + lock.lock(); + + try { + commandBuffer = buffer; + condition.signalAll(); + } finally { + safeUnlock(); + } + } + + /** + * Wait for signalBufferReceived to signal condition + * + * @throws InterruptedException + */ + void await() { + lock.lock(); + + updateInternalTimeout(); + + try { + while (!aborted && !timeout && !condition.await(10, TimeUnit.MILLISECONDS)) { + if (System.currentTimeMillis() > timeoutAt) timeout = true; + } + } catch (InterruptedException ie) { + // should never be interrupted + } finally { + safeUnlock(); + } + } + + /** + * Get the current pending command buffer. + * + * @return + */ + ReadableArray getCommandBuffer() { + return commandBuffer; + } + + + /** + * Get and resolve a DocumentSnapshot from transaction.get(ref); + * + * @param ref + * @param promise + */ + void getDocument(DocumentReference ref, Promise promise) { + updateInternalTimeout(); + + try { + DocumentSnapshot documentSnapshot = firestoreTransaction.get(ref); + WritableMap writableMap = FirestoreSerialize.snapshotToWritableMap(documentSnapshot); + promise.resolve(writableMap); + } catch (FirebaseFirestoreException firestoreException) { + WritableMap jsError = RNFirebaseFirestore.getJSError(firestoreException); + promise.reject(jsError.getString("code"), jsError.getString("message")); + } + } + + /** + * Event map for `firestore_transaction_event` events. + * + * @param error + * @param type + * @return + */ + WritableMap createEventMap(@Nullable FirebaseFirestoreException error, String type) { + WritableMap eventMap = Arguments.createMap(); + + eventMap.putInt("id", transactionId); + eventMap.putString("appName", appName); + + if (error != null) { + eventMap.putString("type", "error"); + eventMap.putMap("error", RNFirebaseFirestore.getJSError(error)); + } else { + eventMap.putString("type", type); + } + + return eventMap; + } + + /* + * ------------- + * INTERNAL API + * ------------- + */ + + private void safeUnlock() { + if (lock.isLocked()) { + lock.unlock(); + } + } + + private void updateInternalTimeout() { + timeoutAt = System.currentTimeMillis() + 15000; + } +} From 3f4c59f81de56d4dcce025c7e5dc2101f4a3ee90 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 23 Feb 2018 03:02:17 +0000 Subject: [PATCH 5/5] [firestore][js] misc transactions internals changes --- lib/modules/firestore/Transaction.js | 14 ++++++++------ lib/modules/firestore/TransactionHandler.js | 20 ++++++++++++-------- lib/modules/firestore/index.js | 3 +++ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/lib/modules/firestore/Transaction.js b/lib/modules/firestore/Transaction.js index 21f7699d..07db5162 100644 --- a/lib/modules/firestore/Transaction.js +++ b/lib/modules/firestore/Transaction.js @@ -8,7 +8,7 @@ import { buildNativeMap } from './utils/serialize'; import type Firestore from './'; import type { TransactionMeta } from './TransactionHandler'; import type DocumentReference from './DocumentReference'; -import type DocumentSnapshot from './DocumentSnapshot'; +import DocumentSnapshot from './DocumentSnapshot'; import { isObject, isString } from '../../utils'; import FieldPath from './FieldPath'; import { getNativeModule } from '../../utils/native'; @@ -24,6 +24,9 @@ type SetOptions = { merge: boolean, }; +// TODO docs state all get requests must be made FIRST before any modifications +// TODO so need to validate that + /** * @class Transaction */ @@ -72,10 +75,9 @@ export default class Transaction { */ get(documentRef: DocumentReference): Promise { // todo validate doc ref - return getNativeModule(this._firestore).transactionGetDocument( - this._meta.id, - documentRef.path - ); + return getNativeModule(this._firestore) + .transactionGetDocument(this._meta.id, documentRef.path) + .then(result => new DocumentSnapshot(this._firestore, result)); } /** @@ -102,7 +104,7 @@ export default class Transaction { type: 'set', path: documentRef.path, data: buildNativeMap(data), - options, + options: options || {}, }); return this; diff --git a/lib/modules/firestore/TransactionHandler.js b/lib/modules/firestore/TransactionHandler.js index b0b5d7c1..d8f0e190 100644 --- a/lib/modules/firestore/TransactionHandler.js +++ b/lib/modules/firestore/TransactionHandler.js @@ -67,7 +67,10 @@ export default class TransactionHandler { reject: null, resolve: null, updateFunction, - stack: new Error().stack.slice(1), + stack: new Error().stack + .split('\n') + .slice(4) + .join('\n'), }; meta.transaction = new Transaction(this._firestore, meta); @@ -91,14 +94,11 @@ export default class TransactionHandler { * Destroys a local instance of a transaction meta * * @param id - * @param pendingAbort Notify native that there's still an transaction in - * progress that needs aborting - this is to handle a JS side - * exception * @private */ - _remove(id, pendingAbort = false) { + _remove(id) { // todo confirm pending arg no longer needed - getNativeModule(this._firestore).transactionDispose(id, pendingAbort); + getNativeModule(this._firestore).transactionDispose(id); // TODO may need delaying to next event loop delete this._pending[id]; } @@ -168,11 +168,15 @@ export default class TransactionHandler { pendingResult = await possiblePromise; } } catch (exception) { - updateFailed = true; // in case the user rejects with nothing + // exception can still be falsey if user `Promise.reject();` 's with no args + // so we track the exception with a updateFailed boolean to ensure no fall-through + updateFailed = true; finalError = exception; } // reject the final promise and remove from native + // update is failed when either the users updateFunction + // throws an error or rejects a promise if (updateFailed) { return reject(finalError); } @@ -183,7 +187,7 @@ export default class TransactionHandler { transaction._pendingResult = pendingResult; // send the buffered update/set/delete commands for native to process - return getNativeModule(this._firestore).transactionProcessUpdateResponse( + return getNativeModule(this._firestore).transactionApplyBuffer( id, transaction._commandBuffer ); diff --git a/lib/modules/firestore/index.js b/lib/modules/firestore/index.js index 3e8d7ecd..cd6a1947 100644 --- a/lib/modules/firestore/index.js +++ b/lib/modules/firestore/index.js @@ -154,6 +154,7 @@ export default class Firestore extends ModuleBase { ) ); } + enableNetwork(): void { throw new Error( INTERNALS.STRINGS.ERROR_UNSUPPORTED_MODULE_METHOD( @@ -162,6 +163,7 @@ export default class Firestore extends ModuleBase { ) ); } + disableNetwork(): void { throw new Error( INTERNALS.STRINGS.ERROR_UNSUPPORTED_MODULE_METHOD( @@ -180,6 +182,7 @@ export default class Firestore extends ModuleBase { enablePersistence(): Promise { throw new Error('Persistence is enabled by default on the Firestore SDKs'); } + settings(): void { throw new Error('firebase.firestore().settings() coming soon'); }