From 503d2debdddd85331ce2e8556bbd50d53f3aa19d Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Mon, 21 May 2018 21:49:57 -0400 Subject: Add conformance to data collection switch for Analytics. (#1296) * Add conformance to data collection switch for Analytics. * Fix method documentation per PR comments. --- Example/Core/Tests/FIRAppTest.m | 38 ++++++ Example/Core/Tests/FIROptionsTest.m | 128 +++++++++++++++++++++ Firebase/Core/FIRAnalyticsConfiguration.m | 15 ++- Firebase/Core/FIRApp.m | 18 +++ Firebase/Core/FIROptions.m | 39 ++++++- .../Private/FIRAnalyticsConfiguration+Internal.h | 10 ++ Firebase/Core/Private/FIROptionsInternal.h | 6 + 7 files changed, 249 insertions(+), 5 deletions(-) diff --git a/Example/Core/Tests/FIRAppTest.m b/Example/Core/Tests/FIRAppTest.m index abf1d38..549c1ab 100644 --- a/Example/Core/Tests/FIRAppTest.m +++ b/Example/Core/Tests/FIRAppTest.m @@ -14,6 +14,7 @@ #import "FIRTestCase.h" +#import #import #import @@ -682,6 +683,43 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; [[FIRApp defaultApp] sendLogsWithServiceName:@"Service" version:@"Version" error:error]; } +#pragma mark - Analytics Flag Tests + +- (void)testAnalyticsSetByGlobalDataCollectionSwitch { + // Test that the global data collection switch triggers setting Analytics when no explicit flag is + // set. + [FIRApp configure]; + + id configurationMock = OCMClassMock([FIRAnalyticsConfiguration class]); + OCMStub([configurationMock sharedInstance]).andReturn(configurationMock); + OCMStub([configurationMock setAnalyticsCollectionEnabled:OCMOCK_ANY persistSetting:OCMOCK_ANY]); + OCMStub([self.optionsInstanceMock isAnalyticsCollectionExpicitlySet]).andReturn(NO); + + // Ensure Analytics is set after the global flag is set. + [[FIRApp defaultApp] setAutomaticDataCollectionEnabled:YES]; + OCMVerify([configurationMock setAnalyticsCollectionEnabled:YES persistSetting:NO]); + + [[FIRApp defaultApp] setAutomaticDataCollectionEnabled:NO]; + OCMVerify([configurationMock setAnalyticsCollectionEnabled:NO persistSetting:NO]); +} + +- (void)testAnalyticsNotSetByGlobalDataCollectionSwitch { + // Test that the global data collection switch doesn't override an explicitly set Analytics flag. + [FIRApp configure]; + + id configurationMock = OCMClassMock([FIRAnalyticsConfiguration class]); + OCMStub([configurationMock sharedInstance]).andReturn(configurationMock); + OCMStub([configurationMock setAnalyticsCollectionEnabled:OCMOCK_ANY persistSetting:OCMOCK_ANY]); + OCMStub([self.optionsInstanceMock isAnalyticsCollectionExpicitlySet]).andReturn(YES); + + // Reject any changes to Analytics when the data collection changes. + [[FIRApp defaultApp] setAutomaticDataCollectionEnabled:YES]; + OCMReject([configurationMock setAnalyticsCollectionEnabled:OCMOCK_ANY persistSetting:OCMOCK_ANY]); + + [[FIRApp defaultApp] setAutomaticDataCollectionEnabled:NO]; + OCMReject([configurationMock setAnalyticsCollectionEnabled:OCMOCK_ANY persistSetting:OCMOCK_ANY]); +} + #pragma mark - Internal Methods - (void)testAuthGetUID { diff --git a/Example/Core/Tests/FIROptionsTest.m b/Example/Core/Tests/FIROptionsTest.m index 064745a..f257cbb 100644 --- a/Example/Core/Tests/FIROptionsTest.m +++ b/Example/Core/Tests/FIROptionsTest.m @@ -428,6 +428,134 @@ extern NSString *const kFIRLibraryVersionID; XCTAssertEqual(mainSizeCount[3], 8); } +- (void)testAnalyticsCollectionGlobalSwitchEnabled { + // Stub the default app, and set the global switch to YES. + id appMock = OCMClassMock([FIRApp class]); + OCMStub([appMock isDefaultAppConfigured]).andReturn(YES); + OCMStub([appMock defaultApp]).andReturn(appMock); + OCMStub([appMock isAutomaticDataCollectionEnabled]).andReturn(YES); + + // With no other settings, Analytics collection should default to the app's flag. + FIROptions *options = [[FIROptions alloc] initInternalWithOptionsDictionary:@{}]; + XCTAssertTrue(options.isAnalyticsCollectionEnabled); + XCTAssertTrue(options.isMeasurementEnabled); + + [appMock stopMocking]; +} + +- (void)testAnalyticsCollectionGlobalSwitchDisabled { + // Stub the default app, and set the global switch to NO. + id appMock = OCMClassMock([FIRApp class]); + OCMStub([appMock isDefaultAppConfigured]).andReturn(YES); + OCMStub([appMock defaultApp]).andReturn(appMock); + OCMStub([appMock isAutomaticDataCollectionEnabled]).andReturn(NO); + + // With no other settings, Analytics collection should default to the app's flag. + FIROptions *options = [[FIROptions alloc] initInternalWithOptionsDictionary:@{}]; + XCTAssertFalse(options.isAnalyticsCollectionEnabled); + XCTAssertFalse(options.isMeasurementEnabled); + + [appMock stopMocking]; +} + +- (void)testAnalyticsCollectionGlobalSwitchOverrideToDisable { + // Stub the default app, and set the global switch to YES. + id appMock = OCMClassMock([FIRApp class]); + OCMStub([appMock isDefaultAppConfigured]).andReturn(YES); + OCMStub([appMock defaultApp]).andReturn(appMock); + OCMStub([appMock isAutomaticDataCollectionEnabled]).andReturn(YES); + + // Test the three Analytics flags that override to disable Analytics collection. + FIROptions *collectionEnabledOptions = [[FIROptions alloc] initInternalWithOptionsDictionary:@{ + kFIRIsAnalyticsCollectionEnabled : @NO + }]; + XCTAssertFalse(collectionEnabledOptions.isAnalyticsCollectionEnabled); + + FIROptions *collectionDeactivatedOptions = + [[FIROptions alloc] initInternalWithOptionsDictionary:@{ + kFIRIsAnalyticsCollectionDeactivated : @YES + }]; + XCTAssertFalse(collectionDeactivatedOptions.isAnalyticsCollectionEnabled); + + FIROptions *measurementEnabledOptions = [[FIROptions alloc] initInternalWithOptionsDictionary:@{ + kFIRIsMeasurementEnabled : @NO + }]; + XCTAssertFalse(measurementEnabledOptions.isAnalyticsCollectionEnabled); +} + +- (void)testAnalyticsCollectionGlobalSwitchOverrideToEnable { + // Stub the default app, and set the global switch to YES. + id appMock = OCMClassMock([FIRApp class]); + OCMStub([appMock isDefaultAppConfigured]).andReturn(YES); + OCMStub([appMock defaultApp]).andReturn(appMock); + OCMStub([appMock isAutomaticDataCollectionEnabled]).andReturn(NO); + + // Test the two Analytics flags that can override and enable collection. + FIROptions *collectionEnabledOptions = [[FIROptions alloc] initInternalWithOptionsDictionary:@{ + kFIRIsAnalyticsCollectionEnabled : @YES + }]; + XCTAssertTrue(collectionEnabledOptions.isAnalyticsCollectionEnabled); + + FIROptions *measurementEnabledOptions = [[FIROptions alloc] initInternalWithOptionsDictionary:@{ + kFIRIsMeasurementEnabled : @YES + }]; + XCTAssertTrue(measurementEnabledOptions.isAnalyticsCollectionEnabled); +} + +- (void)testAnalyticsCollectionExplicitlySet { + NSDictionary *optionsDictionary = @{}; + FIROptions *options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + NSDictionary *analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{}]; + XCTAssertFalse([options isAnalyticsCollectionExpicitlySet]); + + // Test deactivation flag. + optionsDictionary = @{ kFIRIsAnalyticsCollectionDeactivated : @YES }; + options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{}]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); + + // If "deactivated" == NO, that doesn't mean it's explicitly set / enabled so it should be treated + // as if it's not set. + optionsDictionary = @{ kFIRIsAnalyticsCollectionDeactivated : @NO }; + options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{}]; + XCTAssertFalse([options isAnalyticsCollectionExpicitlySet]); + + // Test the collection enabled flag. + optionsDictionary = @{ kFIRIsAnalyticsCollectionEnabled : @YES }; + options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{}]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); + + optionsDictionary = @{ kFIRIsAnalyticsCollectionEnabled : @NO }; + options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{}]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); + + // Test the old measurement flag. + options = [[FIROptions alloc] initInternalWithOptionsDictionary:@{}]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{ + kFIRIsMeasurementEnabled : @YES + }]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); + + options = [[FIROptions alloc] initInternalWithOptionsDictionary:@{}]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{ + kFIRIsMeasurementEnabled : @NO + }]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); + + // For good measure, a combination of all 3 (even if they conflict). + optionsDictionary = + @{ kFIRIsAnalyticsCollectionDeactivated : @YES, + kFIRIsAnalyticsCollectionEnabled : @YES }; + options = [[FIROptions alloc] initInternalWithOptionsDictionary:optionsDictionary]; + analyticsOptions = [options analyticsOptionsDictionaryWithInfoDictionary:@{ + kFIRIsMeasurementEnabled : @NO + }]; + XCTAssertTrue([options isAnalyticsCollectionExpicitlySet]); +} + - (void)testVersionFormat { NSRegularExpression *sLibraryVersionRegex = [NSRegularExpression regularExpressionWithPattern:@"^[0-9]{8,}$" options:0 error:NULL]; diff --git a/Firebase/Core/FIRAnalyticsConfiguration.m b/Firebase/Core/FIRAnalyticsConfiguration.m index 3a5b9f6..33aa168 100644 --- a/Firebase/Core/FIRAnalyticsConfiguration.m +++ b/Firebase/Core/FIRAnalyticsConfiguration.m @@ -47,13 +47,20 @@ } - (void)setAnalyticsCollectionEnabled:(BOOL)analyticsCollectionEnabled { + [self setAnalyticsCollectionEnabled:analyticsCollectionEnabled persistSetting:YES]; +} + +- (void)setAnalyticsCollectionEnabled:(BOOL)analyticsCollectionEnabled + persistSetting:(BOOL)shouldPersist { // Persist the measurementEnabledState. Use FIRAnalyticsEnabledState values instead of YES/NO. FIRAnalyticsEnabledState analyticsEnabledState = analyticsCollectionEnabled ? kFIRAnalyticsEnabledStateSetYes : kFIRAnalyticsEnabledStateSetNo; - NSUserDefaults *userDefaults = [NSUserDefaults standardUserDefaults]; - [userDefaults setObject:@(analyticsEnabledState) - forKey:kFIRAPersistedConfigMeasurementEnabledStateKey]; - [userDefaults synchronize]; + if (shouldPersist) { + NSUserDefaults *userDefaults = [NSUserDefaults standardUserDefaults]; + [userDefaults setObject:@(analyticsEnabledState) + forKey:kFIRAPersistedConfigMeasurementEnabledStateKey]; + [userDefaults synchronize]; + } [self postNotificationName:kFIRAnalyticsConfigurationSetEnabledNotification value:@(analyticsCollectionEnabled)]; diff --git a/Firebase/Core/FIRApp.m b/Firebase/Core/FIRApp.m index 717da4e..b295d50 100644 --- a/Firebase/Core/FIRApp.m +++ b/Firebase/Core/FIRApp.m @@ -16,6 +16,7 @@ #import "FIRApp.h" #import "FIRConfiguration.h" +#import "Private/FIRAnalyticsConfiguration+Internal.h" #import "Private/FIRAppInternal.h" #import "Private/FIRBundleUtil.h" #import "Private/FIRLogger.h" @@ -342,6 +343,23 @@ static NSMutableDictionary *sLibraryVersions; NSString *key = [NSString stringWithFormat:kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat, self.name]; [[NSUserDefaults standardUserDefaults] setBool:automaticDataCollectionEnabled forKey:key]; + + // Core also controls the FirebaseAnalytics flag, so check if the Analytics flags are set + // within FIROptions and change the Analytics value if necessary. Analytics only works with the + // default app, so return if this isn't the default app. + if (self != sDefaultApp) { + return; + } + + // Check if the Analytics flag is explicitly set. If so, no further actions are necessary. + if ([self.options isAnalyticsCollectionExpicitlySet]) { + return; + } + + // The Analytics flag has not been explicitly set, so update with the value being set. + [[FIRAnalyticsConfiguration sharedInstance] + setAnalyticsCollectionEnabled:automaticDataCollectionEnabled + persistSetting:NO]; } - (BOOL)isAutomaticDataCollectionEnabled { diff --git a/Firebase/Core/FIROptions.m b/Firebase/Core/FIROptions.m index 81532c5..2216710 100644 --- a/Firebase/Core/FIROptions.m +++ b/Firebase/Core/FIROptions.m @@ -373,11 +373,48 @@ static NSDictionary *sDefaultOptionsDictionary = nil; } NSNumber *value = self.analyticsOptionsDictionary[kFIRIsMeasurementEnabled]; if (value == nil) { - return YES; // Enable Measurement by default when the key is not in the dictionary. + // TODO: This could probably be cleaned up since FIROptions shouldn't know about FIRApp or have + // to check if it's the default app. The FIROptions instance can't be modified after + // `+configure` is called, so it's not a good place to copy it either in case the flag is + // changed at runtime. + + // If no values are set for Analytics, fall back to the global collection switch in FIRApp. + // Analytics only supports the default FIRApp, so check that first. + if (![FIRApp isDefaultAppConfigured]) { + return NO; + } + + // Fall back to the default app's collection switch when the key is not in the dictionary. + return [FIRApp defaultApp].automaticDataCollectionEnabled; } return [value boolValue]; } +- (BOOL)isAnalyticsCollectionExpicitlySet { + // If it's de-activated, it classifies as explicity set. If not, it's not a good enough indication + // that the developer wants FirebaseAnalytics enabled so continue checking. + if (self.isAnalyticsCollectionDeactivated) { + return YES; + } + + // Check if the current Analytics flag is set. + id collectionEnabledObject = self.analyticsOptionsDictionary[kFIRIsAnalyticsCollectionEnabled]; + if (collectionEnabledObject && [collectionEnabledObject isKindOfClass:[NSNumber class]]) { + // It doesn't matter what the value is, it's explicitly set. + return YES; + } + + // Check if the old measurement flag is set. + id measurementEnabledObject = self.analyticsOptionsDictionary[kFIRIsMeasurementEnabled]; + if (measurementEnabledObject && [measurementEnabledObject isKindOfClass:[NSNumber class]]) { + // It doesn't matter what the value is, it's explicitly set. + return YES; + } + + // No flags are set to explicitly enable or disable FirebaseAnalytics. + return NO; +} + - (BOOL)isAnalyticsCollectionEnabled { if (self.isAnalyticsCollectionDeactivated) { return NO; diff --git a/Firebase/Core/Private/FIRAnalyticsConfiguration+Internal.h b/Firebase/Core/Private/FIRAnalyticsConfiguration+Internal.h index 8ea4cea..be624b4 100644 --- a/Firebase/Core/Private/FIRAnalyticsConfiguration+Internal.h +++ b/Firebase/Core/Private/FIRAnalyticsConfiguration+Internal.h @@ -37,3 +37,13 @@ static NSString *const kFIRAnalyticsConfigurationSetMinimumSessionIntervalNotifi @"FIRAnalyticsConfigurationSetMinimumSessionIntervalNotification"; static NSString *const kFIRAnalyticsConfigurationSetSessionTimeoutIntervalNotification = @"FIRAnalyticsConfigurationSetSessionTimeoutIntervalNotification"; + +@interface FIRAnalyticsConfiguration (Internal) + +/// Sets whether analytics collection is enabled for this app on this device, and a flag to persist +/// the value or not. The setting should not be persisted if being set by the global data collection +/// flag. +- (void)setAnalyticsCollectionEnabled:(BOOL)analyticsCollectionEnabled + persistSetting:(BOOL)shouldPersist; + +@end diff --git a/Firebase/Core/Private/FIROptionsInternal.h b/Firebase/Core/Private/FIROptionsInternal.h index 859ac5c..7bb40fc 100644 --- a/Firebase/Core/Private/FIROptionsInternal.h +++ b/Firebase/Core/Private/FIROptionsInternal.h @@ -61,6 +61,12 @@ extern NSString *const kServiceInfoFileType; + (NSDictionary *)defaultOptionsDictionary; +/** + * Indicates whether or not Analytics collection was explicitly enabled via a plist flag or at + * runtime. + */ +@property(nonatomic, readonly) BOOL isAnalyticsCollectionExpicitlySet; + /** * Whether or not Analytics Collection was enabled. Analytics Collection is enabled unless * explicitly disabled in GoogleService-Info.plist. -- cgit v1.2.3