From 553802790acba5303f590e607077526420823756 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Thu, 3 Jul 2025 18:20:15 +0200 Subject: [PATCH 1/5] feat: MutableCopy deep copy Signed-off-by: Fabrizio Demaria --- Sources/OpenFeature/EvaluationContext.swift | 2 +- Sources/OpenFeature/MutableContext.swift | 4 + Sources/OpenFeature/MutableStructure.swift | 4 + Sources/OpenFeature/Value.swift | 1 + Tests/OpenFeatureTests/EvalContextTests.swift | 80 +++++++++++++++++++ 5 files changed, 90 insertions(+), 1 deletion(-) diff --git a/Sources/OpenFeature/EvaluationContext.swift b/Sources/OpenFeature/EvaluationContext.swift index e5215d7..2f72206 100644 --- a/Sources/OpenFeature/EvaluationContext.swift +++ b/Sources/OpenFeature/EvaluationContext.swift @@ -3,6 +3,6 @@ import Foundation /// Container for arbitrary contextual data that can be used as a basis for dynamic evaluation. public protocol EvaluationContext: Structure { func getTargetingKey() -> String - + func deepCopy() -> EvaluationContext func setTargetingKey(targetingKey: String) } diff --git a/Sources/OpenFeature/MutableContext.swift b/Sources/OpenFeature/MutableContext.swift index b79abeb..4379771 100644 --- a/Sources/OpenFeature/MutableContext.swift +++ b/Sources/OpenFeature/MutableContext.swift @@ -15,6 +15,10 @@ public class MutableContext: EvaluationContext { self.init(structure: MutableStructure(attributes: attributes)) } + public func deepCopy() -> EvaluationContext { + return MutableContext(targetingKey: targetingKey, structure: structure.deepCopy()) + } + public func getTargetingKey() -> String { return self.targetingKey } diff --git a/Sources/OpenFeature/MutableStructure.swift b/Sources/OpenFeature/MutableStructure.swift index 5306e3e..1174cc9 100644 --- a/Sources/OpenFeature/MutableStructure.swift +++ b/Sources/OpenFeature/MutableStructure.swift @@ -24,6 +24,10 @@ public class MutableStructure: Structure { public func asObjectMap() -> [String: AnyHashable?] { return attributes.mapValues(convertValue) } + + public func deepCopy() -> MutableStructure { + MutableStructure(attributes: attributes) + } } extension MutableStructure { diff --git a/Sources/OpenFeature/Value.swift b/Sources/OpenFeature/Value.swift index b13ba72..068bb52 100644 --- a/Sources/OpenFeature/Value.swift +++ b/Sources/OpenFeature/Value.swift @@ -3,6 +3,7 @@ import Foundation /// Values serve as a generic return type for structure data from providers. /// Providers may deal in JSON, protobuf, XML or some other data-interchange format. /// This intermediate representation provides a good medium of exchange. +/// All the enum types must be value types, as Value is expected to support deep copies. public enum Value: Equatable, Codable { case boolean(Bool) case string(String) diff --git a/Tests/OpenFeatureTests/EvalContextTests.swift b/Tests/OpenFeatureTests/EvalContextTests.swift index 03baa7f..07ef646 100644 --- a/Tests/OpenFeatureTests/EvalContextTests.swift +++ b/Tests/OpenFeatureTests/EvalContextTests.swift @@ -136,4 +136,84 @@ final class EvalContextTests: XCTestCase { XCTAssertEqual(ctx.asObjectMap(), expected) } + + func testContextDeepCopyCreatesIndependentCopy() { + // Create original context with various data types + let originalContext = MutableContext(targetingKey: "original-key") + originalContext.add(key: "string", value: .string("original-value")) + originalContext.add(key: "integer", value: .integer(42)) + originalContext.add(key: "boolean", value: .boolean(true)) + originalContext.add(key: "list", value: .list([.string("item1"), .integer(100)])) + originalContext.add(key: "structure", value: .structure([ + "nested-string": .string("nested-value"), + "nested-int": .integer(200) + ])) + + let copiedContext = originalContext.deepCopy() as! MutableContext + + XCTAssertEqual(copiedContext.getTargetingKey(), "original-key") + XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "original-value") + XCTAssertEqual(copiedContext.getValue(key: "integer")?.asInteger(), 42) + XCTAssertEqual(copiedContext.getValue(key: "boolean")?.asBoolean(), true) + XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "item1") + XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 100) + XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["nested-string"]?.asString(), "nested-value") + XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["nested-int"]?.asInteger(), 200) + + originalContext.setTargetingKey(targetingKey: "modified-key") + originalContext.add(key: "string", value: .string("modified-value")) + originalContext.add(key: "new-key", value: .string("new-value")) + + XCTAssertEqual(copiedContext.getTargetingKey(), "original-key") + XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "original-value") + XCTAssertNil(copiedContext.getValue(key: "new-key")) + XCTAssertEqual(originalContext.getTargetingKey(), "modified-key") + XCTAssertEqual(originalContext.getValue(key: "string")?.asString(), "modified-value") + XCTAssertEqual(originalContext.getValue(key: "new-key")?.asString(), "new-value") + } + + func testContextDeepCopyWithEmptyContext() { + let emptyContext = MutableContext() + let copiedContext = emptyContext.deepCopy() as! MutableContext + + XCTAssertEqual(emptyContext.getTargetingKey(), "") + XCTAssertEqual(copiedContext.getTargetingKey(), "") + XCTAssertTrue(emptyContext.keySet().isEmpty) + XCTAssertTrue(copiedContext.keySet().isEmpty) + + emptyContext.setTargetingKey(targetingKey: "test") + emptyContext.add(key: "key", value: .string("value")) + + XCTAssertEqual(copiedContext.getTargetingKey(), "") + XCTAssertTrue(copiedContext.keySet().isEmpty) + } + + func testContextDeepCopyPreservesAllValueTypes() { + let date = Date() + let originalContext = MutableContext(targetingKey: "test-key") + originalContext.add(key: "null", value: .null) + originalContext.add(key: "string", value: .string("test-string")) + originalContext.add(key: "boolean", value: .boolean(false)) + originalContext.add(key: "integer", value: .integer(12345)) + originalContext.add(key: "double", value: .double(3.14159)) + originalContext.add(key: "date", value: .date(date)) + originalContext.add(key: "list", value: .list([.string("list-item"), .integer(999)])) + originalContext.add(key: "structure", value: .structure([ + "struct-key": .string("struct-value"), + "struct-number": .integer(777) + ])) + + let copiedContext = originalContext.deepCopy() as! MutableContext + + XCTAssertTrue(copiedContext.getValue(key: "null")?.isNull() ?? false) + XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "test-string") + XCTAssertEqual(copiedContext.getValue(key: "boolean")?.asBoolean(), false) + XCTAssertEqual(copiedContext.getValue(key: "integer")?.asInteger(), 12345) + XCTAssertEqual(copiedContext.getValue(key: "double")?.asDouble(), 3.14159) + XCTAssertEqual(copiedContext.getValue(key: "date")?.asDate(), date) + XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "list-item") + XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 999) + XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-key"]?.asString(), "struct-value") + XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-number"]?.asInteger(), 777) + } } From 466f0183eae84575015e4fe2abddcf318f7c6ba5 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Thu, 3 Jul 2025 18:20:55 +0200 Subject: [PATCH 2/5] fix: Improves thread safety contract with providers Signed-off-by: Fabrizio Demaria --- Sources/OpenFeature/OpenFeatureAPI.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/OpenFeature/OpenFeatureAPI.swift b/Sources/OpenFeature/OpenFeatureAPI.swift index 55e274e..8a50073 100644 --- a/Sources/OpenFeature/OpenFeatureAPI.swift +++ b/Sources/OpenFeature/OpenFeatureAPI.swift @@ -156,11 +156,11 @@ public class OpenFeatureAPI { self.providerSubject.send(provider) if let initialContext = initialContext { - self.evaluationContext = initialContext + self.evaluationContext = initialContext.deepCopy() } do { - try await provider.initialize(initialContext: initialContext) + try await provider.initialize(initialContext: initialContext?.deepCopy()) self.providerStatus = .ready self.eventHandler.send(.ready) } catch { @@ -177,11 +177,11 @@ public class OpenFeatureAPI { private func updateContext(evaluationContext: EvaluationContext) async { do { - let oldContext = self.evaluationContext - self.evaluationContext = evaluationContext + let oldContext = self.evaluationContext?.deepCopy() + self.evaluationContext = evaluationContext.deepCopy() self.providerStatus = .reconciling eventHandler.send(.reconciling) - try await self.providerSubject.value?.onContextSet(oldContext: oldContext, newContext: evaluationContext) + try await self.providerSubject.value?.onContextSet(oldContext: oldContext, newContext: evaluationContext.deepCopy()) self.providerStatus = .ready eventHandler.send(.contextChanged) } catch { From bf614baaa04f0ea2b25bc706aa2109e57234d71e Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Fri, 4 Jul 2025 11:49:04 +0200 Subject: [PATCH 3/5] test: Thread safe deep copy Signed-off-by: Fabrizio Demaria --- Tests/OpenFeatureTests/EvalContextTests.swift | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/Tests/OpenFeatureTests/EvalContextTests.swift b/Tests/OpenFeatureTests/EvalContextTests.swift index 07ef646..675627f 100644 --- a/Tests/OpenFeatureTests/EvalContextTests.swift +++ b/Tests/OpenFeatureTests/EvalContextTests.swift @@ -146,7 +146,7 @@ final class EvalContextTests: XCTestCase { originalContext.add(key: "list", value: .list([.string("item1"), .integer(100)])) originalContext.add(key: "structure", value: .structure([ "nested-string": .string("nested-value"), - "nested-int": .integer(200) + "nested-int": .integer(200), ])) let copiedContext = originalContext.deepCopy() as! MutableContext @@ -200,7 +200,7 @@ final class EvalContextTests: XCTestCase { originalContext.add(key: "list", value: .list([.string("list-item"), .integer(999)])) originalContext.add(key: "structure", value: .structure([ "struct-key": .string("struct-value"), - "struct-number": .integer(777) + "struct-number": .integer(777), ])) let copiedContext = originalContext.deepCopy() as! MutableContext @@ -216,4 +216,38 @@ final class EvalContextTests: XCTestCase { XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-key"]?.asString(), "struct-value") XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-number"]?.asInteger(), 777) } + + func testContextDeepCopyIsThreadSafe() { + let context = MutableContext(targetingKey: "initial-key") + context.add(key: "initial", value: .string("initial-value")) + + let expectation = XCTestExpectation(description: "Concurrent deep copy operations") + let concurrentQueue = DispatchQueue(label: "test.concurrent", attributes: .concurrent) + let group = DispatchGroup() + + // Perform multiple concurrent operations + for i in 0..<100 { + group.enter() + concurrentQueue.async { + // Modify the context + context.setTargetingKey(targetingKey: "modified-\(i)") + context.add(key: "key-\(i)", value: .integer(Int64(i))) + + // Perform deep copy + let copiedContext = context.deepCopy() + + // Verify the copy is independent + XCTAssertNotEqual(copiedContext.getTargetingKey(), "initial-key") + XCTAssertNotNil(copiedContext.getValue(key: "initial")) + + group.leave() + } + } + + group.notify(queue: .main) { + expectation.fulfill() + } + + wait(for: [expectation], timeout: 5.0) + } } From 26f2fe4207e4ff108ea089c339fe8b33badda882 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Fri, 4 Jul 2025 13:10:36 +0200 Subject: [PATCH 4/5] feat: Make mutable classes thread-safe Signed-off-by: Fabrizio Demaria --- Package.swift | 2 +- Sources/OpenFeature/MutableContext.swift | 35 ++++++++++++++++------ Sources/OpenFeature/MutableStructure.swift | 27 ++++++++++++----- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/Package.swift b/Package.swift index 9d038e0..fea38bf 100644 --- a/Package.swift +++ b/Package.swift @@ -14,7 +14,7 @@ let package = Package( products: [ .library( name: "OpenFeature", - targets: ["OpenFeature"]) + targets: ["OpenFeature"]), ], dependencies: [], targets: [ diff --git a/Sources/OpenFeature/MutableContext.swift b/Sources/OpenFeature/MutableContext.swift index 4379771..904e525 100644 --- a/Sources/OpenFeature/MutableContext.swift +++ b/Sources/OpenFeature/MutableContext.swift @@ -1,8 +1,9 @@ import Foundation -/// The ``MutableContext`` is an ``EvaluationContext`` implementation which is not threadsafe, and whose attributes can +/// The ``MutableContext`` is an ``EvaluationContext`` implementation which is threadsafe, and whose attributes can /// be modified after instantiation. public class MutableContext: EvaluationContext { + private let queue = DispatchQueue(label: "com.openfeature.mutablecontext.queue", qos: .userInitiated) private var targetingKey: String private var structure: MutableStructure @@ -16,38 +17,54 @@ public class MutableContext: EvaluationContext { } public func deepCopy() -> EvaluationContext { - return MutableContext(targetingKey: targetingKey, structure: structure.deepCopy()) + return queue.sync { + MutableContext(targetingKey: targetingKey, structure: structure.deepCopy()) + } } public func getTargetingKey() -> String { - return self.targetingKey + return queue.sync { + self.targetingKey + } } public func setTargetingKey(targetingKey: String) { - self.targetingKey = targetingKey + queue.sync { + self.targetingKey = targetingKey + } } public func keySet() -> Set { - return structure.keySet() + return queue.sync { + structure.keySet() + } } public func getValue(key: String) -> Value? { - return structure.getValue(key: key) + return queue.sync { + structure.getValue(key: key) + } } public func asMap() -> [String: Value] { - return structure.asMap() + return queue.sync { + structure.asMap() + } } public func asObjectMap() -> [String: AnyHashable?] { - return structure.asObjectMap() + return queue.sync { + structure.asObjectMap() + } } } extension MutableContext { @discardableResult public func add(key: String, value: Value) -> MutableContext { - self.structure.add(key: key, value: value) + queue.sync { + self.structure.add(key: key, value: value) + } return self } } diff --git a/Sources/OpenFeature/MutableStructure.swift b/Sources/OpenFeature/MutableStructure.swift index 1174cc9..0b69db5 100644 --- a/Sources/OpenFeature/MutableStructure.swift +++ b/Sources/OpenFeature/MutableStructure.swift @@ -1,8 +1,9 @@ import Foundation -/// The ``MutableStructure`` is a ``Structure`` implementation which is not threadsafe, and whose attributes can +/// The ``MutableStructure`` is a ``Structure`` implementation which is threadsafe, and whose attributes can /// be modified after instantiation. public class MutableStructure: Structure { + private let queue = DispatchQueue(label: "com.openfeature.mutablestructure.queue", qos: .userInitiated) private var attributes: [String: Value] public init(attributes: [String: Value] = [:]) { @@ -10,23 +11,33 @@ public class MutableStructure: Structure { } public func keySet() -> Set { - return Set(attributes.keys) + return queue.sync { + Set(attributes.keys) + } } public func getValue(key: String) -> Value? { - return attributes[key] + return queue.sync { + attributes[key] + } } public func asMap() -> [String: Value] { - return attributes + return queue.sync { + attributes + } } public func asObjectMap() -> [String: AnyHashable?] { - return attributes.mapValues(convertValue) + return queue.sync { + attributes.mapValues(convertValue) + } } public func deepCopy() -> MutableStructure { - MutableStructure(attributes: attributes) + return queue.sync { + MutableStructure(attributes: attributes) + } } } @@ -62,7 +73,9 @@ extension MutableStructure { extension MutableStructure { @discardableResult public func add(key: String, value: Value) -> MutableStructure { - attributes[key] = value + queue.sync { + attributes[key] = value + } return self } } From 1d2ee2e3ca196c423c924483d1bc826fb5cbeaf9 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Fri, 4 Jul 2025 13:17:34 +0200 Subject: [PATCH 5/5] lint: fixes Signed-off-by: Fabrizio Demaria --- Sources/OpenFeature/OpenFeatureAPI.swift | 5 +++- Tests/OpenFeatureTests/EvalContextTests.swift | 25 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/Sources/OpenFeature/OpenFeatureAPI.swift b/Sources/OpenFeature/OpenFeatureAPI.swift index 8a50073..fa71dca 100644 --- a/Sources/OpenFeature/OpenFeatureAPI.swift +++ b/Sources/OpenFeature/OpenFeatureAPI.swift @@ -181,7 +181,10 @@ public class OpenFeatureAPI { self.evaluationContext = evaluationContext.deepCopy() self.providerStatus = .reconciling eventHandler.send(.reconciling) - try await self.providerSubject.value?.onContextSet(oldContext: oldContext, newContext: evaluationContext.deepCopy()) + try await self.providerSubject.value?.onContextSet( + oldContext: oldContext, + newContext: evaluationContext.deepCopy() + ) self.providerStatus = .ready eventHandler.send(.contextChanged) } catch { diff --git a/Tests/OpenFeatureTests/EvalContextTests.swift b/Tests/OpenFeatureTests/EvalContextTests.swift index 675627f..42369dd 100644 --- a/Tests/OpenFeatureTests/EvalContextTests.swift +++ b/Tests/OpenFeatureTests/EvalContextTests.swift @@ -149,7 +149,10 @@ final class EvalContextTests: XCTestCase { "nested-int": .integer(200), ])) - let copiedContext = originalContext.deepCopy() as! MutableContext + guard let copiedContext = originalContext.deepCopy() as? MutableContext else { + XCTFail("Failed to cast to MutableContext") + return + } XCTAssertEqual(copiedContext.getTargetingKey(), "original-key") XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "original-value") @@ -157,7 +160,10 @@ final class EvalContextTests: XCTestCase { XCTAssertEqual(copiedContext.getValue(key: "boolean")?.asBoolean(), true) XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "item1") XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 100) - XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["nested-string"]?.asString(), "nested-value") + XCTAssertEqual( + copiedContext.getValue(key: "structure")?.asStructure()?["nested-string"]?.asString(), + "nested-value" + ) XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["nested-int"]?.asInteger(), 200) originalContext.setTargetingKey(targetingKey: "modified-key") @@ -174,7 +180,10 @@ final class EvalContextTests: XCTestCase { func testContextDeepCopyWithEmptyContext() { let emptyContext = MutableContext() - let copiedContext = emptyContext.deepCopy() as! MutableContext + guard let copiedContext = emptyContext.deepCopy() as? MutableContext else { + XCTFail("Failed to cast to MutableContext") + return + } XCTAssertEqual(emptyContext.getTargetingKey(), "") XCTAssertEqual(copiedContext.getTargetingKey(), "") @@ -203,7 +212,10 @@ final class EvalContextTests: XCTestCase { "struct-number": .integer(777), ])) - let copiedContext = originalContext.deepCopy() as! MutableContext + guard let copiedContext = originalContext.deepCopy() as? MutableContext else { + XCTFail("Failed to cast to MutableContext") + return + } XCTAssertTrue(copiedContext.getValue(key: "null")?.isNull() ?? false) XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "test-string") @@ -213,7 +225,10 @@ final class EvalContextTests: XCTestCase { XCTAssertEqual(copiedContext.getValue(key: "date")?.asDate(), date) XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "list-item") XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 999) - XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-key"]?.asString(), "struct-value") + XCTAssertEqual( + copiedContext.getValue(key: "structure")?.asStructure()?["struct-key"]?.asString(), + "struct-value" + ) XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-number"]?.asInteger(), 777) }