Skip to content

Commit 0882dbc

Browse files
authored
Allow multiple configure calls in an extension. (#4436)
* Allow multiple `configure` calls in an extension. Some extensions have an entry point that gets called multiple times. Instead of having to check if a default app has already been configured, allow multiple `configure` calls if and only if it's in an extension and the FIROptions contents are identical. * Review comments. * Updated comment.
1 parent 85365cb commit 0882dbc

File tree

3 files changed

+168
-4
lines changed

3 files changed

+168
-4
lines changed

Example/Core/Tests/FIRAppTest.m

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#import <FirebaseCore/FIRCoreDiagnosticsConnector.h>
2121
#import <FirebaseCore/FIROptionsInternal.h>
2222

23+
#import <GoogleUtilities/GULAppEnvironmentUtil.h>
24+
2325
NSString *const kFIRTestAppName1 = @"test_app_name_1";
2426
NSString *const kFIRTestAppName2 = @"test-app-name-2";
2527

@@ -214,6 +216,81 @@ - (void)testConfigureWithMultipleApps {
214216
XCTAssertEqualObjects(app.options.APIKey, kCustomizedAPIKey);
215217
}
216218

219+
- (void)testConfigureThrowsAfterConfigured {
220+
FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
221+
GCMSenderID:kGCMSenderID];
222+
[FIRApp configureWithOptions:options];
223+
XCTAssertNotNil([FIRApp defaultApp]);
224+
225+
// A second configure call should throw, since Firebase is already configured.
226+
XCTAssertThrows([FIRApp configureWithOptions:options]);
227+
228+
// Test the same with a custom named app.
229+
[FIRApp configureWithName:kFIRTestAppName1 options:options];
230+
XCTAssertNotNil([FIRApp appNamed:kFIRTestAppName1]);
231+
232+
// A second configure call should throw, since Firebase is already configured.
233+
XCTAssertThrows([FIRApp configureWithName:kFIRTestAppName1 options:options]);
234+
}
235+
236+
- (void)testConfigureDefaultAppInExtension {
237+
id environmentMock = OCMClassMock([GULAppEnvironmentUtil class]);
238+
OCMStub([environmentMock isAppExtension]).andReturn(YES);
239+
240+
// Set up the default app like a standard app.
241+
FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
242+
GCMSenderID:kGCMSenderID];
243+
[FIRApp configureWithOptions:options];
244+
XCTAssertNotNil([FIRApp defaultApp]);
245+
XCTAssertEqual([FIRApp allApps].count, 1);
246+
247+
// Configuring with the same set of options shouldn't throw.
248+
XCTAssertNoThrow([FIRApp configureWithOptions:options]);
249+
250+
// Only 1 app should have been configured still, the default app.
251+
XCTAssertNotNil([FIRApp defaultApp]);
252+
XCTAssertEqual([FIRApp allApps].count, 1);
253+
254+
// Use a set of a different options to call configure again, which should throw.
255+
FIROptions *differentOptions = [[FIROptions alloc] initWithGoogleAppID:@"1:789:ios:789XYZ"
256+
GCMSenderID:kGCMSenderID];
257+
XCTAssertThrows([FIRApp configureWithOptions:differentOptions]);
258+
XCTAssertEqual([FIRApp allApps].count, 1);
259+
260+
// Explicily stop the environmentMock.
261+
[environmentMock stopMocking];
262+
environmentMock = nil;
263+
}
264+
265+
- (void)testConfigureCustomAppInExtension {
266+
id environmentMock = OCMClassMock([GULAppEnvironmentUtil class]);
267+
OCMStub([environmentMock isAppExtension]).andReturn(YES);
268+
269+
// Set up a custom named app like a standard app.
270+
FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
271+
GCMSenderID:kGCMSenderID];
272+
[FIRApp configureWithName:kFIRTestAppName1 options:options];
273+
XCTAssertNotNil([FIRApp appNamed:kFIRTestAppName1]);
274+
XCTAssertEqual([FIRApp allApps].count, 1);
275+
276+
// Configuring with the same set of options shouldn't throw.
277+
XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName1 options:options]);
278+
279+
// Only 1 app should have been configured still.
280+
XCTAssertNotNil([FIRApp appNamed:kFIRTestAppName1]);
281+
XCTAssertEqual([FIRApp allApps].count, 1);
282+
283+
// Use a set of a different options to call configure again, which should throw.
284+
FIROptions *differentOptions = [[FIROptions alloc] initWithGoogleAppID:@"1:789:ios:789XYZ"
285+
GCMSenderID:kGCMSenderID];
286+
XCTAssertThrows([FIRApp configureWithName:kFIRTestAppName1 options:differentOptions]);
287+
XCTAssertEqual([FIRApp allApps].count, 1);
288+
289+
// Explicily stop the environmentMock.
290+
[environmentMock stopMocking];
291+
environmentMock = nil;
292+
}
293+
217294
- (void)testValidName {
218295
XCTAssertNoThrow([FIRApp configureWithName:@"aA1_" options:[FIROptions defaultOptions]]);
219296
XCTAssertNoThrow([FIRApp configureWithName:@"aA1-" options:[FIROptions defaultOptions]]);

Firebase/Core/FIRApp.m

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
#import "Private/FIROptionsInternal.h"
3636
#import "Private/FIRVersion.h"
3737

38+
#import <GoogleUtilities/GULAppEnvironmentUtil.h>
39+
3840
// The kFIRService strings are only here while transitioning CoreDiagnostics from the Analytics
3941
// pod to a Core dependency. These symbols are not used and should be deleted after the transition.
4042
NSString *const kFIRServiceAdMob;
@@ -160,8 +162,9 @@ + (void)configureWithName:(NSString *)name options:(FIROptions *)options {
160162

161163
if ([name isEqualToString:kFIRDefaultAppName]) {
162164
if (sDefaultApp) {
163-
[NSException raise:kFirebaseCoreErrorDomain
164-
format:@"Default app has already been configured."];
165+
// The default app already exixts. Handle duplicate `configure` calls and return.
166+
[self appWasConfiguredTwice:sDefaultApp usingOptions:options];
167+
return;
165168
}
166169

167170
FIRLogDebug(kFIRLoggerCore, @"I-COR000001", @"Configuring the default app.");
@@ -177,8 +180,9 @@ + (void)configureWithName:(NSString *)name options:(FIROptions *)options {
177180

178181
@synchronized(self) {
179182
if (sAllApps && sAllApps[name]) {
180-
[NSException raise:kFirebaseCoreErrorDomain
181-
format:@"App named %@ has already been configured.", name];
183+
// The app already exists. Handle a duplicate `configure` call and return.
184+
[self appWasConfiguredTwice:sAllApps[name] usingOptions:options];
185+
return;
182186
}
183187
}
184188

@@ -200,6 +204,36 @@ + (void)configureWithName:(NSString *)name options:(FIROptions *)options {
200204
}
201205
}
202206

207+
/// Called when `configure` has been called multiple times for the same app. This can either throw
208+
/// an exception (most cases) or ignore the duplicate configuration in situations where it's allowed
209+
/// like an extension.
210+
+ (void)appWasConfiguredTwice:(FIRApp *)app usingOptions:(FIROptions *)options {
211+
// Only extensions should potentially be able to call `configure` more than once.
212+
if (![GULAppEnvironmentUtil isAppExtension]) {
213+
// Throw an exception since this is now an invalid state.
214+
if (app.isDefaultApp) {
215+
[NSException raise:kFirebaseCoreErrorDomain
216+
format:@"Default app has already been configured."];
217+
} else {
218+
[NSException raise:kFirebaseCoreErrorDomain
219+
format:@"App named %@ has already been configured.", app.name];
220+
}
221+
}
222+
223+
// In an extension, the entry point could be called multiple times. As long as the options are
224+
// identical we should allow multiple `configure` calls.
225+
if ([options isEqual:app.options]) {
226+
// Everything is identical but the extension's lifecycle triggered `configure` twice.
227+
// Ignore duplicate calls and return since everything should still be in a valid state.
228+
FIRLogDebug(kFIRLoggerCore, @"I-COR000035",
229+
@"Ignoring second `configure` call in an extension.");
230+
return;
231+
} else {
232+
[NSException raise:kFirebaseCoreErrorDomain
233+
format:@"App named %@ has already been configured.", app.name];
234+
}
235+
}
236+
203237
+ (FIRApp *)defaultApp {
204238
if (sDefaultApp) {
205239
return sDefaultApp;

Firebase/Core/FIROptions.m

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,59 @@ - (void)setAppGroupID:(NSString *)appGroupID {
330330
_appGroupID = [appGroupID copy];
331331
}
332332

333+
#pragma mark - Equality
334+
335+
- (BOOL)isEqual:(id)object {
336+
if (!object || ![object isKindOfClass:[FIROptions class]]) {
337+
return NO;
338+
}
339+
340+
return [self isEqualToOptions:(FIROptions *)object];
341+
}
342+
343+
- (BOOL)isEqualToOptions:(FIROptions *)options {
344+
// Skip any non-FIROptions classes.
345+
if (![options isKindOfClass:[FIROptions class]]) {
346+
return NO;
347+
}
348+
349+
// Check the internal dictionary and custom properties for differences.
350+
if (![options.optionsDictionary isEqualToDictionary:self.optionsDictionary]) {
351+
return NO;
352+
}
353+
354+
// Validate extra properties not contained in the dictionary. Only validate it if one of the
355+
// objects has the property set.
356+
if ((options.deepLinkURLScheme != nil || self.deepLinkURLScheme != nil) &&
357+
![options.deepLinkURLScheme isEqualToString:self.deepLinkURLScheme]) {
358+
return NO;
359+
}
360+
361+
if ((options.appGroupID != nil || self.appGroupID != nil) &&
362+
![options.appGroupID isEqualToString:self.appGroupID]) {
363+
return NO;
364+
}
365+
366+
// Validate the Analytics options haven't changed with the Info.plist.
367+
if (![options.analyticsOptionsDictionary isEqualToDictionary:self.analyticsOptionsDictionary]) {
368+
return NO;
369+
}
370+
371+
// We don't care about the `editingLocked` or `usingOptionsFromDefaultPlist` properties since
372+
// those relate to lifecycle and construction, we only care if the contents of the options
373+
// themselves are equal.
374+
return YES;
375+
}
376+
377+
- (NSUInteger)hash {
378+
// This is strongly recommended for any object that implements a custom `isEqual:` method to
379+
// ensure that dictionary and set behavior matches other `isEqual:` checks.
380+
// Note: `self.analyticsOptionsDictionary` was left out here since it solely relies on the
381+
// contents of the main bundle's `Info.plist`. We should avoid reading that file and the contents
382+
// should be identical.
383+
return self.optionsDictionary.hash ^ self.deepLinkURLScheme.hash ^ self.appGroupID.hash;
384+
}
385+
333386
#pragma mark - Internal instance methods
334387

335388
- (NSDictionary *)analyticsOptionsDictionaryWithInfoDictionary:(NSDictionary *)infoDictionary {

0 commit comments

Comments
 (0)