[Kotlin][client] Support gson and moshi as serialization libraries #3734
Conversation
Summary of changes:
- Added a new 'serializationEngine' option for config.json with 2 valid values: "moshi" (default) and "gson"
- updated to kotlin-client mustache templates to support gson's @SerializedName annotation - this also helps with proguard which can break JSON binding to object in release code
- removed empty {} in the data class body when there is no enums - this fixes annoying visual indicators of empty body in Android Studio IDE when looking at data classes without enums.
Testing:
# Test 1
mvn clean install
./bin/kotlin-client-petstore.sh
cd samples/client/petstore/kotlin
gradle wrapper; ./gradlew check assemble test
success
# Test 2
./bin/kotlin-client-petstore.sh --additional-properties serializationEngine=gson
cd samples/client/petstore/kotlin
gradle wrapper; ./gradlew check assemble test
success
# Test 3
./bin/kotlin-client-petstore.sh --additional-properties serializationEngine=foobar
verified proper error message is displayed:
Exception in thread "main" java.lang.RuntimeException: foobar is an invalid enum property naming option. Please choose from:
moshi
gson
at org.openapitools.codegen.languages.AbstractKotlinCodegen.setSerializationEngine(AbstractKotlinCodegen.java:286)
at org.openapitools.codegen.languages.AbstractKotlinCodegen.processOpts(AbstractKotlinCodegen.java:360)
at org.openapitools.codegen.languages.KotlinClientCodegen.processOpts(KotlinClientCodegen.java:122)
at org.openapitools.codegen.DefaultGenerator.configureGeneratorProperties(DefaultGenerator.java:194)
at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:910)
at org.openapitools.codegen.cmd.Generate.run(Generate.java:400)
at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:60)
parachutemind
changed the title
…ngine to serializationLibrary Fixed the data class mustache template to not add an extra line between the @entity and the variable name Regenerated petstore samples
|
@wing328 any ideas what is going on with |
|
in #3759 I have added an new option called @jimschubert any opinion? |
|
@jmini I already changed my code to include serializationLibrary based on your PR (wing328 brought the suggestion up). See my last commit 38a8dc2. |
wing328
changed the title
wing328
changed the title
Let's deal with this issue separately |
...penapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java
Outdated
Show resolved
Hide resolved
| @@ -202,6 +202,10 @@ | |||
| public static final String ENUM_PROPERTY_NAMING = "enumPropertyNaming"; | |||
| public static final String ENUM_PROPERTY_NAMING_DESC = "Naming convention for enum properties: 'camelCase', 'PascalCase', 'snake_case', 'UPPERCASE', and 'original'"; | |||
|
|
|||
| public static final String SERIALIZATION_LIBRARY = "serializationLibrary"; | |||
There was a problem hiding this comment.
This is great, once merged I will remove org.openapitools.codegen.languages.JavaClientCodegen.SERIALIZATION_LIBRARY and use that instead.
| @@ -202,6 +202,10 @@ | |||
| public static final String ENUM_PROPERTY_NAMING = "enumPropertyNaming"; | |||
| public static final String ENUM_PROPERTY_NAMING_DESC = "Naming convention for enum properties: 'camelCase', 'PascalCase', 'snake_case', 'UPPERCASE', and 'original'"; | |||
|
|
|||
| public static final String SERIALIZATION_LIBRARY = "serializationLibrary"; | |||
| public static final String SERIALIZATION_LIBRARY_DESC = "What serialization library to use: 'moshi' (default), or 'gson'"; | |||
There was a problem hiding this comment.
This description is Kotlin specific. Can you move it to Kotlin Codegen?
For instance for java client the choice will be between gson and jackson
There was a problem hiding this comment.
total brain fart on my part. I got lost in the forest. Commit bfb8701 tries to address this request.
We didn't enforce this (from day one) so we can find a lot of generator specified options/constants in CodegenConstants. I agree we should enforce this by first cleaning it up in the future. |
…degen. I picked this layer instead of KotlinClient to make it available to the server in the future. In this commit, I've also updated kotlin.md to document serializationLibrary kotlin options. Testing: re-run mvn clean install and mvn verify
|
@wing328 can you re-run ci/circleci and travis-ci checks. I think something got crossed between my merge commit and the actual change. |
|
You can close and reopen PR to rerun all tests. |
|
@prisoneroftech thanks for the PR, which has been included in the v4.1.2 release: https://twitter.com/oas_generator/status/1172068944355528705 |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/kotlin-client-petstore.sh,./bin/openapi3/kotlin-client-petstore.shif 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.master,4.1.x,5.0.x. Default:master.@jimschubert
@dr4ke616
@karismann
@Zomzog
Description of the PR
Summary of changes:
Testing:
Test 1
mvn clean install
./bin/kotlin-client-petstore.sh
cd samples/client/petstore/kotlin
gradle wrapper; ./gradlew check assemble test
success
Test 2
./bin/kotlin-client-petstore.sh --additional-properties serializationEngine=gson
cd samples/client/petstore/kotlin
gradle wrapper; ./gradlew check assemble test
success
Test 3
./bin/kotlin-client-petstore.sh --additional-properties serializationEngine=foobar
verified proper error message is displayed:
Test 4
./bin/openapi3/kotlin-client-petstore.sh
cd samples/openapi3/client/petstore/kotlin/
gradle wrapper; ./gradlew check assemble test
Unfortunately, I run into the following error when running the tests above against openapi3. I don't believe this is due to my changes. It seems to be broken in master as well...
Fixes #3414