Skip to content

Hyphen in path variable name results in static path variable name in url #601

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

Closed
ChipCracker opened this issue Jul 23, 2024 · 4 comments · Fixed by #602
Closed

Hyphen in path variable name results in static path variable name in url #601

ChipCracker opened this issue Jul 23, 2024 · 4 comments · Fixed by #602
Labels
area/generator Affects: plugin, CLI, config file. kind/bug Feature doesn't work as expected.
Milestone

Comments

@ChipCracker
Copy link

Description

When generating Swift code from the API spec, path variables that contain a “-” are not recognized as path variables and are instead generated as fixed URL parameters/paths. However the path variable is generated correctly, its just not used:

Example:

/testresult/upload/{qrcode-uuid}:

Becomes (Client.swift):

let path = try converter.renderedPath(
    template: "/testresult/upload/qrcode-uuid",
    parameters: []
)

However, the expected behavior would be:

let path = try converter.renderedPath(
    template: "/testresult/upload/{}",
    parameters: [
        input.path.qrcode_hyphen_uuid
    ]
)

Types.swift:

/// - Remark: Generated from `#/paths/testresult/{qrcode-uuid}/GET/path`.
public struct Path: Sendable, Hashable {
    /// UUID associated with the specific test result
    ///
    /// - Remark: Generated from `#/paths/testresult/{qrcode-uuid}/GET/path/qrcode-uuid`.
    public var qrcode_hyphen_uuid: Swift.String
    /// Creates a new `Path`.
    ///
    /// - Parameters:
    ///   - qrcode_hyphen_uuid: UUID associated with the specific test result
    public init(qrcode_hyphen_uuid: Swift.String) {
        self.qrcode_hyphen_uuid = qrcode_hyphen_uuid
    }
}

Reproduction

openapi: 3.0.3
info:
  title: Example
  description: This is the API definition for the example-app
  contact:
    email: [email protected]
  license:
    name: Apache 2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.11
servers:
  - url: http://localhost:8080/api/v1
tags:
  - name: Test-Result
paths:
  /testresult/{qrcode-uuid}:
    get:
      tags:
        - Test-Result
      summary: "Retrieves a specific test result by UUID as an encrypted ZIP file"
      operationId: "get_test_result"
      x-openapi-router-controller: openapi_server.controllers.test_result_controller
      parameters:
        - in: path
          name: qrcode-uuid
          required: true
          schema:
            type: string
          description: "UUID associated with the specific test result"
      responses:
        '200':
          description: "Successful operation, encrypted test result retrieved"
          content:
            application/zip:
              schema:
                type: string
                format: binary
                description: "Encrypted ZIP file containing the test result"
        '404':
          description: "Test result not found"
        '500':
          description: "Server error"
  /testresult/upload/{qrcodeuuid}:
    put:
      tags:
        - Test-Result
      summary: "Uploads a test result zip file"
      x-openapi-router-controller: openapi_server.controllers.test_result_controller
      parameters:
        - in: path
          name: qrcodeuuid
          required: true
          schema:
            type: string
          description: "UUID associated with the QR code"
        - in: query
          name: study-secret
          required: true
          schema:
            type: string
          description: "the study secret for auth"
      responses:
        '200':
          description: "Successful operation"
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ApiResponse'
        '409':
          description: "Testresult already exists"
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ApiResponse'
      requestBody:
        required: true
        description: "Encrypted ZIP file containing test results"
        content:
          application/zip:
            schema:
              type: string
              format: binary
              description: "Encrypted ZIP file"
  /health:
    get:
      tags:
        - Health Check
      summary: "Health Check Endpoint"
      description: "Returns the health status of the API server"
      operationId: "get_health_status"
      x-openapi-router-controller: openapi_server.controllers.health_check_controller
      responses:
        '200':
          description: "API server is up and running"
          content:
            application/json:
              schema:
                type: object
                properties:
                  status:
                    type: string
                    example: "healthy"
                  timestamp:
                    type: string
                    format: date-time
                    example: "2024-05-29T12:34:56Z"
      

