Port FirestoreSettings to C++.

Firestore/Source/API/FIRDocumentSnapshot.mm

@@ -20,8 +20,6 @@
 
 #include "Firestore/core/src/firebase/firestore/util/warnings.h"
 
-#import "FIRFirestoreSettings.h"
-
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRFieldPath+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -32,6 +30,7 @@
 #include "Firestore/core/src/firebase/firestore/api/document_snapshot.h"
 #include "Firestore/core/src/firebase/firestore/api/firestore.h"
 #include "Firestore/core/src/firebase/firestore/api/input_validation.h"
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/model/field_value.h"
@@ -184,7 +183,7 @@ - (FSTFieldValueOptions *)optionsForServerTimestampBehavior:
       initWithServerTimestampBehavior:InternalServerTimestampBehavior(serverTimestampBehavior)
          timestampsInSnapshotsEnabled:_snapshot.firestore()
                                           ->settings()
-                                          .timestampsInSnapshotsEnabled];
+                                          .timestamps_in_snapshots_enabled()];
   SUPPRESS_END()
 }
 

Firestore/Source/API/FIRFirestore.mm

@@ -26,7 +26,7 @@
 #include <string>
 #include <utility>
 
-#import "FIRFirestore.h"
+#import "FIRFirestoreSettings+Internal.h"
 
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -40,6 +40,7 @@
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
 #include "Firestore/core/src/firebase/firestore/util/delayed_constructor.h"
+#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
 #include "Firestore/core/src/firebase/firestore/util/log.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
