Serialize evaluation context to OFREP #5
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
| case .object(let value): | ||
| let data = try Self.jsonEncoder.encode(value) | ||
| let container = try Self.jsonDecoder.decode(OpenAPIObjectContainer.self, from: data) | ||
| values[key] = container.value |
There was a problem hiding this comment.
I'm not super happy about this approach to convert an any Codable & Sendable into a value to put inside an OpenAPIObjectContainer. @czechboy0 Do you have an idea how this could be improved?
There was a problem hiding this comment.
I think you should be just able to use OpenAPIObjectContainer(unvalidatedValue: value instead of going through the Codable encoding/decoding.
There was a problem hiding this comment.
Thanks for the fast reply 🏎️
unvalidatedValue expects a [String: any Sendable] dictionary, but value in this case is any Codable & Sendable.
There was a problem hiding this comment.
I also tried OpenAPIValueContainer but that fails in the tryCast because value is neither an array nor a dictionary nor a supported primitive value: https://github.com/apple/swift-openapi-runtime/blob/23146bc8710ac5e57abb693113f02dc274cf39b6/Sources/OpenAPIRuntime/Base/OpenAPIValue.swift#L81
There was a problem hiding this comment.
Wait, so what is the underlying type? Unless it's a native Swift container or primitive type then yeah, you'll need to run it through Codable.
I wonder why it's Codable though? Where does that requirement come from?
There was a problem hiding this comment.
It's part of the OpenFeature spec to allow for user-defined objects to be supported in the evaluation API. In the OFREP provider that maps to JSON-coded values but a different provider may encode/decode them differently. That's why I opted for Codable & Sendable as the requirement.
There was a problem hiding this comment.
Perhaps OpenAPIRuntime could support "lazy encodable" values in OpenAPIValueContainer which would be encoded alongside the request. That way I could save the roundtrip.
There was a problem hiding this comment.
If that's something you'd like to see, please open an issue on Swift OpenAPI Generator and we can discuss there.
There was a problem hiding this comment.
Yep, I'll think it through some more but will then likely create an issue. Thanks again for your input 🙏
No description provided.