From 09f6b735a0c54c5b43bb7415b5fff87d48ba379e Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Wed, 10 Jul 2024 15:50:32 +0200 Subject: [PATCH] refactor!: getEvaluation doesnt throw (#158) * refactor!: getEvaluation doesnt throw * chore: update api spec file * fix(provider): throw OpenFeature exceptions on failed evaluations --------- Co-authored-by: Nicklas Lundin --- README.md | 2 +- Sources/Confidence/Confidence.swift | 10 +- Sources/Confidence/FlagEvaluation.swift | 115 ++++++++++-------- .../ConfidenceFeatureProvider.swift | 16 ++- .../ConfidenceProviderTest.swift | 100 ++++++++++++++- .../ConfidenceIntegrationTest.swift | 10 +- Tests/ConfidenceTests/ConfidenceTest.swift | 56 ++++----- .../LocalStorageResolverTest.swift | 17 ++- api/Confidence_public_api.json | 2 +- 9 files changed, 220 insertions(+), 108 deletions(-) diff --git a/README.md b/README.md index f47944e4..de249601 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ In the case of an error, the default value will be returned and the `Evaluation` ```swift let message: String = confidence.getValue(key: "flag-name.message", defaultValue: "default message") -let messageFlag: Evaluation = try confidence.getEvaluation(key: "flag-name.message", defaultValue: "default message") +let messageFlag: Evaluation = confidence.getEvaluation(key: "flag-name.message", defaultValue: "default message") let messageValue = messageFlag.value // message and messageValue are the same diff --git a/Sources/Confidence/Confidence.swift b/Sources/Confidence/Confidence.swift index dc2914cd..29f2b57d 100644 --- a/Sources/Confidence/Confidence.swift +++ b/Sources/Confidence/Confidence.swift @@ -126,8 +126,8 @@ public class Confidence: ConfidenceEventSender { } } - public func getEvaluation(key: String, defaultValue: T) throws -> Evaluation { - try self.cache.evaluate( + public func getEvaluation(key: String, defaultValue: T) -> Evaluation { + self.cache.evaluate( flagName: key, defaultValue: defaultValue, context: getContext(), @@ -136,11 +136,7 @@ public class Confidence: ConfidenceEventSender { } public func getValue(key: String, defaultValue: T) -> T { - do { - return try getEvaluation(key: key, defaultValue: defaultValue).value - } catch { - return defaultValue - } + return getEvaluation(key: key, defaultValue: defaultValue).value } func isStorageEmpty() -> Bool { diff --git a/Sources/Confidence/FlagEvaluation.swift b/Sources/Confidence/FlagEvaluation.swift index 55eddd54..1193f4b4 100644 --- a/Sources/Confidence/FlagEvaluation.swift +++ b/Sources/Confidence/FlagEvaluation.swift @@ -11,6 +11,8 @@ public struct Evaluation { public enum ErrorCode { case providerNotReady case invalidContext + case flagNotFound + case evaluationError } struct FlagResolution: Encodable, Decodable, Equatable { @@ -21,70 +23,85 @@ struct FlagResolution: Encodable, Decodable, Equatable { } extension FlagResolution { + // swiftlint:disable function_body_length func evaluate( flagName: String, defaultValue: T, context: ConfidenceStruct, flagApplier: FlagApplier? = nil - ) throws -> Evaluation { - let parsedKey = try FlagPath.getPath(for: flagName) - if self == FlagResolution.EMPTY { - throw ConfidenceError.flagNotFoundError(key: parsedKey.flag) - } - let resolvedFlag = self.flags.first { resolvedFlag in resolvedFlag.flag == parsedKey.flag } - guard let resolvedFlag = resolvedFlag else { - throw ConfidenceError.flagNotFoundError(key: parsedKey.flag) - } - - if resolvedFlag.resolveReason != .targetingKeyError { - Task { - await flagApplier?.apply(flagName: parsedKey.flag, resolveToken: self.resolveToken) + ) -> Evaluation { + do { + let parsedKey = try FlagPath.getPath(for: flagName) + let resolvedFlag = self.flags.first { resolvedFlag in resolvedFlag.flag == parsedKey.flag } + guard let resolvedFlag = resolvedFlag else { + return Evaluation( + value: defaultValue, + variant: nil, + reason: .error, + errorCode: .flagNotFound, + errorMessage: "Flag '\(parsedKey.flag)' not found in local cache" + ) } - } else { - return Evaluation( - value: defaultValue, - variant: nil, - reason: .targetingKeyError, - errorCode: .invalidContext, - errorMessage: "Invalid targeting key" - ) - } - guard let value = resolvedFlag.value else { - return Evaluation( - value: defaultValue, - variant: resolvedFlag.variant, - reason: resolvedFlag.resolveReason, - errorCode: nil, - errorMessage: nil - ) - } + if resolvedFlag.resolveReason != .targetingKeyError { + Task { + await flagApplier?.apply(flagName: parsedKey.flag, resolveToken: self.resolveToken) + } + } else { + return Evaluation( + value: defaultValue, + variant: nil, + reason: .targetingKeyError, + errorCode: .invalidContext, + errorMessage: "Invalid targeting key" + ) + } - let parsedValue = try getValue(path: parsedKey.path, value: value) - let pathValue: T = getTyped(value: parsedValue) ?? defaultValue + guard let value = resolvedFlag.value else { + return Evaluation( + value: defaultValue, + variant: resolvedFlag.variant, + reason: resolvedFlag.resolveReason, + errorCode: nil, + errorMessage: nil + ) + } - if resolvedFlag.resolveReason == .match { - var resolveReason: ResolveReason = .match - if self.context != context { - resolveReason = .stale + let parsedValue = try getValue(path: parsedKey.path, value: value) + let pathValue: T = getTyped(value: parsedValue) ?? defaultValue + + if resolvedFlag.resolveReason == .match { + var resolveReason: ResolveReason = .match + if self.context != context { + resolveReason = .stale + } + return Evaluation( + value: pathValue, + variant: resolvedFlag.variant, + reason: resolveReason, + errorCode: nil, + errorMessage: nil + ) + } else { + return Evaluation( + value: defaultValue, + variant: resolvedFlag.variant, + reason: resolvedFlag.resolveReason, + errorCode: nil, + errorMessage: nil + ) } - return Evaluation( - value: pathValue, - variant: resolvedFlag.variant, - reason: resolveReason, - errorCode: nil, - errorMessage: nil - ) - } else { + } catch { return Evaluation( value: defaultValue, - variant: resolvedFlag.variant, - reason: resolvedFlag.resolveReason, - errorCode: nil, - errorMessage: nil + variant: nil, + reason: .error, + errorCode: .evaluationError, + errorMessage: error.localizedDescription ) } } + // swiftlint:enable function_body_length // swiftlint:disable:next cyclomatic_complexity private func getTyped(value: ConfidenceValue) -> T? { diff --git a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift index c073864a..b20960cc 100644 --- a/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift +++ b/Sources/ConfidenceProvider/ConfidenceFeatureProvider.swift @@ -134,8 +134,20 @@ public class ConfidenceFeatureProvider: FeatureProvider { } extension Evaluation { - func toProviderEvaluation() -> ProviderEvaluation { - ProviderEvaluation( + func toProviderEvaluation() throws -> ProviderEvaluation { + if let errorCode = self.errorCode { + switch errorCode { + case .providerNotReady: + throw OpenFeatureError.providerNotReadyError + case .invalidContext: + throw OpenFeatureError.invalidContextError + case .flagNotFound: + throw OpenFeatureError.flagNotFoundError(key: self.errorMessage ?? "unknown key") + case .evaluationError: + throw OpenFeatureError.generalError(message: self.errorMessage ?? "unknown error") + } + } + return ProviderEvaluation( value: self.value, variant: self.variant, reason: self.reason.rawValue, diff --git a/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift index edf4945a..66140895 100644 --- a/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift +++ b/Tests/ConfidenceProviderTests/ConfidenceProviderTest.swift @@ -7,10 +7,8 @@ import XCTest @testable import Confidence class ConfidenceProviderTest: XCTestCase { - private var readyExpectation = XCTestExpectation(description: "Ready") - private var errorExpectation = XCTestExpectation(description: "Error") - func testErrorFetchOnInit() async throws { + let readyExpectation = XCTestExpectation(description: "Ready") class FakeClient: ConfidenceResolveClient { func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult { throw ConfidenceError.internalError(message: "test") @@ -28,7 +26,7 @@ class ConfidenceProviderTest: XCTestCase { let cancellable = OpenFeatureAPI.shared.observe().sink { event in if event == .ready { - self.readyExpectation.fulfill() + readyExpectation.fulfill() } else { print(event) } @@ -38,6 +36,7 @@ class ConfidenceProviderTest: XCTestCase { } func testErrorStorageOnInit() async throws { + let errorExpectation = XCTestExpectation(description: "Error") class FakeStorage: Storage { func save(data: Encodable) throws { // no-op @@ -66,7 +65,7 @@ class ConfidenceProviderTest: XCTestCase { let cancellable = OpenFeatureAPI.shared.observe().sink { event in if event == .error { - self.errorExpectation.fulfill() + errorExpectation.fulfill() } else { print(event) } @@ -74,4 +73,95 @@ class ConfidenceProviderTest: XCTestCase { await fulfillment(of: [errorExpectation], timeout: 5.0) cancellable.cancel() } + + func testProviderThrowsOpenFeatureErrors() async throws { + let context = MutableContext(targetingKey: "t") + let readyExpectation = XCTestExpectation(description: "Ready") + let storage = StorageMock() + class FakeClient: ConfidenceResolveClient { + var resolvedValues: [ResolvedValue] = [ + ResolvedValue( + variant: "variant1", + value: .init(structure: ["int": .init(integer: 42)]), + flag: "flagName", + resolveReason: .match) + ] + func resolve(ctx: ConfidenceStruct) async throws -> ResolvesResult { + return .init(resolvedValues: resolvedValues, resolveToken: "token") + } + } + + let confidence = Confidence.Builder(clientSecret: "test") + .withContext(initialContext: ["targeting_key": .init(string: "t")]) + .withFlagResolverClient(flagResolver: FakeClient()) + .withStorage(storage: storage) + .build() + + let provider = ConfidenceFeatureProvider(confidence: confidence, initializationStrategy: .fetchAndActivate) + OpenFeatureAPI.shared.setProvider(provider: provider) + let cancellable = OpenFeatureAPI.shared.observe().sink { event in + if event == .ready { + readyExpectation.fulfill() + } else { + print(event) + } + } + await fulfillment(of: [readyExpectation], timeout: 1.0) + cancellable.cancel() + let evaluation = try provider.getIntegerEvaluation(key: "flagName.int", defaultValue: -1, context: context) + XCTAssertEqual(evaluation.value, 42) + + XCTAssertThrowsError(try provider.getIntegerEvaluation( + key: "flagNotFound.something", + defaultValue: -1, + context: context)) + { error in + if let specificError = error as? OpenFeatureError { + XCTAssertEqual(specificError.errorCode(), ErrorCode.flagNotFound) + } else { + XCTFail("expected a flag not found error") + } + } + } +} + +private class StorageMock: Storage { + var data = "" + var saveExpectation: XCTestExpectation? + private let storageQueue = DispatchQueue(label: "com.confidence.storagemock") + + convenience init(data: Encodable) throws { + self.init() + try self.save(data: data) + } + + func save(data: Encodable) throws { + try storageQueue.sync { + let dataB = try JSONEncoder().encode(data) + self.data = String(decoding: dataB, as: UTF8.self) + + saveExpectation?.fulfill() + } + } + + func load(defaultValue: T) throws -> T where T: Decodable { + try storageQueue.sync { + if data.isEmpty { + return defaultValue + } + return try JSONDecoder().decode(T.self, from: try XCTUnwrap(data.data(using: .utf8))) + } + } + + func clear() throws { + storageQueue.sync { + data = "" + } + } + + func isEmpty() -> Bool { + storageQueue.sync { + return data.isEmpty + } + } } diff --git a/Tests/ConfidenceTests/ConfidenceIntegrationTest.swift b/Tests/ConfidenceTests/ConfidenceIntegrationTest.swift index ad3e3036..32732c10 100644 --- a/Tests/ConfidenceTests/ConfidenceIntegrationTest.swift +++ b/Tests/ConfidenceTests/ConfidenceIntegrationTest.swift @@ -30,8 +30,8 @@ class ConfidenceIntegrationTests: XCTestCase { .withContext(initialContext: ctx) .build() try await confidence.fetchAndActivate() - let intResult = try confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: "1") - let boolResult = try confidence.getEvaluation(key: "\(resolveFlag).my-boolean", defaultValue: false) + let intResult = confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: "1") + let boolResult = confidence.getEvaluation(key: "\(resolveFlag).my-boolean", defaultValue: false) XCTAssertEqual(intResult.reason, .match) @@ -104,7 +104,7 @@ class ConfidenceIntegrationTests: XCTestCase { .build() try await confidence.fetchAndActivate() - let result = try confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) + let result = confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) XCTAssertEqual(result.reason, .match) XCTAssertNotNil(result.variant) @@ -132,7 +132,7 @@ class ConfidenceIntegrationTests: XCTestCase { .build() try await confidence.fetchAndActivate() // When evaluation of the flag happens using date context - let result = try confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) + let result = confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) // Then there is targeting match (non-default targeting) XCTAssertEqual(result.reason, .match) XCTAssertNotNil(result.variant) @@ -162,7 +162,7 @@ class ConfidenceIntegrationTests: XCTestCase { .build() try await confidence.fetchAndActivate() // When evaluation of the flag happens using date context - let result = try confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) + let result = confidence.getEvaluation(key: "\(resolveFlag).my-integer", defaultValue: 1) // Then there is targeting match (non-default targeting) XCTAssertEqual(result.value, 1) XCTAssertEqual(result.reason, .noSegmentMatch) diff --git a/Tests/ConfidenceTests/ConfidenceTest.swift b/Tests/ConfidenceTests/ConfidenceTest.swift index 8444ad12..e26d7d3a 100644 --- a/Tests/ConfidenceTests/ConfidenceTest.swift +++ b/Tests/ConfidenceTests/ConfidenceTest.swift @@ -126,16 +126,13 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() + let emptyEvaluation = confidence.getEvaluation( + key: "flag.size", + defaultValue: "value" + ) - XCTAssertThrowsError( - try confidence.getEvaluation( - key: "flag.size", - defaultValue: "value" - )) { error in - XCTAssertEqual( - error as? ConfidenceError, - ConfidenceError.flagNotFoundError(key: "flag")) - } + XCTAssertEqual(emptyEvaluation.value, "value") + XCTAssertEqual(emptyEvaluation.errorCode, .flagNotFound) client.resolvedValues = [ ResolvedValue( @@ -153,7 +150,7 @@ class ConfidenceTest: XCTestCase { await fulfillment(of: [expectation], timeout: 1) cancellable.cancel() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0) @@ -194,7 +191,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0) @@ -273,7 +270,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0) @@ -314,11 +311,11 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0) - _ = try confidence.getEvaluation( + _ = confidence.getEvaluation( key: "flag.size", defaultValue: 0) @@ -364,7 +361,7 @@ class ConfidenceTest: XCTestCase { try await confidence.fetchAndActivate() confidence.putContext(context: ["hello": .init(string: "world")]) - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0) @@ -405,7 +402,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 0.0) @@ -446,7 +443,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: false) @@ -487,7 +484,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: [:]) @@ -528,7 +525,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 42) @@ -543,7 +540,7 @@ class ConfidenceTest: XCTestCase { XCTAssertEqual(flagApplier.applyCallCount, 1) } - func testProviderThrowsFlagNotFound() async throws { + func testProviderFlagNotFound() async throws { class FakeClient: ConfidenceResolveClient { var resolveStats: Int = 0 var resolvedValues: [ResolvedValue] = [] @@ -562,15 +559,16 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - XCTAssertThrowsError( - try confidence.getEvaluation( - key: "flag.size", - defaultValue: 42 - ) - ) { error in - XCTAssertEqual(error as? ConfidenceError, ConfidenceError.flagNotFoundError(key: "flag")) - } + let evaluation = confidence.getEvaluation( + key: "flag.size", + defaultValue: 42 + ) + XCTAssertEqual(evaluation.value, 42) + XCTAssertNil(evaluation.variant) + XCTAssertEqual(evaluation.reason, .error) + XCTAssertEqual(evaluation.errorCode, .flagNotFound) + XCTAssertEqual(evaluation.errorMessage, "Flag 'flag' not found in local cache") XCTAssertEqual(client.resolveStats, 1) } @@ -595,7 +593,7 @@ class ConfidenceTest: XCTestCase { .build() try await confidence.fetchAndActivate() - let evaluation = try confidence.getEvaluation( + let evaluation = confidence.getEvaluation( key: "flag.size", defaultValue: 42) diff --git a/Tests/ConfidenceTests/LocalStorageResolverTest.swift b/Tests/ConfidenceTests/LocalStorageResolverTest.swift index 85bf5f6b..1097a266 100644 --- a/Tests/ConfidenceTests/LocalStorageResolverTest.swift +++ b/Tests/ConfidenceTests/LocalStorageResolverTest.swift @@ -17,7 +17,7 @@ class LocalStorageResolverTest: XCTestCase { ) XCTAssertNoThrow( - try flagResolution.evaluate( + flagResolution.evaluate( flagName: "flag_name.string", defaultValue: "default", context: [:]) ) } @@ -30,13 +30,12 @@ class LocalStorageResolverTest: XCTestCase { ) let context = ["hey": ConfidenceValue(string: "old value")] - let flagResolution = - FlagResolution(context: context, flags: [resolvedValue], resolveToken: "") - XCTAssertThrowsError( - try flagResolution.evaluate(flagName: "new_flag_name", defaultValue: "default", context: context) - ) { error in - XCTAssertEqual( - error as? ConfidenceError, .flagNotFoundError(key: "new_flag_name")) - } + let flagResolution = FlagResolution(context: context, flags: [resolvedValue], resolveToken: "") + let evaluation = flagResolution.evaluate(flagName: "new_flag_name", defaultValue: "default", context: context) + XCTAssertEqual(evaluation.value, "default") + XCTAssertNil(evaluation.variant) + XCTAssertEqual(evaluation.reason, .error) + XCTAssertEqual(evaluation.errorCode, .flagNotFound) + XCTAssertEqual(evaluation.errorMessage, "Flag 'new_flag_name' not found in local cache") } } diff --git a/api/Confidence_public_api.json b/api/Confidence_public_api.json index 01f9bb32..98200bc9 100644 --- a/api/Confidence_public_api.json +++ b/api/Confidence_public_api.json @@ -16,7 +16,7 @@ }, { "name": "getEvaluation(key:defaultValue:)", - "declaration": "public func getEvaluation(key: String, defaultValue: T) throws -> Evaluation" + "declaration": "public func getEvaluation(key: String, defaultValue: T) -> Evaluation" }, { "name": "getValue(key:defaultValue:)",