diff --git a/Sources/Confidence/Confidence.swift b/Sources/Confidence/Confidence.swift index 7a07a2bf..a794f3d8 100644 --- a/Sources/Confidence/Confidence.swift +++ b/Sources/Confidence/Confidence.swift @@ -17,8 +17,8 @@ public class Confidence: ConfidenceEventSender { private var cache = FlagResolution.EMPTY private var storage: Storage private var cancellables = Set() - private var currentFetchTask: Task<(), Never>? private let debugLogger: DebugLogger? + private let semaphore = DispatchSemaphore(value: 1) // Internal for testing internal let remoteFlagResolver: ConfidenceResolveClient @@ -220,50 +220,60 @@ 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) } @@ -271,22 +281,25 @@ public class Confidence: ConfidenceEventSender { } 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) } @@ -294,18 +307,21 @@ public class Confidence: ConfidenceEventSender { } 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) } @@ -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() } } diff --git a/Tests/ConfidenceTests/ConfidenceContextTests.swift b/Tests/ConfidenceTests/ConfidenceContextTests.swift index 9e087b39..b97ffa88 100644 --- a/Tests/ConfidenceTests/ConfidenceContextTests.swift +++ b/Tests/ConfidenceTests/ConfidenceContextTests.swift @@ -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), @@ -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), @@ -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), @@ -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") ] @@ -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"), diff --git a/Tests/ConfidenceTests/ConfidenceTest.swift b/Tests/ConfidenceTests/ConfidenceTest.swift index 09adda67..6871f0a6 100644 --- a/Tests/ConfidenceTests/ConfidenceTest.swift +++ b/Tests/ConfidenceTests/ConfidenceTest.swift @@ -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() @@ -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 @@ -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") @@ -691,15 +599,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() diff --git a/api/Confidence_public_api.json b/api/Confidence_public_api.json index be9aaed5..da5e3e39 100644 --- a/api/Confidence_public_api.json +++ b/api/Confidence_public_api.json @@ -55,8 +55,8 @@ "declaration": "public func putContext(context: ConfidenceStruct, removeKeys removedKeys: [String] = [])" }, { - "name": "removeKey(key:)", - "declaration": "public func removeKey(key: String)" + "name": "removeContext(key:)", + "declaration": "public func removeContext(key: String)" }, { "name": "withContext(_:)", @@ -245,4 +245,4 @@ } ] } -] \ No newline at end of file +]