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

Add Polymorphism support #144

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ open class KotlinGenerator : SharedCodegen() {
outputFolder = "generated-code${File.separator}android-kotlin-client"
modelTemplateFiles["model.mustache"] = ".kt"
apiTemplateFiles["retrofit2/api.mustache"] = ".kt"

supportsInheritance = true
}

/*
Expand Down Expand Up @@ -125,6 +127,8 @@ open class KotlinGenerator : SharedCodegen() {
"CollectionFormats.kt",
"EnumToValueConverterFactory.kt",
"GeneratedCodeConverters.kt",
"OptimisticHashCode.kt",
"PolymorphicAdapterFactory.kt",
"TypesAdapters.kt",
"WrapperConverterFactory.kt",
"XNullable.kt",
Expand Down Expand Up @@ -202,14 +206,13 @@ open class KotlinGenerator : SharedCodegen() {
return codegenModel
}

@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")
}

if (!codegenModel.isAlias) {
if (!codegenModel.isAlias || codegenModel.parent != null) {
// If we are rendering a model (or enum) we are annotating it with @JsonClass,
// so we need to make sure that we're importing it
codegenModel.imports.add("com.squareup.moshi.JsonClass")
Expand All @@ -222,34 +225,11 @@ open class KotlinGenerator : SharedCodegen() {
break
}
}
}

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
if (supportsInheritance && codegenModel.discriminator != null) {
// Import JsonClass annotation to enable Adapters generations via Kotlin Annotation Processor
codegenModel.imports.add("$toolsPackage.optimisticHashCode")
codegenModel.imports.add("$toolsPackage.Polymorphic")
}
}

Expand Down
198 changes: 194 additions & 4 deletions gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
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
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
Expand Down Expand Up @@ -80,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()
Expand Down Expand Up @@ -259,6 +262,27 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
}

handleXNullable(codegenModel)

// Update all enum properties datatypeWithEnum to use "BaseClass.InnerEnumClass" to reduce ambiguity
CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties ->
properties.filter { it.isEnum }
.onEach { it.datatypeWithEnum = postProcessDataTypeWithEnum(codegenModel, it) }
}

// Fix presence of duplicated variables into CodegenModel instances.
// Apparently swagger-codegen does not really play well with such properties if
// inheritance is enable and models have `allOf` attribute.
CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties ->
val seenProperties = mutableSetOf<CodegenProperty>()
val propertiesIterator = properties.iterator()
while (propertiesIterator.hasNext()) {
val property = propertiesIterator.next()
if (!seenProperties.add(property)) {
propertiesIterator.remove()
}
}
}

return codegenModel
}

Expand All @@ -283,6 +307,116 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
}
}

/**
* Codegen does use the same [CodegenProperty] in the child models, is the property is inherited
*
* This method allows the creation of clones of the [CodegenProperty] instances in case they are
* present due to inheritance.
*
* This additional computation is needed to allow further customisation of the child model
* property definitions without parent model property modifications.
* One example of use-case is the update of isInherited attribute of [CodegenProperty]
*/
private fun decoupleParentProperties(codegenModel: CodegenModel) {
val nameToParentClone = codegenModel.parentVars.map {
it.name to it.clone()
}.toMap()
CodegenModelVar.forEachVarAttribute(codegenModel) { type, properties ->
if (type != CodegenModelVar.PARENT_VARS) {
properties.forEachIndexed { index, property ->
nameToParentClone[property.name]?.let {
properties[index] = it
}
}
}
}
}

