Skip to content

Commit 82323ce

Browse files
authored
Fix a bug in path parameter ordering (#380)
### Motivation Fixes #378. Basically, if the path parameter order was different between the URL template `/a/{a}/b/{b}` and the list of path parameters, for example if `b` came before `a` there, we used the parameter order, not the URL template order, which was _incorrect_. I suspect this bug has been hiding for this long is because most documents use the same order in both. ### Modifications Change the algorithm that composes the URL template and the list of parameters to respect the URL template's order. ### Result The URL template order is now respected, regardless of the path parameter order. ### Test Plan Added a snippet test that deliberately uses a different order between the URL template and the path paramater list. It failed before this fix, passes with it.
1 parent d3a7ba9 commit 82323ce

File tree

2 files changed

+99
-8
lines changed

2 files changed

+99
-8
lines changed

Sources/_OpenAPIGeneratorCore/Translator/Operations/OperationDescription.swift

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,32 @@ extension OperationDescription {
251251
/// For example, `/cats/{}` and `[input.catId]`.
252252
var templatedPathForClient: (String, Expression) {
253253
get throws {
254-
let path = self.path.rawValue
255-
let pathParameters = try allResolvedParameters.filter { $0.location == .path }
256-
// replace "{foo}" with "{}" for each parameter
257-
let template = pathParameters.reduce(into: path) { partialResult, parameter in
258-
partialResult = partialResult.replacingOccurrences(of: "{\(parameter.name)}", with: "{}")
254+
let pathParameterNames = try Set(allResolvedParameters.filter { $0.location == .path }.map(\.name))
255+
var orderedPathParameters: [String] = []
256+
// Replace "{foo}" with "{}" for each parameter and record the order
257+
// in which the parameters are used.
258+
var newComponents: [String] = []
259+
for component in path.components {
260+
guard component.hasPrefix("{") && component.hasSuffix("}") else {
261+
newComponents.append(component)
262+
continue
263+
}
264+
let componentName = String(component.dropFirst().dropLast())
265+
guard pathParameterNames.contains(componentName) else {
266+
throw GenericError(
267+
message:
268+
"Parameter '\(componentName)' used in the path '\(self.path.rawValue)', but not found in the defined list of path parameters."
269+
)
270+
}
271+
orderedPathParameters.append(componentName)
272+
newComponents.append("{}")
259273
}
260-
let names: [Expression] = pathParameters.map { param in
261-
.identifierPattern("input").dot("path").dot(asSwiftSafeName(param.name))
274+
let newPath = OpenAPI.Path(newComponents, trailingSlash: path.trailingSlash)
275+
let names: [Expression] = orderedPathParameters.map { param in
276+
.identifierPattern("input").dot("path").dot(asSwiftSafeName(param))
262277
}
263278
let arrayExpr: Expression = .literal(.array(names))
264-
return (template, arrayExpr)
279+
return (newPath.rawValue, arrayExpr)
265280
}
266281
}
267282

Tests/OpenAPIGeneratorReferenceTests/SnippetBasedReferenceTests.swift

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2157,6 +2157,82 @@ final class SnippetBasedReferenceTests: XCTestCase {
21572157
"""
21582158
)
21592159
}
2160+
func testRequestWithPathParams() throws {
2161+
try self.assertRequestInTypesClientServerTranslation(
2162+
"""
2163+
/foo/a/{a}/b/{b}:
2164+
get:
2165+
parameters:
2166+
- name: b
2167+
in: path
2168+
required: true
2169+
schema:
2170+
type: string
2171+
- name: a
2172+
in: path
2173+
required: true
2174+
schema:
2175+
type: string
2176+
operationId: getFoo
2177+
responses:
2178+
default:
2179+
description: Response
2180+
""",
2181+
types: """
2182+
public struct Input: Sendable, Hashable {
2183+
public struct Path: Sendable, Hashable {
2184+
public var b: Swift.String
2185+
public var a: Swift.String
2186+
public init(
2187+
b: Swift.String,
2188+
a: Swift.String
2189+
) {
2190+
self.b = b
2191+
self.a = a
2192+
}
2193+
}
2194+
public var path: Operations.getFoo.Input.Path
2195+
public init(path: Operations.getFoo.Input.Path) {
2196+
self.path = path
2197+
}
2198+
}
2199+
""",
2200+
client: """
2201+
{ input in
2202+
let path = try converter.renderedPath(
2203+
template: "/foo/a/{}/b/{}",
2204+
parameters: [
2205+
input.path.a,
2206+
input.path.b
2207+
]
2208+
)
2209+
var request: HTTPTypes.HTTPRequest = .init(
2210+
soar_path: path,
2211+
method: .get
2212+
)
2213+
suppressMutabilityWarning(&request)
2214+
return (request, nil)
2215+
}
2216+
""",
2217+
server: """
2218+
{ request, requestBody, metadata in
2219+
let path: Operations.getFoo.Input.Path = .init(
2220+
b: try converter.getPathParameterAsURI(
2221+
in: metadata.pathParameters,
2222+
name: "b",
2223+
as: Swift.String.self
2224+
),
2225+
a: try converter.getPathParameterAsURI(
2226+
in: metadata.pathParameters,
2227+
name: "a",
2228+
as: Swift.String.self
2229+
)
2230+
)
2231+
return Operations.getFoo.Input(path: path)
2232+
}
2233+
"""
2234+
)
2235+
}
21602236

21612237
func testRequestRequiredBodyPrimitiveSchema() throws {
21622238
try self.assertRequestInTypesClientServerTranslation(

0 commit comments

Comments
 (0)