-
Notifications
You must be signed in to change notification settings - Fork 197
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
Extract builderInstantiator interface to prepare for nullability changes #2988
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rust.codegen.client.smithy.generators | ||
|
||
import software.amazon.smithy.model.shapes.MemberShape | ||
import software.amazon.smithy.model.shapes.StructureShape | ||
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext | ||
import software.amazon.smithy.rust.codegen.core.rustlang.Writable | ||
import software.amazon.smithy.rust.codegen.core.rustlang.map | ||
import software.amazon.smithy.rust.codegen.core.rustlang.rust | ||
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate | ||
import software.amazon.smithy.rust.codegen.core.rustlang.writable | ||
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider | ||
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator | ||
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderInstantiator | ||
|
||
fun ClientCodegenContext.builderInstantiator(): BuilderInstantiator = ClientBuilderInstantiator(symbolProvider) | ||
|
||
class ClientBuilderInstantiator(private val symbolProvider: RustSymbolProvider) : BuilderInstantiator { | ||
override fun setField(builder: String, value: Writable, field: MemberShape): Writable { | ||
return setFieldBase(builder, value, field) | ||
} | ||
|
||
override fun finalizeBuilder(builder: String, shape: StructureShape, mapErr: Writable?): Writable = writable { | ||
if (BuilderGenerator.hasFallibleBuilder(shape, symbolProvider)) { | ||
rustTemplate( | ||
"$builder.build()#{mapErr}?", | ||
"mapErr" to ( | ||
mapErr?.map { | ||
rust(".map_err(#T)", it) | ||
} ?: writable { } | ||
), | ||
) | ||
} else { | ||
rust("$builder.build()") | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||
/* | ||||||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||||||
* SPDX-License-Identifier: Apache-2.0 | ||||||
*/ | ||||||
|
||||||
package software.amazon.smithy.rust.codegen.core.smithy.generators | ||||||
|
||||||
import software.amazon.smithy.model.shapes.MemberShape | ||||||
import software.amazon.smithy.model.shapes.StructureShape | ||||||
import software.amazon.smithy.rust.codegen.core.rustlang.Writable | ||||||
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate | ||||||
import software.amazon.smithy.rust.codegen.core.rustlang.writable | ||||||
|
||||||
/** Abstraction for instantiating a builders. | ||||||
* | ||||||
* Builder abstractions vary—clients MAY use `build_with_error_correction`, e.g., and builders can vary in fallibility. | ||||||
* */ | ||||||
interface BuilderInstantiator { | ||||||
/** Set a field on a builder. */ | ||||||
fun setField(builder: String, value: Writable, field: MemberShape): Writable | ||||||
|
||||||
/** Finalize a builder, turning into a built object (or in the case of builders-of-builders, return the builder directly).*/ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
fun finalizeBuilder(builder: String, shape: StructureShape, mapErr: Writable? = null): Writable | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth documenting |
||||||
|
||||||
/** Set a field on a builder using the `$setterName` method. $value will be passed directly. */ | ||||||
fun setFieldBase(builder: String, value: Writable, field: MemberShape) = writable { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
Base denotes we're doing inheritance. Which we kind of are doing manually in the implementations of this interface. |
||||||
rustTemplate("$builder = $builder.${field.setterName()}(#{value})", "value" to value) | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,7 @@ class EventStreamUnmarshallerGenerator( | |
private val unionShape: UnionShape, | ||
) { | ||
private val model = codegenContext.model | ||
private val builderInstantiator = codegenContext.builderInstantiator() | ||
private val symbolProvider = codegenContext.symbolProvider | ||
private val codegenTarget = codegenContext.target | ||
private val runtimeConfig = codegenContext.runtimeConfig | ||
|
@@ -339,6 +340,7 @@ class EventStreamUnmarshallerGenerator( | |
// TODO(EventStream): Errors on the operation can be disjoint with errors in the union, | ||
// so we need to generate a new top-level Error type for each event stream union. | ||
when (codegenTarget) { | ||
// TODO(https://github.com/awslabs/smithy-rs/issues/1970) It should be possible to unify these branches now | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why should it be possible? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we have the builderInstantiator, I think. But it's possible this is more divergence that can't be handled like that. |
||
CodegenTarget.CLIENT -> { | ||
val target = model.expectShape(member.target, StructureShape::class.java) | ||
val parser = protocol.structuredDataParser().errorParser(target) | ||
|
@@ -352,9 +354,19 @@ class EventStreamUnmarshallerGenerator( | |
})?; | ||
builder.set_meta(Some(generic)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside: not sure why we bind the error metadata to a variable named |
||
return Ok(#{UnmarshalledMessage}::Error( | ||
#{OpError}::${member.target.name}(builder.build()) | ||
#{OpError}::${member.target.name}( | ||
#{build} | ||
) | ||
)) | ||
""", | ||
"build" to builderInstantiator.finalizeBuilder( | ||
"builder", target, | ||
mapErr = { | ||
rustTemplate( | ||
"""|err|#{Error}::unmarshalling(format!("{}", err))""", *codegenScope, | ||
) | ||
}, | ||
), | ||
"parser" to parser, | ||
*codegenScope, | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,13 +59,16 @@ import software.amazon.smithy.utils.StringUtils | |
* Class describing a JSON parser section that can be used in a customization. | ||
*/ | ||
sealed class JsonParserSection(name: String) : Section(name) { | ||
data class BeforeBoxingDeserializedMember(val shape: MemberShape) : JsonParserSection("BeforeBoxingDeserializedMember") | ||
data class BeforeBoxingDeserializedMember(val shape: MemberShape) : | ||
JsonParserSection("BeforeBoxingDeserializedMember") | ||
|
||
data class AfterTimestampDeserializedMember(val shape: MemberShape) : JsonParserSection("AfterTimestampDeserializedMember") | ||
data class AfterTimestampDeserializedMember(val shape: MemberShape) : | ||
JsonParserSection("AfterTimestampDeserializedMember") | ||
|
||
data class AfterBlobDeserializedMember(val shape: MemberShape) : JsonParserSection("AfterBlobDeserializedMember") | ||
|
||
data class AfterDocumentDeserializedMember(val shape: MemberShape) : JsonParserSection("AfterDocumentDeserializedMember") | ||
data class AfterDocumentDeserializedMember(val shape: MemberShape) : | ||
JsonParserSection("AfterDocumentDeserializedMember") | ||
} | ||
|
||
/** | ||
|
@@ -100,6 +103,7 @@ class JsonParserGenerator( | |
private val codegenTarget = codegenContext.target | ||
private val smithyJson = CargoDependency.smithyJson(runtimeConfig).toType() | ||
private val protocolFunctions = ProtocolFunctions(codegenContext) | ||
private val builderInstantiator = codegenContext.builderInstantiator() | ||
private val codegenScope = arrayOf( | ||
"Error" to smithyJson.resolve("deserialize::error::DeserializeError"), | ||
"expect_blob_or_null" to smithyJson.resolve("deserialize::token::expect_blob_or_null"), | ||
|
@@ -251,6 +255,7 @@ class JsonParserGenerator( | |
deserializeMember(member) | ||
} | ||
} | ||
|
||
CodegenTarget.SERVER -> { | ||
if (symbolProvider.toSymbol(member).isOptional()) { | ||
withBlock("builder = builder.${member.setterName()}(", ");") { | ||
|
@@ -508,12 +513,14 @@ class JsonParserGenerator( | |
"Builder" to symbolProvider.symbolForBuilder(shape), | ||
) | ||
deserializeStructInner(shape.members()) | ||
// Only call `build()` if the builder is not fallible. Otherwise, return the builder. | ||
if (returnSymbolToParse.isUnconstrained) { | ||
rust("Ok(Some(builder))") | ||
} else { | ||
rust("Ok(Some(builder.build()))") | ||
val builder = builderInstantiator.finalizeBuilder( | ||
"builder", shape, | ||
) { | ||
rustTemplate( | ||
"""|err|#{Error}::custom_source("Response was invalid", err)""", *codegenScope, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look correct to me. This only applies to clients, not servers. It only works because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mapErr defines how to convert the error back into the caller error—since the server never returns an error, it doesn't need to use it. |
||
) | ||
} | ||
rust("Ok(Some(#T))", builder) | ||
} | ||
} | ||
} | ||
|
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.