Paginator codegen using Kotlin Flow #557
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0. | ||
| */ | ||
| package software.amazon.smithy.kotlin.codegen.rendering | ||
|
|
||
| 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.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.integration.KotlinIntegration | ||
| import software.amazon.smithy.kotlin.codegen.model.SymbolProperty | ||
| import software.amazon.smithy.kotlin.codegen.model.expectShape | ||
| import software.amazon.smithy.kotlin.codegen.model.hasTrait | ||
| import software.amazon.smithy.kotlin.codegen.model.toSymbol | ||
| import software.amazon.smithy.kotlin.codegen.utils.getOrNull | ||
| import software.amazon.smithy.kotlin.codegen.utils.toggleFirstCharacterCase | ||
| import software.amazon.smithy.model.Model | ||
| import software.amazon.smithy.model.knowledge.PaginatedIndex | ||
| import software.amazon.smithy.model.knowledge.PaginationInfo | ||
| import software.amazon.smithy.model.shapes.CollectionShape | ||
| import software.amazon.smithy.model.shapes.MapShape | ||
| import software.amazon.smithy.model.shapes.OperationShape | ||
| import software.amazon.smithy.model.shapes.ServiceShape | ||
| import software.amazon.smithy.model.shapes.Shape | ||
| import software.amazon.smithy.model.traits.PaginatedTrait | ||
|
|
||
| /** | ||
| * Generate paginators for supporting operations. See | ||
| * https://awslabs.github.io/smithy/1.0/spec/core/behavior-traits.html#paginated-trait for details. | ||
| */ | ||
| class PaginatorGenerator : KotlinIntegration { | ||
| private val kotlinxFlowSymbol = "kotlinx.coroutines.flow.Flow".toSymbol() | ||
| private val kotlinxFlowGeneratorSymbol = "kotlinx.coroutines.flow.flow".toSymbol() | ||
| private val kotlinxFlowTransformSymbol = "kotlinx.coroutines.flow.transform".toSymbol() | ||
|
|
||
| override fun enabledForService(model: Model, settings: KotlinSettings): Boolean = | ||
| model.operationShapes.any { it.hasTrait<PaginatedTrait>() } | ||
|
|
||
| override fun writeAdditionalFiles(ctx: CodegenContext, delegator: KotlinDelegator) { | ||
| val service = ctx.model.expectShape<ServiceShape>(ctx.settings.service) | ||
| val paginatedIndex = PaginatedIndex.of(ctx.model) | ||
|
|
||
| delegator.useFileWriter("Paginators.kt", "${ctx.settings.pkg.name}.paginator") { writer -> | ||
|
There was a problem hiding this comment. question Should we use There was a problem hiding this comment. personally I don't see the value of the nested package. But as we have opted to do this, singular requires one less byte so that would be my vote. There was a problem hiding this comment. Plural better matches conventions in the ecosystem (e.g., kotlin.sequences, kotlin.streams, kotlin.coroutines, etc.) so |
||
| val paginatedOperations = service.allOperations | ||
| .map { ctx.model.expectShape<OperationShape>(it) } | ||
| .filter { operationShape -> operationShape.hasTrait(PaginatedTrait.ID) } | ||
|
|
||
| paginatedOperations.forEach { paginatedOperation -> | ||
| val paginationInfo = paginatedIndex.getPaginationInfo(service, paginatedOperation).getOrNull() | ||
|
There was a problem hiding this comment. style Doesn't optional have There was a problem hiding this comment. It does but it doesn't throw a There was a problem hiding this comment. Right no I want to keep val paginationInfo = paginatedIndex.getPaginationInfo(...)
.orElseThrow {
CodegenException(...)
}What you have is fine though. |
||
| ?: throw CodegenException("Unexpectedly unable to get PaginationInfo from $service $paginatedOperation") | ||
| val paginationItemInfo = getItemDescriptorOrNull(paginationInfo, ctx) | ||
|
|
||
| renderPaginatorForOperation(writer, ctx, service, paginatedOperation, paginationInfo, paginationItemInfo) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Render paginator(s) for operation | ||
| private fun renderPaginatorForOperation( | ||
| writer: KotlinWriter, | ||
| ctx: CodegenContext, | ||
| service: ServiceShape, | ||
| paginatedOperation: OperationShape, | ||
| paginationInfo: PaginationInfo, | ||
| itemDesc: ItemDescriptor? | ||
| ) { | ||
| val serviceSymbol = ctx.symbolProvider.toSymbol(service) | ||
| val outputSymbol = ctx.symbolProvider.toSymbol(paginationInfo.output) | ||
| val inputSymbol = ctx.symbolProvider.toSymbol(paginationInfo.input) | ||
| val cursorMember = ctx.model.getShape(paginationInfo.inputTokenMember.target).get() | ||
| val cursorSymbol = ctx.symbolProvider.toSymbol(cursorMember) | ||
|
|
||
| renderResponsePaginator( | ||
| writer, | ||
| serviceSymbol, | ||
| paginatedOperation, | ||
| inputSymbol, | ||
| outputSymbol, | ||
| paginationInfo, | ||
| cursorSymbol | ||
| ) | ||
|
|
||
| // Optionally generate paginator when nested item is specified on the trait. | ||
| if (itemDesc != null) { | ||
| renderItemPaginator( | ||
| writer, | ||
| service, | ||
| itemDesc, | ||
| outputSymbol | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Generate the paginator that iterates over responses | ||
| private fun renderResponsePaginator( | ||
| writer: KotlinWriter, | ||
| serviceSymbol: Symbol, | ||
| operationShape: OperationShape, | ||
| inputSymbol: Symbol, | ||
| outputSymbol: Symbol, | ||
| paginationInfo: PaginationInfo, | ||
| cursorSymbol: Symbol | ||
| ) { | ||
| val nextMarkerLiteral = paginationInfo.outputTokenMemberPath.joinToString(separator = "?.") { | ||
| it.defaultName() | ||
| } | ||
| val markerLiteral = paginationInfo.inputTokenMember.defaultName() | ||
|
|
||
| writer.write("") | ||
| writer.dokka("Paginate over [${outputSymbol.name}]") | ||
|
There was a problem hiding this comment. We could probably come up with more complete documentation. just spit balling /**
* Paginate over [Client.Operation] 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.
*
* @return a flow of [outputSymbol]
*/
fun Client.fooPaginated(...): Flow<..> { ... } |
||
| writer | ||
| .addImport(kotlinxFlowSymbol) | ||
| .addImport(kotlinxFlowGeneratorSymbol) | ||
| .addImport(serviceSymbol) | ||
| .addImport(inputSymbol) | ||
| .addImport(outputSymbol) | ||
| .addImport(cursorSymbol) | ||
| .addImportReferences(cursorSymbol, SymbolReference.ContextOption.DECLARE) | ||
| .withBlock( | ||
| "fun #T.#LPaginated(initialRequest: #T): Flow<#T> {", | ||
| "}", | ||
| serviceSymbol, | ||
| operationShape.defaultName(), | ||
| inputSymbol, | ||
| outputSymbol | ||
| ) { | ||
| withBlock("return flow {", "}") { | ||
|
There was a problem hiding this comment. this could just be |
||
| write("var cursor: #F = null", cursorSymbol) | ||
| write("var isFirstPage: Boolean = true") | ||
| write("") | ||
| withBlock("while (isFirstPage || (cursor?.isNotEmpty() == true)) {", "}") { | ||
|
There was a problem hiding this comment. Question: The cursor symbol is detected above but There was a problem hiding this comment.
Yes, according to the Smithy spec on the
This is simply using the model to generate it's specified type. What do you find troublesome, the action to retrieve the type of the member, using a variable to store that, or the notation in There was a problem hiding this comment. All of the above. In cases where we know the one-and-only type of a variable, we typically hardcode that (e.g., There was a problem hiding this comment. is cursor the next token? This is not always a string. See: which is used as a token for DynamoDb::scan There was a problem hiding this comment. Good callout, tracking issue: #565 |
||
| withBlock("val req = initialRequest.copy {", "}") { | ||
| write("this.$markerLiteral = cursor") | ||
| } | ||
| write( | ||
| "val result = this@#1LPaginated.#1L(req)", | ||
| operationShape.defaultName() | ||
| ) | ||
| write("isFirstPage = false") | ||
| write("cursor = result.$nextMarkerLiteral") | ||
| write("emit(result)") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Generate a paginator that iterates over the model-specified item | ||
| private fun renderItemPaginator( | ||
| writer: KotlinWriter, | ||
| serviceShape: ServiceShape, | ||
| itemDesc: ItemDescriptor, | ||
| outputSymbol: Symbol, | ||
| ) { | ||
| writer.write("") | ||
| writer.dokka("Paginate over [${outputSymbol.name}.${itemDesc.itemLiteral}]") | ||
| writer | ||
| .addImport(kotlinxFlowTransformSymbol) | ||
| .addImport(itemDesc.itemSymbol) | ||
| .addImportReferences(itemDesc.itemSymbol, SymbolReference.ContextOption.USE) | ||
| .write( | ||
| """@JvmName("#L#L")""", | ||
| outputSymbol.name.toggleFirstCharacterCase(), | ||
|
There was a problem hiding this comment. Suggestion: Explicitly casting the first character to lowercase would clarify the intent here. There was a problem hiding this comment. This? There was a problem hiding this comment. I'm confused...do we want this to be uppercase? That's not normally how Java/Kotlin methods are named. Assuming not, I think this captures the intent more clearly: outputSymbol.name.replaceFirstChar(Char::lowercaseChar) |
||
| itemDesc.targetMember.defaultName(serviceShape) | ||
| ) | ||
| .withBlock( | ||
| "fun #T<#T>.#L(): #T<#L> =", "", | ||
| kotlinxFlowSymbol, | ||
| outputSymbol, | ||
| itemDesc.itemLiteral, | ||
| kotlinxFlowSymbol, | ||
| itemDesc.collectionLiteral | ||
| ) { | ||
| withBlock("transform() { response -> ", "}") { | ||
| withBlock("response.#L?.forEach {", "}", itemDesc.itemPathLiteral) { | ||
|
There was a problem hiding this comment. question Is this correct for maps? We end up with an There was a problem hiding this comment. Yes, we get a |
||
| write("emit(it)") | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Model info necessary to codegen paginator item | ||
| */ | ||
| private data class ItemDescriptor( | ||
| val collectionLiteral: String, | ||
| val targetMember: Shape, | ||
| val itemLiteral: String, | ||
| val itemPathLiteral: String, | ||
| val itemSymbol: Symbol | ||
| ) | ||
|
|
||
| /** | ||
| * Return an [ItemDescriptor] if model supplies, otherwise null | ||
| */ | ||
| private fun getItemDescriptorOrNull(paginationInfo: PaginationInfo, ctx: CodegenContext): ItemDescriptor? { | ||
| val itemMemberId = paginationInfo.itemsMemberPath?.lastOrNull()?.target ?: return null | ||
|
|
||
| val itemLiteral = paginationInfo.itemsMemberPath!!.last()!!.defaultName() | ||
| val itemPathLiteral = paginationInfo.itemsMemberPath.joinToString(separator = "?.") { it.defaultName() } | ||
| val itemMember = ctx.model.expectShape(itemMemberId) | ||
| 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 | ||
| ) | ||
| else -> error("Unexpected shape type ${itemMember.type}") | ||
| } | ||
|
|
||
| return ItemDescriptor( | ||
| collectionLiteral, | ||
| targetMember, | ||
| itemLiteral, | ||
| itemPathLiteral, | ||
| ctx.symbolProvider.toSymbol(itemMember) | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| software.amazon.smithy.kotlin.codegen.lang.BuiltinPreprocessor | ||
| software.amazon.smithy.kotlin.codegen.lang.DocumentationPreprocessor | ||
| software.amazon.smithy.kotlin.codegen.rendering.PaginatorGenerator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
Move to
RuntimeTypes(or similar).Also need to add a dependency on
kotlinx.coroutines. We can either do that by defining it inKotlinDependencyor alternatively we can add anapi("kotlinx.coroutines:$coroutinesVersion")toruntime-core/build.gradle.ktsfor managing the dependency (sinceruntime-coreis basically guaranteed to always be pulled in). This may be favorable to ensure the version stays in sync with the runtime.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new
objectto house external types, I think it makes sense to keep them separate from our own runtime types. Regarding the dependency aspect, yeah we had discussed this but then I found that everything works as is so was reluctant to add it without knowing why it's needed. I notice that the build file you mention already expresses a dependency oncoroutines-core, which is whereFlowand friends live. Is there something I'm missing other than switching fromimplementationtoapi?