launchdarkly / ios-client-sdk

LaunchDarkly Client-side SDK for iOS (Swift and Obj-C)
https://docs.launchdarkly.com/sdk/client-side/ios
Other
70 stars 84 forks source link

Change how the connection status is changed after updating the user #195

Closed polszacki-tooploox closed 4 years ago

polszacki-tooploox commented 4 years ago

Change how the connection status is changed after updating the user to match the one from the 2.13.9 version of the library.

Here's the code from the version 2.13.9:

- (BOOL)updateUser:(LDUserBuilder *)builder {
    DEBUG_LOGX(@"LDClient updateUser method called");
    if (!self.clientStarted) {
        DEBUG_LOGX(@"LDClient aborted updateUser: client not started");
        return NO;
    }
    if (!builder) {
        DEBUG_LOGX(@"LDClient aborted updateUser: LDUserBuilder is nil");
        return NO;
    }

    if (builder.key.length > 0 && [builder.key isEqualToString:self.ldUser.key]) {
        self.ldUser = [LDUserBuilder compareNewBuilder:builder withUser:self.ldUser];
        [[LDDataManager sharedManager] saveUser:self.ldUser];
    } else {
        self.ldUser = [builder build];
    }

    [[LDClientManager sharedInstance] updateUser];
    return YES;
}

-(void)updateUser {
    if (!self.isOnline) {
        DEBUG_LOGX(@"ClientManager updateUser aborted - manager is offline");
        return;
    }
    if (self.eventSource) {
        [self stopEventSource];
    }
    [[LDPollingManager sharedInstance] stopConfigPolling];

    if ([[[LDClient sharedInstance] ldConfig] streaming]) {
        [self configureEventSource];
    } else {
        [self syncWithServerForConfig];
        [[LDPollingManager sharedInstance] startConfigPolling];
    }
}

In the 2.13.9 version, if the (!self.isOnline) check inside updateUser method from LDClientManager failed, nothing happened to the connection status. From version 2.14.0 when the user is updated from the offline state it will force the offline state after the update is completed. If there are two consecutive calls to update the user and the second call starts when the first operation is in progress, meaning the client is set to offline for a moment to update the user, the second call will start with isOnline set to false. The first call after completing the update will set the client online again. Which is fine. But then the second call finishes and sets the client offline as it was the state the second call was started in.

If there is a valid reason for it to work this way, could you at least provide a solution on how to handle updating user securely?

Console output for two update user calls one after another in quick succession:

2019-10-03 10:32:10 +0000 [LD] Setting online: true
2019-10-03 10:32:12 +0000 [LD] Updating user. Is online: true
2019-10-03 10:32:12 +0000 [LD] Setting online: false // Called from the LD SDK during the update as it’s required to restart the client
2019-10-03 10:32:14 +0000 [LD] Updating user. Is online: false
2019-10-03 10:32:14 +0000 [LD] Setting online: false // Called from the LD SDK after the second update finishes
torchhound commented 4 years ago

Hi @polszacki-tooploox thanks for the PR! We have an internal change to this code that we are currently testing. I will test the behavior you pointed out here and evaluate against our internal spec for this behavior.

torchhound commented 4 years ago

Hi @polszacki-tooploox we just released a new version that changes this code, please test whether this suits your workflow.

polszacki-tooploox commented 4 years ago

It does, thanks for the update.