-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
update ktor multiplatform serialization issue #14045
base: master
Are you sure you want to change the base?
update ktor multiplatform serialization issue #14045
Conversation
@Marek00Malik thanks for the PR. Can you please resolve the merge conflicts when you've time? |
@wing328 sorry for the delay, I'm on holiday and didn't notice your message. On the side, I noticed the upgrade of Kotlin to 1.7.21 which is an excellent idea. But I think it could also be a good idea to add this information in the release notes as this requires some action on the consumer side as well. Currently, the website related to the Kotlin client generator is still mentioning Ktor 1.6.8 and there isn't any were any note on what version of Kotlin this generator is compiled in. |
update ktor multiplatform serialization issue
Samples & fixes to missing libraries in Ktor versions
aebf7d5
to
81d809e
Compare
@wing328 I've pushed the changes on top of the latest master (thus the force push). I will raise this as a separate issue and push tests to Ktor implementations and the Jackson/Gson ones. I also found that if we use Unit as a return response, Moshi has problems deserializing this into a Kotlin type. |
Would it be possible to get this merged in? The milestone is changed at each release :( |
@Marek00Malik thanks for the PR. Can you please PM me via Slack to discuss this further? https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g Sorry for the delay in reviewing this as there are too many PRs |
@@ -154,6 +155,7 @@ import {{packageName}}.auth.* | |||
this.method = requestConfig.method.httpMethod | |||
headers.filter { header -> !UNSAFE_HEADERS.contains(header.key) }.forEach { header -> this.header(header.key, header.value) } | |||
if (requestConfig.method in listOf(RequestMethod.PUT, RequestMethod.POST, RequestMethod.PATCH)) { | |||
this.contentType(Application.Json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for not using the following code to support different content types?
val contentType = (requestConfig.headers[HttpHeaders.ContentType]?.let { ContentType.parse(it) }
?: ContentType.Application.Json)
this.contentType(contentType)
This was used before ktor was broken by #12966
update ktor multiplatform serialization issue - #14044
PR checklist
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.
master
(6.3.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)