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

[Kotlin] fix #20231, OkHttp client can handle a field with a list of files #20274

Merged
merged 8 commits into from
Dec 9, 2024

Conversation

jorgeferdz
Copy link
Contributor

This PR is basically the same one as created a few days ago (#20232) but it removes the following line inside one of the new addPartToMultiPart methods:

if (obj == null) return

This way, the behavior when adding null values is still the same.

issue: Kotlin OkHttp client does not process correctly when a field in the endpoint is defined as an array of lists like this:

...
requestBody:
  content:
    multipart/form-data:
      schema:
        type: object
        properties:
          attachments:
            type: array
            description: attached files
            items:
              type: string
              format: binary
...

If the autogenerated client is used, the server will receive always 1 item in the list but it is not a file or anything and it is trying to serialize like a normal object.

I noticed this issue while developing my own project, so I modified this code (by looking at the Java counterpart that can be seen here) and tested it by building the project in my local machine and adding the plugin to my project manually.

I tried to add some testing (I added the faulty specification) but I was not really sure how to do it. Also by reading the documentation, it didn't find it very clearly what I should do. Any guidance regarding how to add test for that would be very much appreciated.

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.

@wing328 I created the tests and I will add them in a separate PR so it doesn't becomes too big.

@wing328 wing328 added this to the 7.11.0 milestone Dec 9, 2024
@wing328 wing328 merged commit a447b59 into OpenAPITools:master Dec 9, 2024
60 checks passed
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