Skip to content

Suggested fixes for cpp/port_auth #846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 0 additions & 2 deletions Firestore/Example/Tests/SpecTests/FSTMockDatastore.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property(nonatomic) int writeStreamRequestCount;

+ (instancetype)mockDatastoreWithWorkerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue;

#pragma mark - Watch Stream manipulation.

/** Injects an Added WatchChange containing the given targetIDs. */
Expand Down
16 changes: 0 additions & 16 deletions Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

#import "Firestore/Example/Tests/SpecTests/FSTMockDatastore.h"

#include <list>

#import "Firestore/Source/Core/FSTSnapshotVersion.h"
#import "Firestore/Source/Local/FSTQueryData.h"
#import "Firestore/Source/Model/FSTMutation.h"
Expand Down Expand Up @@ -290,20 +288,6 @@ @interface FSTMockDatastore ()

@implementation FSTMockDatastore

+ (instancetype)mockDatastoreWithWorkerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
// This owns the DatabaseInfos since we do not have FirestoreClient instance to own them.
static DatabaseInfo database_info{DatabaseId{"project", "database"}, "persistence", "host",
false};

// Note that we purposely don't bother to cleanup the EmptyCredentialsProvider instances.
static std::list<EmptyCredentialsProvider> credentials_providers;
credentials_providers.emplace_back();

return [[FSTMockDatastore alloc] initWithDatabaseInfo:&database_info
workerDispatchQueue:workerDispatchQueue
credentials:&credentials_providers.back()];
}

#pragma mark - Overridden FSTDatastore methods.

