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

Update mustache style #90

Merged
merged 9 commits into from
Feb 5, 2020
3 changes: 2 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ repos:
- id: trailing-whitespace
exclude: .*\.mustache$
- id: end-of-file-fixer
exclude: .*\.mustache$
exclude: (.*\.mustache|samples/[^/]+/src/main/java/swagger.json)$
- id: pretty-format-json
args: [--autofix, --indent, '4']
exclude: ^samples/[^/]+/src/main/java/swagger.json$
- id: check-yaml
- id: check-executables-have-shebangs
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
Expand Down
15 changes: 7 additions & 8 deletions plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -402,14 +402,20 @@ open class KotlinGenerator : SharedCodegen() {
): CodegenOperation {
val codegenOperation = super.fromOperation(path, httpMethod, operation, definitions, swagger)

retrofitImport.get(codegenOperation.httpMethod)?.let { codegenOperation.imports.add(it) }
retrofitImport[codegenOperation.httpMethod]?.let { codegenOperation.imports.add(it) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a big fan of this syntax [...]?.whatever when the returned type is optional. I generally prefer having a chain of functions. Was this suggested by the IDE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was on the "learn kotlin the Jetbrains way" 🤣
I just accepted the IDE suggestion.

codegenOperation.allParams.forEach { codegenParameter: CodegenParameter ->
codegenParameter.collectionFormat?.let {
val importName = "$toolsPackage.${it.toUpperCase()}"
if (importName !in codegenOperation.imports) {
codegenOperation.imports.add(importName)
}
}

if (codegenParameter.isFile) {
codegenOperation.imports.add("okhttp3.RequestBody")
// The generated Retrofit APIs use RequestBody and not File objects
codegenOperation.imports.remove("File")
}
Comment on lines +414 to +418
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that you're wrapping the "okhttp3.RequestBody" here as otherwise you'll have a bunch of unused imports. On the other hand you're adding a new endpoint to junit-test that is not covered by a test. I'd suggest you add the new endpoint in a separate PR. Ideally we should tackle also the Multipart scenario that I'm pretty sure we don't cover.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd rather keep the already existing endpoint, I'll try to see how expensive would be creating a test for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Creating a realistic test for it seems going a bit beyond the scope of the PR, especially considering that I'm getting the feeling that files are not properly handled (#99 )

Going to offload the effort in a follow-up PR

}

codegenOperation.returnType = when {
Expand All @@ -427,11 +433,6 @@ open class KotlinGenerator : SharedCodegen() {

codegenOperation.imports.add("retrofit2.http.Headers")

codegenOperation.vendorExtensions[X_OPERATION_ID] = operation?.operationId
getHeadersToIgnore().forEach { headerName ->
ignoreHeaderParameter(headerName, codegenOperation)
}

// Let's remove the leading
if (!basePath.isNullOrBlank()) {
codegenOperation.path = codegenOperation.path.removePrefix("/")
Expand Down Expand Up @@ -471,8 +472,6 @@ open class KotlinGenerator : SharedCodegen() {
override fun preprocessSwagger(swagger: Swagger) {
super.preprocessSwagger(swagger)

// Override the swagger version with the one provided from command line.
swagger.info.version = additionalProperties[SPEC_VERSION] as String
this.basePath = swagger.basePath
}

Expand Down
6 changes: 6 additions & 0 deletions plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {

mapXModel(swagger)

// Override the swagger version with the one provided from command line.
swagger.info.version = additionalProperties[SPEC_VERSION] as String

swagger.definitions?.forEach { (name, model) ->
// Ensure that all the models have a title
// The title should give priority to x-model, then title and finally
Expand Down Expand Up @@ -484,6 +487,9 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
): CodegenOperation {
val codegenOperation = super.fromOperation(path, httpMethod, operation, definitions, swagger)
codegenOperation.vendorExtensions[X_OPERATION_ID] = operation?.operationId
getHeadersToIgnore().forEach { headerName ->
ignoreHeaderParameter(headerName, codegenOperation)
}
if (unsafeOperations.contains(operation?.operationId)) {
codegenOperation.vendorExtensions[X_UNSAFE_OPERATION] = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ object KotlinLangUtils {
"array" to "List",
"list" to "List",
"map" to "Map",
"object" to "Map<String,Any?>",
"object" to "Map<String, Any?>",
"binary" to "List<Byte>"
)

Expand All @@ -144,7 +144,7 @@ object KotlinLangUtils {
"List" to "kotlin.collections.List",
"List<Byte>" to "kotlin.collections.List",
"Map" to "kotlin.collections.Map",
"Map<String,Any?>" to "kotlin.collections.Map",
"Map<String, Any?>" to "kotlin.collections.Map",
"Timestamp" to "java.sql.Timestamp",
"UUID" to "java.util.UUID"
)
Expand Down
21 changes: 10 additions & 11 deletions plugin/src/main/resources/kotlin/data_class.mustache
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/**
* {{{description}}}
/**{{#description}}
* {{{description}}}{{/description}}
{{#vars}}
* @property {{{name}}} {{description}}
* @property {{{name}}}{{#description}} {{description}}{{/description}}
{{/vars}}
*/
@JsonClass(generateAdapter = true)
{{#hasVars}}data {{/hasVars}}class {{classname}} {{#hasVars}}(
{{#hasVars}}data {{/hasVars}}class {{classname}}{{#hasVars}}(
{{#requiredVars}}
{{>data_class_req_var}}{{^-last}},
{{/-last}}{{/requiredVars}}{{#hasRequired}}{{#hasOptional}},
Expand All @@ -14,12 +14,11 @@
){{#hasEnums}} {
{{#vars}}
{{#isEnum}}
/**
* {{{description}}}
* Values: {{#allowableValues}}{{#enumVars}}{{&name}}{{^-last}}, {{/-last}}{{/enumVars}}{{/allowableValues}}
/**{{#description}}
* {{{description}}}{{/description}}
* Values:{{#allowableValues}} {{#enumVars}}{{&name}}{{^-last}}, {{/-last}}{{/enumVars}}{{/allowableValues}}
*/
@JsonClass(generateAdapter = false)
enum class {{enumName}}(val value: {{complexType}}){ {{#allowableValues}}{{#enumVars}}
@Json(name = {{{value}}}) {{&name}}({{{value}}}){{^-last}},{{/-last}}{{#-last}}{{/-last}}{{/enumVars}}{{/allowableValues}}
}
{{/isEnum}}{{/vars}}}{{/hasEnums}}{{/hasVars}}
enum class {{enumName}}(val value: {{complexType}}) {
{{#allowableValues}}{{#enumVars}} @Json(name = {{{value}}}) {{&name}}({{{value}}}){{^-last}},{{/-last}}{{{newline}}}{{/enumVars}}{{/allowableValues}} }
{{/isEnum}}{{/vars}}}{{/hasEnums}}{{/hasVars}}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
@Json(name = "{{{baseName}}}") @field:Json(name = "{{{baseName}}}") {{#vendorExtensions.x-nullable}}@XNullable {{/vendorExtensions.x-nullable}}var {{{name}}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{datatype}}}{{/isEnum}} = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}
@Json(name = "{{{baseName}}}") @field:Json(name = "{{{baseName}}}") {{#vendorExtensions.x-nullable}}@XNullable {{/vendorExtensions.x-nullable}}var {{{name}}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{datatype}}}{{/isEnum}} = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
@Json(name = "{{{baseName}}}") @field:Json(name = "{{{baseName}}}") var {{{name}}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{datatype}}}{{/isEnum}}
@Json(name = "{{{baseName}}}") @field:Json(name = "{{{baseName}}}") var {{{name}}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{datatype}}}{{/isEnum}}
9 changes: 4 additions & 5 deletions plugin/src/main/resources/kotlin/enum_class.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/**
* {{{description}}}
/**{{#description}}
* {{{description}}}{{/description}}
* Values: {{#allowableValues}}{{#enumVars}}{{&name}}{{^-last}},{{/-last}}{{/enumVars}}{{/allowableValues}}
*/
@JsonClass(generateAdapter = false)
enum class {{classname}}(val value: {{dataType}}){
{{#allowableValues}}{{#enumVars}} @Json(name = {{{value}}}) {{&name}}({{{value}}}){{^-last}},{{/-last}}
{{/enumVars}}{{/allowableValues}}}
enum class {{classname}}(val value: {{dataType}}) {
{{#allowableValues}}{{#enumVars}} @Json(name = {{{value}}}) {{&name}}({{{value}}}){{^-last}},{{/-last}}{{{newline}}}{{/enumVars}}{{/allowableValues}}}
12 changes: 4 additions & 8 deletions plugin/src/main/resources/kotlin/model.mustache
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
{{>file_header}}
package {{modelPackage}}

{{#imports}}import {{import}}
{{#imports}}{{#-first}}
{{/-first}}import {{import}}
{{/imports}}

{{#models}}
{{#model}}
{{#isAlias}}{{>type_alias}}{{/isAlias}}{{^isAlias}}{{#isEnum}}{{>enum_class}}{{/isEnum}}{{^isEnum}}{{>data_class}}{{/isEnum}}{{/isAlias}}
{{/model}}
{{/models}}
{{#models}}{{#model}}
{{#isAlias}}{{>type_alias}}{{/isAlias}}{{^isAlias}}{{#isEnum}}{{>enum_class}}{{/isEnum}}{{^isEnum}}{{>data_class}}{{/isEnum}}{{/isAlias}}{{/model}}{{/models}}
40 changes: 18 additions & 22 deletions plugin/src/main/resources/kotlin/retrofit2/api.mustache
Original file line number Diff line number Diff line change
@@ -1,47 +1,43 @@
{{>file_header}}
package {{package}}

import okhttp3.RequestBody

{{#imports}}import {{import}}
{{/imports}}

{{#operations}}
@JvmSuppressWildcards
interface {{classname}} {
{{#operation}}
/**{{#summary}} {{{newline}}} * {{{summary}}}{{/summary}}{{#notes}}{{{newline}}} * {{{<br/>}}}{{{notes}}}{{/notes}}
* The endpoint is owned by {{{vendorExtensions.x-team-owners}}}{{^vendorExtensions.x-team-owners}}{{service_name}} service owner{{/vendorExtensions.x-team-owners}}
/**{{#summary}}{{{newline}}} * {{{summary}}}{{/summary}}{{#notes}}{{{newline}}} * {{{<br/>}}}{{{notes}}}{{/notes}}
* The endpoint is owned by {{{vendorExtensions.x-team-owners}}}{{^vendorExtensions.x-team-owners}}{{service_name}} service owner{{/vendorExtensions.x-team-owners}}
{{#allParams}}
* @param {{paramName}} {{description}}{{#required}} (required){{/required}}{{^required}} (optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}
* @param {{paramName}}{{#description}} {{description}}{{/description}} ({{#required}}required{{/required}}{{^required}}optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}{{/required}})
{{/allParams}}
{{#externalDocs}}
* {{description}}
* @see [{{url}}"][{{summary}} Documentation]
* {{description}}
* @see [{{url}}"][{{summary}} Documentation]
{{/externalDocs}}
*/
*/
{{#formParams}}
{{#-first}}
{{#isMultipart}}@retrofit2.http.Multipart{{/isMultipart}}{{^isMultipart}}@retrofit2.http.FormUrlEncoded{{/isMultipart}}
{{#isMultipart}}@retrofit2.http.Multipart{{/isMultipart}}{{^isMultipart}}@retrofit2.http.FormUrlEncoded{{/isMultipart}}
{{/-first}}
{{/formParams}}
{{#vendorExtensions.hasOperationHeaders}}
@Headers(
{{#vendorExtensions.operationHeaders}}"{{first}}: {{second}}"{{^-last}},
{{/-last}}{{#-last}}
){{/-last}}{{/vendorExtensions.operationHeaders}}
{{/vendorExtensions.hasOperationHeaders}}
@{{httpMethod}}("{{{path}}}"){{#vendorExtensions.x-unsafe-operation}}{{#isDeprecated}}
@Deprecated(message = "Deprecated and unsafe to use"){{/isDeprecated}}{{^isDeprecated}}
@Deprecated(message = "Unsafe to use"){{/isDeprecated}}
@Headers(
{{#vendorExtensions.operationHeaders}}"{{first}}: {{second}}"{{^-last}},
{{/-last}}{{#-last}}
){{/-last}}{{/vendorExtensions.operationHeaders}}
{{/vendorExtensions.hasOperationHeaders}}
@{{httpMethod}}("{{{path}}}"){{#vendorExtensions.x-unsafe-operation}}{{#isDeprecated}}
@Deprecated(message = "Deprecated and unsafe to use"){{/isDeprecated}}{{^isDeprecated}}
@Deprecated(message = "Unsafe to use"){{/isDeprecated}}
{{/vendorExtensions.x-unsafe-operation}}{{^vendorExtensions.x-unsafe-operation}}{{#isDeprecated}}
@Deprecated(message = "Deprecated"){{/isDeprecated}}
{{/vendorExtensions.x-unsafe-operation}}
{{vendorExtensions.x-function-qualifiers}} fun {{operationId}}({{^allParams}}){{/allParams}}
{{#allParams}}{{>retrofit2/queryParams}}{{>retrofit2/pathParams}}{{>retrofit2/headerParams}}{{>retrofit2/bodyParams}}{{>retrofit2/formParams}}{{#hasMore}},
{{/hasMore}}{{^hasMore}}
){{/hasMore}}{{/allParams}}: {{{returnType}}}

{{#vendorExtensions.x-function-qualifiers}}{{vendorExtensions.x-function-qualifiers}} {{/vendorExtensions.x-function-qualifiers}}fun {{operationId}}({{#hasParams}}
{{#allParams}} {{>retrofit2/queryParams}}{{>retrofit2/pathParams}}{{>retrofit2/headerParams}}{{>retrofit2/bodyParams}}{{>retrofit2/formParams}}{{#hasMore}},{{{newline}}}{{/hasMore}}{{/allParams}}
{{/hasParams}}): {{{returnType}}}
{{/operation}}
}
{{/operations}}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#isBodyParam}}@retrofit2.http.Body {{paramName}}: {{{dataType}}}{{/isBodyParam}}
{{#isBodyParam}}@retrofit2.http.Body {{paramName}}: {{{dataType}}}{{/isBodyParam}}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#isFormParam}}{{#notFile}}{{#isMultipart}}@retrofit2.http.Part{{/isMultipart}}{{^isMultipart}}@retrofit2.http.Field{{/isMultipart}}("{{baseName}}") {{paramName}}: {{{dataType}}}{{/notFile}}{{#isFile}}{{#isMultipart}}@retrofit2.http.Part{{/isMultipart}}{{^isMultipart}}@retrofit2.http.Field{{/isMultipart}}("{{baseName}}\"; filename=\"{{baseName}}") {{paramName}}: RequestBody{{^required}}?{{/required}} {{/isFile}}{{/isFormParam}}
{{#isFormParam}}{{#notFile}}{{#isMultipart}}@retrofit2.http.Part{{/isMultipart}}{{^isMultipart}}@retrofit2.http.Field{{/isMultipart}}("{{baseName}}") {{paramName}}: {{{dataType}}}{{/notFile}}{{#isFile}}{{#isMultipart}}@retrofit2.http.Part{{/isMultipart}}{{^isMultipart}}@retrofit2.http.Field{{/isMultipart}}("{{baseName}}\"; filename=\"{{baseName}}") {{paramName}}: RequestBody{{^required}}?{{/required}}{{/isFile}}{{/isFormParam}}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package {{packageName}}.tools

import java.lang.reflect.Type
import retrofit2.Converter
import retrofit2.Retrofit
import java.lang.reflect.Type

internal class CollectionFormatConverterFactory : Converter.Factory() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,14 @@ package {{packageName}}.tools
@Retention(AnnotationRetention.RUNTIME)
annotation class CSV


@Target(AnnotationTarget.FIELD, AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
annotation class SSV


@Target(AnnotationTarget.FIELD, AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
annotation class TSV


@Target(AnnotationTarget.FIELD, AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
annotation class PIPES
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package {{packageName}}.tools

import com.squareup.moshi.Json
import java.lang.reflect.Type
import retrofit2.Converter
import retrofit2.Retrofit
import java.lang.reflect.Type

internal class EnumToValueConverterFactory : Converter.Factory() {

Expand All @@ -20,7 +20,7 @@ internal class EnumToValueConverterFactory : Converter.Factory() {
internal class EnumToValueConverter : Converter<Any, String> {
override fun convert(enum: Any): String? {
val enumName = (enum as Enum<*>).name
val jsonAnnotation : Json? = enum.javaClass.getField(enumName).getAnnotation(Json::class.java)
val jsonAnnotation: Json? = enum.javaClass.getField(enumName).getAnnotation(Json::class.java)

// Checking if the Enum is annotated with @Json to get the name.
// If not, fallback to enum default (.toString())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
import retrofit2.Converter
import retrofit2.converter.moshi.MoshiConverterFactory


object GeneratedCodeConverters {
private val moshi = Moshi.Builder()
.add(XNullableAdapterFactory())
Expand Down
14 changes: 7 additions & 7 deletions plugin/src/main/resources/kotlin/tools/TypesAdapters.kt.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import com.squareup.moshi.JsonReader
import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
import com.squareup.moshi.internal.Util
import java.lang.reflect.Type
import java.math.BigDecimal
import org.threeten.bp.DateTimeException
import org.threeten.bp.LocalDate
import org.threeten.bp.ZonedDateTime
import org.threeten.bp.LocalDateTime
import org.threeten.bp.DateTimeException
import org.threeten.bp.ZoneId
import org.threeten.bp.ZonedDateTime
import org.threeten.bp.format.DateTimeFormatter
import java.lang.reflect.Type
import java.math.BigDecimal

/**
* Moshi Factory to handle all the custom types we want to support,
Expand Down Expand Up @@ -42,7 +42,7 @@ class TypesAdapterFactory : JsonAdapter.Factory {
*/
internal abstract class XNullableJsonAdapter<T> : JsonAdapter<T>() {

abstract fun fromNonNullString(nextString: String) : T
abstract fun fromNonNullString(nextString: String): T

override fun fromJson(reader: JsonReader): T? {
return if (reader.peek() != JsonReader.Token.NULL) {
Expand All @@ -57,7 +57,7 @@ internal abstract class XNullableJsonAdapter<T> : JsonAdapter<T>() {
internal class LocalDateAdapter : XNullableJsonAdapter<LocalDate>() {
private val formatter = DateTimeFormatter.ISO_LOCAL_DATE

override fun fromNonNullString(nextString: String) : LocalDate = LocalDate.parse(nextString, formatter)
override fun fromNonNullString(nextString: String): LocalDate = LocalDate.parse(nextString, formatter)

override fun toJson(writer: JsonWriter, value: LocalDate?) {
value?.let { writer.value(it.format(formatter)) }
Expand All @@ -70,7 +70,7 @@ internal class ZonedDateTimeAdapter : XNullableJsonAdapter<ZonedDateTime>() {
override fun fromNonNullString(nextString: String): ZonedDateTime {
return try {
ZonedDateTime.parse(nextString, formatter)
} catch (parseException : DateTimeException) {
} catch (parseException: DateTimeException) {
val localDateTime = LocalDateTime.parse(nextString, formatter)
localDateTime.atZone(ZoneId.of("Z"))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,39 @@
package {{packageName}}.tools

import java.lang.reflect.Type
import okhttp3.RequestBody
import okhttp3.ResponseBody
import retrofit2.Converter
import retrofit2.Retrofit
import java.lang.reflect.Type

internal class WrapperConverterFactory(private vararg val factories: Converter.Factory) : Converter.Factory() {

override fun responseBodyConverter(type: Type,
annotations: Array<Annotation>,
retrofit: Retrofit): Converter<ResponseBody, *>? {
override fun responseBodyConverter(
type: Type,
annotations: Array<Annotation>,
retrofit: Retrofit
): Converter<ResponseBody, *>? {
return factories.mapFirstNonNull {
it.responseBodyConverter(type, annotations, retrofit)
}
}

override fun requestBodyConverter(type: Type,
parameterAnnotations: Array<Annotation>,
methodAnnotations: Array<Annotation>,
retrofit: Retrofit): Converter<*, RequestBody>? {
override fun requestBodyConverter(
type: Type,
parameterAnnotations: Array<Annotation>,
methodAnnotations: Array<Annotation>,
retrofit: Retrofit
): Converter<*, RequestBody>? {
return factories.mapFirstNonNull {
it.requestBodyConverter(type, parameterAnnotations, methodAnnotations, retrofit)
}
}

override fun stringConverter(type: Type,
annotations: Array<Annotation>,
retrofit: Retrofit): Converter<*, String>? {
override fun stringConverter(
type: Type,
annotations: Array<Annotation>,
retrofit: Retrofit
): Converter<*, String>? {
return factories.mapFirstNonNull {
it.stringConverter(type, annotations, retrofit)
}
Expand Down
Loading