Skip to content

Commit f0589f6

Browse files
authored
Fix optional query param parsing (#48)
Fix optional query param parsing ### Motivation Fixes apple/swift-openapi-generator#249. ### Modifications Adds a `decodeIfPresent` method on `URIDecoder` now used by the optional query param getter. ### Result This makes the query param getter return nil instead of throwing an error when it encounters a missing optional query param. ### Test Plan Added new unit tests, hopefully these cover it now. Reviewed by: glbrntt, simonjbeaumont, tib Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (api breakage) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (integration test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. #48
1 parent f0fc91d commit f0589f6

File tree

9 files changed

+180
-5
lines changed

9 files changed

+180
-5
lines changed

Sources/OpenAPIRuntime/Conversion/Converter+Server.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ extension Converter {
134134
dateTranscoder: configuration.dateTranscoder
135135
)
136136
)
137-
let value = try decoder.decode(
137+
let value = try decoder.decodeIfPresent(
138138
T.self,
139139
forKey: name,
140140
from: query

Sources/OpenAPIRuntime/Conversion/CurrencyExtensions.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,9 @@ extension Converter {
322322
explode: Bool?,
323323
name: String,
324324
as type: T.Type,
325-
convert: (String, ParameterStyle, Bool) throws -> T
325+
convert: (String, ParameterStyle, Bool) throws -> T?
326326
) throws -> T? {
327-
guard let query else {
327+
guard let query, !query.isEmpty else {
328328
return nil
329329
}
330330
let (resolvedStyle, resolvedExplode) = try ParameterStyle.resolvedQueryStyleAndExplode(

Sources/OpenAPIRuntime/URICoder/Decoding/URIDecoder.swift

+47
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,28 @@ extension URIDecoder {
8282
}
8383
}
8484

85+
/// Attempt to decode an object from an URI string, if present.
86+
///
87+
/// Under the hood, `URIDecoder` first parses the string into a
88+
/// `URIParsedNode` using `URIParser`, and then uses
89+
/// `URIValueFromNodeDecoder` to decode the `Decodable` value.
90+
///
91+
/// - Parameters:
92+
/// - type: The type to decode.
93+
/// - key: The key of the decoded value. Only used with certain styles
94+
/// and explode options, ignored otherwise.
95+
/// - data: The URI-encoded string.
96+
/// - Returns: The decoded value.
97+
func decodeIfPresent<T: Decodable>(
98+
_ type: T.Type = T.self,
99+
forKey key: String = "",
100+
from data: String
101+
) throws -> T? {
102+
try withCachedParser(from: data) { decoder in
103+
try decoder.decodeIfPresent(type, forKey: key)
104+
}
105+
}
106+
85107
/// Make multiple decode calls on the parsed URI.
86108
///
87109
/// Use to avoid repeatedly reparsing the raw string.
@@ -133,4 +155,29 @@ struct URICachedDecoder {
133155
)
134156
return try decoder.decodeRoot()
135157
}
158+
159+
/// Attempt to decode an object from an URI-encoded string, if present.
160+
///
161+
/// Under the hood, `URICachedDecoder` already has a pre-parsed
162+
/// `URIParsedNode` and uses `URIValueFromNodeDecoder` to decode
163+
/// the `Decodable` value.
164+
///
165+
/// - Parameters:
166+
/// - type: The type to decode.
167+
/// - key: The key of the decoded value. Only used with certain styles
168+
/// and explode options, ignored otherwise.
169+
/// - Returns: The decoded value.
170+
func decodeIfPresent<T: Decodable>(
171+
_ type: T.Type = T.self,
172+
forKey key: String = ""
173+
) throws -> T? {
174+
let decoder = URIValueFromNodeDecoder(
175+
node: node,
176+
rootKey: key[...],
177+
style: configuration.style,
178+
explode: configuration.explode,
179+
dateTranscoder: configuration.dateTranscoder
180+
)
181+
return try decoder.decodeRootIfPresent()
182+
}
136183
}

Sources/OpenAPIRuntime/URICoder/Decoding/URIValueFromNodeDecoder.swift

+14-1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,18 @@ final class URIValueFromNodeDecoder {
7979
}
8080
return value
8181
}
82+
83+
/// Decodes the provided type from the root node.
84+
/// - Parameter type: The type to decode from the decoder.
85+
/// - Returns: The decoded value.
86+
/// - Throws: When a decoding error occurs.
87+
func decodeRootIfPresent<T: Decodable>(_ type: T.Type = T.self) throws -> T? {
88+
// The root is only nil if the node is empty.
89+
if try currentElementAsArray().isEmpty {
90+
return nil
91+
}
92+
return try decodeRoot(type)
93+
}
8294
}
8395

