Skip to content

Commit

Permalink
Merge pull request #79 from cortinico/fix-76-missing-inner-enum-import
Browse files Browse the repository at this point in the history
Fix missing inner enum import on operations
  • Loading branch information
macisamuele committed Jan 2, 2020
2 parents 01fbfbf + 54b94dd commit b1fbc34
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 26 deletions.
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"))
}
}

0 comments on commit b1fbc34

Please sign in to comment.