Skip to content

Commit b43905e

Browse files
authored
Builders of builders (#1342)
This patchset, affectionately called "Builders of builders", lays the groundwork for fully implementing [Constraint traits] in the server SDK generator. [The RFC] illustrates what the end goal looks like, and is recommended prerrequisite reading to understanding this cover letter. This commit makes the sever deserializers work with _unconstrained_ types during request parsing, and only after the entire request is parsed are constraints enforced. Values for a constrained shape are stored in the correspondingly unconstrained shape, and right before the operation input is built, the values are constrained via a `TryFrom<UnconstrainedShape> for ConstrainedShape` implementation that all unconstrained types enjoy. The service owner only interacts with constrained types, the unconstrained ones are `pub(crate)` and for use by the framework only. In the case of structure shapes, the corresponding unconstrained shape is their builders. This is what gives this commit its title: during request deserialization, arbitrarily nested structures are parsed into _builders that hold builders_. Builders keep track of whether their members are constrained or not by storing its members in a `MaybeConstrained` [Cow](https://doc.rust-lang.org/std/borrow/enum.Cow.html)-like `enum` type: ```rust pub(crate) trait Constrained { type Unconstrained; } #[derive(Debug, Clone)] pub(crate) enum MaybeConstrained<T: Constrained> { Constrained(T), Unconstrained(T::Unconstrained), } ``` Consult the documentation for the generator in `ServerBuilderGenerator.kt` for more implementation details and for the differences with the builder types the server has been using, generated by `BuilderGenerator.kt`, which after this commit are exclusively used by clients. Other shape types, when they are constrained, get generated with their correspondingly unconstrained counterparts. Their Rust types are essentially wrapper newtypes, and similarly enjoy `TryFrom` converters to constrain them. See the documentation in `UnconstrainedShapeSymbolProvider.kt` for details and an example. When constraints are not met, the converters raise _constraint violations_. These are currently `enum`s holding the _first_ encountered violation. When a shape is _transitively but not directly_ constrained, newtype wrappers are also generated to hold the nested constrained values. To illustrate their need, consider for example a list of `@length` strings. Upon request parsing, the server deserializers need a way to hold a vector of unconstrained regular `String`s, and a vector of the constrained newtyped `LengthString`s. The former requirement is already satisfied by the generated unconstrained types, but for the latter we need to generate an intermediate constrained `ListUnconstrained(Vec<LengthString>)` newtype that will eventually be unwrapped into the `Vec<LengthString>` the user is handed. This is the purpose of the `PubCrate*` generators: consult the documentation in `PubCrateConstrainedShapeSymbolProvider.kt`, `PubCrateConstrainedCollectionGenerator.kt`, and `PubCrateConstrainedMapGenerator.kt` for more details. As their name implies, all of these types are `pub(crate)`, and the user never interacts with them. For users that would not like their application code to make use of constrained newtypes for their modeled constrained shapes, a `codegenConfig` setting `publicConstrainedTypes` has been added. They opt out of these by setting it to `false`, and use the inner types directly: the framework will still enforce constraints upon request deserialization, but once execution enters an application handler, the user is on their own to honor (or not) the modeled constraints. No user interest has been expressed for this feature, but I expect we will see demand for it. Moreover, it's a good stepping stone for users that want their services to honor constraints, but are not ready to migrate their application code to constrained newtypes. As for how it's implemented, several parts of the codebase inspect the setting and toggle or tweak generators based on its value. Perhaps the only detail worth mentioning in this commit message is that the structure shape builder types are generated by a much simpler and entirely different generator, in `ServerBuilderGeneratorWithoutPublicConstrainedTypes.kt`. Note that this builder _does not_ enforce constraints, except for `required` and `enum`, which are always (and already) baked into the type system. When `publicConstrainedTypes` is disabled, this is the builder that end users interact with, while the one that enforces all constraints, `ServerBuilderGenerator`, is now generated as `pub(crate)` and left for exclusive use by the deserializers. See the relevant documentation for the details and differences among the builder types. As proof that these foundations are sound, this commit also implements the `length` constraint trait on Smithy map and string shapes. Likewise, the `required` and `enum` traits, which were already baked in the generated types as non-`Option`al and `enum` Rust types, respectively, are now also treated like the rest of constraint traits upon request deserialization. See the documentation in `ConstrainedMapGenerator.kt` and `ConstrainedStringGenerator.kt` for details. The rest of the constraint traits and target shapes are left as an exercise to the reader, but hopefully the reader has been convinced that all of them can be enforced within this framework, paving the way for straightforward implementations. The diff is already large as it is. Any reamining work is being tracked in #1401; this and other issues are referenced in the code as TODOs. So as to not give users the impression that the server SDK plugin _fully_ honors constraints as per the Smithy specification, a validator in `ValidateUnsupportedConstraintsAreNotUsed.kt` has been added. This traverses the model and detects yet-unsupported parts of the spec, aborting code generation and printing informative warnings referencing the relevant tracking issues. This is a regression in that models that used constraint traits previously built fine (even though the constraint traits were silently not being honored), and now they will break. To unblock generation of these models, this commit adds another `codegenConfig` setting, `ignoreUnsupportedConstraints`, that users can opt into. Closes #1714. Testing ------- Several Kotlin unit test classes exercising the finer details of the added generators and symbol providers have been added. However, the best way to test is to generate server SDKs from models making use of constraint traits. The biggest assurances come from the newly added `constraints.smithy` model, an "academic" service that _heavily_ exercises constraint traits. It's a `restJson1` service that also tests binding of constrained shapes to different parts of the HTTP message. Deeply nested hierarchies and recursive shapes are also featured. ```sh ./gradlew -P modules='constraints' codegen-server-test:build ``` This model is _additionally_ generated in CI with the `publicConstrainedTypes` setting disabled: ```sh ./gradlew -P modules='constraints_without_public_constrained_types' codegen-server-test:build `````` Similarly, models using currently unsupported constraints are now being generated with the `ignoreUnsupportedConstraints` setting enabled. See `codegen-server-test/build.gradle.kts` for more details. [Constraint traits]: https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html [The RFC]: #1199
1 parent b2528a1 commit b43905e

File tree

113 files changed

+7438
-634
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

113 files changed

+7438
-634
lines changed

CHANGELOG.next.toml

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,3 +214,128 @@ message = "Several breaking changes have been made to errors. See [the upgrade g
214214
references = ["smithy-rs#1926", "smithy-rs#1819"]
215215
meta = { "breaking" = true, "tada" = false, "bug" = false }
216216
author = "jdisanti"
217+
218+
[[smithy-rs]]
219+
message = """
220+
[Constraint traits](https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html) in server SDKs are beginning to be supported. The following are now supported:
221+
222+
* The `length` trait on `string` shapes.
223+
* The `length` trait on `map` shapes.
224+
225+
Upon receiving a request that violates the modeled constraints, the server SDK will reject it with a message indicating why.
226+
227+
Unsupported (constraint trait, target shape) combinations will now fail at code generation time, whereas previously they were just ignored. This is a breaking change to raise awareness in service owners of their server SDKs behaving differently than what was modeled. To continue generating a server SDK with unsupported constraint traits, set `codegenConfig.ignoreUnsupportedConstraints` to `true` in your `smithy-build.json`.
228+
"""
229+
references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401"]
230+
meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" }
231+
author = "david-perez"
232+
233+
[[smithy-rs]]
234+
message = """
235+
Server SDKs now generate "constrained types" for constrained shapes. Constrained types are [newtypes](https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html) that encapsulate the modeled constraints. They constitute a [widespread pattern to guarantee domain invariants](https://www.lpalmieri.com/posts/2020-12-11-zero-to-production-6-domain-modelling/) and promote correctness in your business logic. So, for example, the model:
236+
237+
```smithy
238+
@length(min: 1, max: 69)
239+
string NiceString
240+
```
241+
242+
will now render a `struct NiceString(String)`. Instantiating a `NiceString` is a fallible operation:
243+
244+
```rust
245+
let data: String = ... ;
246+
let nice_string = NiceString::try_from(data).expect("data is not nice");
247+
```
248+
249+
A failed attempt to instantiate a constrained type will yield a `ConstraintViolation` error type you may want to handle. This type's API is subject to change.
250+
251+
Constrained types _guarantee_, by virtue of the type system, that your service's operation outputs adhere to the modeled constraints. To learn more about the motivation for constrained types and how they work, see [the RFC](https://github.com/awslabs/smithy-rs/pull/1199).
252+
253+
If you'd like to opt-out of generating constrained types, you can set `codegenConfig.publicConstrainedTypes` to `false`. Note that if you do, the generated server SDK will still honor your operation input's modeled constraints upon receiving a request, but will not help you in writing business logic code that adheres to the constraints, and _will not prevent you from returning responses containing operation outputs that violate said constraints_.
254+
"""
255+
references = ["smithy-rs#1342", "smithy-rs#1119"]
256+
meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" }
257+
author = "david-perez"
258+
259+
[[smithy-rs]]
260+
message = """
261+
Structure builders in server SDKs have undergone significant changes.
262+
263+
The API surface has been reduced. It is now simpler and closely follows what you would get when using the [`derive_builder`](https://docs.rs/derive_builder/latest/derive_builder/) crate:
264+
265+
1. Builders no longer have `set_*` methods taking in `Option<T>`. You must use the unprefixed method, named exactly after the structure's field name, and taking in a value _whose type matches exactly that of the structure's field_.
266+
2. Builders no longer have convenience methods to pass in an element for a field whose type is a vector or a map. You must pass in the entire contents of the collection up front.
267+
3. Builders no longer implement [`PartialEq`](https://doc.rust-lang.org/std/cmp/trait.PartialEq.html).
268+
269+
Bug fixes:
270+
271+
4. Builders now always fail to build if a value for a `required` member is not provided. Previously, builders were falling back to a default value (e.g. `""` for `String`s) for some shapes. This was a bug.
272+
273+
Additions:
274+
275+
5. A structure `Structure` with builder `Builder` now implements `TryFrom<Builder> for Structure` or `From<Builder> for Structure`, depending on whether the structure [is constrained](https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html) or not, respectively.
276+
277+
To illustrate how to migrate to the new API, consider the example model below.
278+
279+
```smithy
280+
structure Pokemon {
281+
@required
282+
name: String,
283+
@required
284+
description: String,
285+
@required
286+
evolvesTo: PokemonList
287+
}
288+
289+
list PokemonList {
290+
member: Pokemon
291+
}
292+
```
293+
294+
In the Rust code below, note the references calling out the changes described in the numbered list above.
295+
296+
Before:
297+
298+
```rust
299+
let eevee_builder = Pokemon::builder()
300+
// (1) `set_description` takes in `Some<String>`.
301+
.set_description(Some("Su código genético es muy inestable. Puede evolucionar en diversas razas de Pokémon.".to_owned()))
302+
// (2) Convenience method to add one element to the `evolvesTo` list.
303+
.evolves_to(vaporeon)
304+
.evolves_to(jolteon)
305+
.evolves_to(flareon);
306+
307+
// (3) Builder types can be compared.
308+
assert_ne!(eevee_builder, Pokemon::builder());
309+
310+
// (4) Builds fine even though we didn't provide a value for `name`, which is `required`!
311+
let _eevee = eevee_builder.build();
312+
```
313+
314+
After:
315+
316+
```rust
317+
let eevee_builder = Pokemon::builder()
318+
// (1) `set_description` no longer exists. Use `description`, which directly takes in `String`.
319+
.description("Su código genético es muy inestable. Puede evolucionar en diversas razas de Pokémon.".to_owned())
320+
// (2) Convenience methods removed; provide the entire collection up front.
321+
.evolves_to(vec![vaporeon, jolteon, flareon]);
322+
323+
// (3) Binary operation `==` cannot be applied to `pokemon::Builder`.
324+
// assert_ne!(eevee_builder, Pokemon::builder());
325+
326+
// (4) `required` member `name` was not set.
327+
// (5) Builder type can be fallibly converted to the structure using `TryFrom` or `TryInto`.
328+
let _error = Pokemon::try_from(eevee_builder).expect_err("name was not provided");
329+
```
330+
"""
331+
references = ["smithy-rs#1714", "smithy-rs#1342"]
332+
meta = { "breaking" = true, "tada" = true, "bug" = true, "target" = "server" }
333+
author = "david-perez"
334+
335+
[[smithy-rs]]
336+
message = """
337+
Server SDKs now correctly reject operation inputs that don't set values for `required` structure members. Previously, in some scenarios, server SDKs would accept the request and set a default value for the member (e.g. `""` for a `String`), even when the member shape did not have [Smithy IDL v2's `default` trait](https://awslabs.github.io/smithy/2.0/spec/type-refinement-traits.html#smithy-api-default-trait) attached. The `default` trait is [still unsupported](https://github.com/awslabs/smithy-rs/issues/1860).
338+
"""
339+
references = ["smithy-rs#1714", "smithy-rs#1342", "smithy-rs#1860"]
340+
meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "server" }
341+
author = "david-perez"

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientInstantiator.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,34 @@
66
package software.amazon.smithy.rust.codegen.client.smithy.generators
77

88
import software.amazon.smithy.codegen.core.Symbol
9+
import software.amazon.smithy.model.shapes.MemberShape
10+
import software.amazon.smithy.model.shapes.StructureShape
911
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
1012
import software.amazon.smithy.rust.codegen.core.rustlang.rust
1113
import software.amazon.smithy.rust.codegen.core.rustlang.writable
1214
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
15+
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
1316
import software.amazon.smithy.rust.codegen.core.smithy.generators.Instantiator
17+
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName
1418

1519
private fun enumFromStringFn(enumSymbol: Symbol, data: String): Writable = writable {
1620
rust("#T::from($data)", enumSymbol)
1721
}
1822

23+
class ClientBuilderKindBehavior(val codegenContext: CodegenContext) : Instantiator.BuilderKindBehavior {
24+
override fun hasFallibleBuilder(shape: StructureShape): Boolean =
25+
BuilderGenerator.hasFallibleBuilder(shape, codegenContext.symbolProvider)
26+
27+
override fun setterName(memberShape: MemberShape): String = memberShape.setterName()
28+
29+
override fun doesSetterTakeInOption(memberShape: MemberShape): Boolean = true
30+
}
31+
1932
fun clientInstantiator(codegenContext: CodegenContext) =
2033
Instantiator(
2134
codegenContext.symbolProvider,
2235
codegenContext.model,
2336
codegenContext.runtimeConfig,
37+
ClientBuilderKindBehavior(codegenContext),
2438
::enumFromStringFn,
2539
)

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/HttpBoundProtocolGenerator.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
2626
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization
2727
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationSection
2828
import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations
29-
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
29+
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
3030
import software.amazon.smithy.rust.codegen.core.smithy.generators.builderSymbol
3131
import software.amazon.smithy.rust.codegen.core.smithy.generators.error.errorSymbol
3232
import software.amazon.smithy.rust.codegen.core.smithy.generators.http.ResponseBindingGenerator
@@ -332,7 +332,7 @@ class HttpBoundProtocolTraitImplGenerator(
332332
}
333333
}
334334

335-
val err = if (StructureGenerator.hasFallibleBuilder(outputShape, symbolProvider)) {
335+
val err = if (BuilderGenerator.hasFallibleBuilder(outputShape, symbolProvider)) {
336336
".map_err(${format(errorSymbol)}::unhandled)?"
337337
} else ""
338338

0 commit comments

Comments
 (0)