Skip to content

Commit

Permalink
fix: Provider cache and resolver are accessed safely (#107)
Browse files Browse the repository at this point in the history
* fix: Provider cache and resolver are accessed safely

* fix: Fix flaky test

* fix: Fix test testResolveAndApplyIntegerFlagError
  • Loading branch information
fabriziodemaria committed Apr 30, 2024
1 parent 3386dd6 commit d166712
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 10 deletions.
1 change: 0 additions & 1 deletion Sources/Confidence/Confidence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public class Confidence: ConfidenceEventSender {
}
}


public func getContext() -> ConfidenceStruct {
let parentContext = parent?.getContext() ?? [:]
var reconciledCtx = parentContext.filter {
Expand Down
16 changes: 14 additions & 2 deletions Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class ConfidenceFeatureProvider: FeatureProvider {
private let confidence: Confidence?
private var cancellables = Set<AnyCancellable>()
private var currentResolveTask: Task<Void, Never>?
private let confidenceFeatureProviderQueue = DispatchQueue(label: "com.provider.queue")

/// Should not be called externally, use `ConfidenceFeatureProvider.Builder`or init with `Confidence` instead.
init(
Expand Down Expand Up @@ -139,8 +140,10 @@ public class ConfidenceFeatureProvider: FeatureProvider {
try self.storage.save(data: result.resolvedValues.toCacheData(context: context, resolveToken: resolveToken))

if refreshCache {
self.cache = InMemoryProviderCache.from(storage: self.storage)
resolver = LocalStorageResolver(cache: cache)
withLock { provider in
provider.cache = InMemoryProviderCache.from(storage: self.storage)
provider.resolver = LocalStorageResolver(cache: provider.cache)
}
}
}

Expand Down Expand Up @@ -446,6 +449,15 @@ public class ConfidenceFeatureProvider: FeatureProvider {
"Error while executing \"apply\": \(error)")
}
}

private func withLock(callback: @escaping (ConfidenceFeatureProvider) -> Void) {
confidenceFeatureProviderQueue.sync { [weak self] in
guard let self = self else {
return
}
callback(self)
}
}
}

// MARK: Storage
Expand Down
26 changes: 19 additions & 7 deletions Tests/ConfidenceProviderTests/ConfidenceFeatureProviderTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,47 +25,55 @@ class ConfidenceFeatureProviderTest: XCTestCase {
super.setUp()
}

// swiftlint:disable function_body_length
func testSlowFirstResolveWillbeCancelledOnSecondResolve() async throws {
let expectation1 = expectation(description: "First resolve completed")
let expectation2 = expectation(description: "Unlock second resolve")
let expectation3 = expectation(description: "Third resolve completed")
let expectation4 = expectation(description: "Second resolve cancelled")

class FakeClient: XCTestCase, ConfidenceResolveClient {
var callCount = 0
var resolveContexts: [ConfidenceStruct] = []
let expectation1: XCTestExpectation
let expectation2: XCTestExpectation
let expectation3: XCTestExpectation
let expectation4: XCTestExpectation

init(expectation1: XCTestExpectation, expectation2: XCTestExpectation, expectation3: XCTestExpectation) {
init(expectation1: XCTestExpectation, expectation2: XCTestExpectation, expectation3: XCTestExpectation, expectation4: XCTestExpectation) {
self.expectation1 = expectation1
self.expectation2 = expectation2
self.expectation3 = expectation3
self.expectation4 = expectation4
super.init(invocation: nil) // Workaround to use expectations in FakeClient
}

func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult {
callCount += 1
switch callCount {
case 1:
expectation1.fulfill()
if Task.isCancelled {
return .init(resolvedValues: [], resolveToken: "")
XCTFail("Resolve one was cancelled unexpectedly")
} else {
resolveContexts.append(ctx)
expectation1.fulfill()
}
case 2:
await fulfillment(of: [expectation2], timeout: 5.0)
if Task.isCancelled {
expectation4.fulfill()
return .init(resolvedValues: [], resolveToken: "")
}
XCTFail("This task should be cancelled and never reach here")
case 3:
expectation3.fulfill()
if Task.isCancelled {
return .init(resolvedValues: [], resolveToken: "")
XCTFail("Resolve three was cancelled unexpectedly")
} else {
resolveContexts.append(ctx)
expectation3.fulfill()
}
default: XCTFail("We expect only 3 resolve calls")
}
resolveContexts.append(ctx)
return .init(resolvedValues: [], resolveToken: "")
}
}
Expand All @@ -74,7 +82,8 @@ class ConfidenceFeatureProviderTest: XCTestCase {
let client = FakeClient(
expectation1: expectation1,
expectation2: expectation2,
expectation3: expectation3
expectation3: expectation3,
expectation4: expectation4
)
let provider = ConfidenceFeatureProvider(confidence: confidence, session: nil, client: client)
// Initialize allows to start listening for context changes in "confidence"
Expand All @@ -85,10 +94,12 @@ class ConfidenceFeatureProviderTest: XCTestCase {
confidence.putContext(key: "new2", value: ConfidenceValue(string: "value2"))
await fulfillment(of: [expectation3], timeout: 5.0)
expectation2.fulfill() // Allow second resolve to continue, regardless if cancelled or not
await fulfillment(of: [expectation4], 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() throws {
var session = MockedResolveClientURLProtocol.mockedSession(flags: [:])
Expand Down Expand Up @@ -329,6 +340,7 @@ class ConfidenceFeatureProviderTest: XCTestCase {
}

func testResolveAndApplyIntegerFlagError() throws {
flagApplier = FlagApplierMock(expectedApplies: 2)
let resolve: [String: MockedResolveClientURLProtocol.ResolvedTestFlag] = [
"user1": .init(variant: "control", value: .structure(["size": .integer(3)]))
]
Expand Down
4 changes: 4 additions & 0 deletions Tests/ConfidenceProviderTests/Helpers/FlagApplierMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class FlagApplierMock: FlagApplier {
var applyCallCount = 0
var applyExpectation = XCTestExpectation(description: "Flag Applied")

init(expectedApplies: Int = 1) {
applyExpectation.expectedFulfillmentCount = expectedApplies
}

func apply(flagName: String, resolveToken: String) async {
applyCallCount += 1
applyExpectation.fulfill()
Expand Down

0 comments on commit d166712

Please sign in to comment.