You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
AbstractKotlinCodegen.java has some issues related to date types.
The first one is that in the typeMapping it defines Date as java.time.LocalDate which is correct.
But then in importMapping it defines Date as java.time.LocalDate which is incorrect and doesn't match with the typeMapping
It should define Date as java.time.LocalDate in the typeMapping.
The second one is that it defines DateTime as java.time.LocalDateTime in the typeMapping and in the importMapping, which is wrong, it should be java.time.OffsetDateTime.
This can cause bugs because the servers and the clients can (and most probably are) in different timezones.
This is already fixed in KotlinSpringServerCodegen and KotlinClientCodegen but not in AbstractKotlinCodegen.
Finally since java.util.Date is never used, it's not necessary in the Kotlin Client Serializer, so it's removed.
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.
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.1.x, 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.
This PR has lots of changes, because I generate the samples server and openapi3, which were really outdated, because they are "hidden" in the directories bin/configs/other/openapi3/kotlin* and bin/configs/other/kotlin*.
@wing328 Maybe to avoid this problem of the samples server and openapi3 getting outdated, they should be moved to bin/configs/*?
I was going to ask about the DateTime default from java.time.LocalDateTime to java.time.OffsetDateTime, but then I thought about it more and it's really the only type that fully supports the spec's date-time requirement to match rfc3339.
We'll need to consider this a bug fix in that case. Folks may need to pass type-mappings to override for backward compatibility with previously generated code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AbstractKotlinCodegen.javahas some issues related to date types.The first one is that in the
typeMappingit definesDateasjava.time.LocalDatewhich is correct.But then in
importMappingit definesDateasjava.time.LocalDatewhich is incorrect and doesn't match with thetypeMappingIt should define
Dateasjava.time.LocalDatein thetypeMapping.The second one is that it defines
DateTimeasjava.time.LocalDateTimein thetypeMappingand in theimportMapping, which is wrong, it should bejava.time.OffsetDateTime.This can cause bugs because the servers and the clients can (and most probably are) in different timezones.
This is already fixed in
KotlinSpringServerCodegenandKotlinClientCodegenbut not inAbstractKotlinCodegen.Finally since
java.util.Dateis never used, it's not necessary in the Kotlin ClientSerializer, so it's removed.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,5.1.x,6.0.x@jimschubert (2017/09) ❤️, @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03)