Implement MultiDB Support and Tests (#10465)

Author: tom-andersen

Firestore/CHANGELOG.md

@@ -2,6 +2,7 @@
 - [fixed] Fix FAILED_PRECONDITION when writing to a deleted document in a
   transaction (#10431).
 - [fixed] Fixed data race in credentials provider (#10393).
+- [feature] Add MultiDb support.
 - [fixed] Fix Firestore failing to raise initial snapshot from empty local cache
   result (#10437).
 

Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm

@@ -25,9 +25,13 @@
 
 #include "Firestore/core/src/api/query_snapshot.h"
 #include "Firestore/core/src/core/firestore_client.h"
+#include "Firestore/core/src/model/database_id.h"
+#include "Firestore/core/src/util/string_apple.h"
 #include "Firestore/core/test/unit/testutil/app_testing.h"
 
+using firebase::firestore::model::DatabaseId;
 using firebase::firestore::testutil::AppForUnitTesting;
+using firebase::firestore::util::MakeNSString;
 using firebase::firestore::util::MakeString;
 using firebase::firestore::util::TimerId;
 
@@ -36,6 +40,11 @@ @interface FIRDatabaseTests : FSTIntegrationTestCase
 
 @implementation FIRDatabaseTests
 
+- (void)tearDown {
+  [FIRApp resetApps];
+  [super tearDown];
+}
+
 - (void)testCanUpdateAnExistingDocument {
   FIRDocumentReference *doc = [self.db documentWithPath:@"rooms/eros"];
   NSDictionary<NSString *, id> *initialData =
@@ -1744,4 +1753,37 @@ - (void)testWaitForPendingWritesCompletesWhenOfflineIfNoPending {
   [self awaitExpectations];
 }
 
+- (void)testDefaultNamedDbIsSame {
+  [FIRApp configure];
+  FIRApp *app = [FIRApp defaultApp];
+  FIRFirestore *db1 = [FIRFirestore firestore];
+  FIRFirestore *db2 = [FIRFirestore firestoreForApp:app];
+  FIRFirestore *db3 = [FIRFirestore firestoreForApp:app database:@"(default)"];
+  FIRFirestore *db4 = [FIRFirestore firestoreForDatabase:@"(default)"];
+
+  XCTAssertIdentical(db1, db2);
+  XCTAssertIdentical(db1, db3);
+  XCTAssertIdentical(db1, db4);
+}
+
+- (void)testSameNamedDbIsSame {
+  [FIRApp configure];
+  FIRApp *app = [FIRApp defaultApp];
+  FIRFirestore *db1 = [FIRFirestore firestoreForApp:app database:@"myDb"];
+  FIRFirestore *db2 = [FIRFirestore firestoreForDatabase:@"myDb"];
+
+  XCTAssertIdentical(db1, db2);
+}
+
+- (void)testNamedDbHaveDifferentInstance {
+  [FIRApp configure];
+  FIRFirestore *db1 = [FIRFirestore firestore];
+  FIRFirestore *db2 = [FIRFirestore firestoreForDatabase:@"db1"];
+  FIRFirestore *db3 = [FIRFirestore firestoreForDatabase:@"db2"];
+
+  XCTAssertNotIdentical(db1, db2);
+  XCTAssertNotIdentical(db1, db3);
+  XCTAssertNotIdentical(db2, db3);
+}
+
 @end

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

@@ -19,11 +19,13 @@
 #import <XCTest/XCTest.h>
 #include <limits>
 
+#import "FirebaseCore/Extension/FIRAppInternal.h"
 #import "FirebaseCore/Extension/FIROptionsInternal.h"
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 #import "Firestore/Source/API/FIRFieldValue+Internal.h"
 #import "Firestore/Source/API/FIRFilter+Internal.h"
+#import "Firestore/Source/API/FIRFirestore+Internal.h"
 #import "Firestore/Source/API/FIRQuery+Internal.h"
 #include "Firestore/core/test/unit/testutil/app_testing.h"
 
@@ -38,6 +40,11 @@ @interface FIRValidationTests : FSTIntegrationTestCase
 
 @implementation FIRValidationTests
 
+- (void)tearDown {
+  [FIRApp resetApps];
+  [super tearDown];
+}
+
 #pragma mark - FIRFirestoreSettings Validation
 
 - (void)testNilHostFails {
@@ -74,6 +81,27 @@ - (void)testNilFIRAppFails {
        "default FirebaseApp instance.");
 }
 
+- (void)testNilFIRAppDatabaseFails1 {
+  FIRApp *app = AppForUnitTesting();
+  FSTAssertThrows(
+      [FIRFirestore firestoreForApp:app database:nil],
+      @"Database identifier may not be nil. Use '(default)' if you want the default database");
+}
+
+- (void)testNilFIRAppDatabaseFails2 {
+  FSTAssertThrows(
+      [FIRFirestore firestoreForApp:nil database:@"NotNil"],
+      @"FirebaseApp instance may not be nil. Use FirebaseApp.app() if you'd like to use the "
+       "default FirebaseApp instance.");
+}
+
+- (void)testNilDatabaseFails {
+  [FIRApp configure];
+  FSTAssertThrows(
+      [FIRFirestore firestoreForDatabase:nil],
+      @"Database identifier may not be nil. Use '(default)' if you want the default database");
+}
+
 - (void)testNilProjectIDFails {
   FIROptions *options = OptionsForUnitTesting("ignored");
   options.projectID = nil;
@@ -82,8 +110,6 @@ - (void)testNilProjectIDFails {
                   @"FIROptions.projectID must be set to a valid project ID.");
 }
 
-// TODO(b/62410906): Test for firestoreForApp:database: with nil DatabaseID.
-
 - (void)testNilTransactionBlocksFail {
   FSTAssertThrows([self.db runTransactionWithBlock:nil
                                         completion:^(id, NSError *) {

Firestore/Example/Tests/Integration/CMakeLists.txt

@@ -16,7 +16,7 @@ if(NOT FIREBASE_IOS_BUILD_TESTS OR NOT APPLE)
   return()
 endif()
 
-file(GLOB sources *.h *.mm API/*.h API/*.mm)
+file(GLOB sources *.h *.mm API/*.h API/*.mm ../../App/GoogleService-Info.plist)
 
 firebase_ios_add_objc_test(
   firestore_objc_integration_test
@@ -30,3 +30,8 @@ target_link_libraries(
   firestore_core
   firestore_objc_testing
 )
+
+set_target_properties(
+  firestore_objc_integration_test PROPERTIES
+  RESOURCE ../../App/GoogleService-Info.plist
+)

Firestore/Source/API/FIRFirestore+Internal.h

@@ -83,6 +83,30 @@ NS_ASSUME_NONNULL_BEGIN
 @property(nonatomic, assign, readonly) const model::DatabaseId &databaseID;
 @property(nonatomic, strong, readonly) FSTUserDataReader *dataReader;
 
+/**
+ * Creates, caches, and returns named `Firestore` object for the specified `FirebaseApp`. Each
+ * subsequent invocation returns the same `Firestore` object.
+ *
+ * @param app The `FirebaseApp` instance to use for authentication and as a source of the Google
+ * Cloud Project ID for your Firestore Database. If you want the default instance, you should
+ * explicitly set it to `FirebaseApp.app()`.
+ * @param database The database name.
+ *
+ * @return The named `Firestore` instance.
+ */
++ (instancetype)firestoreForApp:(FIRApp *)app
+                       database:(NSString *)database NS_SWIFT_NAME(firestore(app:database:));
+
+/**
+ * Creates, caches, and returns named `Firestore` object for the default _app_. Each subsequent
+ * invocation returns the same `Firestore` object.
+ *
+ * @param database The database name.
+ *
+ * @return The named `Firestore` instance.
+ */
++ (instancetype)firestoreForDatabase:(NSString *)database NS_SWIFT_NAME(firestore(database:));
+
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Source/API/FIRFirestore.mm

@@ -74,6 +74,8 @@
 using firebase::firestore::util::Empty;
 using firebase::firestore::util::Executor;
 using firebase::firestore::util::ExecutorLibdispatch;
+using firebase::firestore::util::kLogLevelDebug;
+using firebase::firestore::util::kLogLevelNotice;
 using firebase::firestore::util::LogSetLevel;
 using firebase::firestore::util::MakeCallback;
 using firebase::firestore::util::MakeNSError;
@@ -83,11 +85,9 @@
 using firebase::firestore::util::SetThrowHandler;
 using firebase::firestore::util::Status;
 using firebase::firestore::util::StatusOr;
+using firebase::firestore::util::StreamReadResult;
 using firebase::firestore::util::ThrowIllegalState;
 using firebase::firestore::util::ThrowInvalidArgument;
-using firebase::firestore::util::kLogLevelDebug;
-using firebase::firestore::util::kLogLevelNotice;
-using firebase::firestore::util::StreamReadResult;
 
 using UserUpdateBlock = id _Nullable (^)(FIRTransaction *, NSError **);
 using UserTransactionCompletion = void (^)(id _Nullable, NSError *_Nullable);
@@ -128,23 +128,6 @@ + (instancetype)firestoreForApp:(FIRApp *)app {
   return [self firestoreForApp:app database:MakeNSString(DatabaseId::kDefault)];
 }
 
-// TODO(b/62410906): make this public
-+ (instancetype)firestoreForApp:(FIRApp *)app database:(NSString *)database {
-  if (!app) {
-    ThrowInvalidArgument("FirebaseApp instance may not be nil. Use FirebaseApp.app() if you'd like "
-                         "to use the default FirebaseApp instance.");
-  }
-  if (!database) {
-    ThrowInvalidArgument("Database identifier may not be nil. Use '%s' if you want the default "
-                         "database",
-                         DatabaseId::kDefault);
-  }
-
-  id<FSTFirestoreMultiDBProvider> provider =
-      FIR_COMPONENT(FSTFirestoreMultiDBProvider, app.container);
-  return [provider firestoreForDatabase:database];
-}
-
 - (instancetype)initWithDatabaseID:(model::DatabaseId)databaseID
                     persistenceKey:(std::string)persistenceKey
            authCredentialsProvider:
@@ -529,6 +512,31 @@ @implementation FIRFirestore (Internal)
   return _firestore->database_id();
 }
 
++ (instancetype)firestoreForApp:(FIRApp *)app database:(NSString *)database {
+  if (!app) {
+    ThrowInvalidArgument("FirebaseApp instance may not be nil. Use FirebaseApp.app() if you'd like "
+                         "to use the default FirebaseApp instance.");
+  }
+  if (!database) {
+    ThrowInvalidArgument("Database identifier may not be nil. Use '%s' if you want the default "
+                         "database",
+                         DatabaseId::kDefault);
+  }
+
+  id<FSTFirestoreMultiDBProvider> provider =
+      FIR_COMPONENT(FSTFirestoreMultiDBProvider, app.container);
+  return [provider firestoreForDatabase:database];
+}
+
++ (instancetype)firestoreForDatabase:(NSString *)database {
+  FIRApp *app = [FIRApp defaultApp];
+  if (!app) {
+    ThrowIllegalState("Failed to get FirebaseApp instance. Please call FirebaseApp.configure() "
+                      "before using Firestore");
+  }
+  return [self firestoreForApp:app database:database];
+}
+
 + (FIRFirestore *)recoverFromFirestore:(std::shared_ptr<Firestore>)firestore {
   return (__bridge FIRFirestore *)firestore->extension();
 }

Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h

@@ -43,22 +43,22 @@ NS_SWIFT_NAME(Firestore)
 - (instancetype)init __attribute__((unavailable("Use a static constructor method.")));
 
 /**
- * Creates, caches, and returns a `Firestore` using the default `FirebaseApp`. Each subsequent
- * invocation returns the same `Firestore` object.
+ * Creates, caches, and returns the default `Firestore` using the default `FirebaseApp`. Each
+ * subsequent invocation returns the same `Firestore` object.
  *
- * @return The `Firestore` instance.
+ * @return The default `Firestore` instance.
  */
 + (instancetype)firestore NS_SWIFT_NAME(firestore());
 
 /**
- * Creates, caches, and returns a `Firestore` object for the specified _app_. Each subsequent
- * invocation returns the same `Firestore` object.
+ * Creates, caches, and returns the default `Firestore` object for the specified _app_. Each
+ * subsequent invocation returns the same `Firestore` object.
  *
  * @param app The `FirebaseApp` instance to use for authentication and as a source of the Google
  * Cloud Project ID for your Firestore Database. If you want the default instance, you should
  * explicitly set it to `FirebaseApp.app()`.
  *
- * @return The `Firestore` instance.
+ * @return The default `Firestore` instance.
  */
 + (instancetype)firestoreForApp:(FIRApp *)app NS_SWIFT_NAME(firestore(app:));