Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[cloud_firestore] Invoke on ui thread only #1701

Merged
kroikie merged 6 commits into from
Jun 4, 2019
Merged

[cloud_firestore] Invoke on ui thread only #1701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eedcb2d
Java channel calls on UI thread
kroikie Jun 4, 2019
ca262db
Update registrar version
kroikie Jun 4, 2019
c7a5b59
update Firestore Firebase reporting string
kroikie Jun 4, 2019
c3fcd79
fix formatting
kroikie Jun 4, 2019
a62b859
Update iOS reporting version
kroikie Jun 4, 2019
a3d391a
Update README to include more change info
kroikie Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/cloud_firestore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 0.12.2

* Ensure that all channel calls to the Dart side from the Java side are done
Copy link
Contributor

@collinjackson collinjackson Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to mention that this is an Android-only change.

Copy link
Contributor Author

@kroikie kroikie Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
**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.

Copy link

@larssn larssn Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this take into account that Firestore's Android SDK will not allow transactions on the UI thread?

Copy link
Contributor Author

@kroikie kroikie Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transaction itself is not being executed on the UI thread, only the message being passed over the channel from native side to Dart side is being done on the UI thread.

## 0.12.1

* Added support for `Source` to `Query.getDocuments()` and `DocumentReference.get()`.
Expand Down
140 changes: 100 additions & 40 deletions ...ndroid/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@

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;
Expand Down Expand Up @@ -53,8 +56,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;
Expand All @@ -72,11 +76,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<String, Object> arguments) {
Expand Down Expand Up @@ -343,58 +348,76 @@ public void onMethodCall(MethodCall call, final Result result) {
final Map<String, Object> arguments = call.arguments();
getFirestore(arguments)
.runTransaction(
new Transaction.Function<Void>() {
new Transaction.Function<Map<String, Object>>() {
@Nullable
@Override
public Void apply(@NonNull Transaction transaction)
throws FirebaseFirestoreException {
public Map<String, Object> 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")
activity.runOnUiThread(
new Runnable() {
@Override
public void success(Object doTransactionResult) {
transactionTCS.trySetResult(
(Map<String, Object>) 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"));
public void run() {
channel.invokeMethod(
"DoTransaction",
arguments,
new Result() {
@SuppressWarnings("unchecked")
@Override
public void success(Object doTransactionResult) {
transactionTCS.trySetResult(
(Map<String, Object>) 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<String, Object> transactionResult =
final Map<String, Object> 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<Map<String, Object>>() {
@Override
public void onComplete(Task<Map<String, Object>> task) {
if (task.isSuccessful()) {
result.success(task.getResult());
} else {
result.error(
"Error performing transaction", task.getException().getMessage(), null);
}
}
});
break;
}
Expand All @@ -408,7 +431,7 @@ protected Void doInBackground(Void... voids) {
try {
DocumentSnapshot documentSnapshot =
transaction.get(getDocumentReference(arguments));
Map<String, Object> snapshotMap = new HashMap<>();
final Map<String, Object> snapshotMap = new HashMap<>();
snapshotMap.put("path", documentSnapshot.getReference().getPath());
if (documentSnapshot.exists()) {
snapshotMap.put("data", documentSnapshot.getData());
Expand All @@ -419,9 +442,21 @@ 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;
}
Expand All @@ -439,9 +474,21 @@ protected Void doInBackground(Void... voids) {
Map<String, Object> data = (Map<String, Object>) 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;
}
Expand All @@ -458,7 +505,14 @@ protected Void doInBackground(Void... voids) {
protected Void doInBackground(Void... voids) {
Map<String, Object> data = (Map<String, Object>) 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();
Expand All @@ -472,7 +526,13 @@ 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();
Expand Down
4 changes: 2 additions & 2 deletions ...src/main/java/io/flutter/plugins/firebase/cloudfirestore/FlutterFirebaseAppRegistrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import java.util.List;

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_NAME = "flutter-fire-fst";
Copy link
Contributor

@collinjackson collinjackson Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be mentioned in changelog

Copy link
Contributor Author

@kroikie kroikie Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

private static final String LIBRARY_VERSION = "0.12.2";

@Override
public List<Component<?>> getComponents() {
Expand Down
2 changes: 1 addition & 1 deletion packages/cloud_firestore/ios/Classes/CloudFirestorePlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#import <Firebase/Firebase.h>

#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;
Expand Down
2 changes: 1 addition & 1 deletion packages/cloud_firestore/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <flutter-dev@googlegroups.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/cloud_firestore
version: 0.12.1
version: 0.12.2

flutter:
plugin:
Expand Down