Cache FirebaseStorage instances (#8178)

Author: schmidt-sebastian

FirebaseStorage/CHANGELOG.md

@@ -1,3 +1,7 @@
+# Unreleased (8.1.0)
+- [changed] Instances are now cached. Repeated invocations of `Storage.storage()`
+  return the same instance and retain the same settings.
+
 # 8.0.0
 - [added] Added `FirebaseStorage.useEmulator()`, which allows the Storage SDK to
   connect to the Cloud Storage for Firebase emulator.

FirebaseStorage/Sources/FIRStorageComponent.m

@@ -22,6 +22,9 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+/** A NSMutableDictionary of FirebaseApp name and bucket names to FIRStorage  instance. */
+typedef NSMutableDictionary<NSString *, FIRStorage *> FIRStorageDictionary;
+
 @interface FIRStorage ()
 // Surface the internal initializer to create instances of FIRStorage.
 - (instancetype)initWithApp:(FIRApp *)app
@@ -31,6 +34,7 @@ - (instancetype)initWithApp:(FIRApp *)app
 @end
 
 @interface FIRStorageComponent () <FIRLibrary>
+@property(nonatomic) FIRStorageDictionary *instances;
 /// Internal initializer.
 - (instancetype)initWithApp:(FIRApp *)app;
 @end
@@ -43,6 +47,7 @@ - (instancetype)initWithApp:(FIRApp *)app {
   self = [super init];
   if (self) {
     _app = app;
+    _instances = [NSMutableDictionary dictionary];
   }
   return self;
 }
@@ -60,6 +65,7 @@ + (void)load {
                                                       isRequired:NO];
   FIRComponentCreationBlock creationBlock =
       ^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) {
+    *isCacheable = YES;
     return [[FIRStorageComponent alloc] initWithApp:container.app];
   };
   FIRComponent *storageProvider =
@@ -74,10 +80,21 @@ + (void)load {
 #pragma mark - FIRStorageInstanceProvider Conformance
 
 - (FIRStorage *)storageForBucket:(NSString *)bucket {
-  // Create an instance of FIRStorage and return it.
-  id<FIRAuthInterop> auth = FIR_COMPONENT(FIRAuthInterop, self.app.container);
-  id<FIRAppCheckInterop> appCheck = FIR_COMPONENT(FIRAppCheckInterop, self.app.container);
-  return [[FIRStorage alloc] initWithApp:self.app bucket:bucket auth:auth appCheck:appCheck];
+  FIRStorageDictionary *instances = [self instances];
+  @synchronized(instances) {
+    FIRStorage *instance = instances[bucket];
+    if (!instance) {
+      // Create an instance of FIRStorage and return it.
+      id<FIRAuthInterop> auth = FIR_COMPONENT(FIRAuthInterop, self.app.container);
+      id<FIRAppCheckInterop> appCheck = FIR_COMPONENT(FIRAppCheckInterop, self.app.container);
+      instance = [[FIRStorage alloc] initWithApp:self.app
+                                          bucket:bucket
+                                            auth:auth
+                                        appCheck:appCheck];
+      instances[bucket] = instance;
+    }
+    return instance;
+  }
 }
 
 @end

FirebaseStorage/Tests/Integration/FIRStorageIntegrationTests.m

@@ -137,6 +137,24 @@ - (void)tearDown {
   [super tearDown];
 }
 
+- (void)testSameInstanceNoBucket {
+  FIRStorage *storage1 = [FIRStorage storageForApp:self.app];
+  FIRStorage *storage2 = [FIRStorage storageForApp:self.app];
+  XCTAssertEqual(storage1, storage2);
+}
+
+- (void)testSameInstanceCustomBucket {
+  FIRStorage *storage1 = [FIRStorage storageForApp:self.app URL:@"gs://foo-bar.appspot.com"];
+  FIRStorage *storage2 = [FIRStorage storageForApp:self.app URL:@"gs://foo-bar.appspot.com"];
+  XCTAssertEqual(storage1, storage2);
+}
+
+- (void)testDiffferentInstance {
+  FIRStorage *storage1 = [FIRStorage storageForApp:self.app];
+  FIRStorage *storage2 = [FIRStorage storageForApp:self.app URL:@"gs://foo-bar.appspot.com"];
+  XCTAssertNotEqual(storage1, storage2);
+}
+
 - (void)testName {
   NSString *aGSURI = [NSString
       stringWithFormat:@"gs://%@.appspot.com/path/to", [[FIRApp defaultApp] options].projectID];

FirebaseStorage/Tests/Unit/FIRStorageComponentTests.m

@@ -56,7 +56,7 @@ - (void)testStorageInstanceCreation {
   XCTAssertNotNil(storage);
 }
 
-/// Tests that the component container does not cache instances of FIRStorageComponent.
+/// Tests that the component container caches instances of FIRStorageComponent.
 - (void)testMultipleComponentInstancesCreated {
   // App isn't used in any of this, so a simple class mock works for simplicity.
   id app = OCMClassMock([FIRApp class]);
@@ -71,8 +71,8 @@ - (void)testMultipleComponentInstancesCreated {
       FIR_COMPONENT(FIRStorageMultiBucketProvider, container);
   XCTAssertNotNil(provider2);
 
-  // Ensure they're different instances.
-  XCTAssertNotEqual(provider1, provider2);
+  // Ensure they're the same instance.
+  XCTAssertEqual(provider1, provider2);
 }
 
 /// Tests that instances of FIRStorage created are different.
@@ -92,8 +92,8 @@ - (void)testMultipleStorageInstancesCreated {
   FIRStorage *storage2 = [provider storageForBucket:@"randomBucket"];
   XCTAssertNotNil(storage2);
 
-  // Ensure that they're different instances but equal objects since everything else matches.
-  XCTAssertNotEqual(storage1, storage2);
+  // Ensure that they're the same instance
+  XCTAssertEqual(storage1, storage2);
   XCTAssertEqualObjects(storage1, storage2);
 
   // Create another bucket with a different provider from above.