- (FSTWatchStream *)createWatchStream {
Expand Down
14 changes: 13 additions & 1 deletion Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,16 @@
#import "Firestore/Example/Tests/Core/FSTSyncEngine+Testing.h"
#import "Firestore/Example/Tests/SpecTests/FSTMockDatastore.h"

#include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h"
#include "Firestore/core/src/firebase/firestore/auth/user.h"
#include "Firestore/core/src/firebase/firestore/core/database_info.h"
#include "Firestore/core/src/firebase/firestore/model/database_id.h"

using firebase::firestore::auth::EmptyCredentialsProvider;
using firebase::firestore::auth::HashUser;
using firebase::firestore::auth::User;
using firebase::firestore::core::DatabaseInfo;
using firebase::firestore::model::DatabaseId;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -83,7 +89,9 @@ @implementation FSTSyncEngineTestDriver {
// ivar is declared as mutable.
std::unordered_map<User, NSMutableArray<FSTOutstandingWrite *> *, HashUser> _outstandingWrites;

DatabaseInfo _databaseInfo;
User _currentUser;
EmptyCredentialsProvider _credentialProvider;
}

- (instancetype)initWithPersistence:(id<FSTPersistence>)persistence
Expand All @@ -106,13 +114,17 @@ - (instancetype)initWithPersistence:(id<FSTPersistence>)persistence

_events = [NSMutableArray array];

_databaseInfo = {DatabaseId{"project", "database"}, "persistence", "host", false};

// Set up the sync engine and various stores.
dispatch_queue_t mainQueue = dispatch_get_main_queue();
FSTDispatchQueue *dispatchQueue = [FSTDispatchQueue queueWith:mainQueue];
_localStore = [[FSTLocalStore alloc] initWithPersistence:persistence
garbageCollector:garbageCollector
initialUser:initialUser];
_datastore = [FSTMockDatastore mockDatastoreWithWorkerDispatchQueue:dispatchQueue];
_datastore = [[FSTMockDatastore alloc] initWithDatabaseInfo:&_databaseInfo
workerDispatchQueue:dispatchQueue
credentials:&_credentialProvider];

_remoteStore = [FSTRemoteStore remoteStoreWithLocalStore:_localStore datastore:_datastore];

Expand Down
6 changes: 5 additions & 1 deletion Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"

#include <memory>
#include <utility>

#import <FirebaseCore/FIRLogger.h>
#import <FirebaseFirestore/FirebaseFirestore-umbrella.h>
Expand All @@ -27,6 +28,7 @@
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
#include "Firestore/core/src/firebase/firestore/util/autoid.h"
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
#include "absl/memory/memory.h"

#import "Firestore/Source/API/FIRFirestore+Internal.h"
#import "Firestore/Source/Core/FSTFirestoreClient.h"
Expand Down Expand Up @@ -141,7 +143,9 @@ - (FIRFirestore *)firestoreWithProjectID:(NSString *)projectID {
FIRSetLoggerLevel(FIRLoggerLevelDebug);
// HACK: FIRFirestore expects a non-nil app, but for tests we cheat.
FIRApp *app = nil;
std::unique_ptr<CredentialsProvider> credentials_provider(new EmptyCredentialsProvider());
std::unique_ptr<CredentialsProvider> credentials_provider =
absl::make_unique<firebase::firestore::auth::EmptyCredentialsProvider>();

FIRFirestore *firestore = [[FIRFirestore alloc] initWithProjectID:util::MakeStringView(projectID)
database:DatabaseId::kDefault
persistenceKey:persistenceKey
Expand Down
4 changes: 3 additions & 1 deletion Firestore/Source/API/FIRFirestore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import "FIRFirestore.h"

#include <memory>
#include <utility>

#import <FirebaseCore/FIRApp.h>
#import <FirebaseCore/FIRAppInternal.h>
Expand Down Expand Up @@ -44,6 +45,7 @@
#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/string_apple.h"
#include "absl/memory/memory.h"

namespace util = firebase::firestore::util;
using firebase::firestore::auth::CredentialsProvider;
Expand Down Expand Up @@ -159,7 +161,7 @@ + (instancetype)firestoreForApp:(FIRApp *)app database:(NSString *)database {
queueWith:dispatch_queue_create("com.google.firebase.firestore", DISPATCH_QUEUE_SERIAL)];

std::unique_ptr<CredentialsProvider> credentials_provider =
std::make_unique<FirebaseCredentialsProvider>(app);
absl::make_unique<FirebaseCredentialsProvider>(app);

NSString *persistenceKey = app.name;

Expand Down
42 changes: 20 additions & 22 deletions Firestore/Source/Core/FSTFirestoreClient.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#import "Firestore/Source/Core/FSTFirestoreClient.h"

#import <future>

#import "Firestore/Source/Core/FSTEventManager.h"
#import "Firestore/Source/Core/FSTSyncEngine.h"
#import "Firestore/Source/Core/FSTTransaction.h"
Expand Down Expand Up @@ -104,35 +106,31 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
_userDispatchQueue = userDispatchQueue;
_workerDispatchQueue = workerDispatchQueue;

dispatch_semaphore_t initialUserAvailable = dispatch_semaphore_create(0);
__block bool initialized = false;
__block User initialUser;
FSTWeakify(self);
auto userChangeListener = ^(User user) {
FSTStrongify(self);
if (self) {
if (!initialized) {
initialUser = user;
initialized = true;
dispatch_semaphore_signal(initialUserAvailable);
} else {
[workerDispatchQueue dispatchAsync:^{
[self userDidChange:user];
}];
}
auto userPromise = std::make_shared<std::promise<User>>();

__weak typeof(self) weakSelf = self;
auto userChangeListener = [initialized = false, userPromise, weakSelf, workerDispatchQueue](User user) mutable {
typeof(self) strongSelf = weakSelf;
if (!strongSelf) return;

if (!initialized) {
initialized = true;
userPromise->set_value(user);
} else {
[workerDispatchQueue dispatchAsync:^{
[strongSelf userDidChange:user];
}];
}
};

_credentialsProvider->SetUserChangeListener(
[userChangeListener](const User &user) { userChangeListener(user); });
_credentialsProvider->SetUserChangeListener(userChangeListener);

// Defer initialization until we get the current user from the userChangeListener. This is
// guaranteed to be synchronously dispatched onto our worker queue, so we will be initialized
// before any subsequently queued work runs.
[_workerDispatchQueue dispatchAsync:^{
dispatch_semaphore_wait(initialUserAvailable, DISPATCH_TIME_FOREVER);

[self initializeWithUser:initialUser usePersistence:usePersistence];
User user = userPromise->get_future().get();
[self initializeWithUser:user usePersistence:usePersistence];
}];
}
return self;
Expand Down Expand Up @@ -237,7 +235,7 @@ - (void)enableNetworkWithCompletion:(nullable FSTVoidErrorBlock)completion {

- (void)shutdownWithCompletion:(nullable FSTVoidErrorBlock)completion {
[self.workerDispatchQueue dispatchAsync:^{
_credentialsProvider->SetUserChangeListener(nullptr);
self->_credentialsProvider->SetUserChangeListener(nullptr);

[self.remoteStore shutdown];
[self.localStore shutdown];
Expand Down