From eedcb2d8d0c3d14309e8a1d7bfdfe1547a2e12f0 Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 10:52:53 -0400 Subject: [PATCH 1/6] Java channel calls on UI thread --- packages/cloud_firestore/CHANGELOG.md | 6 + .../cloudfirestore/CloudFirestorePlugin.java | 139 ++++++++++++------ packages/cloud_firestore/pubspec.yaml | 2 +- 3 files changed, 102 insertions(+), 45 deletions(-) diff --git a/packages/cloud_firestore/CHANGELOG.md b/packages/cloud_firestore/CHANGELOG.md index 931c78189ce5..b21c314c3964 100644 --- a/packages/cloud_firestore/CHANGELOG.md +++ b/packages/cloud_firestore/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.12.2 + +* Ensure that all channel calls to the Dart side from the Java side are done + on the UI thread. This change allows Transactions to work with upcoming + Engine restrictions, which require channel calls be made on the UI thread. + ## 0.12.1 * Added support for `Source` to `Query.getDocuments()` and `DocumentReference.get()`. diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java index d75d6e81dadf..1bd341d7c74a 100644 --- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java +++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java @@ -4,10 +4,14 @@ package io.flutter.plugins.firebase.cloudfirestore; +import android.app.Activity; import android.os.AsyncTask; +import android.util.Log; import android.util.SparseArray; import androidx.annotation.NonNull; import androidx.annotation.Nullable; + +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; @@ -53,8 +57,9 @@ public class CloudFirestorePlugin implements MethodCallHandler { - public static final String TAG = "CloudFirestorePlugin"; + private static final String TAG = "CloudFirestorePlugin"; private final MethodChannel channel; + private final Activity activity; // Handles are ints used as indexes into the sparse array of active observers private int nextListenerHandle = 0; @@ -72,11 +77,12 @@ public static void registerWith(PluginRegistry.Registrar registrar) { registrar.messenger(), "plugins.flutter.io/cloud_firestore", new StandardMethodCodec(FirestoreMessageCodec.INSTANCE)); - channel.setMethodCallHandler(new CloudFirestorePlugin(channel)); + channel.setMethodCallHandler(new CloudFirestorePlugin(channel, registrar.activity())); } - private CloudFirestorePlugin(MethodChannel channel) { + private CloudFirestorePlugin(MethodChannel channel, Activity activity) { this.channel = channel; + this.activity = activity; } private FirebaseFirestore getFirestore(Map arguments) { @@ -343,59 +349,73 @@ public void onMethodCall(MethodCall call, final Result result) { final Map arguments = call.arguments(); getFirestore(arguments) .runTransaction( - new Transaction.Function() { + new Transaction.Function>() { @Nullable @Override - public Void apply(@NonNull Transaction transaction) - throws FirebaseFirestoreException { + public Map apply(@NonNull Transaction transaction) { // Store transaction. int transactionId = (Integer) arguments.get("transactionId"); transactions.append(transactionId, transaction); completionTasks.append(transactionId, transactionTCS); // Start operations on Dart side. - channel.invokeMethod( - "DoTransaction", - arguments, - new Result() { - @SuppressWarnings("unchecked") - @Override - public void success(Object doTransactionResult) { - transactionTCS.trySetResult( - (Map) doTransactionResult); - } - - @Override - public void error( - String errorCode, String errorMessage, Object errorDetails) { - // result.error(errorCode, errorMessage, errorDetails); - transactionTCS.trySetException( - new Exception("Do transaction failed.")); - } - - @Override - public void notImplemented() { - // result.error("DoTransaction not implemented", null, null); - transactionTCS.setException( - new Exception("DoTransaction not implemented")); - } - }); + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + channel.invokeMethod( + "DoTransaction", + arguments, + new Result() { + @SuppressWarnings("unchecked") + @Override + public void success(Object doTransactionResult) { + transactionTCS.trySetResult( + (Map) doTransactionResult); + } + + @Override + public void error( + String errorCode, String errorMessage, Object errorDetails) { + transactionTCS.trySetException( + new Exception("Do transaction failed.")); + } + + @Override + public void notImplemented() { + transactionTCS.trySetException( + new Exception("DoTransaction not implemented")); + } + }); + } + }); // Wait till transaction is complete. try { String timeoutKey = "transactionTimeout"; long timeout = ((Number) arguments.get(timeoutKey)).longValue(); - Map transactionResult = + final Map transactionResult = Tasks.await(transactionTCSTask, timeout, TimeUnit.MILLISECONDS); // Once transaction completes return the result to the Dart side. - result.success(transactionResult); + return transactionResult; } catch (Exception e) { + Log.e(TAG, e.getMessage(), e); result.error("Error performing transaction", e.getMessage(), null); } return null; } - }); + }) + .addOnCompleteListener(new OnCompleteListener>() { + @Override + public void onComplete(Task> task) { + if (task.isSuccessful()) { + result.success(task.getResult()); + } else { + result.error("Error performing transaction", + task.getException().getMessage(), null); + } + } + }); break; } case "Transaction#get": @@ -408,7 +428,7 @@ protected Void doInBackground(Void... voids) { try { DocumentSnapshot documentSnapshot = transaction.get(getDocumentReference(arguments)); - Map snapshotMap = new HashMap<>(); + final Map snapshotMap = new HashMap<>(); snapshotMap.put("path", documentSnapshot.getReference().getPath()); if (documentSnapshot.exists()) { snapshotMap.put("data", documentSnapshot.getData()); @@ -419,9 +439,19 @@ protected Void doInBackground(Void... voids) { metadata.put("hasPendingWrites", documentSnapshot.getMetadata().hasPendingWrites()); metadata.put("isFromCache", documentSnapshot.getMetadata().isFromCache()); snapshotMap.put("metadata", metadata); - result.success(snapshotMap); - } catch (FirebaseFirestoreException e) { - result.error("Error performing Transaction#get", e.getMessage(), null); + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + result.success(snapshotMap); + } + }); + } catch (final FirebaseFirestoreException e) { + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + result.error("Error performing Transaction#get", e.getMessage(), null); + } + }); } return null; } @@ -439,9 +469,19 @@ protected Void doInBackground(Void... voids) { Map data = (Map) arguments.get("data"); try { transaction.update(getDocumentReference(arguments), data); - result.success(null); - } catch (IllegalStateException e) { - result.error("Error performing Transaction#update", e.getMessage(), null); + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + result.success(null); + } + }); + } catch (final IllegalStateException e) { + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + result.error("Error performing Transaction#update", e.getMessage(), null); + } + }); } return null; } @@ -458,7 +498,13 @@ protected Void doInBackground(Void... voids) { protected Void doInBackground(Void... voids) { Map data = (Map) arguments.get("data"); transaction.set(getDocumentReference(arguments), data); - result.success(null); + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + Log.d(TAG, "sending set success"); + result.success(null); + } + }); return null; } }.execute(); @@ -472,7 +518,12 @@ protected Void doInBackground(Void... voids) { @Override protected Void doInBackground(Void... voids) { transaction.delete(getDocumentReference(arguments)); - result.success(null); + activity.runOnUiThread(new Runnable() { + @Override + public void run() { + result.success(null); + } + }); return null; } }.execute(); diff --git a/packages/cloud_firestore/pubspec.yaml b/packages/cloud_firestore/pubspec.yaml index 0198a45b6134..cf8a0df0b2c9 100755 --- a/packages/cloud_firestore/pubspec.yaml +++ b/packages/cloud_firestore/pubspec.yaml @@ -3,7 +3,7 @@ description: Flutter plugin for Cloud Firestore, a cloud-hosted, noSQL database live synchronization and offline support on Android and iOS. author: Flutter Team homepage: https://github.com/flutter/plugins/tree/master/packages/cloud_firestore -version: 0.12.1 +version: 0.12.2 flutter: plugin: From ca262dbf75c71279dd93a99a20b5cef939fb10f2 Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 10:56:06 -0400 Subject: [PATCH 2/6] Update registrar version --- .../firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java index bcb1078a15ea..1529bf6f9b11 100644 --- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java +++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java @@ -8,7 +8,7 @@ public class FlutterFirebaseAppRegistrar implements ComponentRegistrar { private static final String LIBRARY_NAME = "flutter-firebase_cloud_firestore"; - private static final String LIBRARY_VERSION = "0.12.1"; + private static final String LIBRARY_VERSION = "0.12.2"; @Override public List> getComponents() { From c7a5b59a348b2d46d49eb52f352459b4f6e69084 Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 11:02:46 -0400 Subject: [PATCH 3/6] update Firestore Firebase reporting string --- .../firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java index 1529bf6f9b11..efdf7eefcc17 100644 --- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java +++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java @@ -7,7 +7,7 @@ import java.util.List; public class FlutterFirebaseAppRegistrar implements ComponentRegistrar { - private static final String LIBRARY_NAME = "flutter-firebase_cloud_firestore"; + private static final String LIBRARY_NAME = "flutter-fire-fst"; private static final String LIBRARY_VERSION = "0.12.2"; @Override From c3fcd797401624fb4b835a8be9facf0e8f4a0ce1 Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 11:06:28 -0400 Subject: [PATCH 4/6] fix formatting --- .../cloudfirestore/CloudFirestorePlugin.java | 163 +++++++++--------- 1 file changed, 86 insertions(+), 77 deletions(-) diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java index 1bd341d7c74a..0470713ef234 100644 --- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java +++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java @@ -10,7 +10,6 @@ import android.util.SparseArray; import androidx.annotation.NonNull; import androidx.annotation.Nullable; - import com.google.android.gms.tasks.OnCompleteListener; import com.google.android.gms.tasks.OnFailureListener; import com.google.android.gms.tasks.OnSuccessListener; @@ -359,35 +358,38 @@ public Map apply(@NonNull Transaction transaction) { completionTasks.append(transactionId, transactionTCS); // Start operations on Dart side. - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - channel.invokeMethod( - "DoTransaction", - arguments, - new Result() { - @SuppressWarnings("unchecked") - @Override - public void success(Object doTransactionResult) { - transactionTCS.trySetResult( - (Map) doTransactionResult); - } - - @Override - public void error( - String errorCode, String errorMessage, Object errorDetails) { - transactionTCS.trySetException( - new Exception("Do transaction failed.")); - } - - @Override - public void notImplemented() { - transactionTCS.trySetException( - new Exception("DoTransaction not implemented")); - } - }); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + channel.invokeMethod( + "DoTransaction", + arguments, + new Result() { + @SuppressWarnings("unchecked") + @Override + public void success(Object doTransactionResult) { + transactionTCS.trySetResult( + (Map) doTransactionResult); + } + + @Override + public void error( + String errorCode, + String errorMessage, + Object errorDetails) { + transactionTCS.trySetException( + new Exception("Do transaction failed.")); + } + + @Override + public void notImplemented() { + transactionTCS.trySetException( + new Exception("DoTransaction not implemented")); + } + }); + } + }); // Wait till transaction is complete. try { @@ -405,17 +407,18 @@ public void notImplemented() { return null; } }) - .addOnCompleteListener(new OnCompleteListener>() { - @Override - public void onComplete(Task> task) { - if (task.isSuccessful()) { - result.success(task.getResult()); - } else { - result.error("Error performing transaction", - task.getException().getMessage(), null); - } - } - }); + .addOnCompleteListener( + new OnCompleteListener>() { + @Override + public void onComplete(Task> task) { + if (task.isSuccessful()) { + result.success(task.getResult()); + } else { + result.error( + "Error performing transaction", task.getException().getMessage(), null); + } + } + }); break; } case "Transaction#get": @@ -439,19 +442,21 @@ protected Void doInBackground(Void... voids) { metadata.put("hasPendingWrites", documentSnapshot.getMetadata().hasPendingWrites()); metadata.put("isFromCache", documentSnapshot.getMetadata().isFromCache()); snapshotMap.put("metadata", metadata); - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - result.success(snapshotMap); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + result.success(snapshotMap); + } + }); } catch (final FirebaseFirestoreException e) { - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - result.error("Error performing Transaction#get", e.getMessage(), null); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + result.error("Error performing Transaction#get", e.getMessage(), null); + } + }); } return null; } @@ -469,19 +474,21 @@ protected Void doInBackground(Void... voids) { Map data = (Map) arguments.get("data"); try { transaction.update(getDocumentReference(arguments), data); - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - result.success(null); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + result.success(null); + } + }); } catch (final IllegalStateException e) { - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - result.error("Error performing Transaction#update", e.getMessage(), null); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + result.error("Error performing Transaction#update", e.getMessage(), null); + } + }); } return null; } @@ -498,13 +505,14 @@ public void run() { protected Void doInBackground(Void... voids) { Map data = (Map) arguments.get("data"); transaction.set(getDocumentReference(arguments), data); - activity.runOnUiThread(new Runnable() { - @Override - public void run() { - Log.d(TAG, "sending set success"); - result.success(null); - } - }); + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { + Log.d(TAG, "sending set success"); + result.success(null); + } + }); return null; } }.execute(); @@ -518,12 +526,13 @@ public void run() { @Override protected Void doInBackground(Void... voids) { transaction.delete(getDocumentReference(arguments)); - activity.runOnUiThread(new Runnable() { - @Override - public void run() { + activity.runOnUiThread( + new Runnable() { + @Override + public void run() { result.success(null); - } - }); + } + }); return null; } }.execute(); From a62b859d84c261cea89d524577052051b74d1f6d Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 11:22:16 -0400 Subject: [PATCH 5/6] Update iOS reporting version --- packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m b/packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m index 0462ebddf79e..2b46c408ebf2 100644 --- a/packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m +++ b/packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m @@ -7,7 +7,7 @@ #import #define LIBRARY_NAME @"flutter-firebase_cloud_firestore" -#define LIBRARY_VERSION @"0.12.1" +#define LIBRARY_VERSION @"0.12.2" static FlutterError *getFlutterError(NSError *error) { if (error == nil) return nil; From a3d391a46ac411c798eb256ab6d332db5fae09cb Mon Sep 17 00:00:00 2001 From: Arthur Thompson Date: Tue, 4 Jun 2019 14:13:49 -0400 Subject: [PATCH 6/6] Update README to include more change info --- packages/cloud_firestore/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cloud_firestore/CHANGELOG.md b/packages/cloud_firestore/CHANGELOG.md index b21c314c3964..1173ff2927d8 100644 --- a/packages/cloud_firestore/CHANGELOG.md +++ b/packages/cloud_firestore/CHANGELOG.md @@ -2,7 +2,10 @@ * Ensure that all channel calls to the Dart side from the Java side are done on the UI thread. This change allows Transactions to work with upcoming - Engine restrictions, which require channel calls be made on the UI thread. + Engine restrictions, which require channel calls be made on the UI thread. + **Note** this is an Android only change, the iOS implementation was not impacted. +* Updated the Firebase reporting string to `flutter-fire-fst` to be consistent + with other reporting libraries. ## 0.12.1