components:
  schemas:
    ApiResponse:
      type: object
      properties:
        code:
          type: integer
          format: int32
        type:
          type: string
        message:
          type: string
      xml:
        name: ApiResponse

Package version(s)

OpenAPIKit 3.2.0
swift-algorithms 1.2.0
swift-argument-parser 1.5.0
swift-collections 1.1.2
swift-http-types 1.3.0
swift-numerics 1.0.2
swift-openapi-generator 1.2.1
swift-openapi-runtime 1.4.1
swift-openapi-urlsession 1.0.1
Yams 5.1.3

Expected behavior

// Generated by swift-openapi-generator, do not modify.
@_spi(Generated) import OpenAPIRuntime
#if os(Linux)
@preconcurrency import struct Foundation.URL
@preconcurrency import struct Foundation.Data
@preconcurrency import struct Foundation.Date
#else
import struct Foundation.URL
import struct Foundation.Data
import struct Foundation.Date
#endif
import HTTPTypes
/// This is the API definition for the memtest-app
internal struct Client: APIProtocol {
    /// The underlying HTTP client.
    private let client: UniversalClient
    /// Creates a new client.
    /// - Parameters:
    ///   - serverURL: The server URL that the client connects to. Any server
    ///   URLs defined in the OpenAPI document are available as static methods
    ///   on the ``Servers`` type.
    ///   - configuration: A set of configuration values for the client.
    ///   - transport: A transport that performs HTTP operations.
    ///   - middlewares: A list of middlewares to call before the transport.
    internal init(
        serverURL: Foundation.URL,
        configuration: Configuration = .init(),
        transport: any ClientTransport,
        middlewares: [any ClientMiddleware] = []
    ) {
        self.client = .init(
            serverURL: serverURL,
            configuration: configuration,
            transport: transport,
            middlewares: middlewares
        )
    }
    private var converter: Converter {
        client.converter
    }
    /// Retrieves a specific test result by UUID as an encrypted ZIP file
    ///
    /// - Remark: HTTP `GET /testresult/{qrcode-uuid}`.
    /// - Remark: Generated from `#/paths//testresult/{qrcode-uuid}/get(get_test_result)`.
    internal func get_test_result(_ input: Operations.get_test_result.Input) async throws -> Operations.get_test_result.Output {
        try await client.send(
            input: input,
            forOperation: Operations.get_test_result.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/testresult/{}",
                    parameters: [
                        input.path.qrcode_hyphen_uuid
                    ]
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .get
                )
                suppressMutabilityWarning(&request)
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                return (request, nil)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.get_test_result.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/zip"
                        ]
                    )
                    switch chosenContentType {
                    case "application/zip":
                        body = try converter.getResponseBodyAsBinary(
                            OpenAPIRuntime.HTTPBody.self,
                            from: responseBody,
                            transforming: { value in
                                .application_zip(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                case 404:
                    return .notFound(.init())
                case 500:
                    return .internalServerError(.init())
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
    /// Uploads a test result zip file
    ///
    /// - Remark: HTTP `PUT /testresult/upload/{qrcodeuuid}`.
    /// - Remark: Generated from `#/paths//testresult/upload/{qrcodeuuid}/put(upload_test_result)`.
    internal func upload_test_result(_ input: Operations.upload_test_result.Input) async throws -> Operations.upload_test_result.Output {
        try await client.send(
            input: input,
            forOperation: Operations.upload_test_result.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/testresult/upload/{}",
                    parameters: [
                        input.path.qrcode_hyphen_uuid
                    ]
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .put
                )
                suppressMutabilityWarning(&request)
                try converter.setQueryItemAsURI(
                    in: &request,
                    style: .form,
                    explode: true,
                    name: "study-secret",
                    value: input.query.study_hyphen_secret
                )
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                let body: OpenAPIRuntime.HTTPBody?
                switch input.body {
                case let .application_zip(value):
                    body = try converter.setRequiredRequestBodyAsBinary(
                        value,
                        headerFields: &request.headerFields,
                        contentType: "application/zip"
                    )
                }
                return (request, body)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.upload_test_result.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Components.Schemas.ApiResponse.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                case 409:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.upload_test_result.Output.Conflict.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Components.Schemas.ApiResponse.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .conflict(.init(body: body))
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
    /// Health Check Endpoint
    ///
    /// Returns the health status of the API server
    ///
    /// - Remark: HTTP `GET /health`.
    /// - Remark: Generated from `#/paths//health/get(get_health_status)`.
    internal func get_health_status(_ input: Operations.get_health_status.Input) async throws -> Operations.get_health_status.Output {
        try await client.send(
            input: input,
            forOperation: Operations.get_health_status.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/health",
                    parameters: []
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .get
                )
                suppressMutabilityWarning(&request)
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                return (request, nil)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.get_health_status.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Operations.get_health_status.Output.Ok.Body.jsonPayload.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
}

Environment

Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0

Additional information

Currently generated code:

// Generated by swift-openapi-generator, do not modify.
@_spi(Generated) import OpenAPIRuntime
#if os(Linux)
@preconcurrency import struct Foundation.URL
@preconcurrency import struct Foundation.Data
@preconcurrency import struct Foundation.Date
#else
import struct Foundation.URL
import struct Foundation.Data
import struct Foundation.Date
#endif
import HTTPTypes
/// This is the API definition for the memtest-app
public struct Client: APIProtocol {
    /// The underlying HTTP client.
    private let client: UniversalClient
    /// Creates a new client.
    /// - Parameters:
    ///   - serverURL: The server URL that the client connects to. Any server
    ///   URLs defined in the OpenAPI document are available as static methods
    ///   on the ``Servers`` type.
    ///   - configuration: A set of configuration values for the client.
    ///   - transport: A transport that performs HTTP operations.
    ///   - middlewares: A list of middlewares to call before the transport.
    public init(
        serverURL: Foundation.URL,
        configuration: Configuration = .init(),
        transport: any ClientTransport,
        middlewares: [any ClientMiddleware] = []
    ) {
        self.client = .init(
            serverURL: serverURL,
            configuration: configuration,
            transport: transport,
            middlewares: middlewares
        )
    }
    private var converter: Converter {
        client.converter
    }
    /// Retrieves a specific test result by UUID as an encrypted ZIP file
    ///
    /// - Remark: HTTP `GET /testresult/{qrcode-uuid}`.
    /// - Remark: Generated from `#/paths//testresult/{qrcode-uuid}/get(get_test_result)`.
    public func get_test_result(_ input: Operations.get_test_result.Input) async throws -> Operations.get_test_result.Output {
        try await client.send(
            input: input,
            forOperation: Operations.get_test_result.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/testresult/qrcode-uuid",
                    parameters: []
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .get
                )
                suppressMutabilityWarning(&request)
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                return (request, nil)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.get_test_result.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/zip"
                        ]
                    )
                    switch chosenContentType {
                    case "application/zip":
                        body = try converter.getResponseBodyAsBinary(
                            OpenAPIRuntime.HTTPBody.self,
                            from: responseBody,
                            transforming: { value in
                                .application_zip(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                case 404:
                    return .notFound(.init())
                case 500:
                    return .internalServerError(.init())
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
    /// Uploads a test result zip file
    ///
    /// - Remark: HTTP `PUT /testresult/upload/{qrcode-uuid}`.
    /// - Remark: Generated from `#/paths//testresult/upload/{qrcode-uuid}/put(upload_test_result)`.
    public func upload_test_result(_ input: Operations.upload_test_result.Input) async throws -> Operations.upload_test_result.Output {
        try await client.send(
            input: input,
            forOperation: Operations.upload_test_result.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/testresult/upload/qrcode-uuid",
                    parameters: []
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .put
                )
                suppressMutabilityWarning(&request)
                try converter.setQueryItemAsURI(
                    in: &request,
                    style: .form,
                    explode: true,
                    name: "study-secret",
                    value: input.query.study_hyphen_secret
                )
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                let body: OpenAPIRuntime.HTTPBody?
                switch input.body {
                case let .application_zip(value):
                    body = try converter.setRequiredRequestBodyAsBinary(
                        value,
                        headerFields: &request.headerFields,
                        contentType: "application/zip"
                    )
                }
                return (request, body)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.upload_test_result.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Components.Schemas.ApiResponse.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                case 409:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.upload_test_result.Output.Conflict.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Components.Schemas.ApiResponse.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .conflict(.init(body: body))
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
    /// Health Check Endpoint
    ///
    /// Returns the health status of the API server
    ///
    /// - Remark: HTTP `GET /health`.
    /// - Remark: Generated from `#/paths//health/get(get_health_status)`.
    public func get_health_status(_ input: Operations.get_health_status.Input) async throws -> Operations.get_health_status.Output {
        try await client.send(
            input: input,
            forOperation: Operations.get_health_status.id,
            serializer: { input in
                let path = try converter.renderedPath(
                    template: "/health",
                    parameters: []
                )
                var request: HTTPTypes.HTTPRequest = .init(
                    soar_path: path,
                    method: .get
                )
                suppressMutabilityWarning(&request)
                converter.setAcceptHeader(
                    in: &request.headerFields,
                    contentTypes: input.headers.accept
                )
                return (request, nil)
            },
            deserializer: { response, responseBody in
                switch response.status.code {
                case 200:
                    let contentType = converter.extractContentTypeIfPresent(in: response.headerFields)
                    let body: Operations.get_health_status.Output.Ok.Body
                    let chosenContentType = try converter.bestContentType(
                        received: contentType,
                        options: [
                            "application/json"
                        ]
                    )
                    switch chosenContentType {
                    case "application/json":
                        body = try await converter.getResponseBodyAsJSON(
                            Operations.get_health_status.Output.Ok.Body.jsonPayload.self,
                            from: responseBody,
                            transforming: { value in
                                .json(value)
                            }
                        )
                    default:
                        preconditionFailure("bestContentType chose an invalid content type.")
                    }
                    return .ok(.init(body: body))
                default:
                    return .undocumented(
                        statusCode: response.status.code,
                        .init(
                            headerFields: response.headerFields,
                            body: responseBody
                        )
                    )
                }
            }
        )
    }
}
@ChipCracker ChipCracker added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Jul 23, 2024
@czechboy0
Copy link
Contributor

Hi @ChipCracker, thank you for this great bug report!

You are correct, the bug is caused by the hyphen in the parameter name. It isn't getting correctly parsed by the regular expression that splits the path into components.

I have a fix ready locally, let me open a PR shortly.

@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. and removed status/triage Collecting information required to triage the issue. labels Jul 23, 2024
@czechboy0 czechboy0 added this to the Post-1.0 milestone Jul 23, 2024
@ChipCracker
Copy link
Author

Hi @czechboy0

thank you for the swift response and for preparing a fix so quickly!

@czechboy0
Copy link
Contributor

You're welcome, it was only possible because you already did the heavy lifting and identified the bug 🙂

czechboy0 added a commit that referenced this issue Jul 23, 2024
### Motivation

Fixes #601, check out the issue for details.

### Modifications

Adds hyphen to the regular expression that parses out params from the
URL template.

### Result

Correctly handle path params with hyphens in the name.

### Test Plan

Added a unit test.
@czechboy0
Copy link
Contributor

Fixed and released in 1.3.0: https://github.com./apple/swift-openapi-generator/releases/tag/1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/generator Affects: plugin, CLI, config file. kind/bug Feature doesn't work as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants