-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Surface the distinction between an explicit "null" value and an absent value #419
Comments
Hi @madsodgaard, what's the meaning of marking the value as nullable beyond just defining as
and not putting it in the required list? How would you want to see it represented in Swift, if not as a simple optional property of TaskOccurenceDTO? |
@czechboy0 One place where I use this pattern a lot is when designing type:
- string
- "null" since this is a partial update endpoint, I want to differentiate between the user providing a "null" value (thus updating the value of The ideal situtation, in my mind, is that the library could somehow recognize this pattern and just provide an optional property of TaskOccurenceDTO, as it does currently when not specifying the property in the |
Swift OpenAPI Generator delegates JSON parsing to automatic Codable implementations (wherever possible) and to Foundation's JSONDecoder, which doesn't make the distinction between a You'll need to make resetting values somehow an explicit part of the API, not relying on the distinction between |
@czechboy0 Thanks! Of course the fact that Codable does not differentiate between absent values and null is quite challenge to this issue. But, this pattern is so common in APIs today, that perhaps we should find a way to support? By "this pattern", I am referring to PATCH endpoints for partial updates instead of PUT endpoints to replace the entire resource. I have used this property wrapper previously in server-side applications. Do you think such a type or something similar would have a place in the library? (or is it too niché?) It would be used in places where you have a non-required nullable field. @propertyWrapper
struct PotentiallyAbsent<T: Codable>: Codable {
var wrappedValue: T? {
switch state {
case .present(let value):
return value
default:
return nil
}
}
enum State {
case absent
case null
case present(T)
}
var projectedValue: PotentiallyAbsent<T> {
return self
}
var state: State
init(_ state: State) {
self.state = state
}
static var null: PotentiallyAbsent<T> {
.init(.null)
}
static var absent: PotentiallyAbsent<T> {
.init(.absent)
}
static func present(_ value: T) -> PotentiallyAbsent<T> {
.init(.present(value))
}
init(from decoder: Decoder) throws {
fatalError("Attempted to decode `PotentiallyAbsent` as a standalone value")
}
func encode(to encoder: Encoder) throws {
fatalError("Attempted to encode `PotentiallyAbsent` as a standalone value")
}
}
extension KeyedDecodingContainer {
func decode<T>(_ type: PotentiallyAbsent<T>.Type, forKey key: K) throws -> PotentiallyAbsent<T> {
do {
if try self.decodeNil(forKey: key) {
return .init(.null)
} else {
return .init(.present(try decode(T.self, forKey: key)))
}
} catch DecodingError.keyNotFound {
return .init(.absent)
}
}
}
extension KeyedEncodingContainer {
mutating func encode<T>(_ value: PotentiallyAbsent<T>, forKey key: KeyedEncodingContainer<K>.Key) throws {
switch value.state {
case .absent:
break
case .null:
try encodeNil(forKey: key)
case let .present(value):
try encode(value, forKey: key)
}
}
} |
I don't think it's likely for us to try to solve this in Swift OpenAPI Generator. The fix should be in Codable and JSONDecoder. Once it's there, we can see what'd remain for Swift OpenAPI Generator to have to do to support this case. But only doing it here would be a significant amount of work that introduces a lot of complexity. One way this could be better represented that'd work today would be a PATCH call that takes two values, a "toSet" dictionary, which has keys matching the property to set, and the value to set. And if a value is nil for a property, the property isn't changed. And the second value would be called "toReset", which would just be a list of property names whose values to reset to their default. |
Renamed the issue to reflect that this is a missing feature. See my comment above explaining that this goes deeper in the stack we're based on, so would likely need some improvements from JSONDecoder first, but keeping open to allow tracking progress. |
I ran into this (or a similar) issue. The encoded output of my server omits a field with a nil value, but some code consuming this service expects an explicit null value. One idea is this property wrapper: https://stackoverflow.com/a/53142581/67280 |
I think it'd be best to first help Foundation get the desired behavior, and then Swift OpenAPI Generator and all other projects relying on JSONDecoder get it for free. |
When using a schema that has a field from a reference that is both non-required and "nullable" this is a common way of defining it in OpenAPI: 3.1.0
but this currently generates the following API:
This is quite confusing syntax at the callsite, since we have this weird
case2
, instead of just having only an optionalTaskOccurenceDTO
. Is this expected behaviour or could we do something to improve the ergonomics before the 1.0.0 release? I'll happily work on a PR, if you could point me in the right direction to where in the codebase the relevant parts are!The text was updated successfully, but these errors were encountered: