Multipart form properties codegen is not typesafe

[jpsim]

Motivation

One of the main benefits I get from using swift-openapi-generator is type safety for client code.

However, for multipart form upload requests, it seems like the property codegen is not typesafe, where all properties just get a generic HTTPBody payload.

For example, given this OpenAPI document:

/uploadFile:
  post:
    operationId: uploadFile
    requestBody:
      required: true
      content:
        multipart/form-data:
          schema:
            type: object
            properties:
              description:
                type: string
                description: A text description of the uploaded file
              count:
                type: integer
                description: An integer value associated with the upload
              imageFile:
                type: string
                format: binary
                description: The image file to upload
            required:
              - description
              - count
              - imageFile

This code is generated for the request body type:

enum Body: Sendable, Hashable {
    enum MultipartFormPayload: Sendable, Hashable {
        struct DescriptionPayload: Sendable, Hashable {
            var body: OpenAPIRuntime.HTTPBody
            init(body: OpenAPIRuntime.HTTPBody) {
                self.body = body
            }
        }
        case description(OpenAPIRuntime.MultipartPart<Operations.UploadFile.Input.Body.MultipartFormPayload.DescriptionPayload>)
        struct CountPayload: Sendable, Hashable {
            var body: OpenAPIRuntime.HTTPBody
            init(body: OpenAPIRuntime.HTTPBody) {
                self.body = body
            }
        }
        case count(OpenAPIRuntime.MultipartPart<Operations.UploadFile.Input.Body.MultipartFormPayload.CountPayload>)
        struct ImageFilePayload: Sendable, Hashable {
            var body: OpenAPIRuntime.HTTPBody
            init(body: OpenAPIRuntime.HTTPBody) {
                self.body = body
            }
        }
        case imageFile(OpenAPIRuntime.MultipartPart<Operations.UploadFile.Input.Body.MultipartFormPayload.ImageFilePayload>)
        case undocumented(OpenAPIRuntime.MultipartRawPart)
    }
    case multipartForm(OpenAPIRuntime.MultipartBody<Operations.UploadFile.Input.Body.MultipartFormPayload>)
}

So at the call site, you're responsible for producing HTTPBody values that respect the schema, but you get no compile-type safety if you make a mistake:

let multipartBody: MultipartBody<Operations.UploadFile.Input.Body.MultipartFormPayload> = [
    .description(.init(payload: .init(body: "This is a test image upload"))),
    .count(.init(payload: .init(body: "42"))),
    .imageFile(.init(payload: .init(body: .init(fileData)))),
]

Ideally, the codegen would produce type safe property payload initializers:

let multipartBody: MultipartBody<Operations.UploadFile.Input.Body.MultipartFormPayload> = [
    .description(.init(payload: .init(value: "This is a test image upload"))),
    .count(.init(payload: .init(value: 42))),
    .imageFile(.init(payload: .init(value: fileData))),
]

and you'd get a compiler failure if you passed say a Swift.String as the count value.

Proposed solution

Ideally, the codegen would produce type safe property payload initializers:

let multipartBody: MultipartBody<Operations.UploadFile.Input.Body.MultipartFormPayload> = [
    .description(.init(payload: .init(value: "This is a test image upload"))),
    .count(.init(payload: .init(value: 42))),
    .imageFile(.init(payload: .init(value: fileData))),
]

and you'd get a compiler failure if you passed say a Swift.String as the count value.

Alternatives considered

No response

Additional information

No response

[jpsim]

Also, let me know if I should open a new issue for this, but there's also a lack of type safety related to missing a required fields.

In my example above, if you forget to add the count property in multipartBody, the request will just hang until it times out, without ever making an http request. Might be nice to have an alternative where you produce a UploadFileRequestBody struct with typesafe properties up front in an initializer. You wouldn't be able to use this in all cases, but in my case I know all the fields I want to populate up front before performing the request.

And a third (final!) thing on multipart form upload ergonomics, in my case I want to specify a dynamic content type and file name for my image property, but instead of doing .image(custom overrides...) I have to resort to .undocumented(MultipartRawPart(custom overrides...). Ideally I could just pass the filename and content type directly in the codegened .image(...) enum.

[czechboy0]

Hi @jpsim, thanks for the detailed issue(s) 🙂

On the first one, a multipart part of type "string" will always be HTTPBody (an async sequence of chunks), because we don't know if you want to buffer it or not, so we give you the stream always. That's meant for things like multi-GB log files (which could also be a part of type string).

Now, the integer at the top level being HTTPBody is unexpected to me, and we'll need to look into it.

More generally, I think you'll get the best results if your multipart parts are almost like request bodies - each of a different content type (or multiple files). So if I were authoring your example, I'd write it as:

/uploadFile:
  post:
    operationId: uploadFile
    requestBody:
      required: true
      content:
        multipart/form-data:
          schema:
            type: object
            properties:
              metadata:
                type: object
                properties:
                  description:
                    type: string
                    description: A text description of the uploaded file
                  count:
                    type: integer
                    description: An integer value associated with the upload
                required: [description, count]
              imageFile:
                type: string
                format: binary
                description: The image file to upload
            required:
              - metadata
              - imageFile

Although I appreciate that this might be an upstream service's OpenAPI that you might not be able to change.

[jpsim]

On the first one, a multipart part of type "string" will always be HTTPBody (an async sequence of chunks), because we don't know if you want to buffer it or not, so we give you the stream always. That's meant for things like multi-GB log files (which could also be a part of type string).

I appreciate the benefits of having a performant and streaming friendly API, however for small strings it'd be nice to have a typesafe initializer.

Same thing goes for string-backed enums!

[czechboy0]

I agree - we just don't know which strings will be larger or small. The OpenAPI document doesn't include that information.

[jpsim]

That's why I think it should be up to the caller to determine which to use.

For example, for a string description property:

struct DescriptionPayload: Sendable, Hashable {
  var body: OpenAPIRuntime.HTTPBody
    init(uncheckedBody: OpenAPIRuntime.HTTPBody) {
      self.body = body
    }
    init(value: Swift.String) {
      self.body = OpenAPIRuntime.HTTPBody(body)
    }
}

[czechboy0]

I see, so is this the main ask? For the generated multipart part to go from e.g.

struct DescriptionPayload: Sendable, Hashable {
    var body: OpenAPIRuntime.HTTPBody
    init(body: OpenAPIRuntime.HTTPBody) {
        self.body = body
    }
}

to

struct DescriptionPayload: Sendable, Hashable {
    var body: OpenAPIRuntime.HTTPBody
    init(body: OpenAPIRuntime.HTTPBody) {
        self.body = body
    }
+   init(bodyString string: String) {
+       self.body = .init(string)
+   }
}

I think that's reasonable, and is an API-stable change.

[jpsim]

Yes that's the main ask, to have typesafe initializers for each property payload type in addition to the current generic HTTPBody initializer. Not just for strings, but for all types supported by swift-openapi-generator. Integers, booleans, enums, data, dates, etc.

Optionally deprecating the body initializer in favor of a renamed uncheckedBody initializer would also encourage the use of the type safe initializer in most cases, and consumers can still use it for streaming use cases, but there's no schema validation if you do that. Marking an API as deprecated is usually considered to be an API-stable change. But I don't feel strongly about this one.

[jpsim]

Proposed implementation (only for primitive types though): #775