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
fabriziodemaria committed Nov 28, 2024
1 parent a4fdde4 commit e2a2476
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 161 deletions.
107 changes: 58 additions & 49 deletions Sources/Confidence/Confidence.swift
View file
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
View file
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
104 changes: 2 additions & 102 deletions Tests/ConfidenceTests/ConfidenceTest.swift
View file
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 @@ -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()

Expand Down
6 changes: 3 additions & 3 deletions api/Confidence_public_api.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(_:)",
Expand Down Expand Up @@ -245,4 +245,4 @@
}
]
}
]
]

0 comments on commit e2a2476

Please sign in to comment.