From 54a674e7ff8be4c97ad296f6bf066b8641ae95b5 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Mon, 17 Jun 2024 15:43:43 +0200 Subject: [PATCH] fix: Issues with context/secret trigger error (#137) * fix: Missing context emits an error local-event * fix: Non-authenticated fetches trigger error * refactor: no OF context init is allowed * refactor: Renaming test classes * test: [OF] Test errors on init * fix: todo comments * feat: Add docs for public functions * fix: Remove unnecessary check * fix: Rename test class * fix: Formatting --- Sources/Confidence/Confidence.swift | 28 ++++++- .../ConfidenceFeatureProvider.swift | 14 ++-- .../ConfidenceProviderTest.swift | 77 +++++++++++++++++++ ...sts.swift => ConfidenceContextTests.swift} | 2 +- ...roviderTest.swift => ConfidenceTest.swift} | 2 +- 5 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift rename Tests/ConfidenceTests/{ConfidenceTests.swift => ConfidenceContextTests.swift} (99%) rename Tests/ConfidenceTests/{ConfidenceFeatureProviderTest.swift => ConfidenceTest.swift} (99%) diff --git a/Sources/Confidence/Confidence.swift b/Sources/Confidence/Confidence.swift index 2db8b68c..09cc2599 100644 --- a/Sources/Confidence/Confidence.swift +++ b/Sources/Confidence/Confidence.swift @@ -53,19 +53,33 @@ public class Confidence: ConfidenceEventSender { try await self.fetchAndActivate() self.contextReconciliatedChanges.send(context.hash()) } catch { + // TODO: Log errors for debugging } } } .store(in: &cancellables) } - + /** + Activating the cache means that the flag data on disk is loaded into memory, so consumers can access flag values. + Errors can be thrown if something goes wrong access data on disk. + */ public func activate() throws { let savedFlags = try storage.load(defaultValue: FlagResolution.EMPTY) self.cache = savedFlags } + /** + Fetches latest flag evaluations and store them on disk. Regardless of the fetch outcome (success or failure), this + function activates the cache after the fetch. + Activating the cache means that the flag data on disk is loaded into memory, so consumers can access flag values. + Fetching is best-effort, so no error is propagated. Errors can still be thrown if something goes wrong access data on disk. + */ public func fetchAndActivate() async throws { - try await internalFetch() + do { + try await internalFetch() + } catch { + // TODO: Log errors for debugging + } try activate() } @@ -80,9 +94,17 @@ public class Confidence: ConfidenceEventSender { try storage.save(data: resolution) } + /** + Fetches latest flag evaluations and store them on disk. Note that "activate" must be called for this data to be + made available in the app session. + */ public func asyncFetch() { Task { - try await internalFetch() + do { + try await internalFetch() + } catch { + // TODO: Log errors for debugging + } } } diff --git a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift index 5319156a..b611c7f1 100644 --- a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift +++ b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift @@ -39,11 +39,7 @@ public class ConfidenceFeatureProvider: FeatureProvider { } public func initialize(initialContext: OpenFeature.EvaluationContext?) { - guard let initialContext = initialContext else { - return - } - - self.updateConfidenceContext(context: initialContext) + self.updateConfidenceContext(context: initialContext ?? MutableContext(attributes: [:])) if self.initializationStrategy == .activateAndFetchAsync { eventHandler.send(.ready) } @@ -55,8 +51,12 @@ public class ConfidenceFeatureProvider: FeatureProvider { confidence.asyncFetch() } else { Task { - try await confidence.fetchAndActivate() - eventHandler.send(.ready) + do { + try await confidence.fetchAndActivate() + eventHandler.send(.ready) + } catch { + eventHandler.send(.error) + } } } } catch { diff --git a/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift new file mode 100644 index 00000000..edf4945a --- /dev/null +++ b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift @@ -0,0 +1,77 @@ +import Foundation +import ConfidenceProvider +import Combine +import OpenFeature +import XCTest + +@testable import Confidence + +class ConfidenceProviderTest: XCTestCase { + private var readyExpectation = XCTestExpectation(description: "Ready") + private var errorExpectation = XCTestExpectation(description: "Error") + + func testErrorFetchOnInit() async throws { + class FakeClient: ConfidenceResolveClient { + func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult { + throw ConfidenceError.internalError(message: "test") + } + } + + let client = FakeClient() + let confidence = Confidence.Builder(clientSecret: "test") + .withContext(initialContext: ["targeting_key": .init(string: "user1")]) + .withFlagResolverClient(flagResolver: client) + .build() + + let provider = ConfidenceFeatureProvider(confidence: confidence, initializationStrategy: .activateAndFetchAsync) + OpenFeatureAPI.shared.setProvider(provider: provider) + + let cancellable = OpenFeatureAPI.shared.observe().sink { event in + if event == .ready { + self.readyExpectation.fulfill() + } else { + print(event) + } + } + await fulfillment(of: [readyExpectation], timeout: 5.0) + cancellable.cancel() + } + + func testErrorStorageOnInit() async throws { + class FakeStorage: Storage { + func save(data: Encodable) throws { + // no-op + } + + func load(defaultValue: T) throws -> T where T: Decodable { + throw ConfidenceError.internalError(message: "test") + } + + func clear() throws { + // no-op + } + + func isEmpty() -> Bool { + return false + } + } + + let confidence = Confidence.Builder(clientSecret: "test") + .withContext(initialContext: ["targeting_key": .init(string: "user1")]) + .withStorage(storage: FakeStorage()) + .build() + + let provider = ConfidenceFeatureProvider(confidence: confidence, initializationStrategy: .activateAndFetchAsync) + OpenFeatureAPI.shared.setProvider(provider: provider) + + let cancellable = OpenFeatureAPI.shared.observe().sink { event in + if event == .error { + self.errorExpectation.fulfill() + } else { + print(event) + } + } + await fulfillment(of: [errorExpectation], timeout: 5.0) + cancellable.cancel() + } +} diff --git a/Tests/ConfidenceTests/ConfidenceTests.swift b/Tests/ConfidenceTests/ConfidenceContextTests.swift similarity index 99% rename from Tests/ConfidenceTests/ConfidenceTests.swift rename to Tests/ConfidenceTests/ConfidenceContextTests.swift index 1aec5d6f..cf65f4db 100644 --- a/Tests/ConfidenceTests/ConfidenceTests.swift +++ b/Tests/ConfidenceTests/ConfidenceContextTests.swift @@ -2,7 +2,7 @@ import XCTest @testable import Confidence // swiftlint:disable type_body_length -final class ConfidenceTests: XCTestCase { +final class ConfidenceContextTests: XCTestCase { func testWithContext() { let client = RemoteConfidenceResolveClient( options: ConfidenceClientOptions( diff --git a/Tests/ConfidenceTests/ConfidenceFeatureProviderTest.swift b/Tests/ConfidenceTests/ConfidenceTest.swift similarity index 99% rename from Tests/ConfidenceTests/ConfidenceFeatureProviderTest.swift rename to Tests/ConfidenceTests/ConfidenceTest.swift index d400e83c..25543438 100644 --- a/Tests/ConfidenceTests/ConfidenceFeatureProviderTest.swift +++ b/Tests/ConfidenceTests/ConfidenceTest.swift @@ -7,7 +7,7 @@ import XCTest @testable import Confidence @available(macOS 13.0, iOS 16.0, *) -class ConfidenceFeatureProviderTest: XCTestCase { +class ConfidenceTest: XCTestCase { private var flagApplier = FlagApplierMock() private let storage = StorageMock() private var readyExpectation = XCTestExpectation(description: "Ready")