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

Eliminate SDK client generics #1132

Merged
merged 9 commits into from
Feb 1, 2022
59 changes: 58 additions & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,61 @@ author = "jdisanti"
message = "MSRV increased from `1.54` to `1.56.1` per our 2-behind MSRV policy."
references = ["smithy-rs#1130"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "jdisanti"
author = "jdisanti"

[[aws-sdk-rust]]
message = """
Fluent clients for all services no longer have generics, and now use `DynConnector` and `DynMiddleware` to allow
for connector/middleware customization. This should only break references to the client that specified generic types for it.

If you customized the AWS client's connector or middleware with something like the following:
```rust
let client = aws_sdk_s3::Client::with_config(
aws_sdk_s3::client::Builder::new()
.connector(my_custom_connector) // Connector customization
.middleware(my_custom_middleware) // Middleware customization
.default_async_sleep()
.build(),
config
);
```
Then you will need to wrap the custom connector or middleware in
[`DynConnector`](https://docs.rs/aws-smithy-client/0.36.0/aws_smithy_client/erase/struct.DynConnector.html)
and
[`DynMiddleware`](https://docs.rs/aws-smithy-client/0.36.0/aws_smithy_client/erase/struct.DynMiddleware.html)
respectively:
```rust
let client = aws_sdk_s3::Client::with_config(
aws_sdk_s3::client::Builder::new()
.connector(DynConnector::new(my_custom_connector)) // Now with `DynConnector`
.middleware(DynMiddleware::new(my_custom_middleware)) // Now with `DynMiddleware`
.default_async_sleep()
.build(),
config
);
```

If you had functions that took a generic connector, such as the following:
```rust
fn some_function<C, E>(conn: C) -> Result<()>
where
C: aws_smithy_client::bounds::SmithyConnector<Error = E> + Send + 'static,
E: Into<aws_smithy_http::result::ConnectorError>
{
// ...
}
```

Then the generics and trait bounds will no longer be necessary:
```rust
fn some_function(conn: DynConnector) -> Result<()> {
// ...
}
```
Comment on lines +131 to +136
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt a great disturbance in the Force, as if millions of voices where clauses suddenly cried out in terror and were suddenly silenced. I fear something terrible awesome has happened.


Similarly, functions that took a generic middleware can replace the generic with `DynMiddleware` and
remove their trait bounds.
"""
references = ["smithy-rs#1132"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "jdisanti"
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package software.amazon.smithy.rustsdk

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.TitleTrait
import software.amazon.smithy.rust.codegen.rustlang.Attribute
Expand All @@ -23,24 +24,52 @@ import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.smithy.RustCrate
import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.ClientGenerics
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientGenerator
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientSection
import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientGenerator
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientGenerics
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientSection
import software.amazon.smithy.rust.codegen.util.expectTrait
import software.amazon.smithy.rustsdk.AwsRuntimeType.defaultMiddleware

private class Types(runtimeConfig: RuntimeConfig) {
private val smithyClientDep = CargoDependency.SmithyClient(runtimeConfig)
private val smithyHttpDep = CargoDependency.SmithyHttp(runtimeConfig)

val awsTypes = awsTypes(runtimeConfig).asType()
val smithyClientRetry = RuntimeType("retry", smithyClientDep, "aws_smithy_client")
val awsSmithyClient = smithyClientDep.asType()

val defaultMiddleware = runtimeConfig.defaultMiddleware()
val dynConnector = RuntimeType("DynConnector", smithyClientDep, "aws_smithy_client::erase")
val dynMiddleware = RuntimeType("DynMiddleware", smithyClientDep, "aws_smithy_client::erase")
val smithyConnector = RuntimeType("SmithyConnector", smithyClientDep, "aws_smithy_client::bounds")

val connectorError = RuntimeType("ConnectorError", smithyHttpDep, "aws_smithy_http::result")
}

private class AwsClientGenerics(private val types: Types) : FluentClientGenerics {
/** Declaration with defaults set */
override val decl = writable { }

/** Instantiation of the Smithy client generics */
override val smithyInst = writable {
rustTemplate(
"<#{DynConnector}, #{DynMiddleware}<#{DynConnector}>>",
"DynConnector" to types.dynConnector,
"DynMiddleware" to types.dynMiddleware
)
}

/** Instantiation */
override val inst = ""

/** Trait bounds */
override val bounds = writable { }

/** Bounds for generated `send()` functions */
override fun sendBounds(input: Symbol, output: Symbol, error: RuntimeType): Writable = writable { }
Comment on lines +71 to +72
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really surprised this compiles! It generates fn send(...) where { body }. Probably worth including the where in the bounds just so we generate a slightly more normal looking code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, good catch! I'm also surprised 😅

}

class AwsFluentClientDecorator : RustCodegenDecorator {
Expand All @@ -53,12 +82,7 @@ class AwsFluentClientDecorator : RustCodegenDecorator {
val types = Types(codegenContext.runtimeConfig)
FluentClientGenerator(
codegenContext,
generics = ClientGenerics(
connectorDefault = types.dynConnector,
middlewareDefault = types.defaultMiddleware,
retryDefault = types.smithyClientRetry.member("Standard"),
client = types.awsSmithyClient
),
generics = AwsClientGenerics(types),
customizations = listOf(
AwsPresignedFluentBuilderMethod(codegenContext.runtimeConfig),
AwsFluentClientDocs(codegenContext)
Expand Down Expand Up @@ -88,33 +112,34 @@ class AwsFluentClientDecorator : RustCodegenDecorator {
}
}

private class AwsFluentClientExtensions(private val types: Types) {
val clientGenerics = ClientGenerics(
connectorDefault = types.dynConnector,
middlewareDefault = types.defaultMiddleware,
retryDefault = types.smithyClientRetry.member("Standard"),
client = types.awsSmithyClient
)

private class AwsFluentClientExtensions(types: Types) {
private val codegenScope = arrayOf(
"Middleware" to types.defaultMiddleware,
"retry" to types.smithyClientRetry,
"DynConnector" to types.dynConnector,
"aws_smithy_client" to types.awsSmithyClient
"DynMiddleware" to types.dynMiddleware,
"SmithyConnector" to types.smithyConnector,
"ConnectorError" to types.connectorError,
"aws_smithy_client" to types.awsSmithyClient,
"aws_types" to types.awsTypes,
)

fun render(writer: RustWriter) {
writer.rustBlockTemplate("impl<C> Client<C, #{Middleware}, #{retry}::Standard>", *codegenScope) {
writer.rustBlockTemplate("impl Client", *codegenScope) {
rustTemplate(
"""
/// Creates a client with the given service config and connector override.
pub fn from_conf_conn(conf: crate::Config, conn: C) -> Self {
pub fn from_conf_conn<C, E>(conf: crate::Config, conn: C) -> Self
where
C: #{SmithyConnector}<Error = E> + Send + 'static,
E: Into<#{ConnectorError}>,
{
let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default();
let timeout_config = conf.timeout_config.as_ref().cloned().unwrap_or_default();
let sleep_impl = conf.sleep_impl.clone();
let mut builder = #{aws_smithy_client}::Builder::new()
.connector(conn)
.middleware(#{Middleware}::new());
.connector(#{DynConnector}::new(conn))
.middleware(#{DynMiddleware}::new(#{Middleware}::new()));
Comment on lines +141 to +142
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could consider sticking these types into the client generics for a little DRY but not sure if it's worth it

builder.set_retry_config(retry_config.into());
builder.set_timeout_config(timeout_config);
if let Some(sleep_impl) = sleep_impl {
Expand All @@ -123,13 +148,7 @@ private class AwsFluentClientExtensions(private val types: Types) {
let client = builder.build();
Self { handle: std::sync::Arc::new(Handle { client, conf }) }
}
""",
*codegenScope
)
}
writer.rustBlockTemplate("impl Client<#{DynConnector}, #{Middleware}, #{retry}::Standard>", *codegenScope) {
rustTemplate(
"""

/// Creates a new client from a shared config.
##[cfg(any(feature = "rustls", feature = "native-tls"))]
pub fn new(config: &#{aws_types}::config::Config) -> Self {
Expand All @@ -143,7 +162,7 @@ private class AwsFluentClientExtensions(private val types: Types) {
let timeout_config = conf.timeout_config.as_ref().cloned().unwrap_or_default();
let sleep_impl = conf.sleep_impl.clone();
let mut builder = #{aws_smithy_client}::Builder::dyn_https()
.middleware(#{Middleware}::new());
.middleware(#{DynMiddleware}::new(#{Middleware}::new()));
builder.set_retry_config(retry_config.into());
builder.set_timeout_config(timeout_config);
// the builder maintains a try-state. To avoid suppressing the warning when sleep is unset,
Expand All @@ -156,9 +175,7 @@ private class AwsFluentClientExtensions(private val types: Types) {
Self { handle: std::sync::Arc::new(Handle { client, conf }) }
}
""",
"aws_smithy_client" to types.awsSmithyClient,
"aws_types" to types.awsTypes,
"Middleware" to types.defaultMiddleware
*codegenScope,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.smithy.customize.OperationCustomization
import software.amazon.smithy.rust.codegen.smithy.customize.OperationSection
import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientSection
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientSection
import software.amazon.smithy.rust.codegen.smithy.generators.error.errorSymbol
import software.amazon.smithy.rust.codegen.smithy.generators.protocol.MakeOperationGenerator
import software.amazon.smithy.rust.codegen.smithy.protocols.HttpBoundProtocolBodyGenerator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.rust.codegen.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.smithy.RustCrate
import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.ManifestCustomizations
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.config.ConfigCustomization
import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolMap
import software.amazon.smithy.rust.codegen.util.deepMergeWith
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import software.amazon.smithy.rust.codegen.rustlang.writable
import software.amazon.smithy.rust.codegen.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.smithy.generators.client.FluentClientGenerics
import software.amazon.smithy.rust.codegen.smithy.generators.error.errorSymbol
import software.amazon.smithy.rust.codegen.smithy.rustType
import software.amazon.smithy.rust.codegen.util.PANIC
Expand All @@ -45,13 +46,13 @@ class PaginatorGenerator private constructor(
private val symbolProvider: RustSymbolProvider,
service: ServiceShape,
operation: OperationShape,
private val generics: ClientGenerics
private val generics: FluentClientGenerics
) {

companion object {
fun paginatorType(
codegenContext: CodegenContext,
generics: ClientGenerics,
generics: FluentClientGenerics,
operationShape: OperationShape
): RuntimeType? {
return if (operationShape.isPaginated(codegenContext.model)) {
Expand Down Expand Up @@ -79,7 +80,9 @@ class PaginatorGenerator private constructor(
documentation = "Paginators for the service"
)

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

private fun paginatorType(): RuntimeType = RuntimeType.forInlineFun(
paginatorName,
Expand All @@ -91,12 +94,13 @@ 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),

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

// SDK Types
Expand Down Expand Up @@ -125,7 +129,7 @@ class PaginatorGenerator private constructor(
builder: #{Builder}
}

impl${generics.inst} ${paginatorName}${generics.inst} where #{bounds:W} {
impl${generics.inst} ${paginatorName}${generics.inst} #{bounds:W} {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: small inconsistency that one includes the where and the other does not

/// Create a new paginator-wrapper
pub(crate) fn new(handle: std::sync::Arc<crate::client::Handle${generics.inst}>, builder: #{Builder}) -> Self {
Self {
Expand All @@ -143,13 +147,7 @@ class PaginatorGenerator private constructor(
///
/// _Note:_ No requests will be dispatched until the stream is used (eg. with [`.next().await`](tokio_stream::StreamExt::next)).
pub fn send(self) -> impl #{Stream}<Item = std::result::Result<#{Output}, #{SdkError}<#{Error}>>> + Unpin
where
R::Policy: #{client}::bounds::SmithyRetryPolicy<
#{Input}OperationOutputAlias,
#{Output},
#{Error},
#{Input}OperationRetryAlias
>, {
#{send_bounds:W} {
// Move individual fields out of self for the borrow checker
let builder = self.builder;
let handle = self.handle;
Expand Down Expand Up @@ -246,20 +244,14 @@ class PaginatorGenerator private constructor(
/// This is created with [`.items()`]($paginatorName::items)
pub struct ${paginatorName}Items#{generics:W}($paginatorName${generics.inst});

impl ${generics.inst} ${paginatorName}Items${generics.inst} where #{bounds:W} {
impl ${generics.inst} ${paginatorName}Items${generics.inst} #{bounds:W} {
/// Create the pagination stream
///
/// _Note: No requests will be dispatched until the stream is used (eg. with [`.next().await`](tokio_stream::StreamExt::next))._
///
/// To read the entirety of the paginator, use [`.collect::<Result<Vec<_>, _>()`](tokio_stream::StreamExt::collect).
pub fn send(self) -> impl #{Stream}<Item = std::result::Result<${itemType()}, #{SdkError}<#{Error}>>> + Unpin
where
R::Policy: #{client}::bounds::SmithyRetryPolicy<
#{Input}OperationOutputAlias,
#{Output},
#{Error},
#{Input}OperationRetryAlias
>, {
#{send_bounds:W} {
#{fn_stream}::TryFlatMap::new(self.0.send()).flat_map(|page| #{extract_items}(page).unwrap_or_default().into_iter())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0.
*/

package software.amazon.smithy.rust.codegen.smithy.generators
package software.amazon.smithy.rust.codegen.smithy.generators.client

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.MemberShape
Expand All @@ -14,6 +14,7 @@ import software.amazon.smithy.rust.codegen.rustlang.docs
import software.amazon.smithy.rust.codegen.rustlang.documentShape
import software.amazon.smithy.rust.codegen.rustlang.rust
import software.amazon.smithy.rust.codegen.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.smithy.generators.setterName

class FluentClientCore(private val model: Model) {
/** Generate and write Rust code for a builder method that sets a Vec<T> */
Expand Down
Loading