Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: putContext returns after reconciliation #179

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 80 additions & 35 deletions Sources/Confidence/Confidence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Foundation
import Combine
import os

// swiftlint:disable:next type_body_length
public class Confidence: ConfidenceEventSender {
private let clientSecret: String
private var region: ConfidenceRegion
Expand Down Expand Up @@ -48,28 +49,8 @@ public class Confidence: ConfidenceEventSender {
self.remoteFlagResolver = remoteFlagResolver
self.debugLogger = debugLogger
if let visitorId {
putContext(context: ["visitor_id": ConfidenceValue.init(string: visitorId)])
putContextLocal(context: ["visitor_id": ConfidenceValue.init(string: visitorId)])
}

contextChanges().sink { [weak self] context in
guard let self = self else {
return
}
self.currentFetchTask?.cancel()
self.currentFetchTask = Task {
do {
let context = self.getContext()
try await self.fetchAndActivate()
self.contextReconciliatedChanges.send(context.hash())
} catch {
debugLogger?.logMessage(
message: "\(error)",
isWarning: true
)
}
}
}
.store(in: &cancellables)
}

/**
Expand Down Expand Up @@ -214,10 +195,10 @@ public class Confidence: ConfidenceEventSender {
if let contextProducer = producer as? ConfidenceContextProducer {
contextProducer.produceContexts()
.sink { [weak self] context in
guard let self = self else {
return
Task { [weak self] in
guard let self = self else { return }
await self.putContext(context: context)
}
self.putContext(context: context)
}
.store(in: &cancellables)
}
Expand All @@ -238,28 +219,59 @@ public class Confidence: ConfidenceEventSender {
return reconciledCtx
}

public func putContext(key: String, value: ConfidenceValue) {
withLock { confidence in
public func putContext(key: String, value: ConfidenceValue) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
map[key] = value
confidence.contextSubject.value = map
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(message: "Error when putting context: \(error)", isWarning: true)
}
}
}

public func putContext(context: ConfidenceStruct) {

public func putContextLocal(context: ConfidenceStruct, removeKeys removedKeys: [String] = []) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a draft API (not in our internal specs yet). This is to experiment how to solve the integration with the OF layer

withLock { confidence in
var map = confidence.contextSubject.value
for removedKey in removedKeys {
map.removeValue(forKey: removedKey)
}
for entry in context {
map.updateValue(entry.value, forKey: entry.key)
}
confidence.contextSubject.value = map
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
confidence.debugLogger?.logContext(
action: "PutContext",
context: confidence.contextSubject.value)
}
}

public func putContext(context: ConfidenceStruct, removeKeys removedKeys: [String] = []) {
withLock { confidence in
public func putContext(context: ConfidenceStruct) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
for entry in context {
map.updateValue(entry.value, forKey: entry.key)
}
confidence.contextSubject.value = map
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
action: "PutContext & FetchAndActivate",
context: confidence.contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
message: "Error when putting context: \(error)",
isWarning: true)
}
}
}

public func putContext(context: ConfidenceStruct, removeKeys removedKeys: [String] = []) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
for removedKey in removedKeys {
map.removeValue(forKey: removedKey)
Expand All @@ -268,17 +280,35 @@ public class Confidence: ConfidenceEventSender {
map.updateValue(entry.value, forKey: entry.key)
}
confidence.contextSubject.value = map
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
action: "PutContext & FetchAndActivate",
context: confidence.contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
message: "Error when putting context: \(error)",
isWarning: true)
}
}
}

public func removeKey(key: String) {
withLock { confidence in
public func removeContext(key: String) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
map.removeValue(forKey: key)
confidence.contextSubject.value = map
confidence.removedContextKeys.insert(key)
confidence.debugLogger?.logContext(action: "RemoveContext", context: confidence.contextSubject.value)
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
action: "RemoveContextKey & FetchAndActivate",
context: confidence.contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
message: "Error when removing context key: \(error)",
isWarning: true)
}
}
}

Expand All @@ -304,6 +334,21 @@ public class Confidence: ConfidenceEventSender {
callback(self)
}
}

private func withLockAsync(callback: @escaping (Confidence) async -> Void) async {
await withCheckedContinuation { continuation in
contextSubjectQueue.async { [weak self] in
guard let self = self else {
continuation.resume()
return
}
Task {
await callback(self) // Await the async closure
continuation.resume()
}
}
}
}
}

extension Confidence {
Expand Down
4 changes: 2 additions & 2 deletions Sources/Confidence/ConfidenceEventSender.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ public protocol ConfidenceEventSender: ConfidenceContextProvider {
/**
Adds/override entry to local context data
*/
func putContext(key: String, value: ConfidenceValue)
func putContext(key: String, value: ConfidenceValue) async
/**
Removes entry from localcontext data
It hides entries with this key from parents' data (without modifying parents' data)
*/
func removeKey(key: String)
func removeContext(key: String) async
/**
Creates a child event sender instance that maintains access to its parent's data
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public class ConfidenceFeatureProvider: FeatureProvider {
}

private func updateConfidenceContext(context: EvaluationContext, removedKeys: [String] = []) {
confidence.putContext(context: ConfidenceTypeMapper.from(ctx: context), removeKeys: removedKeys)
confidence.putContextLocal(context: ConfidenceTypeMapper.from(ctx: context), removeKeys: removedKeys)
}

public func getBooleanEvaluation(key: String, defaultValue: Bool, context: EvaluationContext?) throws
Expand Down
20 changes: 10 additions & 10 deletions Tests/ConfidenceTests/ConfidenceContextTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testWithContextUpdateParent() {
func testWithContextUpdateParent() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -51,7 +51,7 @@ final class ConfidenceContextTests: XCTestCase {
let confidenceChild: ConfidenceEventSender = confidenceParent.withContext(
["k2": ConfidenceValue(string: "v2")]
)
confidenceParent.putContext(
await confidenceParent.putContext(
key: "k3",
value: ConfidenceValue(string: "v3"))
let expected = [
Expand All @@ -62,7 +62,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testUpdateLocalContext() {
func testUpdateLocalContext() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -80,7 +80,7 @@ final class ConfidenceContextTests: XCTestCase {
parent: nil,
debugLogger: nil
)
confidence.putContext(
await confidence.putContext(
key: "k1",
value: ConfidenceValue(string: "v3"))
let expected = [
Expand All @@ -89,7 +89,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidence.getContext(), expected)
}

func testUpdateLocalContextWithoutOverride() {
func testUpdateLocalContextWithoutOverride() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -110,7 +110,7 @@ final class ConfidenceContextTests: XCTestCase {
let confidenceChild: ConfidenceEventSender = confidenceParent.withContext(
["k2": ConfidenceValue(string: "v2")]
)
confidenceChild.putContext(
await confidenceChild.putContext(
key: "k2",
value: ConfidenceValue(string: "v4"))
let expected = [
Expand All @@ -120,7 +120,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testUpdateParentContextWithOverride() {
func testUpdateParentContextWithOverride() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -141,7 +141,7 @@ final class ConfidenceContextTests: XCTestCase {
let confidenceChild: ConfidenceEventSender = confidenceParent.withContext(
["k2": ConfidenceValue(string: "v2")]
)
confidenceParent.putContext(
await confidenceParent.putContext(
key: "k2",
value: ConfidenceValue(string: "v4"))
let expected = [
Expand Down Expand Up @@ -235,7 +235,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testRemoveContextEntryFromParentAndChildThenUpdate() {
func testRemoveContextEntryFromParentAndChildThenUpdate() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -260,7 +260,7 @@ final class ConfidenceContextTests: XCTestCase {
]
)
confidenceChild.removeKey(key: "k1")
confidenceChild.putContext(key: "k1", value: ConfidenceValue(string: "v4"))
await confidenceChild.putContext(key: "k1", value: ConfidenceValue(string: "v4"))
let expected = [
"k2": ConfidenceValue(string: "v2"),
"k1": ConfidenceValue(string: "v4"),
Expand Down
14 changes: 4 additions & 10 deletions Tests/ConfidenceTests/ConfidenceTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ class ConfidenceTest: XCTestCase {
// Initialize allows to start listening for context changes in "confidence"
// Let the internal "resolve" finish
await fulfillment(of: [resolve1Completed], timeout: 5.0)
confidence.putContext(key: "new", value: ConfidenceValue(string: "value"))
await confidence.putContext(key: "new", value: ConfidenceValue(string: "value"))
await fulfillment(of: [resolve2Started], timeout: 5.0) // Ensure resolve 2 starts before 3
confidence.putContext(key: "new2", value: ConfidenceValue(string: "value2"))
await confidence.putContext(key: "new2", value: ConfidenceValue(string: "value2"))
await fulfillment(of: [resolve3Completed], timeout: 5.0)
resolve2Continues.fulfill() // Allow second resolve to continue, regardless if cancelled or not
await fulfillment(of: [resolve2Cancelled], timeout: 5.0) // Second resolve is cancelled
Expand Down Expand Up @@ -142,14 +142,8 @@ class ConfidenceTest: XCTestCase {
resolveReason: .match)
]

let expectation = expectation(description: "context is synced")
let cancellable = confidence.contextReconciliatedChanges.sink { _ in
expectation.fulfill()
}
confidence.putContext(context: ["targeting_key": .init(string: "user2")])
await fulfillment(of: [expectation], timeout: 1)
cancellable.cancel()

await confidence.putContext(context: ["targeting_key": .init(string: "user2")])
Comment on lines -145 to +146
Copy link
Member Author

@fabriziodemaria fabriziodemaria Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reflects well the change: users can now wait for the context to be reconciled in the flags' cache, before getEvaluation. Before this was not possible, as contextReconciliatedChanges is internal

let evaluation = confidence.getEvaluation(
key: "flag.size",
defaultValue: 0)
Expand Down Expand Up @@ -400,7 +394,7 @@ class ConfidenceTest: XCTestCase {
.build()

try await confidence.fetchAndActivate()
confidence.putContext(context: ["hello": .init(string: "world")])
await confidence.putContext(context: ["hello": .init(string: "world")])
let evaluation = confidence.getEvaluation(
key: "flag.size",
defaultValue: 0)
Expand Down
Loading