Replace dynamic type checks with generic overloads #10
Conversation
Signed-off-by: Si Beaumont <[email protected]>
simonjbeaumont
added
🔨 semver/patch
|
Filed apple/swift-openapi-generator#43 for the issue you mentioned in the note at the end. |
There was a problem hiding this comment.
Thanks, I'm glad you found a way to get rid of the dynamic casts.
Proposed 4 changes, just to remove the C constraint on _StringParameterConvertible, which shouldn't be there, and in fact lead to one instance of the wrong overload being chosen.
Verified that with my proposed changes here, the PR passes both runtime and generator tests (with generator using the branch from apple/swift-openapi-generator#44).
| /// - data: Encoded body data. | ||
| /// - transform: Closure for transforming the Decodable type into a final type. | ||
| /// - Returns: Deserialized body value. | ||
| public func bodyGet<T: Decodable & _StringParameterConvertible, C: _StringParameterConvertible>( |
There was a problem hiding this comment.
| public func bodyGet<T: Decodable & _StringParameterConvertible, C: _StringParameterConvertible>( | |
| public func bodyGet<T: Decodable & _StringParameterConvertible, C>( |
| /// - headerFields: Headers container where to add the Content-Type header. | ||
| /// - transform: Closure for transforming the Encodable value into body content. | ||
| /// - Returns: Data for the serialized body value, or nil if `value` was nil. | ||
| public func bodyAddOptional<T: Encodable & _StringParameterConvertible, C: _StringParameterConvertible>( |
There was a problem hiding this comment.
| public func bodyAddOptional<T: Encodable & _StringParameterConvertible, C: _StringParameterConvertible>( | |
| public func bodyAddOptional<T: Encodable & _StringParameterConvertible, C>( |
| /// - headerFields: Headers container where to add the Content-Type header. | ||
| /// - transform: Closure for transforming the Encodable value into body content. | ||
| /// - Returns: Data for the serialized body value. | ||
| public func bodyAddRequired<T: Encodable & _StringParameterConvertible, C: _StringParameterConvertible>( |
There was a problem hiding this comment.
| public func bodyAddRequired<T: Encodable & _StringParameterConvertible, C: _StringParameterConvertible>( | |
| public func bodyAddRequired<T: Encodable & _StringParameterConvertible, C>( |
| _ value: C, | ||
| headerFields: inout [HeaderField], | ||
| transforming transform: (C) -> EncodableBodyContent<T> | ||
| ) throws -> Data where C: _StringParameterConvertible { |
There was a problem hiding this comment.
| ) throws -> Data where C: _StringParameterConvertible { | |
| ) throws -> Data { |
|
I think that we'll close this for now, because it's not likely to be relevant if we tackle apple/swift-openapi-generator#43. |
Motivation
As described in apple/swift-openapi-generator#41, there are a number of places where we using
as?andas!type casting, which could be replaced with generic overloads.Modifications
Result
Test Plan
The tests continue to pass.
Notes
Whilst implementing this change, I think that we may want to revisit the encoding logic, since I think the knowledge of whether to encode a value as binary data, JSON, or plain text, is at the place where the content-type is set, but we'll defer that to a subsequent PR.