Pass database_id from GetInstance() down to CreateFirestore() (#1270)

Author: milaGGL

firestore/integration_test_internal/src/util/integration_test_util.cc

@@ -37,7 +37,8 @@ struct TestFriend {
   static FirestoreInternal* CreateTestFirestoreInternal(App* app) {
 #if !defined(__ANDROID__)
     return new FirestoreInternal(
-        app, absl::make_unique<credentials::EmptyAuthCredentialsProvider>(),
+        app, /*database_id=*/"(default)",
+        absl::make_unique<credentials::EmptyAuthCredentialsProvider>(),
         absl::make_unique<credentials::EmptyAppCheckCredentialsProvider>());
 #else
     return new FirestoreInternal(app);

firestore/integration_test_internal/src/validation_test.cc

@@ -440,6 +440,14 @@ TEST_F(ValidationTest,
                "database ID.");
 }
 
+TEST_F(ValidationTest,
+       FirestoreGetInstanceWithNoArgumentsReturnsNonNullInstance) {
+  InitResult result;
+  EXPECT_NO_THROW(Firestore::GetInstance(&result));
+  EXPECT_EQ(kInitResultSuccess, result);
+  EXPECT_NE(Firestore::GetInstance(), nullptr);
+}
+
 TEST_F(ValidationTest,
        FirestoreGetInstanceWithNonNullAppReturnsNonNullInstance) {
   InitResult result;
@@ -448,6 +456,22 @@ TEST_F(ValidationTest,
   EXPECT_NE(Firestore::GetInstance(app()), nullptr);
 }
 
+TEST_F(ValidationTest,
+       FirestoreGetInstanceWithAppAndDatabaseNameReturnsNonNullInstance) {
+  InitResult result;
+  EXPECT_NO_THROW(Firestore::GetInstance(app(), "foo", &result));
+  EXPECT_EQ(kInitResultSuccess, result);
+  EXPECT_NE(Firestore::GetInstance(app(), "foo"), nullptr);
+}
+
+TEST_F(ValidationTest,
+       FirestoreGetInstanceWithDatabaseNameReturnsNonNullInstance) {
+  InitResult result;
+  EXPECT_NO_THROW(Firestore::GetInstance("foo", &result));
+  EXPECT_EQ(kInitResultSuccess, result);
+  EXPECT_NE(Firestore::GetInstance("foo"), nullptr);
+}
+
 TEST_F(ValidationTest, CollectionPathsMustBeOddLength) {
   Firestore* db = TestFirestore();
   DocumentReference base_document = db->Document("foo/bar");

firestore/src/common/firestore.cc

@@ -118,45 +118,44 @@ void ValidateDatabase(const char* db_name) {
 
 }  // namespace
 
-Firestore* Firestore::GetInstance(App* app, InitResult* init_result_out) {
-  ValidateApp(app);
-
-  MutexLock lock(*g_firestores_lock);
-
-  Firestore* from_cache = FindFirestoreInCache(app, init_result_out);
-  if (from_cache) {
-    return from_cache;
+Firestore* Firestore::GetInstance(InitResult* init_result_out) {
+  App* app = App::GetInstance();
+  if (!app) {
+    SimpleThrowInvalidArgument(
+        "Failed to get firebase::App instance. Please call "
+        "firebase::App::Create before using Firestore");
   }
+  return Firestore::GetInstance(app, kDefaultDatabase, init_result_out);
+}
 
-  return AddFirestoreToCache(new Firestore(app), init_result_out);
+Firestore* Firestore::GetInstance(App* app, InitResult* init_result_out) {
+  return Firestore::GetInstance(app, kDefaultDatabase, init_result_out);
 }
 
-Firestore* Firestore::GetInstance(InitResult* init_result_out) {
+Firestore* Firestore::GetInstance(const char* db_name,
+                                  InitResult* init_result_out) {
   App* app = App::GetInstance();
   if (!app) {
     SimpleThrowInvalidArgument(
         "Failed to get firebase::App instance. Please call "
         "firebase::App::Create before using Firestore");
   }
-
-  return Firestore::GetInstance(app, init_result_out);
+  return Firestore::GetInstance(app, db_name, init_result_out);
 }
 
 Firestore* Firestore::GetInstance(App* app,
                                   const char* db_name,
                                   InitResult* init_result_out) {
   ValidateApp(app);
   ValidateDatabase(db_name);
-  // TODO(Mila): Implement code
-  return Firestore::GetInstance(app, init_result_out);
-}
 
-Firestore* Firestore::GetInstance(const char* db_name,
-                                  InitResult* init_result_out) {
-  ValidateDatabase(db_name);
-  // TODO(Mila): Implement code
-  App* app = App::GetInstance();
-  return Firestore::GetInstance(app, init_result_out);
+  MutexLock lock(*g_firestores_lock);
+  Firestore* from_cache = FindFirestoreInCache(app, init_result_out);
+  if (from_cache) {
+    return from_cache;
+  }
+
+  return AddFirestoreToCache(new Firestore(app, db_name), init_result_out);
 }
 
 Firestore* Firestore::CreateFirestore(App* app,
@@ -190,8 +189,8 @@ Firestore* Firestore::AddFirestoreToCache(Firestore* firestore,
   return firestore;
 }
 
-Firestore::Firestore(::firebase::App* app)
-    : Firestore{new FirestoreInternal{app}} {}
+Firestore::Firestore(::firebase::App* app, const char* database_id)
+    : Firestore{new FirestoreInternal{app, database_id}} {}
 
 Firestore::Firestore(FirestoreInternal* internal)
     // TODO(wuandy): use make_unique once it is supported for our build here.

firestore/src/include/firebase/firestore.h

@@ -72,6 +72,9 @@ class TransactionManager;
 
 }  // namespace csharp
 
+/** The default name for "unset" database ID in resource names. */
+static const char* kDefaultDatabase = "(default)";
+
 /**
  * @brief Entry point for the Firebase Firestore C++ SDK.
  *
@@ -492,7 +495,8 @@ class Firestore {
   friend class csharp::ApiHeaders;
   friend class csharp::TransactionManager;
 
-  explicit Firestore(::firebase::App* app);
+  explicit Firestore(::firebase::App* app,
+                     const char* database_id = kDefaultDatabase);
   explicit Firestore(FirestoreInternal* internal);
 
   static Firestore* CreateFirestore(::firebase::App* app,

firestore/src/main/firestore_main.cc

@@ -101,17 +101,20 @@ void ValidateDoubleSlash(const char* path) {
 
 }  // namespace
 
-FirestoreInternal::FirestoreInternal(App* app)
-    : FirestoreInternal{app, CreateCredentialsProvider(*app),
+FirestoreInternal::FirestoreInternal(App* app, const char* database_id)
+    : FirestoreInternal{app, database_id, CreateCredentialsProvider(*app),
                         CreateAppCheckCredentialsProvider(*app)} {}
 
 FirestoreInternal::FirestoreInternal(
     App* app,
+    const char* database_id,
     std::unique_ptr<AuthCredentialsProvider> auth_credentials,
     std::unique_ptr<AppCheckCredentialsProvider> app_check_credentials)
     : app_(NOT_NULL(app)),
-      firestore_core_(CreateFirestore(
-          app, std::move(auth_credentials), std::move(app_check_credentials))),
+      firestore_core_(CreateFirestore(app,
+                                      database_id,
+                                      std::move(auth_credentials),
+                                      std::move(app_check_credentials))),
       transaction_executor_(absl::ShareUniquePtr(Executor::CreateConcurrent(
           "com.google.firebase.firestore.transaction", /*threads=*/5))) {
   ApplyDefaultSettings();
@@ -131,13 +134,14 @@ FirestoreInternal::~FirestoreInternal() {
 
 std::shared_ptr<api::Firestore> FirestoreInternal::CreateFirestore(
     App* app,
+    const char* database_id,
     std::unique_ptr<AuthCredentialsProvider> auth_credentials,
     std::unique_ptr<AppCheckCredentialsProvider> app_check_credentials) {
   const AppOptions& opt = app->options();
   return std::make_shared<api::Firestore>(
-      DatabaseId{opt.project_id()}, app->name(), std::move(auth_credentials),
-      std::move(app_check_credentials), CreateWorkerQueue(),
-      CreateFirebaseMetadataProvider(*app), this);
+      DatabaseId{opt.project_id(), database_id}, app->name(),
+      std::move(auth_credentials), std::move(app_check_credentials),
+      CreateWorkerQueue(), CreateFirebaseMetadataProvider(*app), this);
 }
 
 CollectionReference FirestoreInternal::Collection(

firestore/src/main/firestore_main.h

@@ -55,7 +55,7 @@ class Executor;
 class FirestoreInternal {
  public:
   // Note: call `set_firestore_public` immediately after construction.
-  explicit FirestoreInternal(App* app);
+  FirestoreInternal(App* app, const char* database_id);
   ~FirestoreInternal();
 
   FirestoreInternal(const FirestoreInternal&) = delete;
@@ -150,12 +150,14 @@ class FirestoreInternal {
 
   FirestoreInternal(
       App* app,
+      const char* database_id,
       std::unique_ptr<credentials::AuthCredentialsProvider> auth_credentials,
       std::unique_ptr<credentials::AppCheckCredentialsProvider>
           app_check_credentials);
 
   std::shared_ptr<api::Firestore> CreateFirestore(
       App* app,
+      const char* database_id,
       std::unique_ptr<credentials::AuthCredentialsProvider> auth_credentials,
       std::unique_ptr<credentials::AppCheckCredentialsProvider>
           app_check_credentials);