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

Fix missing inner enum import on operations #79

Merged
Merged
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
28 changes: 28 additions & 0 deletions plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,26 @@ class KotlinGenerator : SharedCodegen() {
public override fun listTypeWrapper(listType: String, innerType: String) =
"$listType<$innerType>"

public override fun listTypeUnwrapper(baseType: String) = baseType
.replace(" ", "")
.removeSuffix(">")
.removePrefix("${typeMapping["list"]}<")

public override fun isListTypeWrapped(baseType: String) =
baseType.endsWith('>') && baseType.startsWith("${typeMapping["list"]}<")

@VisibleForTesting
public override fun mapTypeWrapper(mapType: String, innerType: String) =
"$mapType<${typeMapping["string"]}, $innerType>"

public override fun mapTypeUnwrapper(baseType: String) = baseType
.replace(" ", "")
.removeSuffix(">")
.removePrefix("${typeMapping["map"]}<${typeMapping["string"]},")

public override fun isMapTypeWrapped(baseType: String) =
baseType.endsWith('>') && baseType.startsWith("${typeMapping["map"]}<${typeMapping["string"]},")

@VisibleForTesting
public override fun nullableTypeWrapper(baseType: String) =
baseType.safeSuffix("?")
Expand Down Expand Up @@ -407,6 +423,7 @@ class KotlinGenerator : SharedCodegen() {
}

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

codegenOperation.vendorExtensions[X_OPERATION_ID] = operation?.operationId
getHeadersToIgnore().forEach { headerName ->
ignoreHeaderParameter(headerName, codegenOperation)
Expand All @@ -417,6 +434,17 @@ class KotlinGenerator : SharedCodegen() {
codegenOperation.path = codegenOperation.path.removePrefix("/")
}
processTopLevelHeaders(codegenOperation)

// Let's make sure we import all the types, also the inner ones (see #76).
codegenOperation.responses.forEach {
if (it.dataType != null) {
val innerType = resolveInnerType(it.dataType)
if (innerType !in codegenOperation.imports) {
codegenOperation.imports.add(innerType)
}
}
}

return codegenOperation
}

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

/**
* Resolve the inner type from a complex type. E.g:
* List<List<Int>> ---> Int
* Map<String, Map<String, List<Object>>> ---> Object
*/
internal tailrec fun resolveInnerType(
baseType: String
): String {
if (isListTypeWrapped(baseType)) {
return resolveInnerType(listTypeUnwrapper(baseType))
}
if (isMapTypeWrapped(baseType)) {
return resolveInnerType(mapTypeUnwrapper(baseType))
}

return baseType
}

/**
* Determine if the swagger operation consumes mutipart content.
*/
Expand Down Expand Up @@ -468,6 +486,21 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
*/
protected abstract fun listTypeWrapper(listType: String, innerType: String): String

/**
* Abstract function to unwrap a JSON Array type.
* @param baseType A JSON list type (e.g. `List<String>`)
* @return The unwrapped inner type (e.g. `String`).
* @see isListTypeWrapped
*/
protected abstract fun listTypeUnwrapper(baseType: String): String

/**
* Abstract function to check if a type is a JSON Array type.
* @param baseType A JSON type
* @return True if the type is a JSON Array type and can be safely unwrapped with [listTypeUnwrapper]
*/
protected abstract fun isListTypeWrapped(baseType: String): Boolean

/**
* Abstract function to create a type for a JSON Object (A Map from String to values).
* Please note that in JSON Maps have only Strings as keys.
Expand All @@ -478,6 +511,21 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
*/
protected abstract fun mapTypeWrapper(mapType: String, innerType: String): String

/**
* Abstract function to unwrap a JSON Map type.
* @param baseType A JSON map type (e.g. `Map<String, Item>`)
* @return The unwrapped inner type (e.g. `Item`).
* @see isMapTypeWrapped
*/
protected abstract fun mapTypeUnwrapper(baseType: String): String

/**
* Abstract function to check if a type is a JSON Map type.
* @param baseType A JSON type
* @return True if the type is a JSON Map type and can be safely unwrapped with [mapTypeUnwrapper]
*/
protected abstract fun isMapTypeWrapped(baseType: String): Boolean

/**
* Abstract function to create a type from a Nullable type.
* @param baseType A type (e.g. `Integer`).
Expand Down
27 changes: 1 addition & 26 deletions plugin/src/test/java/com/yelp/codegen/KotlinGeneratorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,6 @@ class KotlinGeneratorTest {
assertEquals("com.yelp.test.apis", generator.apiPackage())
}

@Test
fun listTypeWrapper_withSimpleType() {
assertEquals("List<String>", KotlinGenerator().listTypeWrapper("List", "String"))
}

@Test
fun listTypeWrapper_withMultipleNesting() {
assertEquals("List<List<String>>", KotlinGenerator().listTypeWrapper("List", "List<String>"))
}

@Test
fun mapTypeWrapper_withSimpleType() {
assertEquals("Map<String, Any>", KotlinGenerator().mapTypeWrapper("Map", "Any"))
}

@Test
fun mapTypeWrapper_withMultipleNesting() {
assertEquals("HashMap<String, List<Any?>?>", KotlinGenerator().mapTypeWrapper("HashMap", "List<Any?>?"))
}

@Test
fun nullableTypeWrapper_withSimpleType() {
assertEquals("String?", KotlinGenerator().nullableTypeWrapper("String"))
}

@Test
fun escapeUnsafeCharacters_withNothingToEscape() {
assertEquals("Nothing", KotlinGenerator().escapeUnsafeCharacters("Nothing"))
Expand Down Expand Up @@ -154,7 +129,7 @@ class KotlinGeneratorTest {
val generator = KotlinGenerator()
generator.additionalProperties()[GROUP_ID] = "com.yelp"
generator.additionalProperties()[ARTIFACT_ID] = "test"
val sep:String = File.separator
val sep: String = File.separator
assertTrue(generator.modelFileFolder().endsWith("com${sep}yelp${sep}test${sep}models"))
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.yelp.codegen

import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test

class KotlinGenerator_TypeMappersTest {

@Test
fun listTypeWrapper_withSimpleType() {
assertEquals("List<String>", KotlinGenerator().listTypeWrapper("List", "String"))
}

@Test
fun listTypeWrapper_withMultipleNesting() {
assertEquals("List<List<String>>", KotlinGenerator().listTypeWrapper("List", "List<String>"))
}

@Test
fun listTypeUnwrapper_withSimpleType() {
assertEquals("String", KotlinGenerator().listTypeUnwrapper("String"))
}

@Test
fun listTypeUnwrapper_withListType() {
assertEquals("String", KotlinGenerator().listTypeUnwrapper("List<String>"))
}

@Test
fun isListTypeWrapped_withSimpleType() {
assertFalse(KotlinGenerator().isListTypeWrapped("String"))
}

@Test
fun isListTypeWrapped_withListType() {
assertTrue(KotlinGenerator().isListTypeWrapped("List<String>"))
}

@Test
fun isListTypeWrapped_withMapType() {
assertFalse(KotlinGenerator().isListTypeWrapped("Map<String, Int>"))
}

@Test
fun mapTypeWrapper_withSimpleType() {
assertEquals("Map<String, Any>", KotlinGenerator().mapTypeWrapper("Map", "Any"))
}

@Test
fun mapTypeWrapper_withMultipleNesting() {
assertEquals("HashMap<String, List<Any?>?>", KotlinGenerator().mapTypeWrapper("HashMap", "List<Any?>?"))
}

@Test
fun mapTypeUnwrapper_withSimpleType() {
assertEquals("String", KotlinGenerator().mapTypeUnwrapper("String"))
}

@Test
fun mapTypeUnwrapper_withMapType() {
assertEquals("Int", KotlinGenerator().mapTypeUnwrapper("Map<String, Int>"))
}

@Test
fun isMapTypeWrapped_withSimpleType() {
assertFalse(KotlinGenerator().isMapTypeWrapped("String"))
}

@Test
fun isMapTypeWrapped_withListType() {
assertFalse(KotlinGenerator().isMapTypeWrapped("List<String>"))
}

@Test
fun isMapTypeWrapped_withMapType() {
assertTrue(KotlinGenerator().isMapTypeWrapped("Map<String, Int>"))
}

@Test
fun nullableTypeWrapper_withSimpleType() {
assertEquals("String?", KotlinGenerator().nullableTypeWrapper("String"))
}

@Test
fun resolveInnerType_withSimpleType() {
assertEquals("String", KotlinGenerator().resolveInnerType("String"))
}

@Test
fun resolveInnerType_withListType() {
assertEquals("String", KotlinGenerator().resolveInnerType("List<String>"))
}

@Test
fun resolveInnerType_withMapType() {
assertEquals("Int", KotlinGenerator().resolveInnerType("Map<String, Int>"))
}

@Test
fun resolveInnerType_withComplexType() {
assertEquals("Int", KotlinGenerator().resolveInnerType("Map<String, Map<String, List<Map<String, Int>>>>"))
}
}
22 changes: 22 additions & 0 deletions samples/junit-tests/junit_tests_specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,28 @@
]
}
},
"/top_level_enum/nested": {
"get": {
"operationId": "get_top_level_enum_nested",
"responses": {
"200": {
"description": "",
"schema": {
"additionalProperties": {
"additionalProperties": {
"$ref": "#/definitions/top_level_enum"
},
"type": "object"
},
"type": "object"
}
}
},
"tags": [
"resource"
]
}
},
"/top_level_map/{size}": {
"get": {
"operationId": "get_top_level_map",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ interface ResourceApi {
@GET("/top_level_enum")
fun getTopLevelEnum(): Single<TopLevelEnum>

/**
* The endpoint is owned by junittests service owner
*/
@Headers(
"X-Operation-ID: get_top_level_enum_nested"
)
@GET("/top_level_enum/nested")
fun getTopLevelEnumNested(): Single<Map<String, Map<String, TopLevelEnum>>>

/**
* The endpoint is owned by junittests service owner
* @param size (required)
Expand Down
23 changes: 23 additions & 0 deletions samples/junit-tests/src/main/java/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,29 @@
]
}
},
"/top_level_enum/nested": {
"get": {
"operationId": "get_top_level_enum_nested",
"parameters": [],
"responses": {
"200": {
"description": "",
"schema": {
"additionalProperties": {
"additionalProperties": {
"$ref": "#/definitions/top_level_enum"
},
"type": "object"
},
"type": "object"
}
}
},
"tags": [
"resource"
]
}
},
"/top_level_map/{size}": {
"get": {
"operationId": "get_top_level_map",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,18 @@ class TopLevelEnumEndpointTest {
val returned = rule.getApi<ResourceApi>().getTopLevelEnum().blockingGet()
assertEquals(TopLevelEnum.VALUE1, returned)
}

@Test
fun topLevelEnumNestedEndpoint() {
rule.server.enqueue(MockResponse().setBody("""
{
"key1": {
"key2": "TOP_LEVEL_VALUE1"
}
}
""".trimIndent()))

val returned = rule.getApi<ResourceApi>().getTopLevelEnumNested().blockingGet()
assertEquals(TopLevelEnum.VALUE1, returned["key1"]?.get("key2"))
}
}