8496
extension URIValueFromNodeDecoder {
@@ -285,7 +297,8 @@ extension URIValueFromNodeDecoder {
285297
array = try rootValue(in: values)
286298
}
287299
guard array.count == 1 else {
288-
try throwMismatch("Cannot parse a value from a node with multiple values.")
300+
let reason = array.isEmpty ? "an empty node" : "a node with multiple values"
301+
try throwMismatch("Cannot parse a value from \(reason).")
289302
}
290303
let value = array[0]
291304
return value

Sources/OpenAPIRuntime/URICoder/Encoding/URIEncoder.swift

+26
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,30 @@ extension URIEncoder {
8989
let encodedString = try serializer.serializeNode(node, forKey: key)
9090
return encodedString
9191
}
92+
93+
/// Attempt to encode an object into an URI string, if not nil.
94+
///
95+
/// Under the hood, `URIEncoder` first encodes the `Encodable` type
96+
/// into a `URIEncodableNode` using `URIValueToNodeEncoder`, and then
97+
/// `URISerializer` encodes the `URIEncodableNode` into a string based
98+
/// on the configured behavior.
99+
///
100+
/// - Parameters:
101+
/// - value: The value to encode.
102+
/// - key: The key for which to encode the value. Can be an empty key,
103+
/// in which case you still get a key-value pair, like `=foo`.
104+
/// - Returns: The URI string.
105+
func encodeIfPresent(
106+
_ value: (some Encodable)?,
107+
forKey key: String
108+
) throws -> String {
109+
guard let value else {
110+
return ""
111+
}
112+
let encoder = URIValueToNodeEncoder()
113+
let node = try encoder.encodeValue(value)
114+
var serializer = serializer
115+
let encodedString = try serializer.serializeNode(node, forKey: key)
116+
return encodedString
117+
}
92118
}

Sources/OpenAPIRuntime/URICoder/Encoding/URIValueToNodeEncoder+Single.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ extension URISingleValueEncodingContainer: SingleValueEncodingContainer {
5252
}
5353

5454
func encodeNil() throws {
55-
throw URIValueToNodeEncoder.GeneralError.nilNotSupported
55+
// Nil is encoded as no value.
5656
}
5757

5858
func encode(_ value: Bool) throws {

Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift

+36
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,42 @@ final class Test_ServerConverterExtensions: Test_Runtime {
164164
XCTAssertEqual(value, "foo")
165165
}
166166

167+
// | server | get | request query | URI | optional | getOptionalQueryItemAsURI |
168+
func test_getOptionalQueryItemAsURI_string_nil() throws {
169+
let value: String? = try converter.getOptionalQueryItemAsURI(
170+
in: "",
171+
style: nil,
172+
explode: nil,
173+
name: "search",
174+
as: String.self
175+
)
176+
XCTAssertNil(value)
177+
}
178+
179+
// | server | get | request query | URI | optional | getOptionalQueryItemAsURI |
180+
func test_getOptionalQueryItemAsURI_string_notFound() throws {
181+
let value: String? = try converter.getOptionalQueryItemAsURI(
182+
in: "foo=bar",
183+
style: nil,
184+
explode: nil,
185+
name: "search",
186+
as: String.self
187+
)
188+
XCTAssertNil(value)
189+
}
190+
191+
// | server | get | request query | URI | optional | getOptionalQueryItemAsURI |
192+
func test_getOptionalQueryItemAsURI_string_empty() throws {
193+
let value: String? = try converter.getOptionalQueryItemAsURI(
194+
in: "search=",
195+
style: nil,
196+
explode: nil,
197+
name: "search",
198+
as: String.self
199+
)
200+
XCTAssertEqual(value, "")
201+
}
202+
167203
// | server | get | request query | URI | required | getRequiredQueryItemAsURI |
168204
func test_getRequiredQueryItemAsURI_string() throws {
169205
let value: String = try converter.getRequiredQueryItemAsURI(

Tests/OpenAPIRuntimeTests/URICoder/Decoder/Test_URIDecoder.swift

+52
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,56 @@ final class Test_URIDecoder: Test_Runtime {
2828
)
2929
XCTAssertEqual(decodedValue, Foo(bar: "hello world"))
3030
}
31+
32+
func testDecoding_structWithOptionalProperty() throws {
33+
struct Foo: Decodable, Equatable {
34+
var bar: String?
35+
var baz: Int
36+
}
37+
let decoder = URIDecoder(configuration: .formDataExplode)
38+
do {
39+
let decodedValue = try decoder.decode(
40+
Foo.self,
41+
forKey: "",
42+
from: "baz=1&bar=hello+world"
43+
)
44+
XCTAssertEqual(decodedValue, Foo(bar: "hello world", baz: 1))
45+
}
46+
do {
47+
let decodedValue = try decoder.decode(
48+
Foo.self,
49+
forKey: "",
50+
from: "baz=1"
51+
)
52+
XCTAssertEqual(decodedValue, Foo(baz: 1))
53+
}
54+
}
55+
56+
func testDecoding_rootValue() throws {
57+
let decoder = URIDecoder(configuration: .formDataExplode)
58+
do {
59+
let decodedValue = try decoder.decode(
60+
Int.self,
61+
forKey: "root",
62+
from: "root=1"
63+
)
64+
XCTAssertEqual(decodedValue, 1)
65+
}
66+
do {
67+
let decodedValue = try decoder.decodeIfPresent(
68+
Int.self,
69+
forKey: "root",
70+
from: "baz=1"
71+
)
72+
XCTAssertEqual(decodedValue, nil)
73+
}
74+
do {
75+
let decodedValue = try decoder.decodeIfPresent(
76+
Int.self,
77+
forKey: "root",
78+
from: ""
79+
)
80+
XCTAssertEqual(decodedValue, nil)
81+
}
82+
}
3183
}

Tests/OpenAPIRuntimeTests/URICoder/Test_URICodingRoundtrip.swift

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ final class Test_URICodingRoundtrip: Test_Runtime {
2424
var color: SimpleEnum
2525
var empty: String
2626
var date: Date
27+
var maybeFoo: String?
2728
}
2829

2930
enum SimpleEnum: String, Codable, Equatable {

0 commit comments

Comments
 (0)