Skip to content

Commit

Permalink
feat: calculate updated keys and invalidate flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@ioannisj
ioannisj committed Nov 6, 2024
1 parent edd9a05 commit a95c253
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 42 deletions.
83 changes: 68 additions & 15 deletions PostHog/PostHogFeatureFlags.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class PostHogFeatureFlags {
distinctId: String,
anonymousId: String,
groups: [String: String],
callback: @escaping () -> Void
callback: @escaping (Set<String>) -> Void
) {
loadingLock.withLock {
if self.loadingFeatureFlags {
Expand All @@ -99,9 +99,9 @@ class PostHogFeatureFlags {
else {
hedgeLog("Error: Decide response missing correct featureFlags format")

self.notifyAndRelease()
callback([])

return callback()
return self.notifyAndRelease()
}
let errorsWhileComputingFlags = data?["errorsWhileComputingFlags"] as? Bool ?? false

Expand All @@ -127,25 +127,44 @@ class PostHogFeatureFlags {
#endif

self.featureFlagsLock.withLock {
if errorsWhileComputingFlags {
let cachedFeatureFlags = self.getCachedFeatureFlags() ?? [:]
let cachedFeatureFlagsPayloads = self.getCachedFeatureFlagPayload() ?? [:]
let cachedFeatureFlags = self.getCachedFeatureFlags() ?? [:]
let cachedFeatureFlagsPayloads = self.getCachedFeatureFlagPayload() ?? [:]

let newFeatureFlags = cachedFeatureFlags.merging(featureFlags) { _, new in new }
let newFeatureFlagsPayloads = cachedFeatureFlagsPayloads.merging(featureFlagPayloads) { _, new in new }
let newFeatureFlags: [String: Any]
let newFeatureFlagPayloads: [String: Any]

if errorsWhileComputingFlags {
// if not all flags were computed, we upsert flags instead of replacing them
self.setCachedFeatureFlags(newFeatureFlags)
self.setCachedFeatureFlagPayload(newFeatureFlagsPayloads)
newFeatureFlags = cachedFeatureFlags.merging(featureFlags) { _, new in new }
newFeatureFlagPayloads = cachedFeatureFlagsPayloads.merging(featureFlagPayloads) { _, new in new }
} else {
self.setCachedFeatureFlags(featureFlags)
self.setCachedFeatureFlagPayload(featureFlagPayloads)
newFeatureFlags = featureFlags
newFeatureFlagPayloads = featureFlagPayloads
}
}

self.notifyAndRelease()
self.setCachedFeatureFlags(newFeatureFlags)
self.setCachedFeatureFlagPayload(newFeatureFlagPayloads)

// calculate a set of keys that were updated, added or removed
//
// Note: payload changes are not currently tracked, as this update mechanism
// is primarily used for capturing `$feature_flag_called` events tied to flag values.
let newKeys = Set(newFeatureFlags.keys)
let cachedKeys = Set(cachedFeatureFlags.keys)
let addedKeys = newKeys.subtracting(cachedKeys)
let removedKeys = cachedKeys.subtracting(newKeys)
let updatedKeys = cachedKeys.intersection(newKeys).filter {
if let cached = cachedFeatureFlags[$0], let new = newFeatureFlags[$0] {
return !areEqual(cached, new)
}
return false
}
let allUpdatedKeys = removedKeys.union(addedKeys).union(updatedKeys)

return callback()
callback(allUpdatedKeys)
}

return self.notifyAndRelease()
}
}
}
Expand Down Expand Up @@ -252,3 +271,37 @@ class PostHogFeatureFlags {
}
#endif
}

/// Compares two optional `Any` values for equality, best effort
private func areEqual(_ lhs: Any?, _ rhs: Any?) -> Bool {
guard let lhs, let rhs else { return false }

// assume here that different types cannot be equal
guard type(of: lhs) == type(of: rhs) else { return false }

// AnyHashable is handy here. Most types should conform to this
if let lhs = lhs as? AnyHashable, let rhs = rhs as? AnyHashable {
return lhs == rhs
}

// Equatable types is a good next candidate
if let lhs = lhs as? (any Equatable), let rhs = rhs as? (any Equatable) {
return lhs.isEqual(rhs)
}

// Equatable types is a good next candidate
if let lhs = lhs as? NSObject, let rhs = rhs as? NSObject {
return lhs == rhs
}

return false
}

private extension Equatable {
func isEqual(_ other: any Equatable) -> Bool {
guard let other = other as? Self else {
return false
}
return self == other
}
}
4 changes: 2 additions & 2 deletions PostHog/PostHogSDK.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,8 @@ let maxRetryDelay = 30.0
distinctId: storageManager.getDistinctId(),
anonymousId: storageManager.getAnonymousId(),
groups: groups ?? [:],
callback: {
self.flagCallReported.removeAll()
callback: { updatedKeys in
self.flagCallReported.subtract(updatedKeys)
callback()
}
)
Expand Down
28 changes: 14 additions & 14 deletions PostHogTests/PostHogFeatureFlagsTest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -49,7 +49,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -63,7 +63,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -77,7 +77,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -91,7 +91,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -105,7 +105,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -117,7 +117,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -128,7 +128,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group2 = DispatchGroup()
group2.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group2.leave()
})

