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][Client] new library support: Retrofit 2.x #4518

Merged
merged 30 commits into from
Dec 15, 2019

Conversation

javils
Copy link
Contributor

@javils javils commented Nov 17, 2019

#4517

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

PR Description

This PR add support for retrofit2 as a HTTP library in kotlin-client generator. The reason to add this feature is that I want to port some old android java code to kotlin, but I couldn't because retrofit2 was not supported.

@jimschubert @dr4ke616 @karismann @Zomzog @andrewemery

@auto-labeler
Copy link

auto-labeler bot commented Nov 17, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

# Conflicts:
#	bin/kotlin-client-all.sh
#	modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinClientCodegen.java
@wing328 wing328 added this to the 4.2.2 milestone Nov 19, 2019
@@ -8,3 +8,4 @@
./bin/kotlin-client-string.sh
./bin/kotlin-client-threetenbp.sh
./bin/kotlin-client-nullable.sh
./bin/kotlin-client-retrofti2.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: kotlin-client-retrofti2.sh => kotlin-client-retrofit2.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing!
It's solved in this commit: a3b34b5

@wing328
Copy link
Member

wing328 commented Nov 22, 2019

When using --library retrofit2, the following files are missing:

README.md
build.gradle
settings.gradle

Can we also auto-generate these files similar to what we've done for other library options in the kotlin generator?

# Conflicts:
#	modules/openapi-generator/src/main/resources/kotlin-client/data_class_opt_var.mustache
#	modules/openapi-generator/src/main/resources/kotlin-client/data_class_req_var.mustache
@javils
Copy link
Contributor Author

javils commented Nov 30, 2019

When using --library retrofit2, the following files are missing:

README.md
build.gradle
settings.gradle

Can we also auto-generate these files similar to what we've done for other library options in the kotlin generator?

Thanks!
Solved in this commit: f37bfbb

@javils javils requested a review from wing328 November 30, 2019 17:41
@wing328 wing328 modified the milestones: 4.2.2, 4.2.3 Dec 2, 2019
@4brunu
Copy link
Contributor

4brunu commented Dec 2, 2019

@javils can you please run on the terminal sh bin/utils/ensure-up-to-date --batch and commit the file docs/generators/kotlin.md to make the CI pass?
Overall the PR looks ok, but it's a bit scary to merge this, because it changes so many things.


java ${JAVA_OPTS} -jar ${executable} ${ags}

cp CI/samples.ci/client/petstore/kotlin-threetenbp/pom.xml samples/client/petstore/kotlin-threetenbp/pom.xml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javils this like is not related to this sample, can you please remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing!

Fixed here: 6e4566f

@javils
Copy link
Contributor Author

javils commented Dec 3, 2019

@javils can you please run on the terminal sh bin/utils/ensure-up-to-date --batch and commit the file docs/generators/kotlin.md to make the CI pass?
Overall the PR looks ok, but it's a bit scary to merge this, because it changes so many things.

When I run this command I got a lot of modified files and not update the kotlin.md file. Any idea?

@javils
Copy link
Contributor Author

javils commented Dec 5, 2019

Thanks!
For the next PR I know the trick.

@javils
Copy link
Contributor Author

javils commented Dec 9, 2019

@wing328 @jimschubert @dr4ke616 @karismann @Zomzog @andrewemery
Can we merge this? Someone need to check something else?

@wing328
Copy link
Member

wing328 commented Dec 9, 2019

I did a test with the auto-generated Kotlin code (retrofit2) for Petstore but got the following errors:

Starting a Gradle Daemon (subsequent builds will be faster)

> Task :compileKotlin
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (18, 51): Unresolved reference: CSVParams
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (20, 6): No value passed for parameter 'message'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (22, 45): Unresolved reference: CSVParams
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (36, 130): Unresolved reference: MultipartBody
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (14, 22): Parameter name expected
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (14, 79): Expecting comma or ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 5): Expecting ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 6): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 9): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 10): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 11): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 33): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 34): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (29, 20): Parameter name expected
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (29, 32): Expecting comma or ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 5): Expecting ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 6): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 9): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 10): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 11): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 27): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 28): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/infrastructure/ApiClient.kt: (20, 30): Unresolved reference: MoshiConverterFactory

> Task :compileKotlin FAILED

FAILURE: Build failed with an exception.

Can you please take a look?

@javils
Copy link
Contributor Author

javils commented Dec 10, 2019

I did a test with the auto-generated Kotlin code (retrofit2) for Petstore but got the following errors:

Starting a Gradle Daemon (subsequent builds will be faster)

> Task :compileKotlin
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (18, 51): Unresolved reference: CSVParams
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (20, 6): No value passed for parameter 'message'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (22, 45): Unresolved reference: CSVParams
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (36, 130): Unresolved reference: MultipartBody
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (14, 22): Parameter name expected
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (14, 79): Expecting comma or ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 5): Expecting ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 6): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 9): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 10): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 11): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 33): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/StoreApi.kt: (16, 34): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (29, 20): Parameter name expected
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (29, 32): Expecting comma or ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 5): Expecting ')'
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 6): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 9): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 10): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 11): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 27): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/apis/UserApi.kt: (31, 28): Expecting member declaration
e: /private/tmp/re/src/main/kotlin/org/openapitools/client/infrastructure/ApiClient.kt: (20, 30): Unresolved reference: MoshiConverterFactory

> Task :compileKotlin FAILED

FAILURE: Build failed with an exception.

Can you please take a look?

Thanks for your feedback!
I've done some commits to solve the issues :-)

@4brunu
Copy link
Contributor

4brunu commented Dec 11, 2019

@javils Since in your last commit you made a change in a mustache, you also need to update the pet projects, otherwise the CI will fail, because the pet projects are outdated.

@javils
Copy link
Contributor Author

javils commented Dec 11, 2019

@4brunu done!

@javils
Copy link
Contributor Author

javils commented Dec 12, 2019

The build failed 2 times because of this:

An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: http://deb.opera.com/opera stable InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY

@wing328
Copy link
Member

wing328 commented Dec 12, 2019

@javils the shippable failure has been fixed in the master. Can you please merge the latest master into this branch to retrigger the build?

@wing328
Copy link
Member

wing328 commented Dec 13, 2019

I still got errors when testing the Petstore samples (kotlin retrofit2):

> Task :compileKotlin
e: /Users/williamcheng/Code/openapi-generator/samples/client/petstore/kotlin-retrofit2/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (20, 6): No value passed for parameter 'message'
e: /Users/williamcheng/Code/openapi-generator/samples/client/petstore/kotlin-retrofit2/src/main/kotlin/org/openapitools/client/apis/PetApi.kt: (36, 130): Unresolved reference: MultipartBody

Does it work for you locally?

@4brunu
Copy link
Contributor

4brunu commented Dec 14, 2019

I have tested this locally and I think now it's working.
Can anyone confirm? 🙂

@wing328
Copy link
Member

wing328 commented Dec 15, 2019

Tested locally and the result is good. Thanks for the PR.

@wing328 wing328 merged commit bb4af91 into OpenAPITools:master Dec 15, 2019
@wing328 wing328 changed the title Feature/kotlin/library/retrofit [Kotlin][Client] new library support: Retrofit 2.x Dec 15, 2019
@wing328
Copy link
Member

wing328 commented Dec 22, 2019

UPDATE: the library option has been renamed from retrofit2 to jvm-retrofit2

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.

3 participants