Skip to content

Commit c8c3874

Browse files
FIRApp: don't create new component instances while deleting the app (#4918)
* [FIRComponentContainer removeAllComponents] method introduced. * FIRApp library registration tests. * FIRApp: remove all components from container on delete * Run ./scripts/style.sh
1 parent 27c4898 commit c8c3874

File tree

5 files changed

+68
-0
lines changed

5 files changed

+68
-0
lines changed

FirebaseCore/Sources/FIRApp.m

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ - (void)deleteApp:(FIRAppVoidBoolCallback)completion {
287287
if (sAllApps && sAllApps[self.name]) {
288288
FIRLogDebug(kFIRLoggerCore, @"I-COR000006", @"Deleting app named %@", self.name);
289289

290+
// Remove all registered libraries from the container to avoid creating new instances.
291+
[self.container removeAllComponents];
290292
// Remove all cached instances from the container before deleting the app.
291293
[self.container removeAllCachedInstances];
292294

FirebaseCore/Sources/FIRComponentContainer.m

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ - (void)removeAllCachedInstances {
203203
}
204204
}
205205

206+
- (void)removeAllComponents {
207+
@synchronized(self) {
208+
[self.components removeAllObjects];
209+
}
210+
}
211+
206212
@end
207213

208214
NS_ASSUME_NONNULL_END

FirebaseCore/Sources/Private/FIRComponentContainerInternal.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ NS_ASSUME_NONNULL_BEGIN
3737
/// Remove all of the cached instances stored and allow them to clean up after themselves.
3838
- (void)removeAllCachedInstances;
3939

40+
/// Removes all the components. After calling this method no new instances will be created.
41+
- (void)removeAllComponents;
42+
4043
/// Register a class to provide components for the interoperability system. The class should conform
4144
/// to `FIRComponentRegistrant` and provide an array of `FIRComponent` objects.
4245
+ (void)registerAsComponentRegistrant:(Class<FIRLibrary>)klass;

FirebaseCore/Tests/Unit/FIRAppTest.m

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ - (void)tearDown {
8888
}
8989

9090
- (void)testConfigure {
91+
[self
92+
registerLibrariesWithClasses:@ [[FIRTestClassCached class], [FIRTestClassEagerCached class]]];
93+
9194
NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName
9295
isDefaultApp:YES];
9396
[self expectNotificationForObserver:self.observerMock
@@ -102,6 +105,11 @@ - (void)testConfigure {
102105
XCTAssertEqualObjects(app.name, kFIRDefaultAppName);
103106
XCTAssertEqualObjects(app.options.clientID, kClientID);
104107
XCTAssertTrue([FIRApp allApps].count == 1);
108+
109+
// Check the registered libraries instances available.
110+
XCTAssertNotNil(FIR_COMPONENT(FIRTestProtocolCached, app.container));
111+
XCTAssertNotNil(FIR_COMPONENT(FIRTestProtocolEagerCached, app.container));
112+
XCTAssertNil(FIR_COMPONENT(FIRTestProtocol, app.container));
105113
}
106114

107115
- (void)testConfigureWithNoDefaultOptions {
@@ -321,13 +329,22 @@ - (void)testAppNamed {
321329
}
322330

323331
- (void)testDeleteApp {
332+
[self
333+
registerLibrariesWithClasses:@ [[FIRTestClassCached class], [FIRTestClassEagerCached class]]];
334+
324335
NSString *name = NSStringFromSelector(_cmd);
325336
FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
326337
GCMSenderID:kGCMSenderID];
327338
[FIRApp configureWithName:name options:options];
328339
FIRApp *app = [FIRApp appNamed:name];
329340
XCTAssertNotNil(app);
330341
XCTAssertTrue([FIRApp allApps].count == 1);
342+
343+
// Check the registered libraries instances available.
344+
XCTAssertNotNil(FIR_COMPONENT(FIRTestProtocolCached, app.container));
345+
XCTAssertNotNil(FIR_COMPONENT(FIRTestProtocolEagerCached, app.container));
346+
XCTAssertNil(FIR_COMPONENT(FIRTestProtocol, app.container));
347+
331348
[self expectNotificationForObserver:self.observerMock
332349
notificationName:kFIRAppDeleteNotification
333350
object:[FIRApp class]
@@ -342,6 +359,10 @@ - (void)testDeleteApp {
342359
[self waitForExpectations:@[ expectation ] timeout:1];
343360
OCMVerifyAll(self.observerMock);
344361
XCTAssertTrue([FIRApp allApps].count == 0);
362+
363+
// Check no new library instances created after the app delete.
364+
XCTAssertNil(FIR_COMPONENT(FIRTestProtocolCached, app.container));
365+
XCTAssertNil(FIR_COMPONENT(FIRTestProtocolEagerCached, app.container));
345366
}
346367

347368
- (void)testErrorForSubspecConfigurationFailure {
@@ -889,4 +910,10 @@ - (FIROptions *)appOptions {
889910
return [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID GCMSenderID:kGCMSenderID];
890911
}
891912

913+
- (void)registerLibrariesWithClasses:(NSArray<Class> *)classes {
914+
for (Class klass in classes) {
915+
[FIRApp registerInternalLibrary:klass withName:NSStringFromClass(klass) withVersion:@"1.0"];
916+
}
917+
}
918+
892919
@end

FirebaseCore/Tests/Unit/FIRComponentContainerTest.m

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,36 @@ - (void)testRemoveAllCachedInstances {
132132
XCTAssertNotEqual(eagerInstance1, eagerInstance2);
133133
}
134134

135+
- (void)testRemoveAllComponents {
136+
FIRComponentContainer *container =
137+
[self containerWithRegistrants:@ [[FIRTestClass class], [FIRTestClassCached class],
138+
[FIRTestClassEagerCached class],
139+
[FIRTestClassCachedWithDep class]]];
140+
141+
// Retrieve an instance of FIRTestClassCached to ensure it's cached.
142+
id<FIRTestProtocolCached> cachedInstance1 = FIR_COMPONENT(FIRTestProtocolCached, container);
143+
XCTAssertNotNil(cachedInstance1);
144+
id<FIRTestProtocolEagerCached> eagerInstance1 =
145+
FIR_COMPONENT(FIRTestProtocolEagerCached, container);
146+
XCTAssertNotNil(eagerInstance1);
147+
148+
// FIRTestClassEagerCached and FIRTestClassCached instances should be cached at this point.
149+
XCTAssertTrue(container.cachedInstances.count == 2);
150+
151+
// Remove all components.
152+
[container removeAllComponents];
153+
154+
// Remove the instances.
155+
[container removeAllCachedInstances];
156+
157+
// Verify that no new instances are created.
158+
id<FIRTestProtocolCached> cachedInstance2 = FIR_COMPONENT(FIRTestProtocolCached, container);
159+
XCTAssertNil(cachedInstance2);
160+
id<FIRTestProtocolEagerCached> eagerInstance2 =
161+
FIR_COMPONENT(FIRTestProtocolEagerCached, container);
162+
XCTAssertNil(eagerInstance2);
163+
}
164+
135165
#pragma mark - Instantiation Tests
136166

137167
- (void)testEagerInstantiation {

0 commit comments

Comments
 (0)