apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.23k stars 89 forks source link

SOAR-0009 - Typesafe multipart with streaming #369

Closed czechboy0 closed 6 months ago

czechboy0 commented 6 months ago

Motivation

Ready for a review, see rendered at: https://github.com/czechboy0/swift-openapi-generator/blob/hd-soar-0009/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0009.md

Modifications

N/A

Result

N/A

Test Plan

N/A

simonjbeaumont commented 6 months ago

This is now in review and will run until Nov 15th. Forum link: https://forums.swift.org/t/proposal-soar-0009-type-safe-streaming-multipart-support/68331.

nkarczmarzyk commented 6 months ago

+1 It would be an enrichment to have the multipart generator merged in a timely manner. Good work!

simonjbeaumont commented 6 months ago

+1 It would be an enrichment to have the multipart generator merged in a timely manner. Good work!

@nkarczmarzyk Just to keep you reassured, the work has already begun and several PRs have been landed into the runtime package to provide the necessary support. The generator PR will follow.

czechboy0 commented 6 months ago

@nkarczmarzyk As Si said, this work is already ongoing. I'm glad you're looking forward to it, it's a large feature with substantial changes in the generator, so once we release 1.0.0-alpha.1 next week, it would be great if you could give it a try in your project - we want to catch any final issues before publishing 1.0.0 two weeks later.

@simonjbeaumont - can you update the review please? It already finished, so this and the forums post can be updated.

nkarczmarzyk commented 6 months ago

@nkarczmarzyk As Si said, this work is already ongoing. I'm glad you're looking forward to it, it's a large feature with substantial changes in the generator, so once we release 1.0.0-alpha.1 next week, it would be great if you could give it a try in your project - we want to catch any final issues before publishing 1.0.0 two weeks later.

@simonjbeaumont - can you update the review please? It already finished, so this and the forums post can be updated.

I am looking forward to it. I also took a look on the feature (wip) PR https://github.com/apple/swift-openapi-generator/pull/366/ . Great work in there! Seems like a very complex topic. When it is ready for testing, I will give it a try and give feedback. Until then I am trying to implement a workaround.

czechboy0 commented 6 months ago

@nkarczmarzyk As Si said, this work is already ongoing. I'm glad you're looking forward to it, it's a large feature with substantial changes in the generator, so once we release 1.0.0-alpha.1 next week, it would be great if you could give it a try in your project - we want to catch any final issues before publishing 1.0.0 two weeks later. @simonjbeaumont - can you update the review please? It already finished, so this and the forums post can be updated.

I am looking forward to it. I also took a look on the feature (wip) PR #366 . Great work in there! Seems like a very complex topic. When it is ready for testing, I will give it a try and give feedback. Until then I am trying to implement a workaround.

I just got everything working end to end, so you can try the hd-multipart branch of the generator + hd-multipart-converter-methods branch of the runtime library. I'll work to clean up the code and start landing it today.

nkarczmarzyk commented 6 months ago

@czechboy0 I have found https://github.com/czechboy0/swift-openapi-generator/tree/hd-multipart for the generator. Could you please give me an hint where to find hd-multipart-converter-methods for the runtime?

czechboy0 commented 6 months ago

@czechboy0 I have found https://github.com/czechboy0/swift-openapi-generator/tree/hd-multipart for the generator. Could you please give me an hint where to find hd-multipart-converter-methods for the runtime?

Since I wrote that comment, we already landed it all on main - so you can use that branch.

nkarczmarzyk commented 6 months ago

@czechboy0

I really don't want to blow up this pr but currently it seems like something is broken. Using latest of your forked generator (hd-multipart branch) and latest runtime I am getting the following result.

open-api.yaml (v3.0.1):

paths:
[...]
  /Post/addFile:
    post:
      tags:
        - Post
      requestBody:
        content:
          multipart/form-data:
            schema:
              required:
                - File
                - PostId
                - Type
              type: object
              properties:
                PostId:
                  type: string
                  format: uuid
                File:
                  type: string
                  format: binary
                Type:
                  $ref: '#/components/schemas/FileType'
            encoding:
              PostId:
                style: form
              File:
                style: form
              Type:
                style: form
      responses:
        '200':
          description: Success
[...]
components:
  schemas:
    FileType:
      enum:
        - Picture
        - Audio
      type: string
[...]

Generated type:

