From 8728d20ed7300f0caea4a4d8d0f94bbc6c7980d6 Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Mon, 9 Apr 2018 12:08:12 -0400 Subject: [PATCH 1/4] Addition of global data collection switch. --- Example/Core/Tests/FIRAppTest.m | 124 +++++++++++++++++++++++++ Firebase/Core/FIRApp.m | 82 +++++++++++++++- Firebase/Core/Private/FIRAppInternal.h | 16 ++++ Firebase/Core/Public/FIRApp.h | 8 ++ 4 files changed, 229 insertions(+), 1 deletion(-) diff --git a/Example/Core/Tests/FIRAppTest.m b/Example/Core/Tests/FIRAppTest.m index 6825e6a1dd2..f4a2d665686 100644 --- a/Example/Core/Tests/FIRAppTest.m +++ b/Example/Core/Tests/FIRAppTest.m @@ -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 @@ -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 { diff --git a/Firebase/Core/FIRApp.m b/Firebase/Core/FIRApp.m index 3d05f120cc8..0b62e15a0dd 100644 --- a/Firebase/Core/FIRApp.m +++ b/Firebase/Core/FIRApp.m @@ -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"; @@ -227,6 +232,7 @@ - (void)deleteApp:(FIRAppVoidBoolCallback)completion { if (sAllApps && sAllApps[self.name]) { FIRLogDebug(kFIRLoggerCore, @"I-COR000006", @"Deleting app named %@", self.name); [sAllApps removeObjectForKey:self.name]; + [self clearDataCollectionSwitchFromUserDefaults]; if ([self.name isEqualToString:kFIRDefaultAppName]) { sDefaultApp = nil; } @@ -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 { + return YES; + } +} + #pragma mark - private + (void)sendNotificationsToSDKs:(FIRApp *)app { @@ -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 + * 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 + * 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 { + 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]; + 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, diff --git a/Firebase/Core/Private/FIRAppInternal.h b/Firebase/Core/Private/FIRAppInternal.h index b7cf5e88720..a1851b95d09 100644 --- a/Firebase/Core/Private/FIRAppInternal.h +++ b/Firebase/Core/Private/FIRAppInternal.h @@ -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 diff --git a/Firebase/Core/Public/FIRApp.h b/Firebase/Core/Public/FIRApp.h index fb18b75e2f1..1dacfc6a27d 100644 --- a/Firebase/Core/Public/FIRApp.h +++ b/Firebase/Core/Public/FIRApp.h @@ -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 + * can be set once when users have consented to collection. + */ +@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) BOOL automaticDataCollectionEnabled; + @end NS_ASSUME_NONNULL_END From d98a71f88a4b319be5ed0cc6bf8242747af8624e Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Tue, 24 Apr 2018 16:29:25 -0400 Subject: [PATCH 2/4] Added Messaging conformance to data switch. Also formatted code. --- Example/Core/Tests/FIRAppTest.m | 30 +++++++++------- Example/Messaging/Tests/FIRMessagingTest.m | 40 ++++++++++++++++++++++ Firebase/Core/FIRApp.m | 7 ++-- Firebase/Core/Public/FIRApp.h | 3 +- Firebase/Messaging/FIRMessaging+FIRApp.m | 1 + Firebase/Messaging/FIRMessaging.m | 7 ++-- Firebase/Messaging/FIRMessaging_Private.h | 4 +++ 7 files changed, 74 insertions(+), 18 deletions(-) diff --git a/Example/Core/Tests/FIRAppTest.m b/Example/Core/Tests/FIRAppTest.m index f4a2d665686..abf1d38bc43 100644 --- a/Example/Core/Tests/FIRAppTest.m +++ b/Example/Core/Tests/FIRAppTest.m @@ -561,7 +561,8 @@ - (void)testGlobalDataCollectionNoFlags { // Test: No flags set. [FIRApp configure]; OCMStub([self.appClassMock readDataCollectionSwitchFromPlist]).andReturn(nil); - OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]).andReturn(nil); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(nil); XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -570,7 +571,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(nil); XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -579,7 +581,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(nil); XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -588,7 +591,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(@YES); XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -597,7 +601,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(@NO); XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -606,7 +611,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(@YES); XCTAssertTrue([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -615,7 +621,8 @@ - (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); + OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) + .andReturn(@NO); XCTAssertFalse([FIRApp defaultApp].isAutomaticDataCollectionEnabled); } @@ -653,7 +660,7 @@ - (void)testGlobalDataCollectionClearedAfterDelete { }]; // Wait for the delete to complete. - [self waitForExpectations:@[deleteFinished] timeout:1]; + [self waitForExpectations:@[ deleteFinished ] timeout:1]; // Set up the default app again, and check the data collection flag. [FIRApp configure]; @@ -666,14 +673,13 @@ - (void)testGlobalDataCollectionNoDiagnosticsSent { // 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); + 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]; + [[FIRApp defaultApp] sendLogsWithServiceName:@"Service" version:@"Version" error:error]; } #pragma mark - Internal Methods diff --git a/Example/Messaging/Tests/FIRMessagingTest.m b/Example/Messaging/Tests/FIRMessagingTest.m index adc830dd054..61ff1369bd1 100644 --- a/Example/Messaging/Tests/FIRMessagingTest.m +++ b/Example/Messaging/Tests/FIRMessagingTest.m @@ -75,6 +75,46 @@ - (void)testAutoInitEnableFlag { XCTAssertTrue(_messaging.isAutoInitEnabled); } +- (void)testAutoInitEnableFlagOverrideGlobalTrue { + OCMStub([self.mockMessaging isGlobalAutomaticDataCollectionEnabled]).andReturn(YES); + id bundleMock = OCMPartialMock([NSBundle mainBundle]); + OCMStub([bundleMock objectForInfoDictionaryKey:kFIRMessagingPlistAutoInitEnabled]).andReturn(nil); + XCTAssertTrue(self.messaging.isAutoInitEnabled); + + self.messaging.autoInitEnabled = NO; + XCTAssertFalse(self.messaging.isAutoInitEnabled); + [bundleMock stopMocking]; +} + +- (void)testAutoInitEnableFlagOverrideGlobalFalse { + OCMStub([self.mockMessaging isGlobalAutomaticDataCollectionEnabled]).andReturn(YES); + id bundleMock = OCMPartialMock([NSBundle mainBundle]); + OCMStub([bundleMock objectForInfoDictionaryKey:kFIRMessagingPlistAutoInitEnabled]).andReturn(nil); + XCTAssertTrue(self.messaging.isAutoInitEnabled); + + self.messaging.autoInitEnabled = NO; + XCTAssertFalse(self.messaging.isAutoInitEnabled); + [bundleMock stopMocking]; +} + +- (void)testAutoInitEnableGlobalDefaultTrue { + OCMStub([self.mockMessaging isGlobalAutomaticDataCollectionEnabled]).andReturn(YES); + id bundleMock = OCMPartialMock([NSBundle mainBundle]); + OCMStub([bundleMock objectForInfoDictionaryKey:kFIRMessagingPlistAutoInitEnabled]).andReturn(nil); + + XCTAssertTrue(self.messaging.isAutoInitEnabled); + [bundleMock stopMocking]; +} + +- (void)testAutoInitEnableGlobalDefaultFalse { + OCMStub([self.mockMessaging isGlobalAutomaticDataCollectionEnabled]).andReturn(NO); + id bundleMock = OCMPartialMock([NSBundle mainBundle]); + OCMStub([bundleMock objectForInfoDictionaryKey:kFIRMessagingPlistAutoInitEnabled]).andReturn(nil); + + XCTAssertFalse(self.messaging.isAutoInitEnabled); + [bundleMock stopMocking]; +} + #pragma mark - Direct Channel Establishment Testing // Should connect with valid token and application in foreground diff --git a/Firebase/Core/FIRApp.m b/Firebase/Core/FIRApp.m index 0b62e15a0dd..7acd14045dd 100644 --- a/Firebase/Core/FIRApp.m +++ b/Firebase/Core/FIRApp.m @@ -681,7 +681,8 @@ + (nullable NSNumber *)readDataCollectionSwitchFromPlist { 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]; + id plistValue = [[NSBundle mainBundle] + objectForInfoDictionaryKey:kFIRGlobalAppDataCollectionEnabledPlistKey]; if (plistValue && [plistValue isKindOfClass:[NSNumber class]]) { collectionEnabledPlistObject = (NSNumber *)plistValue; } @@ -696,7 +697,9 @@ - (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; } + if (![self isAutomaticDataCollectionEnabled]) { + return; + } NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] initWithDictionary:@{ kFIRAppDiagnosticsConfigurationTypeKey : @(FIRConfigTypeSDK), diff --git a/Firebase/Core/Public/FIRApp.h b/Firebase/Core/Public/FIRApp.h index 1dacfc6a27d..d53e13bfbe9 100644 --- a/Firebase/Core/Public/FIRApp.h +++ b/Firebase/Core/Public/FIRApp.h @@ -119,7 +119,8 @@ NS_SWIFT_NAME(FirebaseApp) * your app's Info.plist. This value is persisted across runs of the app so that it * can be set once when users have consented to collection. */ -@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) BOOL automaticDataCollectionEnabled; +@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) + BOOL automaticDataCollectionEnabled; @end diff --git a/Firebase/Messaging/FIRMessaging+FIRApp.m b/Firebase/Messaging/FIRMessaging+FIRApp.m index 58ae3af9bd5..d48a3b4465e 100644 --- a/Firebase/Messaging/FIRMessaging+FIRApp.m +++ b/Firebase/Messaging/FIRMessaging+FIRApp.m @@ -72,6 +72,7 @@ - (void)configureMessaging:(FIRApp *)app { } self.fcmSenderID = [options.GCMSenderID copy]; + self.globalAutomaticDataCollectionEnabled = [app isAutomaticDataCollectionEnabled]; // Swizzle remote-notification-related methods (app delegate and UNUserNotificationCenter) if ([FIRMessagingRemoteNotificationsProxy canSwizzleMethods]) { diff --git a/Firebase/Messaging/FIRMessaging.m b/Firebase/Messaging/FIRMessaging.m index 339bd7dc3b3..62224206562 100644 --- a/Firebase/Messaging/FIRMessaging.m +++ b/Firebase/Messaging/FIRMessaging.m @@ -75,7 +75,7 @@ NSString *const kFIRMessagingAPNSTokenType = @"APNSTokenType"; // APNS Token type key stored in user info. -static NSString *const kFIRMessagingPlistAutoInitEnabled = +NSString *const kFIRMessagingPlistAutoInitEnabled = @"FirebaseMessagingAutoInitEnabled"; // Auto Init Enabled key stored in Info.plist @interface FIRMessagingMessageInfo () @@ -471,8 +471,9 @@ - (BOOL)isAutoInitEnabled { if (isAutoInitEnabledObject) { return [isAutoInitEnabledObject boolValue]; } - // If none of above exists, we default assume FCM auto init is enabled. - return YES; + + // If none of above exists, we default to the global switch that comes from FIRApp. + return self.isGlobalAutomaticDataCollectionEnabled; } - (void)setAutoInitEnabled:(BOOL)autoInitEnabled { diff --git a/Firebase/Messaging/FIRMessaging_Private.h b/Firebase/Messaging/FIRMessaging_Private.h index 46daee09b46..6bac99d3a08 100644 --- a/Firebase/Messaging/FIRMessaging_Private.h +++ b/Firebase/Messaging/FIRMessaging_Private.h @@ -25,6 +25,7 @@ typedef NS_ENUM(int8_t, FIRMessagingNetworkStatus) { kFIRMessagingReachabilityReachableViaWWAN, }; +FOUNDATION_EXPORT NSString *const kFIRMessagingPlistAutoInitEnabled; FOUNDATION_EXPORT NSString *const kFIRMessagingUserDefaultsKeyAutoInitEnabled; @interface FIRMessagingRemoteMessage () @@ -37,6 +38,9 @@ FOUNDATION_EXPORT NSString *const kFIRMessagingUserDefaultsKeyAutoInitEnabled; #pragma mark - Private API +// The data collection flag from Core. +@property(nonatomic, readwrite, getter=isGlobalAutomaticDataCollectionEnabled) BOOL globalAutomaticDataCollectionEnabled; + - (NSString *)defaultFcmToken; - (FIRMessagingClient *)client; - (FIRMessagingPubSub *)pubsub; From 99c4e0c8540881d0adf94ae8e43d416ffb9c3916 Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Thu, 3 May 2018 11:52:17 -0400 Subject: [PATCH 3/4] Move data collection flag internal until all SDKs conform to it. --- Firebase/Core/Private/FIRAppInternal.h | 12 ++++++++++++ Firebase/Core/Public/FIRApp.h | 9 --------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Firebase/Core/Private/FIRAppInternal.h b/Firebase/Core/Private/FIRAppInternal.h index a1851b95d09..5e14caeb97e 100644 --- a/Firebase/Core/Private/FIRAppInternal.h +++ b/Firebase/Core/Private/FIRAppInternal.h @@ -197,6 +197,18 @@ typedef NSString *_Nullable (^FIRAppGetUIDImplementation)(void); */ - (nullable NSString *)getUID; +/** + * WARNING: THIS SETTING DOES NOT WORK YET. IT WILL BE MOVED TO THE PUBLIC HEADER ONCE ALL SDKS + * CONFORM TO THIS PREFERENCE. DO NOT RELY ON IT. + * + * 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 + * can be set once when users have consented to collection. + */ +@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) + BOOL automaticDataCollectionEnabled; + @end NS_ASSUME_NONNULL_END diff --git a/Firebase/Core/Public/FIRApp.h b/Firebase/Core/Public/FIRApp.h index d53e13bfbe9..fb18b75e2f1 100644 --- a/Firebase/Core/Public/FIRApp.h +++ b/Firebase/Core/Public/FIRApp.h @@ -113,15 +113,6 @@ 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 - * can be set once when users have consented to collection. - */ -@property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) - BOOL automaticDataCollectionEnabled; - @end NS_ASSUME_NONNULL_END From 94239785bf50176755fb6b24429d5d2e0dcd171c Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Wed, 9 May 2018 17:59:39 -0700 Subject: [PATCH 4/4] Formatting in response to code review. --- Firebase/Core/FIRApp.m | 20 ++++++++++---------- Firebase/Core/Private/FIRAppInternal.h | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Firebase/Core/FIRApp.m b/Firebase/Core/FIRApp.m index 7acd14045dd..717da4ee2db 100644 --- a/Firebase/Core/FIRApp.m +++ b/Firebase/Core/FIRApp.m @@ -357,9 +357,9 @@ - (BOOL)isAutomaticDataCollectionEnabled { NSNumber *collectionEnabledPlistValue = [[self class] readDataCollectionSwitchFromPlist]; if (collectionEnabledPlistValue) { return [collectionEnabledPlistValue boolValue]; - } else { - return YES; } + + return YES; } #pragma mark - private @@ -647,8 +647,8 @@ - (NSString *)expectedBundleID { #pragma mark - Reading From Plist & User Defaults /** - * A wrapper to clear the data collection switch from the standard NSUserDefaults for easier testing - * and readability. + * Clears the data collection switch from the standard NSUserDefaults for easier testing and + * readability. */ - (void)clearDataCollectionSwitchFromUserDefaults { NSString *key = @@ -657,8 +657,8 @@ - (void)clearDataCollectionSwitchFromUserDefaults { } /** - * A wrapper to read the data collection switch from the standard NSUserDefaults for easier testing - * and readability. + * Reads the data collection switch from the standard NSUserDefaults for easier testing and + * readability. */ + (nullable NSNumber *)readDataCollectionSwitchFromUserDefaultsForApp:(FIRApp *)app { // Read the object in user defaults, and only return if it's an NSNumber. @@ -667,14 +667,14 @@ + (nullable NSNumber *)readDataCollectionSwitchFromUserDefaultsForApp:(FIRApp *) id collectionEnabledDefaultsObject = [[NSUserDefaults standardUserDefaults] objectForKey:key]; if ([collectionEnabledDefaultsObject isKindOfClass:[NSNumber class]]) { return collectionEnabledDefaultsObject; - } else { - return nil; } + + 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. + * Reads 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; diff --git a/Firebase/Core/Private/FIRAppInternal.h b/Firebase/Core/Private/FIRAppInternal.h index 5e14caeb97e..66979eb3e20 100644 --- a/Firebase/Core/Private/FIRAppInternal.h +++ b/Firebase/Core/Private/FIRAppInternal.h @@ -201,10 +201,10 @@ typedef NSString *_Nullable (^FIRAppGetUIDImplementation)(void); * WARNING: THIS SETTING DOES NOT WORK YET. IT WILL BE MOVED TO THE PUBLIC HEADER ONCE ALL SDKS * CONFORM TO THIS PREFERENCE. DO NOT RELY ON IT. * - * 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 - * can be set once when users have consented to collection. + * 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 can be set once when users have consented to + * collection. */ @property(nonatomic, readwrite, getter=isAutomaticDataCollectionEnabled) BOOL automaticDataCollectionEnabled;