aboutsummaryrefslogtreecommitdiffhomepage
path: root/Firestore
diff options
context:
space:
mode:
authorGravatar Konstantin Varlamov <var-const@users.noreply.github.com>2018-05-10 20:16:50 -0400
committerGravatar GitHub <noreply@github.com>2018-05-10 20:16:50 -0400
commitedd6f60a7385f9280c371df273703f6e104e325f (patch)
tree89b2d18d4f5d4253dd6b87842d7a0182aabd50de /Firestore
parent8cde81689dcfc74c9b59e088bbe00f95ca4bf68d (diff)
Firestore: use C++ Executor for user queue. (#1238)
FSTDispatchQueue enforces serial execution, which is inappropriate for user queue, because a user may configure usage of a concurrent queue in settings, breaking FSTDispatchQueue invariants. Instead, use C++ ExecutorLibdispatch directly.
Diffstat (limited to 'Firestore')
-rw-r--r--Firestore/Example/Tests/Core/FSTQueryListenerTests.mm33
-rw-r--r--Firestore/Source/API/FIRDocumentReference.mm4
-rw-r--r--Firestore/Source/API/FIRFirestore.mm9
-rw-r--r--Firestore/Source/API/FIRQuery.mm4
-rw-r--r--Firestore/Source/Core/FSTFirestoreClient.h19
-rw-r--r--Firestore/Source/Core/FSTFirestoreClient.mm45
-rw-r--r--Firestore/Source/Util/FSTAsyncQueryListener.h7
-rw-r--r--Firestore/Source/Util/FSTAsyncQueryListener.mm14
8 files changed, 74 insertions, 61 deletions
diff --git a/Firestore/Example/Tests/Core/FSTQueryListenerTests.mm b/Firestore/Example/Tests/Core/FSTQueryListenerTests.mm
index 7ae9704..5629075 100644
--- a/Firestore/Example/Tests/Core/FSTQueryListenerTests.mm
+++ b/Firestore/Example/Tests/Core/FSTQueryListenerTests.mm
@@ -15,6 +15,7 @@
*/
#import <XCTest/XCTest.h>
+#include <memory>
#import "Firestore/Source/Core/FSTEventManager.h"
#import "Firestore/Source/Core/FSTQuery.h"
@@ -23,23 +24,29 @@
#import "Firestore/Source/Model/FSTDocumentSet.h"
#import "Firestore/Source/Remote/FSTRemoteEvent.h"
#import "Firestore/Source/Util/FSTAsyncQueryListener.h"
-#import "Firestore/Source/Util/FSTDispatchQueue.h"
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
+#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
+#include "absl/memory/memory.h"
+
+using firebase::firestore::util::internal::ExecutorLibdispatch;
using firebase::firestore::model::DocumentKeySet;
NS_ASSUME_NONNULL_BEGIN
@interface FSTQueryListenerTests : XCTestCase
-@property(nonatomic, strong, readonly) FSTDispatchQueue *asyncQueue;
@end
-@implementation FSTQueryListenerTests
+@implementation FSTQueryListenerTests {
+ std::unique_ptr<ExecutorLibdispatch> _executor;
+}
- (void)setUp {
- _asyncQueue = [FSTDispatchQueue
- queueWith:dispatch_queue_create("FSTQueryListenerTests Queue", DISPATCH_QUEUE_SERIAL)];
+ // TODO(varconst): moving this test to C++, it should be possible to store Executor as a value,
+ // not a pointer, and initialize it in the constructor.
+ _executor = absl::make_unique<ExecutorLibdispatch>(
+ dispatch_queue_create("FSTQueryListenerTests Queue", DISPATCH_QUEUE_SERIAL));
}
- (void)testRaisesCollectionEvents {
@@ -131,12 +138,12 @@ NS_ASSUME_NONNULL_BEGIN
FSTDocument *doc1 = FSTTestDoc("rooms/Eros", 3, @{@"name" : @"Eros"}, NO);
FSTDocument *doc2 = FSTTestDoc("rooms/Eros", 4, @{@"name" : @"Eros2"}, NO);
- __block FSTAsyncQueryListener *listener = [[FSTAsyncQueryListener alloc]
- initWithDispatchQueue:self.asyncQueue
- snapshotHandler:^(FSTViewSnapshot *snapshot, NSError *error) {
- [accum addObject:snapshot];
- [listener mute];
- }];
+ __block FSTAsyncQueryListener *listener =
+ [[FSTAsyncQueryListener alloc] initWithExecutor:_executor.get()
+ snapshotHandler:^(FSTViewSnapshot *snapshot, NSError *error) {
+ [accum addObject:snapshot];
+ [listener mute];
+ }];
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
FSTViewSnapshot *viewSnapshot1 = FSTTestApplyChanges(view, @[ doc1 ], nil);
@@ -148,9 +155,7 @@ NS_ASSUME_NONNULL_BEGIN
// Drain queue
XCTestExpectation *expectation = [self expectationWithDescription:@"Queue drained"];
- [self.asyncQueue dispatchAsync:^{
- [expectation fulfill];
- }];
+ _executor->Execute([=] { [expectation fulfill]; });
[self waitForExpectationsWithTimeout:4.0
handler:^(NSError *_Nullable expectationError) {
diff --git a/Firestore/Source/API/FIRDocumentReference.mm b/Firestore/Source/API/FIRDocumentReference.mm
index da67a5b..5ad606c 100644
--- a/Firestore/Source/API/FIRDocumentReference.mm
+++ b/Firestore/Source/API/FIRDocumentReference.mm
@@ -280,8 +280,8 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
};
FSTAsyncQueryListener *asyncListener =
- [[FSTAsyncQueryListener alloc] initWithDispatchQueue:self.firestore.client.userDispatchQueue
- snapshotHandler:snapshotHandler];
+ [[FSTAsyncQueryListener alloc] initWithExecutor:self.firestore.client.userExecutor
+ snapshotHandler:snapshotHandler];
FSTQueryListener *internalListener =
[firestore.client listenToQuery:query
diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm
index fe461d6..e5f0c12 100644
--- a/Firestore/Source/API/FIRFirestore.mm
+++ b/Firestore/Source/API/FIRFirestore.mm
@@ -45,12 +45,16 @@
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
#include "absl/memory/memory.h"
+#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
+
namespace util = firebase::firestore::util;
using firebase::firestore::auth::CredentialsProvider;
using firebase::firestore::auth::FirebaseCredentialsProvider;
using firebase::firestore::core::DatabaseInfo;
using firebase::firestore::model::DatabaseId;
using firebase::firestore::model::ResourcePath;
+using util::internal::Executor;
+using util::internal::ExecutorLibdispatch;
NS_ASSUME_NONNULL_BEGIN
@@ -272,12 +276,13 @@ extern "C" NSString *const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";
const DatabaseInfo database_info(*self.databaseID, util::MakeStringView(_persistenceKey),
util::MakeStringView(_settings.host), _settings.sslEnabled);
- FSTDispatchQueue *userDispatchQueue = [FSTDispatchQueue queueWith:_settings.dispatchQueue];
+ std::unique_ptr<Executor> userExecutor =
+ absl::make_unique<ExecutorLibdispatch>(_settings.dispatchQueue);
_client = [FSTFirestoreClient clientWithDatabaseInfo:database_info
usePersistence:_settings.persistenceEnabled
credentialsProvider:_credentialsProvider.get()
- userDispatchQueue:userDispatchQueue
+ userExecutor:std::move(userExecutor)
workerDispatchQueue:_workerDispatchQueue];
}
}
diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm
index c83af9c..596f6ac 100644
--- a/Firestore/Source/API/FIRQuery.mm
+++ b/Firestore/Source/API/FIRQuery.mm
@@ -183,8 +183,8 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
};
FSTAsyncQueryListener *asyncListener =
- [[FSTAsyncQueryListener alloc] initWithDispatchQueue:self.firestore.client.userDispatchQueue
- snapshotHandler:snapshotHandler];
+ [[FSTAsyncQueryListener alloc] initWithExecutor:self.firestore.client.userExecutor
+ snapshotHandler:snapshotHandler];
FSTQueryListener *internalListener =
[firestore.client listenToQuery:query
diff --git a/Firestore/Source/Core/FSTFirestoreClient.h b/Firestore/Source/Core/FSTFirestoreClient.h
index 7285e65..94c2284 100644
--- a/Firestore/Source/Core/FSTFirestoreClient.h
+++ b/Firestore/Source/Core/FSTFirestoreClient.h
@@ -15,6 +15,7 @@
*/
#import <Foundation/Foundation.h>
+#include <memory>
#import "Firestore/Source/Core/FSTTypes.h"
#import "Firestore/Source/Core/FSTViewSnapshot.h"
@@ -23,6 +24,7 @@
#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
#include "Firestore/core/src/firebase/firestore/core/database_info.h"
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
+#include "Firestore/core/src/firebase/firestore/util/executor.h"
@class FIRDocumentReference;
@class FIRDocumentSnapshot;
@@ -50,14 +52,15 @@ NS_ASSUME_NONNULL_BEGIN
/**
* Creates and returns a FSTFirestoreClient with the given parameters.
*
- * All callbacks and events will be triggered on the provided userDispatchQueue.
+ * All callbacks and events will be triggered on the provided userExecutor.
*/
-+ (instancetype)clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
- usePersistence:(BOOL)usePersistence
- credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
- credentialsProvider // no passing ownership
- userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
- workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue;
++ (instancetype)
+clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
+ usePersistence:(BOOL)usePersistence
+ credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
+ credentialsProvider // no passing ownership
+ userExecutor:(std::unique_ptr<firebase::firestore::util::internal::Executor>)userExecutor
+ workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue;
- (instancetype)init __attribute__((unavailable("Use static constructor method.")));
@@ -111,7 +114,7 @@ NS_ASSUME_NONNULL_BEGIN
* Dispatch queue for user callbacks / events. This will often be the "Main Dispatch Queue" of the
* app but the developer can configure it to a different queue if they so choose.
*/
-@property(nonatomic, strong, readonly) FSTDispatchQueue *userDispatchQueue;
+- (firebase::firestore::util::internal::Executor *)userExecutor;
@end
diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm
index 658cf57..93d1f7c 100644
--- a/Firestore/Source/Core/FSTFirestoreClient.mm
+++ b/Firestore/Source/Core/FSTFirestoreClient.mm
@@ -18,6 +18,7 @@
#include <future> // NOLINT(build/c++11)
#include <memory>
+#include <utility>
#import "FIRFirestoreErrors.h"
#import "Firestore/Source/API/FIRDocumentReference+Internal.h"
@@ -58,6 +59,8 @@ using firebase::firestore::core::DatabaseInfo;
using firebase::firestore::model::DatabaseId;
using firebase::firestore::model::DocumentKeySet;
+using firebase::firestore::util::internal::Executor;
+
NS_ASSUME_NONNULL_BEGIN
@interface FSTFirestoreClient () {
@@ -68,7 +71,7 @@ NS_ASSUME_NONNULL_BEGIN
usePersistence:(BOOL)usePersistence
credentialsProvider:
(CredentialsProvider *)credentialsProvider // no passing ownership
- userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+ userExecutor:(std::unique_ptr<Executor>)userExecutor
workerDispatchQueue:(FSTDispatchQueue *)queue NS_DESIGNATED_INITIALIZER;
@property(nonatomic, assign, readonly) const DatabaseInfo *databaseInfo;
@@ -91,18 +94,24 @@ NS_ASSUME_NONNULL_BEGIN
@end
-@implementation FSTFirestoreClient
+@implementation FSTFirestoreClient {
+ std::unique_ptr<Executor> _userExecutor;
+}
+
+- (Executor *)userExecutor {
+ return _userExecutor.get();
+}
+ (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
usePersistence:(BOOL)usePersistence
credentialsProvider:
(CredentialsProvider *)credentialsProvider // no passing ownership
- userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+ userExecutor:(std::unique_ptr<Executor>)userExecutor
workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
return [[FSTFirestoreClient alloc] initWithDatabaseInfo:databaseInfo
usePersistence:usePersistence
credentialsProvider:credentialsProvider
- userDispatchQueue:userDispatchQueue
+ userExecutor:std::move(userExecutor)
workerDispatchQueue:workerDispatchQueue];
}
@@ -110,12 +119,12 @@ NS_ASSUME_NONNULL_BEGIN
usePersistence:(BOOL)usePersistence
credentialsProvider:
(CredentialsProvider *)credentialsProvider // no passing ownership
- userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+ userExecutor:(std::unique_ptr<Executor>)userExecutor
workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
if (self = [super init]) {
_databaseInfo = databaseInfo;
_credentialsProvider = credentialsProvider;
- _userDispatchQueue = userDispatchQueue;
+ _userExecutor = std::move(userExecutor);
_workerDispatchQueue = workerDispatchQueue;
auto userPromise = std::make_shared<std::promise<User>>();
@@ -230,9 +239,7 @@ NS_ASSUME_NONNULL_BEGIN
[self.workerDispatchQueue dispatchAsync:^{
[self.remoteStore disableNetwork];
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(nil);
- }];
+ _userExecutor->Execute([=] { completion(nil); });
}
}];
}
@@ -241,9 +248,7 @@ NS_ASSUME_NONNULL_BEGIN
[self.workerDispatchQueue dispatchAsync:^{
[self.remoteStore enableNetwork];
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(nil);
- }];
+ _userExecutor->Execute([=] { completion(nil); });
}
}];
}
@@ -255,9 +260,7 @@ NS_ASSUME_NONNULL_BEGIN
[self.remoteStore shutdown];
[self.persistence shutdown];
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(nil);
- }];
+ _userExecutor->Execute([=] { completion(nil); });
}
}];
}
@@ -338,18 +341,14 @@ NS_ASSUME_NONNULL_BEGIN
[self.workerDispatchQueue dispatchAsync:^{
if (mutations.count == 0) {
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(nil);
- }];
+ _userExecutor->Execute([=] { completion(nil); });
}
} else {
[self.syncEngine writeMutations:mutations
completion:^(NSError *error) {
// Dispatch the result back onto the user dispatch queue.
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(error);
- }];
+ _userExecutor->Execute([=] { completion(error); });
}
}];
}
@@ -366,9 +365,7 @@ NS_ASSUME_NONNULL_BEGIN
completion:^(id _Nullable result, NSError *_Nullable error) {
// Dispatch the result back onto the user dispatch queue.
if (completion) {
- [self.userDispatchQueue dispatchAsync:^{
- completion(result, error);
- }];
+ _userExecutor->Execute([=] { completion(result, error); });
}
}];
}];
diff --git a/Firestore/Source/Util/FSTAsyncQueryListener.h b/Firestore/Source/Util/FSTAsyncQueryListener.h
index 4888268..06471d8 100644
--- a/Firestore/Source/Util/FSTAsyncQueryListener.h
+++ b/Firestore/Source/Util/FSTAsyncQueryListener.h
@@ -18,6 +18,8 @@
#import "Firestore/Source/Core/FSTViewSnapshot.h"
+#include "Firestore/core/src/firebase/firestore/util/executor.h"
+
NS_ASSUME_NONNULL_BEGIN
@class FSTDispatchQueue;
@@ -28,9 +30,8 @@ NS_ASSUME_NONNULL_BEGIN
*/
@interface FSTAsyncQueryListener : NSObject
-- (instancetype)initWithDispatchQueue:(FSTDispatchQueue *)dispatchQueue
- snapshotHandler:(FSTViewSnapshotHandler)snapshotHandler
- NS_DESIGNATED_INITIALIZER;
+- (instancetype)initWithExecutor:(firebase::firestore::util::internal::Executor*)executor
+ snapshotHandler:(FSTViewSnapshotHandler)snapshotHandler NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
diff --git a/Firestore/Source/Util/FSTAsyncQueryListener.mm b/Firestore/Source/Util/FSTAsyncQueryListener.mm
index b72ac57..426594b 100644
--- a/Firestore/Source/Util/FSTAsyncQueryListener.mm
+++ b/Firestore/Source/Util/FSTAsyncQueryListener.mm
@@ -18,16 +18,18 @@
#import "Firestore/Source/Util/FSTDispatchQueue.h"
+using firebase::firestore::util::internal::Executor;
+
@implementation FSTAsyncQueryListener {
volatile BOOL _muted;
FSTViewSnapshotHandler _snapshotHandler;
- FSTDispatchQueue *_dispatchQueue;
+ Executor *_executor;
}
-- (instancetype)initWithDispatchQueue:(FSTDispatchQueue *)dispatchQueue
- snapshotHandler:(FSTViewSnapshotHandler)snapshotHandler {
+- (instancetype)initWithExecutor:(Executor *)executor
+ snapshotHandler:(FSTViewSnapshotHandler)snapshotHandler {
if (self = [super init]) {
- _dispatchQueue = dispatchQueue;
+ _executor = executor;
_snapshotHandler = snapshotHandler;
}
return self;
@@ -40,11 +42,11 @@
// users just want to turn on notifications "forever" and don't want to have
// to keep track of our handle to keep them going.
return ^(FSTViewSnapshot *_Nullable snapshot, NSError *_Nullable error) {
- [self->_dispatchQueue dispatchAsync:^{
+ _executor->Execute([self, snapshot, error] {
if (!self->_muted) {
self->_snapshotHandler(snapshot, error);
}
- }];
+ });
};
}