Skip to content

Commit

Permalink
fix: correctly codegen paginators for types which require fully-quali…
Browse files Browse the repository at this point in the history
…fied names (#1249)
  • Loading branch information
ianbotsf authored Feb 27, 2025
1 parent 462967b commit 72155f7
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changes/13a47747-197a-4b88-bc3b-393af5f5127a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "13a47747-197a-4b88-bc3b-393af5f5127a",
"type": "bugfix",
"description": "Correctly generate paginators for item type names which collide with other used types (e.g., an item type `com.foo.Flow` which conflicts with `kotlinx.coroutines.flow.Flow`)"
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@ import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolReference
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.core.CodegenContext
import software.amazon.smithy.kotlin.codegen.core.ExternalTypes
import software.amazon.smithy.kotlin.codegen.core.KotlinDelegator
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.core.defaultName
import software.amazon.smithy.kotlin.codegen.core.withBlock
import software.amazon.smithy.kotlin.codegen.core.*
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
import software.amazon.smithy.kotlin.codegen.model.*
Expand Down Expand Up @@ -54,7 +49,7 @@ class PaginatorGenerator : KotlinIntegration {
paginatedOperations.forEach { paginatedOperation ->
val paginationInfo = paginatedIndex.getPaginationInfo(service, paginatedOperation).getOrNull()
?: throw CodegenException("Unexpectedly unable to get PaginationInfo from $service $paginatedOperation")
val paginationItemInfo = getItemDescriptorOrNull(paginationInfo, ctx)
val paginationItemInfo = getItemDescriptorOrNull(paginationInfo, ctx, writer)

renderPaginatorForOperation(ctx, writer, paginatedOperation, paginationInfo, paginationItemInfo)
}
Expand Down Expand Up @@ -264,7 +259,11 @@ private data class ItemDescriptor(
/**
* Return an [ItemDescriptor] if model supplies, otherwise null
*/
private fun getItemDescriptorOrNull(paginationInfo: PaginationInfo, ctx: CodegenContext): ItemDescriptor? {
private fun getItemDescriptorOrNull(
paginationInfo: PaginationInfo,
ctx: CodegenContext,
writer: KotlinWriter,
): ItemDescriptor? {
val itemMemberId = paginationInfo.itemsMemberPath?.lastOrNull()?.target ?: return null

val itemLiteral = paginationInfo.itemsMemberPath!!.last()!!.defaultName()
Expand All @@ -273,15 +272,18 @@ private fun getItemDescriptorOrNull(paginationInfo: PaginationInfo, ctx: Codegen
val isSparse = itemMember.isSparse
val (collectionLiteral, targetMember) = when (itemMember) {
is MapShape -> {
val symbol = ctx.symbolProvider.toSymbol(itemMember)
val entryExpression = symbol.expectProperty(SymbolProperty.ENTRY_EXPRESSION) as String
entryExpression to itemMember
val keySymbol = ctx.symbolProvider.toSymbol(itemMember.key)
val valueSymbol = ctx.symbolProvider.toSymbol(itemMember.value)
val valueSuffix = if (isSparse || valueSymbol.isNullable) "?" else ""
val elementExpression = writer.format("Map.Entry<#T, #T#L>", keySymbol, valueSymbol, valueSuffix)
elementExpression 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
val suffix = if (isSparse || symbol.isNullable) "?" else ""
val elementExpression = writer.format("#T#L", symbol, suffix)
elementExpression to target
}
else -> error("Unexpected shape type ${itemMember.type}")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,130 @@ class PaginatorGeneratorTest {
actual.shouldContainOnlyOnceWithDiff(expectedImports)
}

@Test
fun testRenderPaginatorWithItemRequiringFullName() {
val testModelWithItems = """
namespace com.test
use aws.protocols#restJson1
service FlowService {
operations: [ListFlows]
}
@paginated(
inputToken: "Marker",
outputToken: "NextMarker",
pageSize: "MaxItems",
items: "Flows"
)
@readonly
@http(method: "GET", uri: "/flows", code: 200)
operation ListFlows {
input: ListFlowsRequest,
output: ListFlowsResponse
}
structure ListFlowsRequest {
@httpQuery("FlowVersion")
FlowVersion: String,
@httpQuery("Marker")
Marker: String,
@httpQuery("MasterRegion")
MasterRegion: String,
@httpQuery("MaxItems")
MaxItems: Integer
}
structure ListFlowsResponse {
Flows: FlowList,
NextMarker: String
}
list FlowList {
member: Flow
}
structure Flow {
Name: String
}
""".toSmithyModel()
val testContextWithItems = testModelWithItems.newTestContext("FlowService", "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<KotlinIntegration> = 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 = """
/**
* Paginate over [ListFlowsResponse] results.
*
* When this operation is called, a [kotlinx.coroutines.Flow] is created. Flows are lazy (cold) so no service
* calls are made until the flow is collected. This also means there is no guarantee that the request is valid
* until then. Once you start collecting the flow, the SDK will lazily load response pages by making service
* calls until there are no pages left or the flow is cancelled. If there are errors in your request, you will
* see the failures only after you start collection.
* @param initialRequest A [ListFlowsRequest] to start pagination
* @return A [kotlinx.coroutines.flow.Flow] that can collect [ListFlowsResponse]
*/
public fun TestClient.listFlowsPaginated(initialRequest: ListFlowsRequest = ListFlowsRequest { }): kotlinx.coroutines.flow.Flow<ListFlowsResponse> =
flow {
var cursor: kotlin.String? = initialRequest.marker
var hasNextPage: Boolean = true
while (hasNextPage) {
val req = initialRequest.copy {
this.marker = cursor
}
val result = [email protected](req)
cursor = result.nextMarker
hasNextPage = cursor?.isNotEmpty() == true
emit(result)
}
}
/**
* Paginate over [ListFlowsResponse] results.
*
* When this operation is called, a [kotlinx.coroutines.Flow] is created. Flows are lazy (cold) so no service
* calls are made until the flow is collected. This also means there is no guarantee that the request is valid
* until then. Once you start collecting the flow, the SDK will lazily load response pages by making service
* calls until there are no pages left or the flow is cancelled. If there are errors in your request, you will
* see the failures only after you start collection.
* @param block A builder block used for DSL-style invocation of the operation
* @return A [kotlinx.coroutines.flow.Flow] that can collect [ListFlowsResponse]
*/
public fun TestClient.listFlowsPaginated(block: ListFlowsRequest.Builder.() -> Unit): kotlinx.coroutines.flow.Flow<ListFlowsResponse> =
listFlowsPaginated(ListFlowsRequest.Builder().apply(block).build())
/**
* This paginator transforms the flow returned by [listFlowsPaginated]
* to access the nested member [Flow]
* @return A [kotlinx.coroutines.flow.Flow] that can collect [Flow]
*/
@JvmName("listFlowsResponseFlow")
public fun kotlinx.coroutines.flow.Flow<ListFlowsResponse>.flows(): kotlinx.coroutines.flow.Flow<Flow> =
transform() { response ->
response.flows?.forEach {
emit(it)
}
}
""".trimIndent()

actual.shouldContainOnlyOnceWithDiff(expectedCode)
}

@Test
fun testRenderPaginatorWithSparseItem() {
val testModelWithItems = """
Expand Down

0 comments on commit 72155f7

Please sign in to comment.