Skip to content

[kotlin] Recognize more json media types in response#10176

Closed
sheepdreamofandroids wants to merge 2 commits intoOpenAPITools:masterfrom
sheepdreamofandroids:kotlin-client/json-variations
Closed

[kotlin] Recognize more json media types in response#10176
sheepdreamofandroids wants to merge 2 commits intoOpenAPITools:masterfrom
sheepdreamofandroids:kotlin-client/json-variations

Conversation

@sheepdreamofandroids
Copy link
Copy Markdown
Contributor

Instead of matching the mediatype against "application/json" it now matches with "application/*json" so vendor specific json mediatypes like "application/vnd.shopping.cart.v2+json" will also be recognized and deserialized. It doesn't map mediatypes to different models.
This would accept malformed mediatypes as well but that doesn't seem harmful.
I'm using a similarly patched template in a project.

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 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    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*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.
    @jimschubert
    @dr4ke616
    @karismann
    @Zomzog
    @andrewemery
    @4brunu
    @yutaka0m

@sheepdreamofandroids sheepdreamofandroids changed the title Recognize more json media types in response [kotlin] Recognize more json media types in response Aug 18, 2021
@sheepdreamofandroids
Copy link
Copy Markdown
Contributor Author

I don't think the failure in AppVeyor is related to my changes.

@4brunu
Copy link
Copy Markdown
Contributor

4brunu commented Aug 19, 2021

Hi, thanks for opening this PR.
The CI failure is not related to this PR.
I have no experience vendor specific json media types, so it's hard for me to review it.
Looking at the code it makes sense, and I understand the reasoning behind it.
Did you tested this with any vendor specific json media type?

@sheepdreamofandroids
Copy link
Copy Markdown
Contributor Author

Hi @4brunu . My only experience with vendor specific mediatypes is an in-house api using one.

There is some documentation here: https://www.rfc-editor.org/rfc/rfc6838.html#section-4.2.8 which suggests that the used condition should work. Alternatively maybe even only checking for a "/json" or "+json" suffix would be enough. But I prefer another PR if that is ever necessary.

This PR is actually a subset of the changes we made to the templates that are running in production here. The other changes are about the same change for the request body and sending requests fully asynchronous. Those will be separate PR's.

@sheepdreamofandroids
Copy link
Copy Markdown
Contributor Author

I see that I made a mistake when copying some of the changes.
I'll create a new PR that corrects those.

@sheepdreamofandroids sheepdreamofandroids deleted the kotlin-client/json-variations branch August 23, 2021 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants