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

[Polymorphism Prep] Expose private KotlinGenerator methods in SharedCodegen #104

Open
wants to merge 6 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 @@ -202,8 +202,7 @@ 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")
Expand All @@ -224,35 +223,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
*
Expand Down
105 changes: 101 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,13 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
}

handleXNullable(codegenModel)

// Update all enum properties datatypeWithEnum to use "BaseClass.InnerEnumClass" to reduce ambiguity
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 having a hard time understanding the use case of this.

CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties ->
properties.filter { it.isEnum }
.onEach { it.datatypeWithEnum = postProcessDataTypeWithEnum(codegenModel, it) }
}

return codegenModel
}

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

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 ->
// 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 +409,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 +453,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 +603,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 +632,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

Comment on lines +3 to +5
Copy link
Collaborator

Choose a reason for hiding this comment

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

This class is probably not needed at all and the same behavior can be achieved with an extension function:

fun CodegenModel.forEachVarAttribute(
        action: (MutableList<CodegenProperty>) -> Unit
) {
    listOf<MutableList<CodegenProperty>>(
            this.allVars,
            this.optionalVars,
            this.parentVars,
            this.readOnlyVars,
            this.readWriteVars,
            this.requiredVars,
            this.vars).forEach { action(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.

The enums and stuff are mostly needed to ensure that we can have information on which list of variables we're working on.
This is something needed in the usages that will be proposed on the subsequent PR

internal enum class CodegenModelVar {
macisamuele marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the CodegenModelVar here is unused. Can you please remove 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.

I'd rather prefer having it as it will be used in the subsequent branch :)

) {
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
}
}
}