Skip to content

[kotlin] [multiplatform] [jvm-ktor] Fix formdata file upload #21056

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kalinjul
Copy link
Contributor

@kalinjul kalinjul commented Apr 8, 2025

The currently generated code for multipart/form-data does not compile for binary parameters.
See #9423, #19644 and #18094. It also fails when there's a list of files to be uploaded.

This PR includes 2 changes:

  1. KotlinClientCodegen was changed to respect container types (to fix lists of files).
  2. KotlinClientCodegen will now set the dataType to Ktor's FormPart instead of InputProvider
  3. The api.mustache was changed accordingly.

No. 2 has some implications:

  • The parameter name must be passed by the caller (When creating a FormPart like FormPart("document", inputProvider)
  • The headers are still not set correctly, e.g. mime type and filename have to be set by the caller.

This may not be the most convenient solution to upload files, but it should suit every application as it leaves FormPart creation to the caller.
I think this would be a good improvement to the current state.

Sample Yaml:

openapi: 3.0.1
info:
  title: Test API
  version: '1'
paths:
  /some/path/method/:
    post:
      requestBody:
        content:
          multipart/form-data:
            schema:
              type: object
              properties:
                singledocument:
                  type: "string"
                  format: "binary"
                documents:
                  type: "array"
                  items:
                    type: "string"
                    format: "binary"
                isAccepted:
                  type: "boolean"
      responses:
        204:
          description: 'Processed.'

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03) @stefankoppier (2022/06) @e5l (2024/10)

@kalinjul kalinjul changed the title [kotlin-multiplatform] Fix formdata file upload [kotlin] [multiplatform] [jvm-ktor] Fix formdata file upload Apr 9, 2025
@wing328
Copy link
Member

wing328 commented Apr 9, 2025

thanks for the PR

using the spec provided, I did a test with

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g kotlin -i /tmp/kotlin-form.yaml -o /tmp/kotlin-form/ --library multiplatform --additional-properties dateLibrary=kotlinx-datetime

but i don't see any change in the code (only changes in the doc - DefaultApi.md)

am I testing your change with the right configurations/options?

@kalinjul
Copy link
Contributor Author

kalinjul commented Apr 9, 2025

That seems strange, i get the following diff vs. the generated code using master:

--- out_master/src/commonMain/kotlin/org/openapitools/client/apis/DefaultApi.kt 2025-04-09 10:37:15
+++ out/src/commonMain/kotlin/org/openapitools/client/apis/DefaultApi.kt        2025-04-09 10:33:53
@@ -49,15 +49,15 @@
      * @param isAccepted  (optional)
      * @return void
      */
-    open suspend fun somePathMethodPost(singledocument: io.ktor.client.request.forms.InputProvider? = null, documents: io.ktor.client.request.forms.InputProvider? = null, isAccepted: kotlin.Boolean? = null): HttpResponse<Unit> {
+    open suspend fun somePathMethodPost(singledocument: io.ktor.client.request.forms.FormPart<io.ktor.client.request.forms.InputProvider>? = null, documents: kotlin.collections.List<io.ktor.client.request.forms.FormPart<io.ktor.client.request.forms.InputProvider>>? = null, isAccepted: kotlin.Boolean? = null): HttpResponse<Unit> {
 
         val localVariableAuthNames = listOf<String>()
 
         val localVariableBody = 
             formData {
-                singledocument?.apply { append("singledocument", singledocument) }
+                singledocument?.apply { append(singledocument) }
                 documents?.onEach {
-                    append("documents[]", it)
+                    append(it)
                 }
                 isAccepted?.apply { append("isAccepted", isAccepted) }
             }

Used the same command with the yaml from this PR

@kalinjul
Copy link
Contributor Author

kalinjul commented Apr 9, 2025

@wing328 : could you restart the ci, i solved the merge conflicts and somehow the ci has an error now..

@wing328 wing328 closed this Apr 13, 2025
@wing328 wing328 reopened this Apr 13, 2025
@wing328
Copy link
Member

wing328 commented Apr 13, 2025

restarted (by closing and reopening the PR)

@wing328 wing328 closed this Apr 23, 2025
@wing328 wing328 reopened this Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants