Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions Example/Core/Tests/FIRAppTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ + (BOOL)validateAppID:(NSString *)appID;
+ (BOOL)validateAppIDFormat:(NSString *)appID withVersion:(NSString *)version;
+ (BOOL)validateAppIDFingerprint:(NSString *)appID withVersion:(NSString *)version;

+ (nullable NSNumber *)readDataCollectionSwitchFromPlist;
+ (nullable NSNumber *)readDataCollectionSwitchFromUserDefaultsForApp:(FIRApp *)app;

@end

@interface FIRAppTest : FIRTestCase
Expand Down Expand Up @@ -552,6 +555,127 @@ - (void)testAppIDFingerprintInvalid {
[FIRApp validateAppIDFingerprint:@"1:1337:ios:deadbeef:ab" withVersion:kGoodVersionV1]);
}

#pragma mark - Automatic Data Collection Tests

- (void)testGlobalDataCollectionNoFlags {
// Test: No flags set.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(nil);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(nil);

XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionPlistSetEnabled {
// Test: Plist set to enabled, no override.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(@YES);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(nil);

XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionPlistSetDisabled {
// Test: Plist set to disabled, no override.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(@NO);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(nil);

XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionUserSpecifiedEnabled {
// Test: User specified as enabled, no plist value.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(nil);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(@YES);

XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionUserSpecifiedDisabled {
// Test: User specified as disabled, no plist value.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(nil);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(@NO);

XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionUserOverriddenEnabled {
// Test: User specified as enabled, with plist set as disabled.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(@NO);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(@YES);

XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionUserOverriddenDisabled {
// Test: User specified as disabled, with plist set as enabled.
[FIRApp configure];
OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(@YES);
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(@NO);

XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionWriteToDefaults {
id defaultsMock = OCMPartialMock([NSUserDefaults standardUserDefaults]);
[FIRApp configure];

FIRApp *app = [FIRApp defaultApp];
app.automaticDataCollectionEnabled = YES;
NSString *key =
[NSString stringWithFormat:kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat, app.name];
OCMVerify([defaultsMock setObject:@YES forKey:key]);

[FIRApp defaultApp].automaticDataCollectionEnabled = NO;
OCMVerify([defaultsMock setObject:@NO forKey:key]);

[defaultsMock stopMocking];
}

- (void)testGlobalDataCollectionClearedAfterDelete {
// Configure and disable data collection for the default FIRApp.
[FIRApp configure];
FIRApp *app = [FIRApp defaultApp];
app.automaticDataCollectionEnabled = NO;
XCTAssertFalse(app.isAutomaticDataCollectionEnabled);

// Delete the app, and verify that the switch was reset.
XCTestExpectation *deleteFinished =
[self expectationWithDescription:@"The app should successfully delete."];
[app deleteApp:^(BOOL success) {
if (success) {
[deleteFinished fulfill];
}
}];

// Wait for the delete to complete.
[self waitForExpectations:@[deleteFinished] timeout:1];

// Set up the default app again, and check the data collection flag.
[FIRApp configure];
XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled);
}

- (void)testGlobalDataCollectionNoDiagnosticsSent {
[FIRApp configure];

// Stub out reading from user defaults since stubbing out the BOOL has issues. If the data
// collection switch is disabled, the `sendLogs` call should return immediately and not fire a
// notification.
OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(@NO);
OCMReject([self.notificationCenterMock postNotificationName:kFIRAppDiagnosticsNotification
object:OCMOCK_ANY
userInfo:OCMOCK_ANY]);
NSError *error = [NSError errorWithDomain:@"com.firebase" code:42 userInfo:nil];
[[FIRApp defaultApp] sendLogsWithServiceName:@"Service"
version:@"Version"
error:error];
}

#pragma mark - Internal Methods

- (void)testAuthGetUID {
Expand Down
82 changes: 81 additions & 1 deletion Firebase/Core/FIRApp.m
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
NSString *const kFIRAppNameKey = @"FIRAppNameKey";
NSString *const kFIRGoogleAppIDKey = @"FIRGoogleAppIDKey";

NSString *const kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat =
@"/google/firebase/global_data_collection_enabled:%@";
NSString *const kFIRGlobalAppDataCollectionEnabledPlistKey =
@"FirebaseAutomaticDataCollectionEnabled";

NSString *const kFIRAppDiagnosticsNotification = @"FIRAppDiagnosticsNotification";

NSString *const kFIRAppDiagnosticsConfigurationTypeKey = @"ConfigType";
Expand Down Expand Up @@ -227,6 +232,7 @@ - (void)deleteApp:(FIRAppVoidBoolCallback)completion {
if (sAllApps && sAllApps[self.name]) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.name can be nil. We should check it before accessing the dictionary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We throw an exception if a user tries to configure the app with a nil or empty name, which should catch this. If you're still concerned, we can add it in a separate PR since it's unrelated to this change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I think we should be careful of accessing dictionary when the provided key is nil. I'm concerned that self.name might be nil at some point during the entire app life cycle.

FIRLogDebug(kFIRLoggerCore, @"I-COR000006", @"Deleting app named %@", self.name);
[sAllApps removeObjectForKey:self.name];
[self clearDataCollectionSwitchFromUserDefaults];
if ([self.name isEqualToString:kFIRDefaultAppName]) {
sDefaultApp = nil;
}
Expand Down Expand Up @@ -332,6 +338,30 @@ - (FIROptions *)options {
return [_options copy];
}

- (void)setAutomaticDataCollectionEnabled:(BOOL)automaticDataCollectionEnabled {
NSString *key =
[NSString stringWithFormat:kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat, self.name];
[[NSUserDefaults standardUserDefaults] setBool:automaticDataCollectionEnabled forKey:key];
}

- (BOOL)isAutomaticDataCollectionEnabled {
// Check if it's been manually set before in code, and use that as the higher priority value.
NSNumber *defaultsObject = [[self class] readDataCollectionSwitchFromUserDefaultsForApp:self];
if (defaultsObject) {
return [defaultsObject boolValue];
}

// Read the Info.plist to see if the flag is set. If it's not set, it should default to `YES`.
// As per the implementation of `readDataCollectionSwitchFromPlist`, it's a cached value and has
// no performance impact calling multiple times.
NSNumber *collectionEnabledPlistValue = [[self class] readDataCollectionSwitchFromPlist];
if (collectionEnabledPlistValue) {
return [collectionEnabledPlistValue boolValue];
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for else

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return YES;
}
}

#pragma mark - private

+ (void)sendNotificationsToSDKs:(FIRApp *)app {
Expand Down Expand Up @@ -613,11 +643,61 @@ - (NSString *)expectedBundleID {
}

// end App ID validation
#pragma mark

#pragma mark - Reading From Plist & User Defaults

/**
* A wrapper to clear the data collection switch from the standard NSUserDefaults for easier testing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Clears the data collection ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* and readability.
*/
- (void)clearDataCollectionSwitchFromUserDefaults {
NSString *key =
[NSString stringWithFormat:kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat, self.name];
[[NSUserDefaults standardUserDefaults] removeObjectForKey:key];
}

/**
* A wrapper to read the data collection switch from the standard NSUserDefaults for easier testing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Reads the data collection...
Please remove "A wrapper to" here and below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* and readability.
*/
+ (nullable NSNumber *)readDataCollectionSwitchFromUserDefaultsForApp:(FIRApp *)app {
// Read the object in user defaults, and only return if it's an NSNumber.
NSString *key =
[NSString stringWithFormat:kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat, app.name];
id collectionEnabledDefaultsObject = [[NSUserDefaults standardUserDefaults] objectForKey:key];
if ([collectionEnabledDefaultsObject isKindOfClass:[NSNumber class]]) {
return collectionEnabledDefaultsObject;
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for else

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return nil;
}
}

/**
* A wrapper to read the data collection switch from the Info.plist for easier testing and
* readability. Will only read once from the plist and return the cached value.
*/
+ (nullable NSNumber *)readDataCollectionSwitchFromPlist {
static NSNumber *collectionEnabledPlistObject;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
// Read the data from the `Info.plist`, only assign it if it's there and an NSNumber.
id plistValue = [[NSBundle mainBundle] objectForInfoDictionaryKey:kFIRGlobalAppDataCollectionEnabledPlistKey];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ident +4 from previous line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what happened here but I think this was out of date - it's split onto two lines now, even indenting +4 leaves it over 100 chars.

if (plistValue && [plistValue isKindOfClass:[NSNumber class]]) {
collectionEnabledPlistObject = (NSNumber *)plistValue;
}
});

return collectionEnabledPlistObject;
}

#pragma mark - Sending Logs

- (void)sendLogsWithServiceName:(NSString *)serviceName
version:(NSString *)version
error:(NSError *)error {
// If the user has manually turned off data collection, return and don't send logs.
if (![self isAutomaticDataCollectionEnabled]) { return; }

NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] initWithDictionary:@{
kFIRAppDiagnosticsConfigurationTypeKey : @(FIRConfigTypeSDK),
kFIRAppDiagnosticsSDKNameKey : serviceName,
Expand Down
16 changes: 16 additions & 0 deletions Firebase/Core/Private/FIRAppInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,22 @@ extern NSString *const kFIRAppIsDefaultAppKey;
extern NSString *const kFIRAppNameKey;
extern NSString *const kFIRGoogleAppIDKey;

/**
* The format string for the User Defaults key used for storing the data collection enabled flag.
* This includes formatting to append the Firebase App's name.
*/
extern NSString *const kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat;

/**
* The plist key used for storing the data collection enabled flag.
*/
extern NSString *const kFIRGlobalAppDataCollectionEnabledPlistKey;

/**
* A notification fired containing diagnostic information when SDK errors occur.
*/
extern NSString *const kFIRAppDiagnosticsNotification;

/** @var FIRAuthStateDidChangeInternalNotification
@brief The name of the @c NSNotificationCenter notification which is posted when the auth state
changes (e.g. a new token has been produced, a user logs in or out). The object parameter of
Expand Down
8 changes: 8 additions & 0 deletions Firebase/Core/Public/FIRApp.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ NS_SWIFT_NAME(FirebaseApp)
*/
@property(nonatomic, copy, readonly) FIROptions *options;

/**
* Gets or sets whether automatic data collection is enabled for all products.
* Defaults to YES unless FirebaseAutomaticDataCollectionEnabled is set to NO in
* your app's Info.plist. This value is persisted across runs of the app so that it
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: fits previous line. Please fill the whole row up to 100 chars per line.

* can be set once when users have consented to collection.
*/
@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) BOOL automaticDataCollectionEnabled;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ident +4


@end

NS_ASSUME_NONNULL_END