Skip to content
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

Nicer Multipart Payloads? #720

Open
MahdiBM opened this issue Jan 27, 2025 · 7 comments
Open

Nicer Multipart Payloads? #720

MahdiBM opened this issue Jan 27, 2025 · 7 comments
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.

Comments

@MahdiBM
Copy link
Contributor

MahdiBM commented Jan 27, 2025

Description

We currently have codes like this:

    func createThing(
        _ input: OpenAPI.Operations.createThing.Input
    ) async throws -> OpenAPI.Operations.createThing.Output {
        guard case let .multipartForm(payload) = input.body else {
            throw Abort(.badRequest, reason: "Missing multipart form data.")
        }

        var title: String?
        var subtitle: String?
        var kind: Kind?

        for try await part in payload {
            switch part {
            case let .title(payload):
                title = try await payload.payload.body.collectAsString(upTo: 1024)
            case let .subtitle(payload):
                subtitle = try await payload.payload.body.collectAsString(upTo: 1024)
            case let .kind(payload):
                let kindString = try await payload.payload.body.collectAsString(upTo: 1024)
                kind = Kind(rawValue: kindString)
            default: break 
        }
        
        guard let title else {
            throw Abort(.badRequest, reason: "Title is required")
        }
        guard let kind else {
            throw Abort(.badRequest, reason: "Kind is required")
        }

        let thing = Thing(
            title: title,
            subtitle: subtitle,
            kind: kind
        )
        ...
}

The code ... looks pretty redundant / bad ...
What can we do to avoid this kind of redundant code?
I can see how it can be hard to handle with multipart streaming APIs, but it's still pretty suboptimal ...

Reproduction

Package version(s)

generator 1.7.0
runtime 1.8.0

Expected behavior

To not need to declare variables like title 4 different times just to initialize an struct with.

Environment

6.0.3 RELEASE

Additional information

No response

@MahdiBM MahdiBM added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Jan 27, 2025
@czechboy0
Copy link
Contributor

Can you share more of your OpenAPI document? I'm curious why you're using multipart for short strings.

For example, for uploading files, I'd expect one part for the raw data, and one part for a JSON "metadata" schema, where you can add new keys easily.

I'd like to learn more about your API design approach here.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jan 27, 2025

Can try to share the openapi later, not at my desk right now.

We're using multipart since we're sending images as well. What i sent above is not the whole multipart payload. The actual payload has 35 fields which as you can imagine only makes things worse.

@czechboy0
Copy link
Contributor

How many of the 35 fields are raw bytes and how many are structured (JSON-encodable) data?

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jan 27, 2025

We have 2-3 images at most.

I see your suggestions though. Discord also does the same thing with multipart payloads and allows you to send a single "json" multipart field, followed by image files. This is a good recommendation and we should move to doing that tbh.

It still won't be pretty to work with multipart payloads. maybe we'll have at most (for now) 4 fields, but that'll still look bad since you'll need to do the dance with writing to an optional and unwrapping etc...

I can only think of a macro to generate the boilerplate code ... not sure what else openapi-generator could do.

@czechboy0
Copy link
Contributor

If you're buffering everything, you can use the Array(collecting...) to get all the parts in an array. Then you can access the parts even more easily, without the for loop.

Try to consolidate the JSON values and if you still have a suggestion of code you'd like to get generated, let's talk.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jan 27, 2025

Not buffering everything and trying to properly leverage async sequences, so that suggestion won't work 🙂

@czechboy0
Copy link
Contributor

Ok, that's better. Then I think once you consolidate the JSON fields, you'll be using the multipart API as-designed.

Suggestions welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

2 participants