[...]
    /// - Remark: HTTP `POST /Post/addFile`.
    /// - Remark: Generated from `#/paths//Post/addFile/post`.
    public enum post_sol_Post_sol_addFile {
        public static let id: Swift.String = "post/Post/addFile"
        public struct Input: Sendable, Hashable {
            /// Creates a new `Input`.
            public init() {}
        }
[...]

It is also possible that I am doing something wrong. I am quite new to the world of swift.

czechboy0 commented 6 months ago

@nkarczmarzyk Try to remove the encoding key, since none of the 3 parts are URL-encoded strings, this won't have an effect. If it still doesn't work then, I'll try to repro locally.

czechboy0 commented 6 months ago

Also, the request body must be marked as required, multipart bodies cannot be optional (which is the default).

czechboy0 commented 6 months ago

One more question, is the generator emitting any diagnostics when running? That could hint at why it's skipping the content.

czechboy0 commented 6 months ago

@nkarczmarzyk I verified in your example, that you just need to add required: true to the request body and then it works.

nkarczmarzyk commented 6 months ago

@czechboy0 Thanks for the advice. Setting required: true results in a correct generation. I will continue testing 👍

nkarczmarzyk commented 6 months ago

Hey @czechboy0 hope you had a great weekend. I am off testing again. Currently its very complicated to initialise the components in the correct way but I will report if I got more results.

Currently my code looks like this:

        let body = HTTPBody()

        let postIdPayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.PostIdPayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.PostIdPayload(body: body))

        let filePayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.FilePayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.FilePayload(body: body))

        let typePayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload._TypePayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload._TypePayload(body: body))

        let multipartBody: OpenAPIRuntime.MultipartBody<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload> = .init(
            [.PostId(postIdPayload), .File(filePayload), ._Type(typePayload)])

        let response = try await ClientProvider.sharedClient.post_sol_Post_sol_addFile(body: .multipartForm(multipartBody))
nkarczmarzyk commented 6 months ago

@czechboy0 I just found out that you are not passing the correct parameter type in https://github.com/apple/swift-openapi-generator/blob/main/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0009.md.

Instead of: let response = try await client.uploadPhoto(body: multipartBody)

It should be: let response = try await client.uploadPhoto(body: .multipartForm(multipartBody))

nkarczmarzyk commented 6 months ago

@czechboy0 And actually I do not fully understand the aspect of type safety. The generated types look like this:

[...]
                    public struct PostIdPayload: Sendable, Hashable {
                        public var body: OpenAPIRuntime.HTTPBody
                        /// Creates a new `PostIdPayload`.
                        ///
                        /// - Parameters:
                        ///   - body:
                        public init(body: OpenAPIRuntime.HTTPBody) {
                            self.body = body
                        }
                    }
                    case PostId(OpenAPIRuntime.MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.PostIdPayload>)
                    /// - Remark: Generated from `#/paths/Post/addFile/POST/requestBody/multipartForm/File`.
                    public struct FilePayload: Sendable, Hashable {
                        public var body: OpenAPIRuntime.HTTPBody
                        /// Creates a new `FilePayload`.
                        ///
                        /// - Parameters:
                        ///   - body:
                        public init(body: OpenAPIRuntime.HTTPBody) {
                            self.body = body
                        }
                    }
                    case File(OpenAPIRuntime.MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.FilePayload>)
                    /// - Remark: Generated from `#/paths/Post/addFile/POST/requestBody/multipartForm/Type`.
                    public struct _TypePayload: Sendable, Hashable {
                        public var body: OpenAPIRuntime.HTTPBody
                        /// Creates a new `_TypePayload`.
                        ///
                        /// - Parameters:
                        ///   - body:
                        public init(body: OpenAPIRuntime.HTTPBody) {
                            self.body = body
                        }
                    }
                    case _Type(OpenAPIRuntime.MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload._TypePayload>)
[...]

Couldn't I just pass any HTTPBody as payload, which is independent of the OpenAPI definition type?

czechboy0 commented 6 months ago

Streaming types, such as raw strings and raw bytes are represented by the HTTPBody type. If you instead had, for example, a JSON object as a multipart part, you'd get a fully typesafe Codable struct generated.

czechboy0 commented 6 months ago

Hey @czechboy0 hope you had a great weekend. I am off testing again. Currently its very complicated to initialise the components in the correct way but I will report if I got more results.

Currently my code looks like this:

        let body = HTTPBody()

        let postIdPayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.PostIdPayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.PostIdPayload(body: body))

        let filePayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.FilePayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload.FilePayload(body: body))

        let typePayload: MultipartPart<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload._TypePayload> = MultipartPart(payload: Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload._TypePayload(body: body))

        let multipartBody: OpenAPIRuntime.MultipartBody<Operations.post_sol_Post_sol_addFile.Input.Body.multipartFormPayload> = .init(
            [.PostId(postIdPayload), .File(filePayload), ._Type(typePayload)])

        let response = try await ClientProvider.sharedClient.post_sol_Post_sol_addFile(body: .multipartForm(multipartBody))

Right, I'd probably write it like this instead:

let response = try await ClientProvider.sharedClient.post_sol_Post_sol_addFile(body: .multipartForm([
    .PostId(.init(payload: .init(body: "my-post-id"))),
    .File(.init(payload: .init(body: "<file contents go here>"), filename: "file.txt")),
    ._Type(.init(payload: .init(body: "my-type"))),
]))