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

Remove the need for operation type aliasing in codegen #1710

Merged
merged 6 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,11 @@ let response = client.some_operation()
references = ["smithy-rs#1647", "smithy-rs#1112"]
meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "client"}
author = "Velfi"

[[smithy-rs]]
message = """
Removed the need to generate operation output and retry aliases in codegen.
"""
references = ["smithy-rs#967"]
Velfi marked this conversation as resolved.
Show resolved Hide resolved
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
author = "Velfi"
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ private class AwsClientGenerics(private val types: Types) : FluentClientGenerics
override val bounds = writable { }

/** Bounds for generated `send()` functions */
override fun sendBounds(input: Symbol, output: Symbol, error: RuntimeType): Writable = writable { }
override fun sendBounds(operation: Symbol, operationOutput: Symbol, operationError: RuntimeType, retryPolicy: Any): Writable = writable { }
Velfi marked this conversation as resolved.
Show resolved Hide resolved

override fun toGenericsGenerator(): GenericsGenerator {
return GenericsGenerator()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,15 @@ class PaginatorGenerator private constructor(
service: ServiceShape,
operation: OperationShape,
private val generics: FluentClientGenerics,
retryPolicyType: Any = RustType.Unit,
Velfi marked this conversation as resolved.
Show resolved Hide resolved
) {

companion object {
fun paginatorType(
coreCodegenContext: CoreCodegenContext,
generics: FluentClientGenerics,
operationShape: OperationShape,
retryPolicyType: Any,
): RuntimeType? {
return if (operationShape.isPaginated(coreCodegenContext.model)) {
PaginatorGenerator(
Expand All @@ -63,6 +65,7 @@ class PaginatorGenerator private constructor(
coreCodegenContext.serviceShape,
operationShape,
generics,
retryPolicyType,
).paginatorType()
} else {
null
Expand All @@ -82,7 +85,8 @@ class PaginatorGenerator private constructor(
)

private val inputType = symbolProvider.toSymbol(operation.inputShape(model))
private val outputType = operation.outputShape(model)
private val outputShape = operation.outputShape(model)
private val outputType = symbolProvider.toSymbol(outputShape)
private val errorType = operation.errorSymbol(model, symbolProvider, CodegenTarget.CLIENT)

private fun paginatorType(): RuntimeType = RuntimeType.forInlineFun(
Expand All @@ -95,12 +99,12 @@ class PaginatorGenerator private constructor(
"generics" to generics.decl,
"bounds" to generics.bounds,
"page_size_setter" to pageSizeSetter(),
"send_bounds" to generics.sendBounds(inputType, symbolProvider.toSymbol(outputType), errorType),
"send_bounds" to generics.sendBounds(symbolProvider.toSymbol(operation), outputType, errorType, retryPolicyType),

// Operation Types
"operation" to symbolProvider.toSymbol(operation),
"Input" to inputType,
"Output" to symbolProvider.toSymbol(outputType),
"Output" to outputType,
"Error" to errorType,
"Builder" to operation.inputShape(model).builderSymbol(symbolProvider),

Expand All @@ -118,7 +122,7 @@ class PaginatorGenerator private constructor(
/** Generate the paginator struct & impl **/
private fun generate() = writable {
val outputTokenLens = NestedAccessorGenerator(symbolProvider).generateBorrowingAccessor(
outputType,
outputShape,
paginationInfo.outputTokenMemberPath,
)
val inputTokenMember = symbolProvider.toMemberName(paginationInfo.inputTokenMember)
Expand Down Expand Up @@ -173,7 +177,7 @@ class PaginatorGenerator private constructor(
let done = match resp {
Ok(ref resp) => {
let new_token = #{output_token}(resp);
let is_empty = ${nextTokenEmpty("new_token")};
let is_empty = new_token.map(|token| token.is_empty()).unwrap_or(true);
if !is_empty && new_token == input.$inputTokenMember.as_ref() {
let _ = tx.send(Err(#{SdkError}::ConstructionFailure("next token did not change, aborting paginator. This indicates an SDK or AWS service bug.".into()))).await;
return;
Expand Down Expand Up @@ -259,18 +263,14 @@ class PaginatorGenerator private constructor(

""",
"extract_items" to NestedAccessorGenerator(symbolProvider).generateOwnedAccessor(
outputType,
outputShape,
paginationInfo.itemsMemberPath,
),
*codegenScope,
)
}
}

private fun nextTokenEmpty(token: String): String {
return "$token.map(|token|token.is_empty()).unwrap_or(true)"
}

private fun pageSizeSetter() = writable {
paginationInfo.pageSizeMember.orNull()?.also {
val memberName = symbolProvider.toMemberName(it)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class FluentClientGenerator(
client = CargoDependency.SmithyClient(codegenContext.runtimeConfig).asType(),
),
private val customizations: List<FluentClientCustomization> = emptyList(),
private val retryPolicyType: RuntimeType? = null,
private val retryPolicyType: Any = RustType.Unit,
) {
companion object {
fun clientOperationFnName(operationShape: OperationShape, symbolProvider: RustSymbolProvider): String =
Expand Down Expand Up @@ -274,7 +274,6 @@ class FluentClientGenerator(
"client" to clientDep.asType(),
"bounds" to generics.bounds,
) {
val inputType = symbolProvider.toSymbol(operation.inputShape(model))
val outputType = symbolProvider.toSymbol(operation.outputShape(model))
val errorType = operation.errorSymbol(model, symbolProvider, CodegenTarget.CLIENT)

Expand Down Expand Up @@ -322,14 +321,14 @@ class FluentClientGenerator(
"OperationOutput" to outputType,
"SdkError" to runtimeConfig.smithyHttp().member("result::SdkError"),
"SdkSuccess" to runtimeConfig.smithyHttp().member("result::SdkSuccess"),
"send_bounds" to generics.sendBounds(inputType, outputType, errorType),
"send_bounds" to generics.sendBounds(operationSymbol, outputType, errorType, retryPolicyType),
"customizable_op_type_params" to rustTypeParameters(
symbolProvider.toSymbol(operation),
retryPolicyType ?: RustType.Unit,
retryPolicyType,
generics.toGenericsGenerator(),
),
)
PaginatorGenerator.paginatorType(codegenContext, generics, operation)?.also { paginatorType ->
PaginatorGenerator.paginatorType(codegenContext, generics, operation, retryPolicyType)?.also { paginatorType ->
rustTemplate(
"""
/// Create a paginator for this request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface FluentClientGenerics {
val bounds: Writable

/** Bounds for generated `send()` functions */
fun sendBounds(input: Symbol, output: Symbol, error: RuntimeType): Writable
fun sendBounds(input: Symbol, output: Symbol, error: RuntimeType, retryPolicy: Any): Writable

/** Convert this `FluentClientGenerics` into the more general `GenericsGenerator` */
fun toGenericsGenerator(): GenericsGenerator
Expand Down Expand Up @@ -70,21 +70,22 @@ data class FlexibleClientGenerics(
}

/** Bounds for generated `send()` functions */
override fun sendBounds(input: Symbol, output: Symbol, error: RuntimeType): Writable = writable {
override fun sendBounds(operation: Symbol, operationOutput: Symbol, operationError: RuntimeType, retryPolicy: Any): Writable = writable {
rustTemplate(
"""
where
R::Policy: #{client}::bounds::SmithyRetryPolicy<
#{Input}OperationOutputAlias,
#{Output},
#{Error},
#{Input}OperationRetryAlias
#{Operation},
#{OperationOutput},
#{OperationError},
#{RetryPolicy}
>
""",
"client" to client,
"Input" to input,
"Output" to output,
"Error" to error,
"Operation" to operation,
"OperationOutput" to operationOutput,
"OperationError" to operationError,
"RetryPolicy" to retryPolicy,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
package software.amazon.smithy.rust.codegen.smithy.generators.protocol

import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.rustlang.Attribute
import software.amazon.smithy.rust.codegen.rustlang.RustType
import software.amazon.smithy.rust.codegen.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.rustlang.docLink
import software.amazon.smithy.rust.codegen.rustlang.rust
Expand All @@ -30,7 +28,7 @@ import software.amazon.smithy.rust.codegen.util.inputShape
* Used to generate payloads that will go into HTTP bodies for HTTP requests (used by clients)
* and responses (used by servers).
*
* **Note:** There is only one real implementation of this interface. The other implementation is test only.
* **Note:** There is only one real implementation of this interface. The other implementation is test-only.
* All protocols use the same class.
*
* Different protocols (e.g. JSON vs. XML) need to use different functionality to generate payload bodies.
Expand All @@ -39,7 +37,7 @@ interface ProtocolPayloadGenerator {
data class PayloadMetadata(val takesOwnership: Boolean)

/**
* Code generation needs to handle whether or not [generatePayload] takes ownership of the input or output
* Code generation needs to handle whether [generatePayload] takes ownership of the input or output
* for a given operation shape.
*
* Most operations will use the HTTP payload as a reference, but for operations that will consume the entire stream
Expand All @@ -62,7 +60,7 @@ interface ProtocolPayloadGenerator {
/**
* Protocol Trait implementation generator
*
* **Note:** There is only one real implementation of this interface. The other implementation is test only.
* **Note:** There is only one real implementation of this interface. The other implementation is test-only.
* All protocols use the same class.
*
* Protocols implement one of two traits to enable parsing HTTP responses:
Expand All @@ -82,7 +80,7 @@ open class ProtocolGenerator(
/**
* `Protocol` contains all protocol specific information. Each smithy protocol, e.g. RestJson, RestXml, etc. will
* have their own implementation of the protocol interface which defines how an input shape becomes and http::Request
* and an output shape is build from an http::Response.
* and an output shape is build from an `http::Response`.
*/
private val protocol: Protocol,
/**
Expand Down Expand Up @@ -117,9 +115,6 @@ open class ProtocolGenerator(
val builderGenerator = BuilderGenerator(model, symbolProvider, operationShape.inputShape(model))
builderGenerator.render(inputWriter)

// generate type aliases for the fluent builders
renderTypeAliases(inputWriter, operationShape, customizations, inputShape)

// impl OperationInputShape { ... }
val operationName = symbolProvider.toSymbol(operationShape).name
inputWriter.implBlock(inputShape, symbolProvider) {
Expand Down Expand Up @@ -172,31 +167,4 @@ open class ProtocolGenerator(
) {
traitGenerator.generateTraitImpls(operationWriter, operationShape, emptyList())
}

private fun renderTypeAliases(
inputWriter: RustWriter,
operationShape: OperationShape,
customizations: List<OperationCustomization>,
inputShape: StructureShape,
) {
// TODO(https://github.com/awslabs/smithy-rs/issues/976): Callers should be able to invoke
// buildOperationType* directly to get the type rather than depending on these aliases.
// These are used in fluent clients.
val operationTypeOutput = buildOperationTypeOutput(inputWriter, operationShape)
val operationTypeRetry = buildOperationTypeRetry(inputWriter, customizations)
val inputPrefix = symbolProvider.toSymbol(inputShape).name

inputWriter.rust(
"""
##[doc(hidden)] pub type ${inputPrefix}OperationOutputAlias = $operationTypeOutput;
##[doc(hidden)] pub type ${inputPrefix}OperationRetryAlias = $operationTypeRetry;
""",
)
}

private fun buildOperationTypeOutput(writer: RustWriter, shape: OperationShape): String =
writer.format(symbolProvider.toSymbol(shape))

private fun buildOperationTypeRetry(writer: RustWriter, customizations: List<OperationCustomization>): String =
(customizations.firstNotNullOfOrNull { it.retryType() } ?: RustType.Unit).let { writer.format(it) }
}