From f78a313dffb642d9a6301975a383bc3e3fcde871 Mon Sep 17 00:00:00 2001 From: Yakov Manshin Date: Sun, 12 May 2024 03:50:38 +0200 Subject: [PATCH] [#130] Support for Optional Values (#143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Previously, feature-flag stores returned `nil` when the value was not found * This behavior made it impossible to distinguish between the absence of a value and a literal `nil` value * While this distinction wasn’t important in the majority of cases, it could be sometimes * For that reason, optional values were not supported in YMFF * #132 / #139 changed the protocols for feature-flag stores so they throw errors instead of returning `nil` for nonexistent values * This change made it possible to accept `nil` as a valid feature-flag value --- .../FeatureFlagResolver+Error.swift | 5 - .../FeatureFlagResolver.swift | 24 +- .../Cases/FeatureFlagResolverTests.swift | 228 +++++++++++++++--- 3 files changed, 202 insertions(+), 55 deletions(-) diff --git a/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver+Error.swift b/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver+Error.swift index 03ac538..a099511 100644 --- a/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver+Error.swift +++ b/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver+Error.swift @@ -23,11 +23,6 @@ extension FeatureFlagResolver { /// The feature-flag store has thrown an error. case storeError(any Swift.Error) - /// Currently, optional values are not supported by the `FeatureFlagResolver`. - /// - /// - Note: Support for optional values will be added in [#130](https://github.com/yakovmanshin/YMFF/issues/130). - case optionalValuesNotAllowed - } } diff --git a/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver.swift b/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver.swift index a9de3c7..4f3d560 100644 --- a/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver.swift +++ b/Sources/YMFF/FeatureFlagResolver/FeatureFlagResolver.swift @@ -54,10 +54,7 @@ final public class FeatureFlagResolver { extension FeatureFlagResolver: FeatureFlagResolverProtocol { public func value(for key: FeatureFlagKey) async throws -> Value { - let retrievedValue: Value = try await retrieveFirstValue(forKey: key) - try validateValue(retrievedValue) - - return retrievedValue + try await retrieveFirstValue(forKey: key) } public func setValue(_ newValue: Value, toMutableStoreUsing key: FeatureFlagKey) async throws { @@ -66,8 +63,6 @@ extension FeatureFlagResolver: FeatureFlagResolverProtocol { throw Error.noStoreAvailable } - try validateValue(newValue) - for store in getStores() { if case .failure(let error) = await store.value(forKey: key) as Result, @@ -118,10 +113,7 @@ extension FeatureFlagResolver: FeatureFlagResolverProtocol { extension FeatureFlagResolver: SynchronousFeatureFlagResolverProtocol { public func valueSync(for key: FeatureFlagKey) throws -> Value { - let retrievedValue: Value = try retrieveFirstValueSync(forKey: key) - try validateValue(retrievedValue) - - return retrievedValue + try retrieveFirstValueSync(forKey: key) } public func setValueSync(_ newValue: Value, toMutableStoreUsing key: FeatureFlagKey) throws { @@ -130,8 +122,6 @@ extension FeatureFlagResolver: SynchronousFeatureFlagResolverProtocol { throw Error.noStoreAvailable } - try validateValue(newValue) - for store in getSyncStores() { if case .failure(let error) = store.valueSync(forKey: key) as Result, @@ -253,14 +243,4 @@ extension FeatureFlagResolver { throw Error.valueNotFoundInStores(key: key) } - func validateValue(_ value: Value) throws { - if valueIsOptional(value) { - throw Error.optionalValuesNotAllowed - } - } - - func valueIsOptional(_ value: Value) -> Bool { - value is ExpressibleByNilLiteral - } - } diff --git a/Tests/YMFFTests/Cases/FeatureFlagResolverTests.swift b/Tests/YMFFTests/Cases/FeatureFlagResolverTests.swift index 0eb29ab..64e148c 100644 --- a/Tests/YMFFTests/Cases/FeatureFlagResolverTests.swift +++ b/Tests/YMFFTests/Cases/FeatureFlagResolverTests.swift @@ -244,33 +244,73 @@ final class FeatureFlagResolverTests: XCTestCase { } } - func test_value_optionalValue() async { + func test_value_optionalValue_nonNil() async { let store = FeatureFlagStoreMock() configuration.stores = [store] - store.value_result = .success((123 as Int?)!) + let optionalInt: Int? = 123 + store.value_result = .success(optionalInt as Any) do { - let _: Int? = try await resolver.value(for: "TEST_key1") - XCTFail("Expected an error") - } catch FeatureFlagResolver.Error.optionalValuesNotAllowed { + let value: Int? = try await resolver.value(for: "TEST_key1") + XCTAssertEqual(store.value_invocationCount, 1) XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssert(type(of: value) == Optional.self) + XCTAssertEqual(value, 123) } catch { XCTFail("Unexpected error: \(error)") } } - func test_valueSync_optionalValue() { + func test_valueSync_optionalValue_nonNil() { let store = SynchronousFeatureFlagStoreMock() configuration.stores = [store] - store.valueSync_result = .success((123 as Int?)!) + let optionalInt: Int? = 123 + store.valueSync_result = .success(optionalInt as Any) do { - let _: Int? = try resolver.valueSync(for: "TEST_key1") - XCTFail("Expected an error") - } catch FeatureFlagResolver.Error.optionalValuesNotAllowed { + let value: Int? = try resolver.valueSync(for: "TEST_key1") + XCTAssertEqual(store.valueSync_invocationCount, 1) XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssert(type(of: value) == Optional.self) + XCTAssertEqual(value, 123) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_value_optionalValue_nil() async { + let store = FeatureFlagStoreMock() + configuration.stores = [store] + let optionalString: String? = nil + store.value_result = .success(optionalString as Any) + + do { + let value: String? = try await resolver.value(for: "TEST_key1") + + XCTAssertEqual(store.value_invocationCount, 1) + XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssert(type(of: value) == Optional.self) + XCTAssertNil(value) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_valueSync_optionalValue_nil() { + let store = SynchronousFeatureFlagStoreMock() + configuration.stores = [store] + let optionalString: String? = nil + store.valueSync_result = .success(optionalString as Any) + + do { + let value: String? = try resolver.valueSync(for: "TEST_key1") + + XCTAssertEqual(store.valueSync_invocationCount, 1) + XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssert(type(of: value) == Optional.self) + XCTAssertNil(value) } catch { XCTFail("Unexpected error: \(error)") } @@ -438,37 +478,169 @@ final class FeatureFlagResolverTests: XCTestCase { } } - func test_setValue_singleMutableStore_existingValue_optionalValue() async { + func test_setValue_singleMutableStore_existingValue_nonNilToNonNil() async { let store = MutableFeatureFlagStoreMock() configuration.stores = [store] - store.value_result = .success("TEST_value1") + let optionalInt: Int? = 123 + store.value_result = .success(optionalInt as Any) do { - try await resolver.setValue(456 as Int?, toMutableStoreUsing: "TEST_key1") - XCTFail("Expected an error") - } catch FeatureFlagResolver.Error.optionalValuesNotAllowed { - XCTAssertEqual(store.value_invocationCount, 0) - XCTAssertTrue(store.value_keys.isEmpty) - XCTAssertEqual(store.setValue_invocationCount, 0) - XCTAssertTrue(store.setValue_keyValuePairs.isEmpty) + let newValue: Int? = 456 + try await resolver.setValue(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.value_invocationCount, 1) + XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValue_invocationCount, 1) + XCTAssertEqual(store.setValue_keyValuePairs.count, 1) + XCTAssertEqual(store.setValue_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValue_keyValuePairs[0].1 as! Int?, 456) } catch { XCTFail("Unexpected error: \(error)") } } - func test_setValueSync_singleSyncMutableStore_existingValue_optionalValue() { + func test_setValueSync_singleSyncMutableStore_existingValue_nonNilToNonNil() { let store = SynchronousMutableFeatureFlagStoreMock() configuration.stores = [store] - store.valueSync_result = .success("TEST_value1") + let optionalInt: Int? = 123 + store.valueSync_result = .success(optionalInt as Any) do { - try resolver.setValueSync(456 as Int?, toMutableStoreUsing: "TEST_key1") - XCTFail("Expected an error") - } catch FeatureFlagResolver.Error.optionalValuesNotAllowed { - XCTAssertEqual(store.valueSync_invocationCount, 0) - XCTAssertTrue(store.valueSync_keys.isEmpty) - XCTAssertEqual(store.setValueSync_invocationCount, 0) - XCTAssertTrue(store.setValueSync_keyValuePairs.isEmpty) + let newValue: Int? = 456 + try resolver.setValueSync(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.valueSync_invocationCount, 1) + XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValueSync_invocationCount, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs.count, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValueSync_keyValuePairs[0].1 as! Int?, 456) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValue_singleMutableStore_existingValue_nilToNonNil() async { + let store = MutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = 123 + store.value_result = .success(optionalInt as Any) + + do { + let newValue: Int? = nil + try await resolver.setValue(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.value_invocationCount, 1) + XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValue_invocationCount, 1) + XCTAssertEqual(store.setValue_keyValuePairs.count, 1) + XCTAssertEqual(store.setValue_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValue_keyValuePairs[0].1 as! Int?, nil) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValueSync_singleSyncMutableStore_existingValue_nilToNonNil() { + let store = SynchronousMutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = 123 + store.valueSync_result = .success(optionalInt as Any) + + do { + let newValue: Int? = nil + try resolver.setValueSync(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.valueSync_invocationCount, 1) + XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValueSync_invocationCount, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs.count, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValueSync_keyValuePairs[0].1 as! Int?, nil) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValue_singleMutableStore_existingValue_nonNilToNil() async { + let store = MutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = nil + store.value_result = .success(optionalInt as Any) + + do { + let newValue: Int? = 456 + try await resolver.setValue(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.value_invocationCount, 1) + XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValue_invocationCount, 1) + XCTAssertEqual(store.setValue_keyValuePairs.count, 1) + XCTAssertEqual(store.setValue_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValue_keyValuePairs[0].1 as! Int?, 456) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValueSync_singleSyncMutableStore_existingValue_nonNilToNil() { + let store = SynchronousMutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = nil + store.valueSync_result = .success(optionalInt as Any) + + do { + let newValue: Int? = 456 + try resolver.setValueSync(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.valueSync_invocationCount, 1) + XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValueSync_invocationCount, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs.count, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValueSync_keyValuePairs[0].1 as! Int?, 456) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValue_singleMutableStore_existingValue_nilToNil() async { + let store = MutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = nil + store.value_result = .success(optionalInt as Any) + + do { + let newValue: Int? = nil + try await resolver.setValue(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.value_invocationCount, 1) + XCTAssertEqual(store.value_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValue_invocationCount, 1) + XCTAssertEqual(store.setValue_keyValuePairs.count, 1) + XCTAssertEqual(store.setValue_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValue_keyValuePairs[0].1 as! Int?, nil) + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + func test_setValueSync_singleSyncMutableStore_existingValue_nilToNil() { + let store = SynchronousMutableFeatureFlagStoreMock() + configuration.stores = [store] + let optionalInt: Int? = nil + store.valueSync_result = .success(optionalInt as Any) + + do { + let newValue: Int? = nil + try resolver.setValueSync(newValue, toMutableStoreUsing: "TEST_key1") + + XCTAssertEqual(store.valueSync_invocationCount, 1) + XCTAssertEqual(store.valueSync_keys, ["TEST_key1"]) + XCTAssertEqual(store.setValueSync_invocationCount, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs.count, 1) + XCTAssertEqual(store.setValueSync_keyValuePairs[0].0, "TEST_key1") + XCTAssertEqual(store.setValueSync_keyValuePairs[0].1 as! Int?, nil) } catch { XCTFail("Unexpected error: \(error)") }