Expand Down Expand Up @@ -171,7 +171,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
let group = DispatchGroup()
group.enter()

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -195,7 +195,7 @@ class PostHogFeatureFlagsTest: QuickSpec {

server.returnReplay = true

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -221,7 +221,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
server.returnReplay = true
server.returnReplayWithVariant = true

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand All @@ -248,7 +248,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
server.returnReplayWithVariant = true
server.replayVariantValue = false

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand Down Expand Up @@ -277,7 +277,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
server.replayVariantName = "recording-platform"
server.replayVariantValue = ["flag": "recording-platform-check", "variant": "web"]

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand Down Expand Up @@ -306,7 +306,7 @@ class PostHogFeatureFlagsTest: QuickSpec {
server.replayVariantName = "recording-platform"
server.replayVariantValue = ["flag": "recording-platform-check", "variant": "mobile"]

sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: {
sut.loadFeatureFlags(distinctId: "distinctId", anonymousId: "anonymousId", groups: ["group": "value"], callback: { _ in
group.leave()
})

Expand Down
82 changes: 71 additions & 11 deletions PostHogTests/PostHogSDKTest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,25 +859,85 @@ class PostHogSDKTest: QuickSpec {
expect(event.first!.event).to(equal("$feature_flag_called"))
}

it("captures a second $feature_flag_called when feature flag is called after flags are reloaded") {
let sut = self.getSut(sendFeatureFlagEvent: true)
_ = sut.getFeatureFlag("some_key")
it("captures a second $feature_flag_called if reloaded flag changed value ") {
let sut = self.getSut(
preloadFeatureFlags: false,
sendFeatureFlagEvent: true,
flushAt: 2
)

let event = getBatchedEvents(server)
expect(event.first!.event).to(equal("$feature_flag_called"))
sut.reloadFeatureFlags {
DispatchQueue.main.async {
// this should be captured with value true
_ = sut.getFeatureFlag(server.flagKey)
}
server.flagValue = false
sut.reloadFeatureFlags {
DispatchQueue.main.async {
// this should be captured with value false
_ = sut.getFeatureFlag(server.flagKey)
}
}
}

server.start()
let batches = getBatchedEvents(server)
expect(batches.count).to(equal(2))
expect(batches[0].event).to(equal("$feature_flag_called"))
expect(batches[1].event).to(equal("$feature_flag_called"))
expect(batches[0].properties["$feature_flag_response"] as? Bool).to(beTrue())
expect(batches[1].properties["$feature_flag_response"] as? Bool).to(beFalse())
}

let group = DispatchGroup()
it("does not capture a second $feature_flag_called if reloaded flag did not change value ") {
let sut = self.getSut(
preloadFeatureFlags: false,
sendFeatureFlagEvent: true,
flushAt: 2
)

sut.reloadFeatureFlags {
_ = sut.getFeatureFlag("some_key")
sut.flush()
DispatchQueue.main.async {
// this should be captured
_ = sut.getFeatureFlag(server.flagKey)
}
sut.reloadFeatureFlags {
DispatchQueue.main.async {
// this should not be captured
_ = sut.getFeatureFlag(server.flagKey)
sut.flush()
}
}
}

let secondEvent = getBatchedEvents(server)
let batches = getBatchedEvents(server)
expect(batches.count).to(equal(1))
expect(batches[0].event).to(equal("$feature_flag_called"))
expect(batches[0].properties["$feature_flag_response"] as? Bool).to(beTrue())
}

it("does captures a second $feature_flag_called if reloaded flag did not change value, but keys were not preloaded") {
let sut = self.getSut(
preloadFeatureFlags: false,
sendFeatureFlagEvent: true,
flushAt: 2
)

// this should capture with value nil
_ = sut.getFeatureFlag(server.flagKey)

sut.reloadFeatureFlags {
DispatchQueue.main.async {
// this should capture with value true
_ = sut.getFeatureFlag(server.flagKey)
}
}

expect(secondEvent.first!.event).to(equal("$feature_flag_called"))
let batches = getBatchedEvents(server)
expect(batches.count).to(equal(2))
expect(batches[0].event).to(equal("$feature_flag_called"))
expect(batches[1].event).to(equal("$feature_flag_called"))
expect(batches[0].properties["$feature_flag_response"] as? Bool).to(beNil())
expect(batches[1].properties["$feature_flag_response"] as? Bool).to(beTrue())
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions PostHogTests/TestUtils/MockPostHogServer.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ class MockPostHogServer {
public var returnReplayWithMultiVariant = false
public var replayVariantName = "myBooleanRecordingFlag"
public var replayVariantValue: Any = true
public var flagKey = "flag-key"
public var flagValue: Bool = true

init(port _: Int = 9001) {
stub(condition: isPath("/decide")) { _ in
var flags = [
self.flagKey: self.flagValue,
"bool-value": true,
"string-value": "test",
"disabled-flag": false,
Expand Down

0 comments on commit a95c253

Please sign in to comment.