From 67283d1678f17b65a30f8b6a771ccd373215b130 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Tue, 9 Jan 2018 16:59:26 -0800 Subject: Refactor user defaults update handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This updates the handling logic for user defaults to break apart config changes that require a check for pausing or resuming the events lib’s telemetry collection from config changes that can happen with no update to the pause/resume state. It also ports the optimization from https://github.com/mapbox/mapbox-gl-native/pull/10803 so that the call to pause and resume is not performed unless the user defaults store has new values that have not yet been applied to the events library. --- platform/ios/src/MGLMapboxEvents.m | 89 ++++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/platform/ios/src/MGLMapboxEvents.m b/platform/ios/src/MGLMapboxEvents.m index 59eb070449..15cbf50fb6 100644 --- a/platform/ios/src/MGLMapboxEvents.m +++ b/platform/ios/src/MGLMapboxEvents.m @@ -3,6 +3,11 @@ #import "NSProcessInfo+MGLAdditions.h" static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; +static NSString * const MGLMapboxAccountType = @"MGLMapboxAccountType"; +static NSString * const MGLMapboxMetricsEnabled = @"MGLMapboxMetricsEnabled"; +static NSString * const MGLMapboxMetricsDebugLoggingEnabled = @"MGLMapboxMetricsDebugLoggingEnabled"; +static NSString * const MGLTelemetryAccessToken = @"MGLTelemetryAccessToken"; +static NSString * const MGLTelemetryBaseURL = @"MGLTelemetryBaseURL"; @interface MGLMapboxEvents () @@ -17,10 +22,10 @@ static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; + (void)initialize { if (self == [MGLMapboxEvents class]) { NSBundle *bundle = [NSBundle mainBundle]; - NSNumber *accountTypeNumber = [bundle objectForInfoDictionaryKey:@"MGLMapboxAccountType"]; - [[NSUserDefaults standardUserDefaults] registerDefaults:@{@"MGLMapboxAccountType": accountTypeNumber ?: @0, - @"MGLMapboxMetricsEnabled": @YES, - @"MGLMapboxMetricsDebugLoggingEnabled": @NO}]; + NSNumber *accountTypeNumber = [bundle objectForInfoDictionaryKey:MGLMapboxAccountType]; + [[NSUserDefaults standardUserDefaults] registerDefaults:@{MGLMapboxAccountType: accountTypeNumber ?: @0, + MGLMapboxMetricsEnabled: @YES, + MGLMapboxMetricsDebugLoggingEnabled: @NO}]; } } @@ -41,19 +46,19 @@ static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; self = [super init]; if (self) { _eventsManager = [[MMEEventsManager alloc] init]; - _eventsManager.debugLoggingEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsDebugLoggingEnabled"]; - _eventsManager.accountType = [[NSUserDefaults standardUserDefaults] integerForKey:@"MGLMapboxAccountType"]; - _eventsManager.metricsEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsEnabled"]; + _eventsManager.debugLoggingEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:MGLMapboxMetricsDebugLoggingEnabled]; + _eventsManager.accountType = [[NSUserDefaults standardUserDefaults] integerForKey:MGLMapboxAccountType]; + _eventsManager.metricsEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:MGLMapboxMetricsEnabled]; // Because we can get here [MGLAccountManaber +load] it is possible that values from user defaults will be read and // applied here. These checks and local accessToken and baseURL assigns work around that fact. If user default value // are set, they are stored on the local properties here and then only applied later on once MMEEventsManager is // fully initialized (once -[MMEEventsManager initializeWithAccessToken:userAgentBase:hostSDKVersion:] is called. - if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:@"MGLTelemetryAccessToken"]) { - self.accessToken = [[NSUserDefaults standardUserDefaults] objectForKey:@"MGLTelemetryAccessToken"]; + if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:MGLTelemetryAccessToken]) { + self.accessToken = [[NSUserDefaults standardUserDefaults] objectForKey:MGLTelemetryAccessToken]; } - if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:@"MGLTelemetryBaseURL"]) { - self.baseURL = [NSURL URLWithString:[[NSUserDefaults standardUserDefaults] objectForKey:@"MGLTelemetryBaseURL"]]; + if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:MGLTelemetryBaseURL]) { + self.baseURL = [NSURL URLWithString:[[NSUserDefaults standardUserDefaults] objectForKey:MGLTelemetryBaseURL]]; } [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(userDefaultsDidChange:) name:NSUserDefaultsDidChangeNotification object:nil]; @@ -67,28 +72,48 @@ static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; - (void)userDefaultsDidChange:(NSNotification *)notification { dispatch_async(dispatch_get_main_queue(), ^{ - self.eventsManager.metricsEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsEnabled"]; - self.eventsManager.accountType = [[NSUserDefaults standardUserDefaults] integerForKey:@"MGLMapboxAccountType"]; - self.eventsManager.debugLoggingEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsDebugLoggingEnabled"]; - + [self updateNonDisablingConfigurationValues]; + [self updateDisablingConfigurationValuesWithNotification:notification]; + }); +} - // It is possible that MGLTelemetryAccessToken is set and userDefaultsDidChange: is called before setupWithAccessToken: is called. - // In that case, setting the access token here will be a noop. In practice, that's fine because the access token value - // will be resolved when setupWithAccessToken: is called eventually - if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:@"MGLTelemetryAccessToken"]) { - self.eventsManager.accessToken = [[NSUserDefaults standardUserDefaults] objectForKey:@"MGLTelemetryAccessToken"]; - } +- (void)updateNonDisablingConfigurationValues { + self.eventsManager.debugLoggingEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsDebugLoggingEnabled"]; + + // It is possible that MGLTelemetryAccessToken is set and userDefaultsDidChange: is called before setupWithAccessToken: is called. + // In that case, setting the access token here will be a noop. In practice, that's fine because the access token value + // will be resolved when setupWithAccessToken: is called eventually + if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:MGLTelemetryAccessToken]) { + self.eventsManager.accessToken = [[NSUserDefaults standardUserDefaults] objectForKey:MGLTelemetryAccessToken]; + } + + // It is possible that MGLTelemetryBaseURL is set and userDefaultsDidChange: is called before setupWithAccessToken: is called. + // In that case, setting baseURL here will be a noop. In practice, that's fine because the baseURL value + // will be resolved when setupWithAccessToken: is called eventually + if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:MGLTelemetryBaseURL]) { + NSURL *baseURL = [NSURL URLWithString:[[NSUserDefaults standardUserDefaults] objectForKey:MGLTelemetryBaseURL]]; + self.eventsManager.baseURL = baseURL; + } +} + +- (void)updateDisablingConfigurationValuesWithNotification:(NSNotification *)notification { + // Guard against over calling pause / resume if the values this implementation actually + // cares about have not changed. We guard becasue the pause and resume method checks CoreLocation's + // authorization status and that can drag on the main thread if done too many times (e.g. if the host + // app heavily uses the user defaults API and this method is called very frequently) + if ([[notification object] respondsToSelector:@selector(objectForKey:)]) { + NSUserDefaults *userDefaults = [notification object]; - // It is possible that MGLTelemetryBaseURL is set and userDefaultsDidChange: is called before setupWithAccessToken: is called. - // In that case, setting baseURL here will be a noop. In practice, that's fine because the baseURL value - // will be resolved when setupWithAccessToken: is called eventually - if ([[[[NSUserDefaults standardUserDefaults] dictionaryRepresentation] allKeys] containsObject:@"MGLTelemetryBaseURL"]) { - NSURL *baseURL = [NSURL URLWithString:[[NSUserDefaults standardUserDefaults] objectForKey:@"MGLTelemetryBaseURL"]]; - self.eventsManager.baseURL = baseURL; - } + NSInteger accountType = [userDefaults integerForKey:MGLMapboxAccountType]; + BOOL metricsEnabled = [userDefaults boolForKey:MGLMapboxMetricsEnabled]; - [self.eventsManager pauseOrResumeMetricsCollectionIfRequired]; - }); + if (accountType != self.eventsManager.accountType || metricsEnabled != self.eventsManager.metricsEnabled) { + self.eventsManager.accountType = accountType; + self.eventsManager.metricsEnabled = metricsEnabled; + + [self.eventsManager pauseOrResumeMetricsCollectionIfRequired]; + } + } } + (void)setupWithAccessToken:(NSString *)accessToken { @@ -126,7 +151,7 @@ static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; BOOL metricsEnabledSettingShownInAppFlag = [shownInAppNumber boolValue]; if (!metricsEnabledSettingShownInAppFlag && - [[NSUserDefaults standardUserDefaults] integerForKey:@"MGLMapboxAccountType"] == 0) { + [[NSUserDefaults standardUserDefaults] integerForKey:MGLMapboxAccountType] == 0) { // Opt-out is not configured in UI, so check for Settings.bundle id defaultEnabledValue; NSString *appSettingsBundle = [[NSBundle mainBundle] pathForResource:@"Settings" ofType:@"bundle"]; @@ -136,7 +161,7 @@ static NSString * const MGLAPIClientUserAgentBase = @"MapboxEventsiOS"; NSDictionary *settings = [NSDictionary dictionaryWithContentsOfFile:[appSettingsBundle stringByAppendingPathComponent:@"Root.plist"]]; NSArray *preferences = settings[@"PreferenceSpecifiers"]; for (NSDictionary *prefSpecification in preferences) { - if ([prefSpecification[@"Key"] isEqualToString:@"MGLMapboxMetricsEnabled"]) { + if ([prefSpecification[@"Key"] isEqualToString:MGLMapboxMetricsEnabled]) { defaultEnabledValue = prefSpecification[@"DefaultValue"]; } } -- cgit v1.2.1