Skip to content

Commit

Permalink
fix: context modifiers are serialized
Browse files Browse the repository at this point in the history
  • Loading branch information
fabriziodemaria committed Dec 2, 2024
1 parent a4fdde4 commit f4ec841
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 162 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"images" : [
{
"filename" : "ConfidenceLogo.png",
"idiom" : "universal",
"platform" : "ios",
"size" : "1024x1024"
Expand Down
107 changes: 58 additions & 49 deletions Sources/Confidence/Confidence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public class Confidence: ConfidenceEventSender {
private var cache = FlagResolution.EMPTY
private var storage: Storage
private var cancellables = Set<AnyCancellable>()
private var currentFetchTask: Task<(), Never>?
private let debugLogger: DebugLogger?
private let semaphore = DispatchSemaphore(value: 1)

// Internal for testing
internal let remoteFlagResolver: ConfidenceResolveClient
Expand Down Expand Up @@ -220,92 +220,108 @@ public class Confidence: ConfidenceEventSender {
}

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


public func putContextLocal(context: ConfidenceStruct, removeKeys removedKeys: [String] = []) {
withLock { confidence in
var map = confidence.contextSubject.value
// Maybe use the semaphore instead
withSemaphore { [weak self] in
guard let self = self else {
return
}
var map = 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(
contextSubject.value = map
debugLogger?.logContext(
action: "PutContext",
context: confidence.contextSubject.value)
context: contextSubject.value)
}
}

public func putContext(context: ConfidenceStruct) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
await withSemaphoreAsync { [weak self] in
guard let self = self else {
return
}
var map = contextSubject.value
for entry in context {
map.updateValue(entry.value, forKey: entry.key)
}
confidence.contextSubject.value = map
contextSubject.value = map
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
try await fetchAndActivate()
debugLogger?.logContext(
action: "PutContext & FetchAndActivate",
context: confidence.contextSubject.value)
context: contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
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
await withSemaphoreAsync { [weak self] in
guard let self = self else {
return
}
var map = 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
contextSubject.value = map
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
debugLogger?.logContext(
action: "PutContext & FetchAndActivate",
context: confidence.contextSubject.value)
context: contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
debugLogger?.logMessage(
message: "Error when putting context: \(error)",
isWarning: true)
}
}
}

public func removeContext(key: String) async {
await withLockAsync { confidence in
var map = confidence.contextSubject.value
await withSemaphoreAsync { [weak self] in
guard let self = self else {
return
}
var map = contextSubject.value
map.removeValue(forKey: key)
confidence.contextSubject.value = map
confidence.removedContextKeys.insert(key)
contextSubject.value = map
removedContextKeys.insert(key)
do {
try await self.fetchAndActivate()
confidence.debugLogger?.logContext(
debugLogger?.logContext(
action: "RemoveContextKey & FetchAndActivate",
context: confidence.contextSubject.value)
context: contextSubject.value)
} catch {
confidence.debugLogger?.logMessage(
debugLogger?.logMessage(
message: "Error when removing context key: \(error)",
isWarning: true)
}
Expand All @@ -326,28 +342,21 @@ public class Confidence: ConfidenceEventSender {
)
}

private func withLock(callback: @escaping (Confidence) -> Void) {
contextSubjectQueue.sync { [weak self] in
guard let self = self else {
return
func withSemaphoreAsync(callback: @escaping () async -> Void) async {
await withCheckedContinuation { continuation in
DispatchQueue.global().async {
self.semaphore.wait()
continuation.resume()
}
callback(self)
}
await callback()
semaphore.signal()
}

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()
}
}
}
func withSemaphore(callback: @escaping () -> Void) {
self.semaphore.wait()
callback()
semaphore.signal()
}
}

Expand Down
14 changes: 7 additions & 7 deletions Tests/ConfidenceTests/ConfidenceContextTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ final class ConfidenceContextTests: XCTestCase {
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testRemoveContextEntry() {
func testRemoveContextEntry() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -169,14 +169,14 @@ final class ConfidenceContextTests: XCTestCase {
parent: nil,
debugLogger: nil
)
confidence.removeKey(key: "k2")
await confidence.removeContext(key: "k2")
let expected = [
"k1": ConfidenceValue(string: "v1")
]
XCTAssertEqual(confidence.getContext(), expected)
}

func testRemoveContextEntryFromParent() {
func testRemoveContextEntryFromParent() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -197,14 +197,14 @@ final class ConfidenceContextTests: XCTestCase {
let confidenceChild: ConfidenceEventSender = confidenceParent.withContext(
["k2": ConfidenceValue(string: "v2")]
)
confidenceChild.removeKey(key: "k1")
await confidenceChild.removeContext(key: "k1")
let expected = [
"k2": ConfidenceValue(string: "v2")
]
XCTAssertEqual(confidenceChild.getContext(), expected)
}

func testRemoveContextEntryFromParentAndChild() {
func testRemoveContextEntryFromParentAndChild() async {
let client = RemoteConfidenceResolveClient(
options: ConfidenceClientOptions(
credentials: ConfidenceClientCredentials.clientSecret(secret: ""), timeoutIntervalForRequest: 10),
Expand All @@ -228,7 +228,7 @@ final class ConfidenceContextTests: XCTestCase {
"k1": ConfidenceValue(string: "v3"),
]
)
confidenceChild.removeKey(key: "k1")
await confidenceChild.removeContext(key: "k1")
let expected = [
"k2": ConfidenceValue(string: "v2")
]
Expand Down Expand Up @@ -259,7 +259,7 @@ final class ConfidenceContextTests: XCTestCase {
"k1": ConfidenceValue(string: "v3"),
]
)
confidenceChild.removeKey(key: "k1")
await confidenceChild.removeContext(key: "k1")
await confidenceChild.putContext(key: "k1", value: ConfidenceValue(string: "v4"))
let expected = [
"k2": ConfidenceValue(string: "v2"),
Expand Down
105 changes: 2 additions & 103 deletions Tests/ConfidenceTests/ConfidenceTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import XCTest
class ConfidenceTest: XCTestCase {
private var flagApplier = FlagApplierMock()
private let storage = StorageMock()
private var readyExpectation = XCTestExpectation(description: "Ready")
override func setUp() {
try? storage.clear()

Expand All @@ -20,96 +19,6 @@ class ConfidenceTest: XCTestCase {
super.setUp()
}

// swiftlint:disable function_body_length
func testSlowFirstResolveWillbeCancelledOnSecondResolve() async throws {
let resolve1Completed = expectation(description: "First resolve completed")
let resolve2Started = expectation(description: "Second resolve has started")
let resolve2Continues = expectation(description: "Unlock second resolve")
let resolve2Cancelled = expectation(description: "Second resolve cancelled")
let resolve3Completed = expectation(description: "Third resolve completed")

class FakeClient: XCTestCase, ConfidenceResolveClient {
var callCount = 0
var resolveContexts: [ConfidenceStruct] = []
let resolve1Completed: XCTestExpectation
let resolve2Started: XCTestExpectation
let resolve2Continues: XCTestExpectation
let resolve2Cancelled: XCTestExpectation
let resolve3Completed: XCTestExpectation

init(
resolve1Completed: XCTestExpectation,
resolve2Started: XCTestExpectation,
resolve2Continues: XCTestExpectation,
resolve2Cancelled: XCTestExpectation,
resolve3Completed: XCTestExpectation
) {
self.resolve1Completed = resolve1Completed
self.resolve2Started = resolve2Started
self.resolve2Continues = resolve2Continues
self.resolve2Cancelled = resolve2Cancelled
self.resolve3Completed = resolve3Completed
super.init(invocation: nil) // Workaround to use expectations in FakeClient
}

func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult {
callCount += 1
switch callCount {
case 1:
if Task.isCancelled {
XCTFail("Resolve one was cancelled unexpectedly")
} else {
resolveContexts.append(ctx)
resolve1Completed.fulfill()
}
case 2:
resolve2Started.fulfill()
await fulfillment(of: [resolve2Continues], timeout: 5.0)
if Task.isCancelled {
resolve2Cancelled.fulfill()
return .init(resolvedValues: [], resolveToken: "")
}
XCTFail("This task should be cancelled and never reach here")
case 3:
if Task.isCancelled {
XCTFail("Resolve three was cancelled unexpectedly")
} else {
resolveContexts.append(ctx)
resolve3Completed.fulfill()
}
default: XCTFail("We expect only 3 resolve calls")
}
return .init(resolvedValues: [], resolveToken: "")
}
}
let client = FakeClient(
resolve1Completed: resolve1Completed,
resolve2Started: resolve2Started,
resolve2Continues: resolve2Continues,
resolve2Cancelled: resolve2Cancelled,
resolve3Completed: resolve3Completed
)
let confidence = Confidence.Builder.init(clientSecret: "")
.withContext(initialContext: ["targeting_key": .init(string: "user1")])
.withFlagResolverClient(flagResolver: client)
.build()

try await confidence.fetchAndActivate()
// Initialize allows to start listening for context changes in "confidence"
// Let the internal "resolve" finish
await fulfillment(of: [resolve1Completed], timeout: 5.0)
await confidence.putContext(key: "new", value: ConfidenceValue(string: "value"))
await fulfillment(of: [resolve2Started], timeout: 5.0) // Ensure resolve 2 starts before 3
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
XCTAssertEqual(3, client.callCount)
XCTAssertEqual(2, client.resolveContexts.count)
XCTAssertEqual(confidence.getContext(), client.resolveContexts[1])
}
// swiftlint:enable function_body_length

func testRefresh() async throws {
class FakeClient: ConfidenceResolveClient {
var resolveStats: Int = 0
Expand Down Expand Up @@ -370,8 +279,7 @@ class ConfidenceTest: XCTestCase {
var resolvedValues: [ResolvedValue] = []
func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult {
if self.resolveStats == 1 {
let expectation = expectation(description: "never fullfil")
await fulfillment(of: [expectation])
throw ConfidenceError.internalError(message: "test")
}
self.resolveStats += 1
return .init(resolvedValues: resolvedValues, resolveToken: "token")
Expand Down Expand Up @@ -406,7 +314,6 @@ class ConfidenceTest: XCTestCase {
XCTAssertEqual(evaluation.reason, .stale)
XCTAssertEqual(evaluation.variant, "control")
XCTAssertEqual(client.resolveStats, 1)
await fulfillment(of: [flagApplier.applyExpectation], timeout: 1)
XCTAssertEqual(flagApplier.applyCallCount, 1)
}

Expand Down Expand Up @@ -691,15 +598,7 @@ class ConfidenceTest: XCTestCase {
XCTAssertEqual(flagApplier.applyCallCount, 0)
}

func testConcurrentActivate() async {
for _ in 1...100 {
Task {
await concurrentActivate()
}
}
}

private func concurrentActivate() async {
func concurrentActivate() async {
let confidence = Confidence.Builder(clientSecret: "test")
.build()

Expand Down
Loading

0 comments on commit f4ec841

Please sign in to comment.