notify observers about changes rooting from an identify call

[diederich]

When setting the user via identify, any observers were not notified about flag changes, although the underlying store changed.

This PR fixes that by calling into the "notify machinery" when swapping out the underlying data.

Requirements

• [-] I have added test coverage for new or changed functionality
• [-] I have followed the repository's pull request submission guidelines
• [-] I have validated my changes against all supported platform versions

[diederich]

👋 Hey there - we just realised that calling identify did not call the feature-flag changed observers. I'm not sure this is by design or was an oversight. If this is by design - is there any other way to be notified of changes?

[lukeredpath]

I’ve just run into this problem - we have an observer that runs for the duration of an app session and an account switch feature that triggers a call to identify - the observer does not detect changes when calling identify. The observer is still working though and will pick up a change if I toggle a flag on and off.

We are using streaming mode - I wonder if this problem also affects polling mode.

[lukeredpath]

For anyone interested in a workaround while this PR is open, I have taken advantage of the fact that the client is taken offline temporarily whenever you call .identify - this means you can create a connection state observer and whenever the state changes to polling or streaming to indicate it is back online again, you can notify your observers of the current flag value.

[jasdev]

@gwhelanLD, @louis-launchdarkly — sorry for the @, but any chance we could get some eyes on this when y’all get a moment? 🙏🏽

[louis-launchdarkly]

Hello @diederich, @lukeredpath, @jasdev Apologize and thank you for reaching out, for looking at this, and for waiting. @gwhelanLD and I had discussed this, and here are a few thoughts:

1. The flagChangeNotifier.notifyObservers() method is currently used when the flag is changed from a Sync (i.e. someone modified the flag in LaunchDarkly, and the SDK received the change, which is different from this proposal, when the flag values are different, due to change in User context. So just calling notifyObservers may create confusion here.

2. However, it is reasonable to want to know "Did identify change the current flag values in the SDK?". Similarly, some users may not want to know as there is an identify event already to note that flag context is being replaced by identify.

3. To serve the diverging needs, it seems that it is helpful to have means to control this behavior. We will need to evaluate more before we can propose a solution, but we are looking into it.

Filed internally as 128729

[fischman-bcny]

@louis-launchdarkly any update on this / internal 128729 ?

[brianmichel]

Hey @louis-launchdarkly any chance we can get this merged? We'd love to delete our internal fork and use the main LD library, happy to help out here in any way, just let me know :D

[louis-launchdarkly]

Hello @brianmichel, I am really sorry for the late reply. We are currently doing an audit of all the Client-side SDKs from LaunchDarkly, and are trying to line up the behavior of each SDK with this request in mind. That will inform can we merge this or adopt the change in the new callback design.

[patriknyblad]

Just wanted to chime in here and say that we also have issues with this and have been debugging what is going on this week. In the end we implemented a patch in our middle layer code that triggers all observers when we update the context.

If the LDClient got all the updated flags internally when changing context... shouldn't the observers also be notified?
Surely this must be an important bug to fix asap?

[brianmichel]

@louis-launchdarkly @gwhelanLD any updates here?

[brianmichel]

Actually, it looks like this is resolved in the latest v9 commits?

ios-client-sdk/LaunchDarkly/LaunchDarkly/LDClient.swift

Lines 307 to 312 in 0fedd19

	let cachedContextFlags = self.flagCache.retrieveFeatureFlags(contextKey: self.context.fullyQualifiedHashedKey()) ?? [:]
	let oldItems = flagStore.storedItems.featureFlags
	flagStore.replaceStore(newStoredItems: cachedContextFlags)
	flagChangeNotifier.notifyObservers(oldFlags: oldItems, newFlags: flagStore.storedItems.featureFlags)
	self.service.context = self.context
	self.service.clearFlagResponseCache()

[louis-launchdarkly]

Hello @patriknyblad and @brianmichel, sorry for the late reply to this issue. From the investigation, this is missed from the iOS and not some intentional behavior, and this was fixed in both https://github.com/launchdarkly/ios-client-sdk/releases/tag/8.3.1 and https://github.com/launchdarkly/ios-client-sdk/releases/tag/9.2.1

Can you please verify that this fixed your issue?

[brianmichel]

Hello @patriknyblad and @brianmichel, sorry for the late reply to this issue. From the investigation, this is missed from the iOS and not some intentional behavior, and this was fixed in both https://github.com/launchdarkly/ios-client-sdk/releases/tag/8.3.1 and https://github.com/launchdarkly/ios-client-sdk/releases/tag/9.2.1

Can you please verify that this fixed your issue?

@louis-launchdarkly I don't think this is fixed exactly, see #331 that I just filed

[patriknyblad]

@louis-launchdarkly We have been able to remove our local fix after updating the SDK. We were in contact with LD support directly and the issue is fixed for us 🙏

[keelerm84]

Closing this PR as the fix from a previous commit seems to have done the trick for everyone.