override fun postProcessAllModels(objs: Map<String, Any>): Map<String, Any> {
val postProcessedModels = super.postProcessAllModels(objs)

// postProcessedModel does contain a map like Map<ModelName, ContentOfTheModelAsPassedToMustache>
// ContentOfTheModelAsPassedToMustache would look like the following
// {
// <model_name>: {
// <codegen constants>
// "models": [
// {
// "importPath": <String instance>,
// "model": <CodegenModel instance>
// }
// ]
// }
// }
postProcessedModels.values
.asSequence()
.filterIsInstance<Map<String, Any>>()
.mapNotNull { it["models"] }
.filterIsInstance<List<Map<String, Any>>>()
.mapNotNull { it[0]["model"] }
.filterIsInstance<CodegenModel>()
.forEach { codegenModel ->
if (
supportsInheritance &&
// Having a single child usually means override of description or attributes that do not change the model
// So we should NOT mark the model with hasChildren
(codegenModel.children?.size ?: 0) > 1
) {

// Codegen does update the children attribute but does not keep hasChildren consistent
codegenModel.hasChildren = true
}

if (codegenModel.parentModel == null) {
codegenModel.allVars?.forEach { it.isInherited = false }
}

if (supportsInheritance && codegenModel.parentModel != null) {
val parentPropertyNames = codegenModel.parentModel.allVars.map { it.name }.toSet()

// Update parentVars (as codegen does not do it while updating the parents)
codegenModel.parentVars?.addAll(
codegenModel.parentModel.allVars.map {
val clonedProperty = it.clone()
clonedProperty.isInherited = true
clonedProperty
}
)

decoupleParentProperties(codegenModel)

// Update all the *Vars attributes to keep track of parent-inherited parameters
CodegenModelVar.forEachVarAttribute(codegenModel) { type, properties ->
if (type != CodegenModelVar.PARENT_VARS) {
properties.forEach {
it.isInherited = it.name in parentPropertyNames
}
}
}
// Objects with a single item in allOf are recognized as aliases, as this is usually done
// to override the content of metadata (ie. description).
// The tool does already help avoid a new type creation by creating a type alias
// In the case of Polymorphism this is actually a problem as the user would expect an
// instance of a different class, even if the content is the same of the parent class
if (codegenModel.isAlias) {
codegenModel.isAlias = false
codegenModel.hasVars = codegenModel.allVars.isNotEmpty()
addRequiredImports(codegenModel)
}

// codegenModel.hasEnums will be set to true even if the current model does not specify
// an enum value but the parent does.
// In order to avoid to consider this model as a model with enums we're re-evaluating it.
codegenModel.hasEnums = codegenModel.vars.any { it.isEnum }
}

// 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.
Expand Down Expand Up @@ -368,7 +502,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()
}

/**
Expand Down Expand Up @@ -412,7 +546,7 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
* or `items` at the top level (Arrays).
* Their returned type would be a `Map<String, Any?>` or `List<Any?>`, 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).
*
* ```
Expand Down Expand Up @@ -562,6 +696,13 @@ 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]
*/
@VisibleForTesting
internal abstract fun addRequiredImports(codegenModel: CodegenModel)

private fun defaultListType() = typeMapping["list"] ?: ""

private fun defaultMapType() = typeMapping["map"] ?: ""
Expand All @@ -584,4 +725,53 @@ 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
}
}
}

/**
* Small helper to ensurer that the hasMore attribute is properly
* defined within a list of [CodegenProperty]s
*/
internal fun List<CodegenProperty>.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<CodegenParameter>.fixHasMoreParameter() {
this.forEachIndexed { index, item ->
item.hasMore = index != (this.size - 1)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.yelp.codegen.utils

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<CodegenProperty>) -> Unit
) {
values().forEach {
action(it, it.value(codegenModel))
}
}
}

internal fun value(codegenModel: CodegenModel): MutableList<CodegenProperty> {
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
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**{{#description}}
* {{{description}}}{{/description}}
{{#vars}}
{{#allVars}}
* @property {{{name}}}{{#description}} {{description}}{{/description}}
{{/vars}}
{{/allVars}}
*/
@JsonClass(generateAdapter = true)
{{#hasVars}}data {{/hasVars}}class {{classname}}{{#hasVars}}(
Expand All @@ -11,7 +11,7 @@
{{/-last}}{{/requiredVars}}{{#hasRequired}}{{#hasOptional}},
{{/hasOptional}}{{/hasRequired}}{{#optionalVars}}{{>data_class_opt_var}}{{^-last}},
{{/-last}}{{/optionalVars}}
){{#hasEnums}} {
){{#parent}} : {{parent}}({{#parentVars}}{{name}} = {{name}}{{#hasMore}}, {{/hasMore}}{{/parentVars}}){{/parent}}{{#hasEnums}} {
{{#vars}}
{{#isEnum}}
/**{{#description}}
Expand Down
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}}{{#isInherited}}override {{/isInherited}}{{#hasChildren}}open {{/hasChildren}}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}}}") {{#isInherited}}override {{/isInherited}}{{#hasChildren}}open {{/hasChildren}}var {{{name}}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{datatype}}}{{/isEnum}}
Loading