From 630fbf22bfcb55f884d143ad94858f6b9752c3ae Mon Sep 17 00:00:00 2001 From: 0marperez <60363173+0marperez@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:17:56 -0600 Subject: [PATCH] fix: paginator generator nullability bug caused by documents (#1155) --- .../f71f083b-6e5f-4a3c-9069-464b1f9f6d36.json | 5 + .../codegen/core/KotlinSymbolProvider.kt | 2 +- .../codegen/rendering/PaginatorGenerator.kt | 20 +- .../rendering/PaginatorGeneratorTest.kt | 457 ++++++++++++++++++ 4 files changed, 475 insertions(+), 9 deletions(-) create mode 100644 .changes/f71f083b-6e5f-4a3c-9069-464b1f9f6d36.json diff --git a/.changes/f71f083b-6e5f-4a3c-9069-464b1f9f6d36.json b/.changes/f71f083b-6e5f-4a3c-9069-464b1f9f6d36.json new file mode 100644 index 000000000..0055ce0ac --- /dev/null +++ b/.changes/f71f083b-6e5f-4a3c-9069-464b1f9f6d36.json @@ -0,0 +1,5 @@ +{ + "id": "f71f083b-6e5f-4a3c-9069-464b1f9f6d36", + "type": "bugfix", + "description": "Fix paginator generator `List<*>` & `Map.Entry` nullability" +} diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt index 1f7eb9408..e6dbd0dca 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt @@ -166,7 +166,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli val fullyQualifiedKeyType = keyReference.fullName val valueReference = toSymbol(shape.value) - val valueSuffix = if (valueReference.isNullable) "?" else "" + val valueSuffix = if (valueReference.isNullable || shape.isSparse) "?" else "" val valueType = "${valueReference.name}$valueSuffix" val fullyQualifiedValueType = "${valueReference.fullName}$valueSuffix" diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGenerator.kt index d96274492..7ac76fee7 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGenerator.kt @@ -272,18 +272,22 @@ private fun getItemDescriptorOrNull(paginationInfo: PaginationInfo, ctx: Codegen val itemMember = ctx.model.expectShape(itemMemberId) val isSparse = itemMember.isSparse val (collectionLiteral, targetMember) = when (itemMember) { - is MapShape -> - ctx.symbolProvider.toSymbol(itemMember) - .expectProperty(SymbolProperty.ENTRY_EXPRESSION) as String to itemMember - is CollectionShape -> - ctx.symbolProvider.toSymbol(ctx.model.expectShape(itemMember.member.target)).name to ctx.model.expectShape( - itemMember.member.target, - ) + is MapShape -> { + val symbol = ctx.symbolProvider.toSymbol(itemMember) + val entryExpression = symbol.expectProperty(SymbolProperty.ENTRY_EXPRESSION) as String + entryExpression to itemMember + } + is CollectionShape -> { + val target = ctx.model.expectShape(itemMember.member.target) + val symbol = ctx.symbolProvider.toSymbol(target) + val literal = symbol.name + if (symbol.isNullable || isSparse) "?" else "" + literal to target + } else -> error("Unexpected shape type ${itemMember.type}") } return ItemDescriptor( - collectionLiteral + if (isSparse) "?" else "", + collectionLiteral, targetMember, itemLiteral, itemPathLiteral, diff --git a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGeneratorTest.kt b/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGeneratorTest.kt index 1c69df382..7b08a6d86 100644 --- a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGeneratorTest.kt +++ b/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/PaginatorGeneratorTest.kt @@ -762,4 +762,461 @@ class PaginatorGeneratorTest { actual.shouldContainOnlyOnceWithDiff(expected) } + + @Test + fun testRenderPaginatorWithDocumentList() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationList, + NextMarker: String + } + + list FunctionConfigurationList { + member: FunctionConfiguration + } + + document FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfiguration") + public fun Flow.functions(): Flow = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } + + @Test + fun testRenderPaginatorWithSparseDocumentList() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationList, + NextMarker: String + } + + @sparse + list FunctionConfigurationList { + member: FunctionConfiguration + } + + document FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfiguration") + public fun Flow.functions(): Flow = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } + + @Test + fun testRenderPaginatorWithDocumentMap() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationMap, + NextMarker: String + } + + map FunctionConfigurationMap { + key: String + value: FunctionConfiguration + } + + document FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfigurationMap") + public fun Flow.functions(): Flow> = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } + + @Test + fun testRenderPaginatorWithSparseDocumentMap() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationMap, + NextMarker: String + } + + @sparse + map FunctionConfigurationMap { + key: String + value: FunctionConfiguration + } + + document FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfigurationMap") + public fun Flow.functions(): Flow> = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } + + @Test + fun testRenderPaginatorWithSparseStringMap() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationMap, + NextMarker: String + } + + @sparse + map FunctionConfigurationMap { + key: String + value: FunctionConfiguration + } + + string FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfigurationMap") + public fun Flow.functions(): Flow> = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } + + @Test + fun testRenderPaginatorWithStringMap() { + val testModelWithItems = """ + namespace com.test + + use aws.protocols#restJson1 + + service Lambda { + operations: [ListFunctions] + } + + @paginated( + inputToken: "Marker", + outputToken: "NextMarker", + pageSize: "MaxItems", + items: "Functions" + ) + @readonly + @http(method: "GET", uri: "/functions", code: 200) + operation ListFunctions { + input: ListFunctionsRequest, + output: ListFunctionsResponse + } + + structure ListFunctionsRequest { + @httpQuery("FunctionVersion") + FunctionVersion: String, + @httpQuery("Marker") + Marker: String, + @httpQuery("MasterRegion") + MasterRegion: String, + @httpQuery("MaxItems") + MaxItems: Integer + } + + structure ListFunctionsResponse { + Functions: FunctionConfigurationMap, + NextMarker: String + } + + map FunctionConfigurationMap { + key: String + value: FunctionConfiguration + } + + string FunctionConfiguration + """.toSmithyModel() + val testContextWithItems = testModelWithItems.newTestContext("Lambda", "com.test") + + val codegenContextWithItems = object : CodegenContext { + override val model: Model = testContextWithItems.generationCtx.model + override val symbolProvider: SymbolProvider = testContextWithItems.generationCtx.symbolProvider + override val settings: KotlinSettings = testContextWithItems.generationCtx.settings + override val protocolGenerator: ProtocolGenerator = testContextWithItems.generator + override val integrations: List = testContextWithItems.generationCtx.integrations + } + + val unit = PaginatorGenerator() + unit.writeAdditionalFiles(codegenContextWithItems, testContextWithItems.generationCtx.delegator) + + testContextWithItems.generationCtx.delegator.flushWriters() + val testManifest = testContextWithItems.generationCtx.delegator.fileManifest as MockManifest + val actual = testManifest.expectFileString("src/main/kotlin/com/test/paginators/Paginators.kt") + + val expectedCode = """ + @JvmName("listFunctionsResponseFunctionConfigurationMap") + public fun Flow.functions(): Flow> = + transform() { response -> + response.functions?.forEach { + emit(it) + } + } + """.trimIndent() + actual.shouldContainOnlyOnceWithDiff(expectedCode) + } }