Skip to content

Commit

Permalink
Add get_foo() -> &Option<Foo> accessors for builders and fluent build…
Browse files Browse the repository at this point in the history
…ers. (#2792)

This allows testing, as well as making decisions while creating
builders. The references are not mutable, so these props remain read
only keeping the Builder invariants.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
Some operations are built up using various logic (for instance, choosing
an S3 bucket based on a logged in user). It is a lot of overhead to test
at the mock level, and the Debug formatting is typically not stable.
<!--- If it fixes an open issue, please link to the issue here -->
Closes #2791 

## Description
<!--- Describe your changes in detail -->
1. Add `get_foo(&self) -> &Option<Foo>` to Builders
2. Add `as_input(&self) -> &OperationInputBuilder` to Fluent Builders
3. Add `get_foo(&self) -> &Option<Foo>` to FluentBuilder which delegates
to Builders


## Testing
<!--- Please describe in detail how you tested your changes -->
Included unit tests, as well as [proof of concept
tests](DavidSouther/aws-doc-sdk-examples@017e8a2)
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [X] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [X] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: John DiSanti <[email protected]>
  • Loading branch information
DavidSouther and jdisanti authored Jun 21, 2023
1 parent df62f5c commit e6293b2
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 11 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"

[[aws-sdk-rust]]
message = "Add accessors to Builders"
references = ["smithy-rs#2791"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "davidsouther"

[[smithy-rs]]
message = "Add accessors to Builders"
references = ["smithy-rs#2791"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"}
author = "davidsouther"

[[smithy-rs]]
message = "Avoid intermediate vec allocations in AggregatedBytes::to_vec."
author = "yotamofek"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.docs
import software.amazon.smithy.rust.codegen.core.rustlang.documentShape
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName

class FluentClientCore(private val model: Model) {
Expand Down Expand Up @@ -67,4 +68,15 @@ class FluentClientCore(private val model: Model) {
write("self")
}
}

/**
* Generate and write Rust code for a getter method that returns a reference to the inner data.
*/
fun RustWriter.renderGetterHelper(member: MemberShape, memberName: String, coreType: RustType) {
documentShape(member, model)
deprecatedShape(member)
withBlockTemplate("pub fn $memberName(&self) -> &#{CoreType} {", "}", "CoreType" to coreType) {
write("self.inner.$memberName()")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations
import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
import software.amazon.smithy.rust.codegen.core.smithy.generators.getterName
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName
import software.amazon.smithy.rust.codegen.core.smithy.rustType
import software.amazon.smithy.rust.codegen.core.util.inputShape
Expand Down Expand Up @@ -409,6 +410,14 @@ class FluentClientGenerator(
}
}

rust("/// Access the ${operationSymbol.name} as a reference.\n")
withBlockTemplate(
"pub fn as_input(&self) -> &#{Inner} {", "}",
"Inner" to symbolProvider.symbolForBuilder(input),
) {
write("&self.inner")
}

if (smithyRuntimeMode.generateMiddleware) {
val middlewareScope = arrayOf(
*preludeScope,
Expand Down Expand Up @@ -630,6 +639,9 @@ class FluentClientGenerator(
val setterName = member.setterName()
val optionalInputType = outerType.asOptional()
with(core) { renderInputHelper(member, setterName, optionalInputType) }

val getterName = member.getterName()
with(core) { renderGetterHelper(member, getterName, optionalInputType) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ class FluentClientGeneratorTest {
}
operation SayHello { input: TestInput }
structure TestInput {}
structure TestInput {
foo: String,
byteValue: Byte,
}
""".asSmithyModel()

@Test
Expand Down Expand Up @@ -76,4 +79,50 @@ class FluentClientGeneratorTest {
test = test,
)
}

@Test
fun `generate inner builders`() {
val test: (ClientCodegenContext, RustCrate) -> Unit = { codegenContext, rustCrate ->
rustCrate.integrationTest("inner_builder") {
val moduleName = codegenContext.moduleUseName()
rustTemplate(
"""
##[test]
fn test() {
let connector = #{TestConnection}::<#{SdkBody}>::new(Vec::new());
let config = $moduleName::Config::builder()
.endpoint_resolver("http://localhost:1234")
#{set_http_connector}
.build();
let smithy_client = aws_smithy_client::Builder::new()
.connector(connector.clone())
.middleware_fn(|r| r)
.build_dyn();
let client = $moduleName::Client::with_config(smithy_client, config);
let say_hello_fluent_builder = client.say_hello().byte_value(4).foo("hello!");
assert_eq!(*say_hello_fluent_builder.get_foo(), Some("hello!".to_string()));
let input = say_hello_fluent_builder.as_input();
assert_eq!(*input.get_byte_value(), Some(4));
}
""",
"TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig)
.withFeature("test-util").toType()
.resolve("test_connection::TestConnection"),
"SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig),
"set_http_connector" to writable {
if (codegenContext.smithyRuntimeMode.generateOrchestrator) {
rust(".http_connector(connector.clone())")
}
},
)
}
}
clientIntegrationTest(model, TestCodegenSettings.middlewareModeTestParams, test = test)
clientIntegrationTest(
model,
TestCodegenSettings.orchestratorModeTestParams,
test = test,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ class OperationBuildError(private val runtimeConfig: RuntimeConfig) {
// Setter names will never hit a reserved word and therefore never need escaping.
fun MemberShape.setterName() = "set_${this.memberName.toSnakeCase()}"

// Getter names will never hit a reserved word and therefore never need escaping.
fun MemberShape.getterName() = "get_${this.memberName.toSnakeCase()}"

class BuilderGenerator(
private val model: Model,
private val symbolProvider: RustSymbolProvider,
Expand Down Expand Up @@ -210,6 +213,28 @@ class BuilderGenerator(
}
}

/**
* Render a `get_foo` method. This is useful as a target for code generation, because the argument type
* is the same as the resulting member type, and is always optional.
*/
private fun renderBuilderMemberGetterFn(
writer: RustWriter,
outerType: RustType,
member: MemberShape,
memberName: String,
) {
// TODO(https://github.com/awslabs/smithy-rs/issues/1302): This `asOptional()` call is superfluous except in
// the case where the shape is a `@streaming` blob, because [StreamingTraitSymbolProvider] always generates
// a non `Option`al target type: in all other cases the client generates `Option`al types.
val inputType = outerType.asOptional()

writer.documentShape(member, model)
writer.deprecatedShape(member)
writer.rustBlock("pub fn ${member.getterName()}(&self) -> &${inputType.render(true)}") {
rust("&self.$memberName")
}
}

private fun renderBuilder(writer: RustWriter) {
writer.docs("A builder for #D.", structureSymbol)
metadata.additionalAttributes.render(writer)
Expand Down Expand Up @@ -240,6 +265,7 @@ class BuilderGenerator(
}

renderBuilderMemberSetterFn(this, outerType, member, memberName)
renderBuilderMemberGetterFn(this, outerType, member, memberName)
}
writeCustomizations(customizations, BuilderSection.AdditionalMethods(shape))
renderBuildFn(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ internal class BuilderGeneratorTest {
unitTest("generate_builders") {
rust(
"""
let my_struct = MyStruct::builder().byte_value(4).foo("hello!").build();
let my_struct_builder = MyStruct::builder().byte_value(4).foo("hello!");
assert_eq!(*my_struct_builder.get_byte_value(), Some(4));
let my_struct = my_struct_builder.build();
assert_eq!(my_struct.foo.unwrap(), "hello!");
assert_eq!(my_struct.bar, 0);
""",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,22 @@ class UnconstrainedCollectionGeneratorTest {
val model =
"""
namespace test
use aws.protocols#restJson1
use smithy.framework#ValidationException
@restJson1
service TestService {
operations: ["Operation"]
}
@http(uri: "/operation", method: "POST")
operation Operation {
input: OperationInputOutput
output: OperationInputOutput
errors: [ValidationException]
}
structure OperationInputOutput {
list: ListA
}
Expand Down Expand Up @@ -103,7 +103,7 @@ class UnconstrainedCollectionGeneratorTest {
let c_builder = crate::model::StructureC::builder();
let list_b_unconstrained = crate::unconstrained::list_b_unconstrained::ListBUnconstrained(vec![c_builder]);
let list_a_unconstrained = crate::unconstrained::list_a_unconstrained::ListAUnconstrained(vec![list_b_unconstrained]);
let _list_a: crate::constrained::MaybeConstrained<crate::constrained::list_a_constrained::ListAConstrained> = list_a_unconstrained.into();
""",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ class UnconstrainedMapGeneratorTest {
val model =
"""
namespace test
use aws.protocols#restJson1
use smithy.framework#ValidationException
@restJson1
service TestService {
operations: ["Operation"]
}
@http(uri: "/operation", method: "POST")
operation Operation {
input: OperationInputOutput
output: OperationInputOutput
errors: [ValidationException]
}
structure OperationInputOutput {
map: MapA
}
Expand Down

0 comments on commit e6293b2

Please sign in to comment.