@@ -65,6 +66,7 @@ @interface FIRFirestore ()
 
 @implementation FIRFirestore {
   DelayedConstructor<Firestore> _firestore;
+  FIRFirestoreSettings *_settings;
 }
 
 + (NSMutableDictionary<NSString *, FIRFirestore *> *)instances {
@@ -160,16 +162,26 @@ - (instancetype)initWithProjectID:(std::string)projectID
 
     _dataConverter = [[FSTUserDataConverter alloc] initWithDatabaseID:&_firestore->database_id()
                                                          preConverter:block];
+    // Use the property setter so the default settings get plumbed into _firestoreClient.
+    self.settings = [[FIRFirestoreSettings alloc] init];
   }
   return self;
 }
 
 - (FIRFirestoreSettings *)settings {
-  return _firestore->settings();
+  // Disallow mutation of our internal settings
+  return [_settings copy];
 }
 
 - (void)setSettings:(FIRFirestoreSettings *)settings {
-  _firestore->set_settings(settings);
+  if (![settings isEqual:_settings]) {
+    _settings = settings;
+    _firestore->set_settings([settings internalSettings]);
+
+    std::unique_ptr<util::Executor> user_executor =
+        absl::make_unique<util::ExecutorLibdispatch>(settings.dispatchQueue);
+    _firestore->set_user_executor(std::move(user_executor));
+  }
 }
 
 - (FIRCollectionReference *)collectionWithPath:(NSString *)collectionPath {

Firestore/Source/API/FIRFirestoreSettings+Internal.h

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2019 Google
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import "FIRFirestoreSettings.h"
+
+#import <Foundation/Foundation.h>
+
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface FIRFirestoreSettings (Internal)
+
+/** Converts this FIRFirestoreSettings instance into an api::Settings object. */
+- (firebase::firestore::api::Settings)internalSettings;
+
+@end
+
+NS_ASSUME_NONNULL_END

Firestore/Source/API/FIRFirestoreSettings.mm

@@ -17,11 +17,16 @@
 #import "FIRFirestoreSettings.h"
 
 #include "Firestore/core/src/firebase/firestore/api/input_validation.h"
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
+#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "Firestore/core/src/firebase/firestore/util/warnings.h"
+#include "absl/memory/memory.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
+using firebase::firestore::api::Settings;
 using firebase::firestore::api::ThrowInvalidArgument;
+using firebase::firestore::util::MakeString;
 
 static NSString *const kDefaultHost = @"firestore.googleapis.com";
 static const BOOL kDefaultSSLEnabled = YES;
@@ -114,6 +119,16 @@ - (void)setCacheSizeBytes:(int64_t)cacheSizeBytes {
   _cacheSizeBytes = cacheSizeBytes;
 }
 
+- (Settings)internalSettings {
+  Settings settings;
+  settings.set_host(MakeString(_host));
+  settings.set_ssl_enabled(_sslEnabled);
+  settings.set_persistence_enabled(_persistenceEnabled);
+  settings.set_timestamps_in_snapshots_enabled(_timestampsInSnapshotsEnabled);
+  settings.set_cache_size_bytes(_cacheSizeBytes);
+  return settings;
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Source/Core/FSTFirestoreClient.h

@@ -23,6 +23,7 @@
 
 #include "Firestore/core/src/firebase/firestore/api/document_reference.h"
 #include "Firestore/core/src/firebase/firestore/api/document_snapshot.h"
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
 #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/core/listen_options.h"
@@ -35,7 +36,6 @@
 
 @class FIRDocumentReference;
 @class FIRDocumentSnapshot;
-@class FIRFirestoreSettings;
 @class FIRQuery;
 @class FIRQuerySnapshot;
 @class FSTDatabaseID;
@@ -66,7 +66,7 @@ NS_ASSUME_NONNULL_BEGIN
  * All callbacks and events will be triggered on the provided userExecutor.
  */
 + (instancetype)clientWithDatabaseInfo:(const core::DatabaseInfo &)databaseInfo
-                              settings:(FIRFirestoreSettings *)settings
+                              settings:(const api::Settings &)settings
                    credentialsProvider:
                        (auth::CredentialsProvider *)credentialsProvider  // no passing ownership
                           userExecutor:(std::unique_ptr<util::Executor>)userExecutor

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -22,7 +22,6 @@
 #include <utility>
 
 #import "FIRFirestoreErrors.h"
-#import "FIRFirestoreSettings.h"
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -42,6 +41,7 @@
 #import "Firestore/Source/Remote/FSTSerializerBeta.h"
 #import "Firestore/Source/Util/FSTClasses.h"
 
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
 #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"
@@ -60,6 +60,7 @@
 using firebase::firestore::FirestoreErrorCode;
 using firebase::firestore::api::DocumentReference;
 using firebase::firestore::api::DocumentSnapshot;
+using firebase::firestore::api::Settings;
 using firebase::firestore::api::SnapshotMetadata;
 using firebase::firestore::auth::CredentialsProvider;
 using firebase::firestore::auth::User;
@@ -97,7 +98,7 @@ @interface FSTFirestoreClient () {
 }
 
 - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                            settings:(FIRFirestoreSettings *)settings
+                            settings:(const Settings &)settings
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
                         userExecutor:(std::unique_ptr<Executor>)userExecutor
@@ -142,7 +143,7 @@ - (AsyncQueue *)workerQueue {
 }
 
 + (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                              settings:(FIRFirestoreSettings *)settings
+                              settings:(const Settings &)settings
                    credentialsProvider:
                        (CredentialsProvider *)credentialsProvider  // no passing ownership
                           userExecutor:(std::unique_ptr<Executor>)userExecutor
@@ -155,7 +156,7 @@ + (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
 }
 
 - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                            settings:(FIRFirestoreSettings *)settings
+                            settings:(const Settings &)settings
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
                         userExecutor:(std::unique_ptr<Executor>)userExecutor
@@ -199,15 +200,15 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
   return self;
 }
 
-- (void)initializeWithUser:(const User &)user settings:(FIRFirestoreSettings *)settings {
+- (void)initializeWithUser:(const User &)user settings:(const Settings &)settings {
   // Do all of our initialization on our own dispatch queue.
   _workerQueue->VerifyIsCurrentQueue();
   LOG_DEBUG("Initializing. Current user: %s", user.uid());
 
   // Note: The initialization work must all be synchronous (we can't dispatch more work) since
   // external write/listen operations could get queued to run before that subsequent work
   // completes.
-  if (settings.isPersistenceEnabled) {
+  if (settings.persistence_enabled()) {
     Path dir = [FSTLevelDB storageDirectoryForDatabaseInfo:*self.databaseInfo
                                         documentsDirectory:[FSTLevelDB documentsDirectory]];
 
@@ -219,7 +220,7 @@ - (void)initializeWithUser:(const User &)user settings:(FIRFirestoreSettings *)s
     Status levelDbStatus =
         [FSTLevelDB dbWithDirectory:std::move(dir)
                          serializer:serializer
-                          lruParams:LruParams::WithCacheSize(settings.cacheSizeBytes)
+                          lruParams:LruParams::WithCacheSize(settings.cache_size_bytes())
                                 ptr:&ldb];
     if (!levelDbStatus.ok()) {
       // If leveldb fails to start then just throw up our hands: the error is unrecoverable.

Firestore/core/src/firebase/firestore/api/CMakeLists.txt

@@ -39,6 +39,8 @@ cc_select(
 cc_library(
   firebase_firestore_api
   SOURCES
+    settings.cc
+    settings.h
     snapshot_metadata.cc
     snapshot_metadata.h
   DEPENDS

Firestore/core/src/firebase/firestore/api/firestore.h

@@ -29,6 +29,7 @@
 #include <utility>
 #include "dispatch/dispatch.h"
 
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
 #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
@@ -39,7 +40,6 @@ NS_ASSUME_NONNULL_BEGIN
 @class FIRApp;
 @class FIRCollectionReference;
 @class FIRFirestore;
-@class FIRFirestoreSettings;
 @class FIRQuery;
 @class FIRTransaction;
 @class FIRWriteBatch;
@@ -86,8 +86,10 @@ class Firestore {
     return extension_;
   }
 
-  FIRFirestoreSettings* settings() const;
-  void set_settings(FIRFirestoreSettings* settings);
+  const Settings& settings() const;
+  void set_settings(const Settings& settings);
+
+  void set_user_executor(std::unique_ptr<util::Executor> user_executor);
 
   FIRCollectionReference* GetCollection(absl::string_view collection_path);
   DocumentReference GetDocument(absl::string_view document_path);
@@ -111,13 +113,14 @@ class Firestore {
   std::string persistence_key_;
   FSTFirestoreClient* client_ = nil;
 
-  // Ownership will be transferred to `FSTFirestoreClient` as soon as the
-  // client is created.
+  // Ownership of these will be transferred to `FSTFirestoreClient` as soon as
+  // the client is created.
+  std::unique_ptr<util::Executor> user_executor_;
   std::unique_ptr<util::AsyncQueue> worker_queue_;
 
   void* extension_ = nullptr;
 
-  FIRFirestoreSettings* settings_ = nil;
+  Settings settings_;
 
   mutable std::mutex mutex_;
 };

Firestore/core/src/firebase/firestore/api/firestore.mm

@@ -16,7 +16,6 @@
 
 #include "Firestore/core/src/firebase/firestore/api/firestore.h"
 
-#import "FIRFirestoreSettings.h"
 #import "Firestore/Source/API/FIRCollectionReference+Internal.h"
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -27,6 +26,7 @@
 #import "Firestore/Source/Core/FSTQuery.h"
 
 #include "Firestore/core/src/firebase/firestore/api/document_reference.h"
+#include "Firestore/core/src/firebase/firestore/api/settings.h"
 #include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h"
 #include "Firestore/core/src/firebase/firestore/core/transaction.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
@@ -61,31 +61,35 @@
       persistence_key_{std::move(persistence_key)},
       worker_queue_{std::move(worker_queue)},
       extension_{extension} {
-  settings_ = [[FIRFirestoreSettings alloc] init];
 }
 
 AsyncQueue* Firestore::worker_queue() {
   return [client_ workerQueue];
 }
 
-FIRFirestoreSettings* Firestore::settings() const {
+const Settings& Firestore::settings() const {
   std::lock_guard<std::mutex> lock{mutex_};
-  // Disallow mutation of our internal settings
-  return [settings_ copy];
+  return settings_;
 }
 
-void Firestore::set_settings(FIRFirestoreSettings* settings) {
+void Firestore::set_settings(const Settings& settings) {
   std::lock_guard<std::mutex> lock{mutex_};
-  // As a special exception, don't throw if the same settings are passed
-  // repeatedly. This should make it more friendly to create a Firestore
-  // instance.
-  if (client_ && ![settings_ isEqual:settings]) {
+  if (client_) {
     HARD_FAIL(
         "Firestore instance has already been started and its settings can "
         "no longer be changed. You can only set settings before calling any "
         "other methods on a Firestore instance.");
   }
-  settings_ = [settings copy];
+  settings_ = settings;
+}
+
+void Firestore::set_user_executor(
+    std::unique_ptr<util::Executor> user_executor) {
+  std::lock_guard<std::mutex> lock{mutex_};
+  HARD_ASSERT(!client_ && user_executor,
+              "set_user_executor() must be called with a valid executor, "
+              "before the client is initialized.");
+  user_executor_ = std::move(user_executor);
 }
 
 FIRCollectionReference* Firestore::GetCollection(
@@ -175,24 +179,15 @@
   std::lock_guard<std::mutex> lock{mutex_};
 
   if (!client_) {
-    // These values are validated elsewhere; this is just double-checking:
-    HARD_ASSERT(settings_.host, "FirestoreSettings.host cannot be nil.");
-    HARD_ASSERT(settings_.dispatchQueue,
-                "FirestoreSettings.dispatchQueue cannot be nil.");
-
-    DatabaseInfo database_info(database_id_, persistence_key_,
-                               util::MakeString(settings_.host),
-                               settings_.sslEnabled);
-
-    std::unique_ptr<Executor> user_executor =
-        absl::make_unique<ExecutorLibdispatch>(settings_.dispatchQueue);
+    DatabaseInfo database_info(database_id_, persistence_key_, settings_.host(),
+                               settings_.ssl_enabled());
 
     HARD_ASSERT(worker_queue_, "Expected non-null worker queue");
     client_ =
         [FSTFirestoreClient clientWithDatabaseInfo:database_info
                                           settings:settings_
                                credentialsProvider:credentials_provider_.get()
-                                      userExecutor:std::move(user_executor)
+                                      userExecutor:std::move(user_executor_)
                                        workerQueue:std::move(worker_queue_)];
   }
 }