From 2ffc7d620789a0eca72083e68e8ea44cac1918d2 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Sun, 26 Jan 2020 18:05:42 +0100 Subject: [PATCH 1/6] Move addRequiredImports into SharedCodegen --- .../src/main/java/com/yelp/codegen/KotlinGenerator.kt | 2 +- .../plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt index 0e3e7901..0850c031 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt @@ -203,7 +203,7 @@ open class KotlinGenerator : SharedCodegen() { } @VisibleForTesting - internal fun addRequiredImports(codegenModel: CodegenModel) { + override fun addRequiredImports(codegenModel: CodegenModel) { // If there are any vars, we will mark them with the @Json annotation so we have to make sure to import it if (codegenModel.allVars.isNotEmpty() || codegenModel.isEnum) { codegenModel.imports.add("com.squareup.moshi.Json") diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt index d717ad23..c77c19d5 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt @@ -562,6 +562,12 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { */ protected abstract fun nullableTypeWrapper(baseType: String): String + /** + * Hook that allows to add the needed imports for a given [CodegenModel] + * This is needed as we might be modifying models in [postProcessAllModels] + */ + internal abstract fun addRequiredImports(codegenModel: CodegenModel) + private fun defaultListType() = typeMapping["list"] ?: "" private fun defaultMapType() = typeMapping["map"] ?: "" From 398dc23b45506b5a9109b8cc55ed2d7d9b1c645c Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Sat, 25 Jan 2020 19:39:38 +0100 Subject: [PATCH 2/6] Define helper to iterate over all the var attributes of a CodegenModel instance --- .../com/yelp/codegen/utils/CodegenModelVar.kt | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt new file mode 100644 index 00000000..96154d74 --- /dev/null +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt @@ -0,0 +1,38 @@ +import io.swagger.codegen.CodegenModel +import io.swagger.codegen.CodegenProperty + +internal enum class CodegenModelVar { + ALL_VARS, + OPTIONAL_VARS, + PARENT_VARS, + READ_ONLY_VARS, + READ_WRITE_VARS, + REQUIRED_VARS, + VARS; + + companion object { + /** + * Allow the execution of an action on all the var attributes + */ + fun forEachVarAttribute( + codegenModel: CodegenModel, + action: (CodegenModelVar, MutableList) -> Unit + ) { + values().forEach { + action(it, it.value(codegenModel)) + } + } + } + + internal fun value(codegenModel: CodegenModel): MutableList { + return when (this) { + ALL_VARS -> codegenModel.allVars + OPTIONAL_VARS -> codegenModel.optionalVars + PARENT_VARS -> codegenModel.parentVars + READ_ONLY_VARS -> codegenModel.readOnlyVars + READ_WRITE_VARS -> codegenModel.readWriteVars + REQUIRED_VARS -> codegenModel.requiredVars + VARS -> codegenModel.vars + } + } +} From 7a2369f9f8966ec4afc90a931e548b88872fad82 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Sun, 26 Jan 2020 17:15:09 +0100 Subject: [PATCH 3/6] Move postProcessDataTypeWithEnum in SharedCodegen and apply it to all CodegenModel vars attributes --- .../java/com/yelp/codegen/KotlinGenerator.kt | 29 -------------- .../java/com/yelp/codegen/SharedCodegen.kt | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt index 0850c031..91d06d22 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt @@ -224,35 +224,6 @@ open class KotlinGenerator : SharedCodegen() { } } - override fun postProcessModelProperty(model: CodegenModel, property: CodegenProperty) { - super.postProcessModelProperty(model, property) - - if (property.isEnum) { - property.datatypeWithEnum = postProcessDataTypeWithEnum(model.classname, property) - } - } - - /** - * When handling inner enums, we want to prefix their class name, when using them, with their containing class, - * to avoid name conflicts. - */ - private fun postProcessDataTypeWithEnum(modelClassName: String, codegenProperty: CodegenProperty): String { - val name = "$modelClassName.${codegenProperty.enumName}" - - val baseType = if (codegenProperty.isContainer) { - val type = checkNotNull(typeMapping[codegenProperty.containerType]) - "$type<$name>" - } else { - name - } - - return if (codegenProperty.isNullable()) { - nullableTypeWrapper(baseType) - } else { - baseType - } - } - /** * Returns the swagger type for the property * diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt index c77c19d5..e7384bd4 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt @@ -259,6 +259,16 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { } handleXNullable(codegenModel) + + // Update all enum properties datatypeWithEnum to use "BaseClass.InnerEnumClass" to reduce ambiguity + CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties -> + properties.forEach { + if (it.isEnum) { + it.datatypeWithEnum = this.postProcessDataTypeWithEnum(codegenModel, it) + } + } + } + return codegenModel } @@ -590,4 +600,33 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { * Nullable type are either not required or x-nullable annotated properties. */ internal fun CodegenProperty.isNullable() = !this.required || this.vendorExtensions[X_NULLABLE] == true + + override fun postProcessModelProperty(model: CodegenModel, property: CodegenProperty) { + super.postProcessModelProperty(model, property) + + if (property.isEnum) { + property.datatypeWithEnum = this.postProcessDataTypeWithEnum(model, property) + } + } + + /** + * When handling inner enums, we want to prefix their class name, when using them, with their containing class, + * to avoid name conflicts. + */ + private fun postProcessDataTypeWithEnum(codegenModel: CodegenModel, codegenProperty: CodegenProperty): String { + val name = "${codegenModel.classname}.${codegenProperty.enumName}" + + val baseType = if (codegenProperty.isContainer) { + val type = checkNotNull(typeMapping[codegenProperty.containerType]) + "$type<$name>" + } else { + name + } + + return if (codegenProperty.isNullable()) { + nullableTypeWrapper(baseType) + } else { + baseType + } + } } From 4a7a4274b497bf795c970f004b17aa4b401793bb Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Tue, 4 Feb 2020 21:36:53 +0100 Subject: [PATCH 4/6] Ensure correctness of hasMore attribute of CodegenParameter and CodegenProperty --- .../java/com/yelp/codegen/SharedCodegen.kt | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt index e7384bd4..a2eda750 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt @@ -4,6 +4,7 @@ import com.yelp.codegen.utils.safeSuffix import io.swagger.codegen.CodegenConfig import io.swagger.codegen.CodegenModel import io.swagger.codegen.CodegenOperation +import io.swagger.codegen.CodegenParameter import io.swagger.codegen.CodegenProperty import io.swagger.codegen.CodegenType import io.swagger.codegen.DefaultCodegen @@ -293,6 +294,37 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { } } + override fun postProcessAllModels(objs: Map): Map { + val postProcessedModels = super.postProcessAllModels(objs) + + // postProcessedModel does contain a map like Map + // ContentOfTheModelAsPassedToMustache would look like the following + // { + // : { + // + // "models": [ + // { + // "importPath": , + // "model": + // } + // ] + // } + // } + postProcessedModels.values + .asSequence() + .filterIsInstance>() + .mapNotNull { it["models"] } + .filterIsInstance>>() + .mapNotNull { it[0]["model"] } + .filterIsInstance() + .forEach { codegenModel -> + // Ensure that after all the processing done on the CodegenModel.*Vars, hasMore does still make sense + CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties -> properties.fixHasMoreProperty() } + } + + return postProcessedModels + } + /** * Private method to investigate all the properties of a models, filter only the [RefProperty] and eventually * propagate the `x-nullable` vendor extension. @@ -378,7 +410,7 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { } // If we removed the last parameter of the Operation, we should update the `hasMore` flag. - codegenOperation.allParams.lastOrNull()?.hasMore = false + codegenOperation.allParams.fixHasMoreParameter() } /** @@ -630,3 +662,23 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { } } } + +/** + * Small helper to ensurer that the hasMore attribute is properly + * defined within a list of [CodegenProperty]s + */ +internal fun List.fixHasMoreProperty() { + this.forEachIndexed { index, item -> + item.hasMore = index != (this.size - 1) + } +} + +/** + * Small helper to ensurer that the hasMore attribute is properly + * defined within a list of [CodegenParameter]s + */ +internal fun List.fixHasMoreParameter() { + this.forEachIndexed { index, item -> + item.hasMore = index != (this.size - 1) + } +} From bde67fba9cb059f4054cda0109dfce2c7fdea0d5 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Tue, 4 Feb 2020 21:39:58 +0100 Subject: [PATCH 5/6] Ensure that non-null resources (because if Java) are actually checked before usage (to limit kotlin warnings) --- .../plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt index a2eda750..b73e0d52 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt @@ -82,7 +82,7 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { * Returns the /main/resources directory to access the .mustache files */ protected val resourcesDirectory: File - get() = File(this.javaClass.classLoader.getResource(templateDir).path.safeSuffix(File.separator)) + get() = File(this.javaClass.classLoader.getResource(templateDir)!!.path.safeSuffix(File.separator)) override fun processOpts() { super.processOpts() @@ -454,7 +454,7 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { * or `items` at the top level (Arrays). * Their returned type would be a `Map` or `List`, where `Any?` will be the aliased type. * - * The method will call [KotlinAndroidGenerator.resolvePropertyType] that will perform a check if the model + * The method will call [KotlinGenerator.resolvePropertyType] that will perform a check if the model * is aliasing to a 'x-nullable' annotated model and compute the proper type (adding a `?` if needed). * * ``` From cdd09a38d2dce00a2b1f7e5c35b614584aae4c97 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Wed, 28 Oct 2020 22:20:45 +0000 Subject: [PATCH 6/6] PR Feedbacks --- .../main/java/com/yelp/codegen/KotlinGenerator.kt | 1 - .../main/java/com/yelp/codegen/SharedCodegen.kt | 14 +++++++------- .../java/com/yelp/codegen/utils/CodegenModelVar.kt | 2 ++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt index 91d06d22..bbd5fd4b 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt @@ -202,7 +202,6 @@ open class KotlinGenerator : SharedCodegen() { return codegenModel } - @VisibleForTesting override fun addRequiredImports(codegenModel: CodegenModel) { // If there are any vars, we will mark them with the @Json annotation so we have to make sure to import it if (codegenModel.allVars.isNotEmpty() || codegenModel.isEnum) { diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt index b73e0d52..3cc6a116 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt @@ -1,5 +1,7 @@ package com.yelp.codegen +import com.google.common.annotations.VisibleForTesting +import com.yelp.codegen.utils.CodegenModelVar import com.yelp.codegen.utils.safeSuffix import io.swagger.codegen.CodegenConfig import io.swagger.codegen.CodegenModel @@ -81,8 +83,8 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { /** * Returns the /main/resources directory to access the .mustache files */ - protected val resourcesDirectory: File - get() = File(this.javaClass.classLoader.getResource(templateDir)!!.path.safeSuffix(File.separator)) + protected val resourcesDirectory: File? + get() = javaClass.classLoader.getResource(templateDir)?.path?.safeSuffix(File.separator)?.let { File(it) } override fun processOpts() { super.processOpts() @@ -263,11 +265,8 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { // Update all enum properties datatypeWithEnum to use "BaseClass.InnerEnumClass" to reduce ambiguity CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties -> - properties.forEach { - if (it.isEnum) { - it.datatypeWithEnum = this.postProcessDataTypeWithEnum(codegenModel, it) - } - } + properties.filter { it.isEnum } + .onEach { it.datatypeWithEnum = postProcessDataTypeWithEnum(codegenModel, it) } } return codegenModel @@ -608,6 +607,7 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig { * Hook that allows to add the needed imports for a given [CodegenModel] * This is needed as we might be modifying models in [postProcessAllModels] */ + @VisibleForTesting internal abstract fun addRequiredImports(codegenModel: CodegenModel) private fun defaultListType() = typeMapping["list"] ?: "" diff --git a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt index 96154d74..9a50796b 100644 --- a/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt +++ b/gradle-plugin/plugin/src/main/java/com/yelp/codegen/utils/CodegenModelVar.kt @@ -1,3 +1,5 @@ +package com.yelp.codegen.utils + import io.swagger.codegen.CodegenModel import io.swagger.codegen.CodegenProperty