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 1 commit
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
45 changes: 21 additions & 24 deletions Sources/Confidence/Confidence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,6 @@
if let visitorId {
putContext(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 @@ -238,22 +218,24 @@
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
try! await self.fetchAndActivate()

Check failure on line 226 in Sources/Confidence/Confidence.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Force Try Violation: Force tries should be avoided (force_try)
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
}
}

public func putContext(context: ConfidenceStruct) {
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
try! await self.fetchAndActivate()

Check failure on line 238 in Sources/Confidence/Confidence.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Force Try Violation: Force tries should be avoided (force_try)
confidence.debugLogger?.logContext(action: "PutContext", context: confidence.contextSubject.value)
}
}
Expand Down Expand Up @@ -304,6 +286,21 @@
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
2 changes: 1 addition & 1 deletion Sources/Confidence/ConfidenceEventSender.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ 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)
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